Wine-GE: Match launcher naming schemes on extract

[sonic2kk]

Implements #294.

Overview

This adds an extra step to Wine-GE installation which renames the extraction folder to be consistent with what the current launcher would name it.

Screenshots for Lutris and Heroic respectively:

Implementation

We currently keep the name of the top-level archive (lutris-GE-ProtonX-YY-x86_64), but Lutris and Heroic do not use this name. For Lutris specifically, this apparently breaks auto-updating. For Heroic, I figured if we're making a change to be consistent for one launcher (Lutris) we should do it for all of them :-)

The launcher check is similar to what we use for DXVK (and elsewhere I believe), essentially checking for strong clues in the path to infer the launcher.

I am not sure what Bottles uses, as I don't use it, but if we need to make a change there it should be easy enough to add.

I could not see a straightforward way to set the extract name for the top-level folder with tarfile, it seems the general idea for that is read and extract each file into a specific folder, which I thought was overkill. I think it is more straightforward to just rename the folder after extraction is finished, though this does mean the folder name will not be updated until extraction completes (could be about 10 seconds or more). I doubt any users would see this, though.

Considerations

I'm thinking since we've re-used this snippet quite a bit that it could be its own utility function, probably taking install_dir and returning an enum value so we could do a check like if get_launcher_from_install_dir(install_dir) == Launchers.HEROIC. We do some checks like this already, checking on strings with install_loc.get('launcher'), but in ctmods we just pass down install_dir. Though we could allow these install_loc checks to use the enum as well, it may be slightly nicer than checking directly on the string 🙂

Not really a big deal either way though, just a thought I had during development that I wanted to share.

---

Thanks!

[DavidoTek]

Thanks!

I think it is more straightforward to just rename the folder after extraction is finished, though this does mean the folder name will not be updated until extraction completes (could be about 10 seconds or more).

I think that is fine. There probably is some way to do that with tarfile but is seems the solution you choose is way more clean.

Considerations

Good idea. Having that would probably also simplify adding new launchers or allow for different ways of detecting the launcher in case we would need that in the future.

[sonic2kk]

I'll take a look at adding a util function for getting the Launcher type from the install_dir.

What's our minimum Python version these days? If it's >= 3.10 we could use a match for this, but a standard if/else is fine if not :-)

[sonic2kk]

Okay, I created a new util function called get_launcher_from_installdir. For safety it uses str.lower on the install_dir given, which is mainly helpful when matching Steam, since the path have an upper or lowercase Steam, for example .local/share/Steam or .root/steam (there are other lowercase paths as well).

Usage is like this, but I need to do further testing on it so this is just an example:

# Returns a new Enum, DataStructure#Launcher
launcher = get_launcher_from_installdir(install_dir)  # i.e. /home/gaben/.local/share/lutris/runners
if launcher == Launcher.STEAM:
    print('No good, it's full of Steam!')
elif launcher == Launcher.LUTRIS:
    print('Remember winesteam?')
elif launcher == Launcher.HEROIC:
    print('When did I get so many GOG games...')
elif launcher == Launcher.BOTTLES:
    print('Tamika approves')

We could easily extend this in future for, say, PlayOnLinux by adding another launcher to the Launcher enum and then adding a check.

A match statement might be slightly cleaner in my personal opinion, but I'm not sure if they're faster or anything, so it isn't a huge deal. If we could use it though it might be a nice little bit of syntax sugar :-)

[DavidoTek]

I'll take a look at adding a util function for getting the Launcher type from the install_dir.

Great, thanks.

What's our minimum Python version these days? If it's >= 3.10 we could use a match for this, but a standard if/else is fine if not :-)
A match statement might be slightly cleaner in my personal opinion, but I'm not sure if they're faster or anything, so it isn't a huge deal. If we could use it though it might be a nice little bit of syntax sugar :-)

I agree that a match would be cleaner, but...
We're currently still bound to Python 3.8 due to the AppImage build which uses packages from Ubuntu 20.04 (Focal).
It should be possible to update this, but I would prefer to stay on a slightly older version for compatibility reason, at least as long as it is officially supported by Canonical (non-ESM support goes until April 2025). Though I don't think anyone who runs games will be using an old operating system (most likely non-LTS or rolling release).
EDIT: I just checked, Ubuntu 22.04 (Jammy) is 18 months old by now, this would be an option and it also includes Python 3.10.
-> I would leave it as is right now. If we update the AppImage to also use Python 3.10 we could do a general refactoring of the code I guess.

---

For safety it uses str.lower on the install_dir given

Ah, that makes sense.

Usage is like this, but I need to do further testing on it so this is just an example:

Looks good.

We could easily extend this in future

Good, that saves us quite some manual work then

[sonic2kk]

We're currently still bound to Python 3.8 due to the AppImage build which uses packages from Ubuntu 20.04 (Focal).
It should be possible to update this, but I would prefer to stay on a slightly older version for compatibility reason, at least as long as it is officially supported by Canonical (non-ESM support goes until April 2025). Though I don't think anyone who runs games will be using an old operating system (most likely non-LTS or rolling release).
EDIT: I just checked, Ubuntu 22.04 (Jammy) is 18 months old by now, this would be an option and it also includes Python 3.10.
-> I would leave it as is right now. If we update the AppImage to also use Python 3.10 we could do a general refactoring of the code I guess.

I think leaving as-is is fine for now, we can revisit in future like you said. I was just curious :-) Thanks for explaining

[sonic2kk]

I should note that I'm testing this right now, but having weird difficulties with GitHub when trying to download (also happens when trying to leave comments and push commits), but in the couple of tests I've done so far, this PR appears to still work as expected with the new refactor :-)

I will also need to test the ctmods I updated to use the new util function (DXVK, D8VK, vkd3d)

[sonic2kk]

Tested DXVK and vkd3d-proton, both work as expected.

D8VK is broken as the main branch has not had any commits in so long that all artifacts have expired for that branch (https://github.com/AlpyneDreams/d8vk/actions?query=branch%3Amain). Removing the check in the D8VK ctmod's fetch_releases for artifact['expired'], ProtonUp-Qt will list all of the workflows, but it can't download them, as the artifact has expired and when we pass it to nightly.link, it tells us as much.

D8VK does have releases now that we could fall back to, but if the main branch got a commit right after I posted this comment, it would start working again. There is a PR to merge D8VK into DXVK (doitsujin/dxvk#3411), but it's a work-in-progress at the moment, so it may be worth adding a "normal" D8VK ctmod (since the current one is "nightly").

I haven't had the time yet (hence the lack of PRs here 😅) but at some point I would like to make a "common" DXVK ctmod similar to what we have for Proton-tkg. Until then we could probably just have a separate ctmod for a non-nightly D8VK.

That is, entirely, a separate issue to this PR though, as this behaviour is also on master :-)

[DavidoTek]

I made two changes to the code:

• Changed the order of the Launcher enum (UNKNOWN = 0) to match the other enums
• Removed the redudant or original_name in get_launcher_extract_dirname of Wine-GE

Looks very good and will be merged. Thanks.

---

D8VK is broken as the main branch has not had any commits in so long that all artifacts have expired for that branch

Right, that could be fixed if the merger of D8VK is delayed.

D8VK does have releases now that we could fall back to, but if the main branch got a commit right after I posted this comment, it would start working again

I haven't had the time yet (hence the lack of PRs here 😅) but at some point I would like to make a "common" DXVK ctmod similar to what we have for Proton-tkg. Until then we could probably just have a separate ctmod for a non-nightly D8VK.

I see, currently only DXVK Async is based on the DXVK ctmod. Seems like D8VK is using the same release format as DXVK so this seems quite trivial to do. There's no hurry 😄

---

One thing I noticed is that the function get_launcher_from_installdir is kinda redundant as we already have the function get_install_location_from_directory_name which returns a dict with an attribute "launcher" that contains a string with the launcher name. We also lose the ability remember custom set install directories here.
I noticed that when testing this PR and it wouldn't work when I set a random directory as a custom Lutris install directory (Lutris is currently not installed).
This would be something for a future PR though. We probably need to refractor get_install_location_from_directory_name and maybe change the CtInstaller parameter install_dir to install_loc (dict/struct).

[sonic2kk]

Thanks for the updates! They make sense.

---

I see, currently only DXVK Async is based on the DXVK ctmod. Seems like D8VK is using the same release format as DXVK so this seems quite trivial to do. There's no hurry

Yup and that's due for a refactor with #302, though it still uses the subclass format. I structured DXVK in that PR to be compatible with both GitHub and GitLab, and tried to keep in mind nightly.link builds when structuring the function to fetch and filter releases. It might not be perfect, but I did keep in mind making all of this as flexible as possible to allow for more of these common ctmods in a way that is still maintainable, without a whole bunch of conditional logic.

It should be pretty straightforward still to make the subclass for D8VK, as that PR's refactor is structured in such a way that the implementation of how we download should be irrelevant to the subclasses. I'll probably look at it after that PR though in case there are further refactors needed to how we handle sessions.

I am not sure yet how we will handle DXVK Nightly, perhaps we can just override some methods, I haven't looked too deeply yet :-)

---

This would be something for a future PR though. We probably need to refractor get_install_location_from_directory_name and maybe change the CtInstaller parameter install_dir to install_loc (dict/struct).

This makes sense, that way ctmods are more aware of launcher information. Then we can check on the launcher name instead of the path, which is much better imo. I wonder if we could then set the launcher type to the Launcher enum, though that might require a pretty significant refactor depending on how it's used, I recall a few places in the games list I think off the top of my head...

Should be feasible either way, just a bit of work, though I think using install_loc in ctmods overall is cleaner than using install_dir :-)

---

Thanks!