From 505088e0535948419deba6669d756d0f260d03f9 Mon Sep 17 00:00:00 2001 From: GeoffreyCoulaud Date: Fri, 19 May 2023 17:35:31 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Task=20fix=20+=20progress=20bar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/game.py | 2 +- src/importer/importer.py | 36 +++++--------- src/importer/sources/lutris_source.py | 2 +- src/utils/task.py | 69 +++++++++++++++++++++++++-- src/window.py | 2 +- 5 files changed, 79 insertions(+), 32 deletions(-) diff --git a/src/game.py b/src/game.py index c568c2e..9f3952c 100644 --- a/src/game.py +++ b/src/game.py @@ -60,7 +60,7 @@ class Game(Gtk.Box): blacklisted = None game_cover = None - def __init__(self, win, data, allow_side_effects=False, **kwargs): + def __init__(self, win, data, allow_side_effects=True, **kwargs): super().__init__(**kwargs) self.win = shared.win diff --git a/src/importer/importer.py b/src/importer/importer.py index 1034979..dd517ec 100644 --- a/src/importer/importer.py +++ b/src/importer/importer.py @@ -3,7 +3,7 @@ import logging from requests import HTTPError from gi.repository import Adw, Gio, Gtk -from .task import make_task_thread_func_closure +from .task import Task from .create_dialog import create_dialog from .steamgriddb import SGDBAuthError, SGDBError, SGDBHelper @@ -41,7 +41,7 @@ class Importer: @property def progress(self): try: - progress = 1 - self.n_tasks_created / self.n_tasks_done + progress = self.n_tasks_done / self.n_tasks_created except ZeroDivisionError: progress = 1 return progress @@ -62,30 +62,12 @@ class Importer: # (If SGDB auth is bad, cancel all SGDB tasks) self.sgdb_cancellable = Gio.Cancellable() - """ - # Create a task for each source - def make_closure(source): - def closure(task, obj, _data, cancellable): - self.source_task_thread_func(task, obj, (source,), cancellable) - - return closure - for source in self.sources: - self.n_source_tasks_created += 1 logging.debug("Importing games from source %s", source.id) task = Task.new(None, None, self.source_task_callback, (source,)) - closure = make_closure(source) - task.run_in_thread(closure) - """ - - for source in self.sources: self.n_source_tasks_created += 1 - logging.debug("Importing games from source %s", source.id) - task = Gio.Task.new(None, None, self.source_task_callback, (source,)) - closure = make_task_thread_func_closure( - self.source_task_thread_func, (source,) - ) - task.run_in_thread(closure) + task.set_task_data((source,)) + task.run_in_thread(self.source_task_thread_func) def create_dialog(self): """Create the import dialog""" @@ -105,6 +87,9 @@ class Importer: self.import_dialog.present() def update_progressbar(self): + logging.debug( + "Progressbar updated (%f)", self.progress + ) # TODO why progress not workie? self.progressbar.set_fraction(self.progress) def source_task_thread_func(self, _task, _obj, data, _cancellable): @@ -151,12 +136,12 @@ class Importer: # Start sgdb lookup for game # HACK move to its own manager - task = Gio.Task.new( + task = Task.new( None, self.sgdb_cancellable, self.sgdb_task_callback, (game,) ) - closure = make_task_thread_func_closure(self.sgdb_task_thread_func, (game,)) self.n_sgdb_tasks_created += 1 - task.run_in_thread(closure) + task.set_task_data((game,)) + task.run_in_thread(self.sgdb_task_thread_func) def source_task_callback(self, _obj, _result, data): """Source import callback""" @@ -213,6 +198,7 @@ class Importer: self.dialog_response_callback, "open_preferences", "import", + None, ) elif self.n_games_added == 1: diff --git a/src/importer/sources/lutris_source.py b/src/importer/sources/lutris_source.py index 5a707ce..4e755ba 100644 --- a/src/importer/sources/lutris_source.py +++ b/src/importer/sources/lutris_source.py @@ -76,7 +76,7 @@ class LutrisSourceIterator(SourceIterator): "executable": self.source.executable_format.format(game_id=row[2]), "developer": None, # TODO get developer metadata on Lutris } - game = Game(self.source.win, values) + game = Game(self.source.win, values, allow_side_effects=False) # Save official image image_path = self.source.cache_location / "coverart" / f"{row[2]}.jpg" diff --git a/src/utils/task.py b/src/utils/task.py index 356ce9a..a698e8b 100644 --- a/src/utils/task.py +++ b/src/utils/task.py @@ -1,7 +1,68 @@ -def make_task_thread_func_closure(func, data): - """Prepare a Gio.TaskThreadFunc with its data bound in a closure""" +from gi.repository import Gio +from functools import wraps - def closure(task, obj, _data, cancellable): - func(task, obj, data, cancellable) + +def create_task_thread_func_closure(func, data): + """Wrap a Gio.TaskThreadFunc with the given data in a closure""" + + def closure(task, source_object, _data, cancellable): + func(task, source_object, data, cancellable) return closure + + +def decorate_set_task_data(task): + """Decorate Gio.Task.set_task_data to replace it""" + + def decorator(original_method): + @wraps(original_method) + def new_method(task_data): + task.__task_data = task_data + pass + + return new_method + + return decorator + + +def decorate_run_in_thread(task): + """Decorate Gio.Task.run_in_thread to pass the task data correctly + Creates a closure around task_thread_func with the task data available.""" + + def decorator(original_method): + @wraps(original_method) + def new_method(task_thread_func): + closure = create_task_thread_func_closure( + task_thread_func, task.__task_data + ) + original_method(closure) + + return new_method + + return decorator + + +class Task: + """Wrapper around Gio.Task to patch task data not being passed""" + + @classmethod + def new(cls, source_object, cancellable, callback, callback_data): + """Create a new, monkey-patched Gio.Task. + The `set_task_data` and `run_in_thread` methods are decorated. + + As of 2023-05-19, pygobject does not work well with Gio.Task, so to pass data + the only viable way it to create a closure with the thread function and its data. + This class is supposed to make Gio.Task comply with its expected behaviour + per the docs: + + http://lazka.github.io/pgi-docs/#Gio-2.0/classes/Task.html#Gio.Task.set_task_data + + This code may break if pygobject overrides change in the future. + We need to manually pass `self` to the decorators since it's otherwise bound but + not accessible from Python's side. + """ + + task = Gio.Task.new(source_object, cancellable, callback, callback_data) + task.set_task_data = decorate_set_task_data(task)(task.set_task_data) + task.run_in_thread = decorate_run_in_thread(task)(task.run_in_thread) + return task diff --git a/src/window.py b/src/window.py index b456b9e..ba98c34 100644 --- a/src/window.py +++ b/src/window.py @@ -114,7 +114,7 @@ class CartridgesWindow(Adw.ApplicationWindow): ): path.unlink(missing_ok=True) else: - Game(game, allow_side_effects=True).update() + Game(game).update() # Connect search entries self.search_bar.connect_entry(self.search_entry)