CtInfo: Update Games List after Batch Update #350
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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! :-)