From f56f4dd0e4e81bbf10c563c394cc60573dc5369a Mon Sep 17 00:00:00 2001 From: Daniel Johnson Date: Sat, 13 Jan 2024 06:04:55 -0500 Subject: [PATCH] Work around the 'double files' bug. If you click Continue fas enough, you can trigger the prepare_game_files() method to run twice, on two threads concurrently. This results in doubled up files and a duplicate files page in the navigation stack. This is hard to fix simply or without UI ugliness, so this commit tolerates it. It adjusts prepare_game_files() to be thread-safer (!) by committing its changes only at the end. The GIL should make this atomic, probably, so the last preparation wins instead of being combined with the other. The navigation stack will now refuse to add a duplicate page; it still exits the old and re-enters but does not put the second entry in the history, so you'll go back further when you click the Back button. That should hide the problem with minimal code churn and risk. --- lutris/gui/widgets/navigation_stack.py | 2 +- lutris/installer/installer.py | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lutris/gui/widgets/navigation_stack.py b/lutris/gui/widgets/navigation_stack.py index c1a88cd95..47a4c040a 100644 --- a/lutris/gui/widgets/navigation_stack.py +++ b/lutris/gui/widgets/navigation_stack.py @@ -64,7 +64,7 @@ class NavigationStack(Gtk.Stack): In addition, this updates the navigation state so navigate_back() and such work, they may call the presenter again. """ - if self.current_navigated_page_presenter: + if self.current_navigated_page_presenter and self.current_navigated_page_presenter != page_presenter: self.navigation_stack.append(self.current_navigated_page_presenter) self._update_back_button() diff --git a/lutris/installer/installer.py b/lutris/installer/installer.py index b4170f8a3..8143510c5 100644 --- a/lutris/installer/installer.py +++ b/lutris/installer/installer.py @@ -161,11 +161,11 @@ class LutrisInstaller: # pylint: disable=too-many-instance-attributes installer_file_id = file.id installer_file_url = file.url break - self.files = [file.copy() for file in self.script_files if file.id != installer_file_id] - self.extra_file_paths = [] + files = [file.copy() for file in self.script_files if file.id != installer_file_id] + extra_file_paths = [] # Run variable substitution on the URLs from the script - for file in self.files: + for file in files: file.set_url(self.interpreter._substitute(file.url)) if is_moddb_url(file.url): file.set_url(ModDB().transform_url(file.url)) @@ -178,7 +178,7 @@ class LutrisInstaller: # pylint: disable=too-many-instance-attributes installer_files = self.service.get_patch_files(self, installer_file_id) else: content_files, extra_files = self.service.get_installer_files(self, installer_file_id, extras) - self.extra_file_paths = [path for f in extra_files for path in f.get_dest_files_by_id().values()] + extra_file_paths = [path for f in extra_files for path in f.get_dest_files_by_id().values()] installer_files = content_files + extra_files except UnavailableGameError as ex: logger.error("Game not available: %s", ex) @@ -186,15 +186,21 @@ class LutrisInstaller: # pylint: disable=too-many-instance-attributes if installer_files: for installer_file in installer_files: - self.files.append(installer_file) + files.append(installer_file) else: # Failed to get the service game, put back a user provided file logger.debug("Unable to get files from service. Setting %s to manual.", installer_file_id) - self.files.insert(0, InstallerFile(self.game_slug, installer_file_id, { + files.insert(0, InstallerFile(self.game_slug, installer_file_id, { "url": installer_file_url, "filename": "" })) + # Commit changes only at the end; this is more robust in this method is runner + # my two threads concurrently- the GIL can probably save us. It's not desirable + # to do this, but this is the easiest workaround. + self.files = files + self.extra_file_paths = extra_file_paths + def install_extras(self): # Copy extras to game folder; this updates the installer script, so it needs # be called just once, before launching the installers commands.