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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
3 participants