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

Lists Nonexistent lutris-fshack-wine-7.2-2 and gets stuck extracting it at 99% #256

Closed FOSSProponent9436 closed 1 year ago

FOSSProponent9436 commented 1 year ago

Describe the bug
The two lutris wine variants have been replaced with one: https://github.com/lutris/wine/releases/tag/lutris-wine-7.2-2

Yet lutris-fshack-wine-7.2-2 is still in proton up qt even though it doesn't exist to my knowledge (but doesn't install successfully).

To Reproduce
Steps to reproduce the behavior:

  1. Launch ProtonUp-QT
  2. Select lutris in install for:
  3. Press add version
  4. Select Lutris wine in compatibility tool dropdown
  5. Select lutris-fshack-wine-7.2-2 in version dropdown
  6. Attempt to install
  7. Stuck at 99% extracting

Expected behavior
Lutris Fshack v7.2-2 should not be listed because it does not exist to my knowledge. Even so, it shouldn't get stuck at 99% when trying to install it.

Screenshots
Lutris FSHack

Desktop (please complete the following information):

Additional context

New build of Lutris Wine based on Wine Staging 7.2. It now comes only in a single variant. The 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. Previously, 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. In the next Lutris version, this environment variable will be moved to a dedicated option.

Terminal output
Nothing additional outputs when attempting to install, even when it is stuck at 99%

sonic2kk commented 1 year ago

The fshack build does indeed not exist for Lutris-Wine 7.2.2. It seems ProtonUp-Qt always assumes there is an fshack build available, as when building the version list it doesn't actually check for an fshack build but instead just adds the regular builds based on the release tags and then adds an fshack build:

https://github.com/DavidoTek/ProtonUp-Qt/blob/b10c7da4b352fe5fa2a1e842aa845fc69aa828a6/pupgui2/resources/ctmods/ctmod_lutriswine.py#L127-L129

I imagine it's done this way because fshack builds are just listed as separate release assets and they are not separate tagged releases, so ProtonUp-Qt just assumes there will always be an accompanying fshack build.

This seems tricky. Fixing this cleanly might require actually checking the release assets, which would be a bit of a pain when it comes to api calls...

Even so, it shouldn't get stuck at 99% when trying to install it.

When a build doesn't exist or can't be found, I guess in general ProtonUp-Qt should return an error. Cases like this one shouldn't happen in the first place, but having better catches in place for this is not a bad idea.

FOSSProponent9436 commented 1 year ago

The fshack build does indeed not exist for Lutris-Wine 7.2.2. It seems ProtonUp-Qt always assumes there is an fshack build available, as when building the version list it doesn't actually check for an fshack build but instead just adds the regular builds based on the release tags and then adds an fshack build:

https://github.com/DavidoTek/ProtonUp-Qt/blob/b10c7da4b352fe5fa2a1e842aa845fc69aa828a6/pupgui2/resources/ctmods/ctmod_lutriswine.py#L127-L129

I imagine it's done this way because fshack builds are just listed as separate release assets and they are not separate tagged releases, so ProtonUp-Qt just assumes there will always be an accompanying fshack build.

This seems tricky. Fixing this cleanly might require actually checking the release assets, which would be a bit of a pain when it comes to api calls...

Even so, it shouldn't get stuck at 99% when trying to install it.

When a build doesn't exist or can't be found, I guess in general ProtonUp-Qt should return an error. Cases like this one shouldn't happen in the first place, but having better catches in place for this is not a bad idea.

What if you used a hack like only listing fshack if version is less than 7.2-2? would that be possible

sonic2kk commented 1 year ago

It would, but as you mentioned, it's a hack. Ideally we'd want a more robust way to check this - Possibly by checking the release assets.

It would probably also be a good idea for us to not download builds that don't exist (maybe this comes back from some response with a 404 or something?) and at least log that the build didn't exist. Better handling of this kind of issue would feed in a little bit to the discussion in #247.

sonic2kk commented 1 year ago

Actually, I just tested on v2.8.0 Flatpak and on latest main and ProtonUp-Qt seems to download the regular Lutris-Wine 7.2-2 build and correctly extracts it for me. Maybe however it gets the release from __fetch_github_data is "handling" this. I noticed because I checked what rs.get was returning for the fshack download, and it was returning a 200 response. The archive downloaded to /tmp/pupgui2.<blah> is wine-lutris-7.2-2-x86_64.tar.xz.

I have not yet tried the AppImage, perhaps for some reason this issue is exclusive to the AppImage but I am unsure.

So the priority to resolve this would be to ensure we don't list builds that don't actually exist in the first place.

sonic2kk commented 1 year ago

Our release dictionary does have assets tied to it, which we can check the name key of to see if it has -fshack. Investigating a solution.

sonic2kk commented 1 year ago

Got a PR up to fix this issue mostly. It doesn't fix the extraction getting stuck at 99% (I couldn't replicate that problem, sorry) but it should fix the invalid 7.2-2 fshack build showing up, and should also fix the issue for any future Lutris-Wine builds, such as a build based on Wine 8 which would presumably not include an fshack build since it can be toggled now with an environment variable.

DavidoTek commented 1 year ago

The two lutris wine variants have been replaced with one: https://github.com/lutris/wine/releases/tag/lutris-wine-7.2-2

Interesting...

What if you used a hack like only listing fshack if version is less than 7.2-2?

That seems like the simplest solution. I don't think all versions follow the same scheme though, e.g. lutris-ge-lol-6.16-4

Ideally we'd want a more robust way to check this - Possibly by checking the release assets.

Yeah, that would be the best. I wonder how many API calls that would be for every version in the list.

There are two additional alternatives I could think of:

  1. If the user selects fshack, try to download it. If it doesn't exist, just download the normal version. Maybe show a dialog with the info which one was installed
  2. Just remove fshack altogether. I'm not sure though how important older versions of Lutris Wine are.
  3. A combination of 1. and 2.: Hide fshack behind advanced mode. If a user tries to download a new version without fshack, just download the normal version.

Still, it seems like just checking if the release is newer than version 7.2-2 / date x.y.z might be the easiest.