-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
CtInfo: Update Games List after Batch Update #350
base: main
Are you sure you want to change the base?
CtInfo: Update Games List after Batch Update #350
Conversation
Thanks!
Good catch!
Interesting idea. It would make sense to do that as you're done with that compatibility tool after a batch update.
The boolean parameter of ProtonUp-Qt/pupgui2/pupgui2.py Line 229 in 5e5fd9d
That only makes sense if we leave this PR as is. If you decide to not update the ctinfo game list, but instead close the ctinfo dialog after a batch update, we still need to have
(only if we close the UI after a batch update)
That also means we need to add # When a batch update is completed
ctbu_dialog.batch_update_complete.connect(self.on_batch_update_complete)
def on_batch_update_complete(self):
self.batch_update_complete.emit(True) # call update_ui and reload steam app list
self.ui.close()
# When clicking refresh games
self.update_game_list(cached=False)
def update_game_list(self, cached=True):
... # code which reloads the cached app list
self.batch_update_complete.emit(False) # call update_ui but use cached app list there
# pupgui2.py#MainWindow
def update_ui(self, cached=False):
...
steam_app_list = get_steam_app_list(..., cached=cached) |
Overview
This PR fixes the CtInfo dialog's game list not updating after a batch update. If you perform a batch update, the game list still shows the old values. The used/unused and game count indicators will update correctly on the main menu, and re-opening or refreshing the CtInfo dialog will update the values. But they are not immediately updated after a batch update.
When we do a batch update to move the games in use by one compatibility tool to another, aside from actually doing this, there are some other steps:
update_game_list_steam
update_game_list_steam
, at the end, we call emit a signal to notify that the batch update is complete. This is received by the main window and updates athe compatibility tool usage information (i.e. immediately update to reflect if a tool is unused after a batch update)Problem
Currently though, the call to
update_game_list_steam
is incorrect, for two reasons:update_game_list_steam
.a. By extension, we were only calling
batch_update_complete.emit
inside ofupdate_games_list_steam
, when it should've been called inupdate_game_list
so that the signal is correctly emitted if we ever implement batch update for other launchers.Solution
The fix for this is to first, call
btn_refresh_games_clicked
, which is the method connected on click of the CtInfo dialog and callsupdate_game_list
withcached=False
. Then to fix the last issue,batch_update_complete.emit
was moved to the bottom ofupdate_game_list
. I wasn't sure if it should go inupdate_game_list
orupdate_game_list_ui
, but I figured keeping it more "top-level" in update_game_list is fine. It does technically have the job of performing a Qt-related action, and updating the UI on the main menu, so if you'd prefer it elsewhere that's fine. 🙂This does mean there is a bit of delay after pressing the "Batch Update" button, as it will "hang" a bit before the dialog actually closes while the CtInfo dialog updates. Once this is updated, the Batch Update dialog will close and the user will be returned to the updated
Concerns
Two minor concerns here:
We're calling
batch_update_complete.emit(True)
unconditionally. We were doing this before already, but only with Steam, but now we're doing it for every launcher when we open a CtInfo dialog. I have not noticed any performance impact to this. We could possibly addbatch_update_complete=False
as a default parameter toupdate_game_list
, and then we could change the emit call toself.batch_update_complete.emit(batch_update_complete)
. Then for the Batch Update dialog's signal, we can to change to connecting like this:ctbu_dialog.batch_update_complete.connect(lambda: self.update_game_list(cached=False, batch_update_complete=True)
.My other concern is, after a Batch Update, the CtInfo dialog is always going to be empty since we're moving every game. There may not be much reason to keep the dialog open. Maybe we should close the CtInfo dialog after a batch update? We'd have to make sure we still call
self.batch_update_complete.emit(True)
in CtInfo so that the main menu information updates, but that should be fine to do. It would invalidate this PR but it may be better UX - or unexpected, depending on your point of view 😅Thanks! :-)