Skip to content
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

Fix: Remove sideloaded games from recent list when uninstalled #4178

Merged
merged 1 commit into from
Mar 1, 2025

Conversation

arielj
Copy link
Collaborator

@arielj arielj commented Dec 15, 2024

We have an issue that we can only manually remove elements from the recent list in the tray icon when they are either installed or part of the games in the library, but uninstalled sideloaded games disappear completely.

This PR doesn't fix the full problem but I think it's good enough for most use cases:

  • It does remove the item when uninstalled
  • It does NOT cleanup the list if it has old items (I don't think it's worth including code to check the list and remove entries, we can handle those cases manually)

To clean the list with already old elements, we can instruct users to edit the file at ~/.config/heroic/store/config.json and remove elements they don't want from the "recent" key.

Fixes #4170


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@arielj arielj added the pr:ready-for-review Feature-complete, ready for the grind! :P label Dec 15, 2024
@arielj arielj requested review from a team, flavioislima, CommandMC, Nocccer and imLinguin and removed request for a team December 15, 2024 01:19
@@ -128,6 +129,7 @@ export async function uninstall({
notify({ title, body: i18next.t('notify.uninstalled') })

removeShortcutsUtil(gameInfo)
removeRecentGame(appName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add awaits and await removeNonSteamGame({ gameInfo }) as well ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand we intentionally don't await for these things, they can happen async and makes it faster

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could still wrap 'em in an await Promise.all([ ... ]) to at least run them in parallel

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't those already running in parallel? I think we really don't care about waiting for the result of those calls at all

is there any benefit I'm missing?

Copy link
Member

@Etaash-mathamsetty Etaash-mathamsetty Dec 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it would be better if everything was removed by the time this function returned, but it's not necessary. Also the other uninstall functions use awaits

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think we need await there tbh, I agree with Ariel that this type of thing is fine to be async since it will take less than a second and should not block anything

@arielj arielj force-pushed the remove-sideload-from-recent branch from 887f895 to f513f37 Compare March 1, 2025 19:19
@arielj arielj force-pushed the remove-sideload-from-recent branch from f513f37 to 7d5eb0c Compare March 1, 2025 19:22
@flavioislima flavioislima merged commit a2be468 into main Mar 1, 2025
9 checks passed
@flavioislima flavioislima deleted the remove-sideload-from-recent branch March 1, 2025 19:26
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators Mar 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recent entries in tray icon can't be removed after uninstalling a sideloaded game
4 participants