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

[FEATURE] Refactor the Runner classes to use a single instance per runner #235

Open
Arcitec opened this issue Sep 12, 2024 · 2 comments · May be fixed by #246
Open

[FEATURE] Refactor the Runner classes to use a single instance per runner #235

Arcitec opened this issue Sep 12, 2024 · 2 comments · May be fixed by #246
Labels
enhancement New feature or request

Comments

@Arcitec
Copy link
Collaborator

Arcitec commented Sep 12, 2024

From a comment during our discussions:


The only "unsolved" things that we've talked about earlier, are that STL always shows up in the "Only Installed" mode, and that "Only Installed" and "All Versions" are 2 separate UI panels.

Neither issue is particularly important. All other runners offer a way to reinstall after you remove them in the "Only Installed" view, so might as well keep STL like that (doing its version check/download ability) in that tab too. People can see the download icon and realize it's not installed. It's a nice feature and a really minor issue, which is only really solvable with some big refactoring of the app.

Since you wanted to refactor the app later to make 1 single class instance handle both "Only Installed" and "All Versions" lists, I have some ideas for that:

  • Only instantiate one class instance per Runner.
  • Every Runner class internally has version_rows (a List of Gtk Widgets for all of the web-based + any locally installed versions).
  • The List[Gtk.Widget] version_rows is a list of all rows, and whenever the user toggles "Only Installed", you loop through that and set the row's visible state based on whether it's installed.
  • And hiding the Load More button in "Only Installed" view.

That way, a runner's class would know the entire state in a single class instance which can seamlessly change the visible rows based on which mode is active. This way, the runner's class-state and UI rows all use the same memory and there's no state desync issues.

@Arcitec Arcitec added the enhancement New feature or request label Sep 12, 2024
@Arcitec
Copy link
Collaborator Author

Arcitec commented Sep 12, 2024

Here are some other notes from our discussions about that:


Theoretically, we could have a menu entry at the top left above Steam, called:

| Available Updates |
| ----------------- |
| Steam             |
| Lutris            |
| etc...            |

When this is clicked, the app:

  • Loops through all installed apps.
  • Loops through all runners for all installed apps.
  • If any of them has a newer version available, add them to a list.
  • Then present the list to the user all in one view (on the right side of the UI where Runners list normally is):
| Steam: Proton-GE 9.3  |
| System                |
| --------------------- |
| Lutris: Wine-GE 8.5-1 |
| Flatpak               |

etc.

Clicking a row jumps directly to that app and expands that runner's list view.


So in short: This would let people check every update at the same time. And yes it would take a moment for the app to query updates for every runner, sure, but it could be optimized so that runner classes don't download lists of updates if the user doesn't have at least 1 installed version on disk already:

  • So the class would basically lazy-initialize itself, only checking if there's anything on disk.
  • If not, remember is_initialized = false and return "there are no updates for this runner/app combo".
  • If there is a runner installed locally for that app, then download the list of release versions for that app, set is_initialized = true and then return either "there is an update: Version 23.45" or "there are no updates for this runner/app combo".
  • So then it doesn't have to do any real work if there's nothing installed locally. This would speed up the "all updates" check significantly.
  • And if the user later clicks that app + that runner manually to see what versions are available, then it would see if (!is_initialized) and would THEN do the rest of initialization to grab the list of latest versions for the regular GUI.

This would let a single class instance handle Only Installed, All and Updates. :)


The necessary restructuring of the runner classes would fit together with the planned refactoring to having a single instance per "runner class + app combo".

So the single class instances per each runner + app combo would contain:

  • List of runner_versions with flags for is_installed. The is_installed flag would be used to determine if the row should be set_visible() or not in Only Installed mode.
  • A lazy initialization feature which sets is_initialized = false for the mode that ONLY looks for locally installed versions and doesn't query the server yet.
  • A getter like get_runner_versions() that checks if (!is_initialized) and does the rest of initialization (checking GitHub API for available versions, mainly), and then caches and returns runner_versions with the list of available versions.

Actually here's a better idea, with a basic Python prototype to show what I mean:

from pathlib import Path

class Runner:
    pass

class STL(Runner):
    all_versions: list[str]

    def __init__(self, app_location: Path) -> None:
        # Do not query any GitHub API here. Do fast initialization without that!
        self.all_versions = []

    def is_installed(self) -> bool:
        # Do something to check `app_location` to see if this Runner is installed.
        return False
    
    def get_versions(self, installed_only: bool, load_more: bool = False) -> list[str]:
        if installed_only:
            # Build a list of locally installed versions and return it.
            return ["Proton-GE 5.4", "Proton-GE 10.3"]
        else:
            # Only fetch from GitHub's API if we want more or we have NOTHING yet.
            if load_more or len(self.all_versions) < 1:
                # Build or extend `all_versions` from GitHub's API query.
                self.all_versions.append("Proton-GE 10.4")

            # Also ensure that the list contains all locally installed versions.

            return self.all_versions
        
    def install(self, version: str) -> bool:
        # Install the requested version.
        return True

    def remove(self, version: str) -> bool:
        # Remove the requested version.
        return True
    
# Now to check for updates in all runners:
runners = [STL("~/.local/share/Steam")]
for runner in runners:
    # Skips this runner if it's not installed, to avoid GitHub API queries.
    if not runner.is_installed():
        continue

    # Now ask the runner for the list of available + locally installed versions.
    all_versions = runner.get_versions(False)

    # Check if there's an update available for this runner + app combo.
    if len(all_versions) > 0 and not all_versions[0].is_installed:
        # There is an update!
        pass
    else:
        # There is no update, so we rapidly skip this in "Available Updates" mode.

That is the rough idea. This kind of structure would allow to query "Only Installed", "All Versions", and "Is Installed" (to only do the API check for "All Versions" for updates if the user has installed that runner).

It also has install() and remove() which takes the requested version as an argument, so that a single class instance can handle the install/uninstall for all version rows of the runner.

Of course STL would not follow that exact setup since it is a special case where there's only 1 available version. :slight_smile: But the other runners that use GitHub Releases could use this format.


That would then allow us to implement an Available Updates panel:

image

image


Clicking on an available update would then jump to Steam > Proton-Ge (expanded view) for example. So it's like a shortcut and overview of all updates.

There's no need to allow installing directly from the unified update list.

Furthermore, "Available Updates" would be the default app panel when at least 1 runner is installed. Otherwise, it would be hidden.

@Arcitec
Copy link
Collaborator Author

Arcitec commented Sep 12, 2024

The comments above are the raw notes and ideas. The further down the discussion (above), the newer the idea. The newest idea is at the bottom.

@Vysp3r Vysp3r linked a pull request Sep 22, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant