search

DavidoTek / ProtonUp-Qt

Install and manage GE-Proton, Luxtorpeda & more for Steam and Wine-GE & more for Lutris with this graphical user interface.
https://davidotek.github.io/protonup-qt
GNU General Public License v3.0
1.18k stars 39 forks source link

Lutris-Wine: Only show fshack build when the release has it #258

Closed sonic2kk closed 1 year ago

sonic2kk commented 1 year ago

Fixes #256, mostly. It doesn't fix an issue with the download/extraction getting stuck at 99%, because I couldn't reproduce that, but it should fix the user getting to the point where they could download an invalid build.

Background

Right now ProtonUp-Qt assumes that Lutris-Wine builds all have an associated fshack build. The fshack is a Wine patch, and I guess for compatibility reasons, older Wine builds came patched with and without it. As of Lutris-Wine 7.2-2 (distinct from Lutris-Wine 7.2, which does have the fshack build available), this is no longer required. The fshack in Lutris-Wine >= 7.2-2 can be controlled with an environment variable: WINE_DISABLE_FULLSCREEN_HACK.

This is a problem because ProtonUp-Qt assumes that there is an fshack build for all Lutris-Wine release tags, and simply creates the release tags list as follows:

tags.extend((release['tag_name'], release['tag_name'].replace('lutris-', 'lutris-fshack-')))

This is valid for Lutris-Wine <= 7.2, because they do all contain fshack release variants, at least up to the first page of releases I scrolled through.

The problem here is that ProtonUp-Qt points to an invalid wine-lutris-fshack-7.2-2 release. In #256, the user noted that when selected, ProtonUp-Qt will attempt to download this release and then get stuck at 99%. In my tests, this didn't happen from v2.8.0 Flatpak or from main @ b10c7da4b352fe5fa2a1e842aa845fc69aa828a6, but the user was using the AppImage which I have not tested. Instead, when the non-existent lutris-fshack-7.2-2 build was selected, ProtonUp-Qt simply downloaded 7.2-2 (https://github.com/DavidoTek/ProtonUp-Qt/issues/256#issuecomment-1591795287). As well, rs.get for the non-existent build still returned a 200 response, likely because it just fetched the actual 7.2-2 build.

So even though in my tests there was no issue with getting stuck at 99%, ProtonUp-Qt is still listing an invalid lutris-fshack-7.2-2 build. Any future Lutris-Wine builds that come out would also have this issue, as they presumably also would not have an fshack build. Right now 7.2-2 is the latest release and I am not sure if there are plans for a new one anytime soon based on Wine 8, but if such a build was released, that means it would also have this issue, and two invalid fshack releases would be displayed.

For Lutris, fshack builds are meant to be the default, as noted in the release notes for Lutris-Wine 7.2.

[Lutris-Wine 7.2] Comes in 2 variants, the plain version, and the fshack version, which is the default one

So it is not unreasonable that a user looking through a release list with this knowledge may want to select the Lutris-Wine 7.2-2 fshack build. This is also why we can't really just remove the fshack logic altogether, because for older builds, it's probably the build a user would want to pick.

Solution

The fix I went with was to check the release['assets'], which contains a list of dictionaries representing release assets. These are zips and tarballs, and do not include things like the source code zip/tar or checksums from what I can see. We loop through the release['assets'] and if we find one with lutris-fshack in its name, we add an fshack build to the list of tags and break from the loop.

This fix successfully removes the stray fshack build, and should work for future releases. To test and ensure no regressions, I downloaded Lutris-Wine 7.2-2, 7.2, fshack-7.2, and fshack-6.21 as a spot-check to ensure older releases still worked as expected. All of them downloaded without issue. The older GE-LoL builds also still display and download as expected, so in my tests I found no regressions.

image

image

Below are two examples of release assets to better show the structure of a release, which may help the approach I took make a little more sense. I have provided examples of Lutris-Wine 7.2-2, which does not contain an fshack release, and Lutris-Wine 7.2, which does include an fshack release.

Lutris-Wine 7.2-2 Release (No fshack) ```json { "url": "https://api.github.com/repos/lutris/wine/releases/70579699", "assets_url": "https://api.github.com/repos/lutris/wine/releases/70579699/assets", "upload_url": "https://uploads.github.com/repos/lutris/wine/releases/70579699/assets{?name,label}", "html_url": "https://github.com/lutris/wine/releases/tag/lutris-wine-7.2-2", "id": 70579699, "author": { "login": "tannisroot", "id": 10602045, "node_id": "MDQ6VXNlcjEwNjAyMDQ1", "avatar_url": "https://avatars.githubusercontent.com/u/10602045?v=4", "gravatar_id": "", "url": "https://api.github.com/users/tannisroot", "html_url": "https://github.com/tannisroot", "followers_url": "https://api.github.com/users/tannisroot/followers", "following_url": "https://api.github.com/users/tannisroot/following{/other_user}", "gists_url": "https://api.github.com/users/tannisroot/gists{/gist_id}", "starred_url": "https://api.github.com/users/tannisroot/starred{/owner}{/repo}", "subscriptions_url": "https://api.github.com/users/tannisroot/subscriptions", "organizations_url": "https://api.github.com/users/tannisroot/orgs", "repos_url": "https://api.github.com/users/tannisroot/repos", "events_url": "https://api.github.com/users/tannisroot/events{/privacy}", "received_events_url": "https://api.github.com/users/tannisroot/received_events", "type": "User", "site_admin": False }, "node_id": "RE_kwDOB3pf5s4ENPXz", "tag_name": "lutris-wine-7.2-2", "target_commitish": "lutris-7.2-2", "name": "Lutris Wine 7.2-2", "draft": False, "prerelease": False, "created_at": "2022-07-06T18:40:14Z", "published_at": "2022-07-06T18:42:12Z", "assets": [ { "url": "https://api.github.com/repos/lutris/wine/releases/assets/70769014", "id": 70769014, "node_id": "RA_kwDOB3pf5s4EN9l2", "name": "wine-lutris-7.2-2-x86_64.tar.xz", "label": None, "uploader": { "login": "tannisroot", "id": 10602045, "node_id": "MDQ6VXNlcjEwNjAyMDQ1", "avatar_url": "https://avatars.githubusercontent.com/u/10602045?v=4", "gravatar_id": "", "url": "https://api.github.com/users/tannisroot", "html_url": "https://github.com/tannisroot", "followers_url": "https://api.github.com/users/tannisroot/followers", "following_url": "https://api.github.com/users/tannisroot/following{/other_user}", "gists_url": "https://api.github.com/users/tannisroot/gists{/gist_id}", "starred_url": "https://api.github.com/users/tannisroot/starred{/owner}{/repo}", "subscriptions_url": "https://api.github.com/users/tannisroot/subscriptions", "organizations_url": "https://api.github.com/users/tannisroot/orgs", "repos_url": "https://api.github.com/users/tannisroot/repos", "events_url": "https://api.github.com/users/tannisroot/events{/privacy}", "received_events_url": "https://api.github.com/users/tannisroot/received_events", "type": "User", "site_admin": False }, "content_type": "application/x-xz", "state": "uploaded", "size": 288951088, "download_count": 492737, "created_at": "2022-07-06T18:37:28Z", "updated_at": "2022-07-06T18:37:44Z", "browser_download_url": "https://github.com/lutris/wine/releases/download/lutris-wine-7.2-2/wine-lutris-7.2-2-x86_64.tar.xz" } ], "tarball_url": "https://api.github.com/repos/lutris/wine/tarball/lutris-wine-7.2-2", "zipball_url": "https://api.github.com/repos/lutris/wine/zipball/lutris-wine-7.2-2", "body": "New build of Lutris Wine based on Wine Staging 7.2.\r\nIt now comes only in a single variant.\r\nThe reason for this is because you can now disable `fshack` functionality (which allows Wine to scale games running in a non-native resolution on its own) via an environment variable "WINE_DISABLE_FULLSCREEN_HACK=1", leaving the scaling up to your monitor or GPU.\r\nPreviously, to avoid using `fshack`, you had to switch to a non-fshack Wine version, which we labeled as just `lutris`. But seeing how such variants serve no purpose anymore, they are now retired, along with `fshack` naming, as there is no need to distinguish 2 different variants anymore.\r\nIn the next Lutris version, this environment variable will be moved to a dedicated option.\r\n\r\nChanges:\r\n- Add an option to disable `fshack` functionality via an environment variable "WINE_DISABLE_FULLSCREEN_HACK=1".\r\n- The fake FSR resolution option has been renamed from `WINE_FULLSCREEN_FAKE_CURRENT_RES" to `WINE_FULLSCREEN_FSR_CUSTOM_MODE`, following the naming change in GE builds.\r\n- Added "shared resources" patches that some games and apps requires when running through DXVK, notably Epic Games Overlay. FiveM doesn\"t work for some reason, however.\r\n\r\nBig thanks to @Tk-Glitch and @GloriousEggroll for creating the base for these builds.", "reactions": { "url": "https://api.github.com/repos/lutris/wine/releases/70579699/reactions", "total_count": 14, "+1": 6, "-1": 0, "laugh": 1, "hooray": 2, "confused": 0, "heart": 2, "rocket": 2, "eyes": 1 }, "mentions_count": 2 } ```
Lutris-Wine 7.2-2 (has fshack release) ```json { "url": "https: //api.github.com/repos/lutris/wine/releases/62073480", "assets_url": "https://api.github.com/repos/lutris/wine/releases/62073480/assets", "upload_url": "https://uploads.github.com/repos/lutris/wine/releases/62073480/assets{?name,label}", "html_url": "https://github.com/lutris/wine/releases/tag/lutris-wine-7.2", "id": 62073480, "author": { "login": "tannisroot", "id": 10602045, "node_id": "MDQ6VXNlcjEwNjAyMDQ1", "avatar_url": "https://avatars.githubusercontent.com/u/10602045?v=4", "gravatar_id": "", "url": "https://api.github.com/users/tannisroot", "html_url": "https://github.com/tannisroot", "followers_url": "https://api.github.com/users/tannisroot/followers", "following_url": "https://api.github.com/users/tannisroot/following{/other_user}", "gists_url": "https://api.github.com/users/tannisroot/gists{/gist_id}", "starred_url": "https://api.github.com/users/tannisroot/starred{/owner}{/repo}", "subscriptions_url": "https://api.github.com/users/tannisroot/subscriptions", "organizations_url": "https://api.github.com/users/tannisroot/orgs", "repos_url": "https://api.github.com/users/tannisroot/repos", "events_url": "https://api.github.com/users/tannisroot/events{/privacy}", "received_events_url": "https://api.github.com/users/tannisroot/received_events", "type": "User", "site_admin": False }, "node_id": "RE_kwDOB3pf5s4DsyqI", "tag_name": "lutris-wine-7.2", "target_commitish": "lutris-fshack-6.21-4", "name": "Lutris Wine 7.2", "draft": False, "prerelease": False, "created_at": "2022-03-06T21:16:05Z", "published_at": "2022-03-17T17:47:06Z", "assets": [ { "url": "https://api.github.com/repos/lutris/wine/releases/assets/59747775", "id": 59747775, "node_id": "RA_kwDOB3pf5s4Dj62_", "name": "wine-lutris-7.2-x86_64.tar.xz", "label": None, "uploader": { "login": "tannisroot", "id": 10602045, "node_id": "MDQ6VXNlcjEwNjAyMDQ1", "avatar_url": "https://avatars.githubusercontent.com/u/10602045?v=4", "gravatar_id": "", "url": "https://api.github.com/users/tannisroot", "html_url": "https://github.com/tannisroot", "followers_url": "https://api.github.com/users/tannisroot/followers", "following_url": "https://api.github.com/users/tannisroot/following{/other_user}", "gists_url": "https://api.github.com/users/tannisroot/gists{/gist_id}", "starred_url": "https://api.github.com/users/tannisroot/starred{/owner}{/repo}", "subscriptions_url": "https://api.github.com/users/tannisroot/subscriptions", "organizations_url": "https://api.github.com/users/tannisroot/orgs", "repos_url": "https://api.github.com/users/tannisroot/repos", "events_url": "https://api.github.com/users/tannisroot/events{/privacy}", "received_events_url": "https://api.github.com/users/tannisroot/received_events", "type": "User", "site_admin": False }, "content_type": "application/x-xz", "state": "uploaded", "size": 95005032, "download_count": 57215, "created_at": "2022-03-17T07:49:45Z", "updated_at": "2022-03-17T07:49:52Z", "browser_download_url": "https://github.com/lutris/wine/releases/download/lutris-wine-7.2/wine-lutris-7.2-x86_64.tar.xz" }, { "url": "https://api.github.com/repos/lutris/wine/releases/assets/59744700", "id": 59744700, "node_id": "RA_kwDOB3pf5s4Dj6G8", "name": "wine-lutris-fshack-7.2-x86_64.tar.xz", "label": None, "uploader": { "login": "tannisroot", "id": 10602045, "node_id": "MDQ6VXNlcjEwNjAyMDQ1", "avatar_url": "https://avatars.githubusercontent.com/u/10602045?v=4", "gravatar_id": "", "url": "https://api.github.com/users/tannisroot", "html_url": "https://github.com/tannisroot", "followers_url": "https://api.github.com/users/tannisroot/followers", "following_url": "https://api.github.com/users/tannisroot/following{/other_user}", "gists_url": "https://api.github.com/users/tannisroot/gists{/gist_id}", "starred_url": "https://api.github.com/users/tannisroot/starred{/owner}{/repo}", "subscriptions_url": "https://api.github.com/users/tannisroot/subscriptions", "organizations_url": "https://api.github.com/users/tannisroot/orgs", "repos_url": "https://api.github.com/users/tannisroot/repos", "events_url": "https://api.github.com/users/tannisroot/events{/privacy}", "received_events_url": "https://api.github.com/users/tannisroot/received_events", "type": "User", "site_admin": False }, "content_type": "application/x-xz", "state": "uploaded", "size": 95066400, "download_count": 213804, "created_at": "2022-03-17T07:01:22Z", "updated_at": "2022-03-17T07:01:29Z", "browser_download_url": "https://github.com/lutris/wine/releases/download/lutris-wine-7.2/wine-lutris-fshack-7.2-x86_64.tar.xz" } ], "tarball_url": "https://api.github.com/repos/lutris/wine/tarball/lutris-wine-7.2", "zipball_url": "https://api.github.com/repos/lutris/wine/zipball/lutris-wine-7.2", "body": "New build of Lutris Wine based on Wine Staging 7.2.\r\nComes in 2 variants, the plain version, and the `fshack` version, which is the default one.\r\nLearn more about fshack here: https://github.com/lutris/docs/blob/master/WineBuilds.md\r\n\r\nChanges:\r\n- Added support for making use of Proton EAC runtime.\r\n- Fixed wrong reported version.\r\n\r\nBig thanks to @Tk-Glitch and @GloriousEggroll for creating the base for these builds.", "reactions": { "url": "https://api.github.com/repos/lutris/wine/releases/62073480/reactions", "total_count": 6, "+1": 1, "-1": 0, "laugh": 1, "hooray": 1, "confused": 0, "heart": 1, "rocket": 1, "eyes": 1 }, "mentions_count": 2 } ```

Drawbacks

This fix assumes there would only ever be one fshack build per-release. In my opinion, this is almost guaranteed to be the case because fshack will likely not be distributed anymore as a separate build, since it can be toggled with the single Lutris-Wine build.

This would not apply to older releases, which still function as expected and to my knowledge never distributed more than one fshack build.

Bit of a lengthy explanation but it took a while to get the fix figured out in my head, and I wanted to make sure my approach was clear.

sonic2kk commented 1 year ago

I should note that this probably isn't a new issue, but would probably have been present since Lutris-Wine 7.2-2 released, and has only now been caught. I looked at the history of the Lutris-Wine ctmod and even though it was updated in #145, the logic was just changed from two appends to an extend, so the core logic was probably the same. The initial logic was written 2 years ago (see here), and back then there was no 7.2-2, so at the time this fshack check worked.

In other words this wasn't a breakage recently or anything, it was just a change in upstream behaviour that we didn't account for yet :-)

DavidoTek commented 1 year ago

ProtonUp-Qt will attempt to download this release and then get stuck at 99%.

That happend to me once too, wasn't able to replicate it again

The fix I went with was to check the release['assets'], which contains a list of dictionaries representing release assets

That's great. That doesn't even require any additional API calls. Also if they introduce fshack again, it will still be available

This fix assumes there would only ever be one fshack build per-release

Yeah, I don't think it would make sense to change it. They will probably just do it as in the older releases.

Thanks!

sonic2kk commented 1 year ago

That's great. That doesn't even require any additional API calls.

That was my main concern about investigating this, that it would need an extra API call and quite a large refactor. Instead we were able to re-use the release object that we already got :-)