-
-
Notifications
You must be signed in to change notification settings - Fork 451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Automatic creation of steam shortcuts for uninstalled games #4235
base: main
Are you sure you want to change the base?
Add support for Automatic creation of steam shortcuts for uninstalled games #4235
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This process is something that can take quiet a lot of time. For example, I have over 600 games, turning this on would start a loop doing adding the games one by one in the background and if a game takes 0.5s to be added (we have to get pcgamingwiki data, then download the images, edit steam files, save) it would take 5 minutes to complete with no visible indication of the progress (and no way to stop it)
I think a better approach to do something like this at least as an initial version of this feature would be to add a button Add all games to Steam
that opens a dialog and shows progress. That way it's more manageable and can solve some of the issues I mentioned in other comments. I imagine a dialog shows up telling you You are about to add these games to Steam
or something like that, add some information about not closing the dialog maybe, and a confirm
button. then that action could loop through the libraries and start adding games in batches maybe and show some progress indicator.
I think this would still need some rework, would the pcgamingwiki and steamgrid be ok with this amount of requests in a short period of time? would they stop us with some rate limiting? what happens if some of those requests fail? the code calls getWikiGameInfo
which also tries to get howlongtobeat info, gamesdbinfo, it means more requests are happening for each game (these requests are fine for a single game, but in a loop of large libraries it's going to be a problem)
@@ -443,6 +445,10 @@ export async function refresh(): Promise<ExecResult> { | |||
} | |||
} | |||
|
|||
for (const game of gamesObjects) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe, instead of checking addSteamShortcutsUninstalled
for every game in addShortcuts
, you could check that once here and then if it's true you do the loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, won't this trigger one system notification per game? it can be really annoying to turn this on, restart heroic and start seeing maybe hundreds of notifications popping up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's also A LOT of work happening for every game: getting wiki game info, downloading the images, checking if steam files exist, etc
I think using addShortcuts
in a loop is not scalable (there's users with 1000+ games), we would need a different method, something that adds all games and then saves the shortcuts.vdf once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, having each library running its own for
loop and all of them writing to the shortcuts.vdf
file at the same time in parallel can create a bunch of race conditions (what if while the gog loops is trying to add a game, the legendary loop saved a file, then the gog loops overrides it?)
or if I refresh the library while the games are being added, then this would run again for the same store in parallel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
finally, I don't think it's scalable to wait for all the shortcuts to be added to continue with the refresh call, this will delay showing the library in heroic
@@ -385,6 +386,7 @@ export async function refresh(): Promise<ExecResult> { | |||
const filteredApiArray = gameApiArray.filter( | |||
(entry) => entry.platform_id === 'gog' | |||
) | |||
console.log(filteredApiArray.length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.log(filteredApiArray.length) |
} | ||
title={t( | ||
'setting.addgamestosteamuninstalled', | ||
'Add uninstalled games to Steam automatically' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this feature seems to add all games
to steam, even installed ones that were not added before
I have around 600 games aswell (jej epic games ;) but yeah its currently a bit naive. running this sync doesnt cause any rate limit issues but to speed up the process running this async would be a good idea. I still like this to be part of the refresh process to be honest. Don't see the value of being it a separate thing and making it extra work for the user. Would like this to be a set and forget thing Can split in 2 tho. First run the local refresh so results are shown quick and then run the steam shortcuts. I assumed there would already be a process indicator for import and refresh (something somethings assumptions). If this isn't the case i can look into giving these processes an process indicator. il look into optimizing this further |
continuation of #4234 fix #3378