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.24k stars 40 forks source link

Bump steam library version #157

Closed sonic2kk closed 1 year ago

sonic2kk commented 1 year ago

Fixes #155.

The steam library for Python was updated yesterday to address an issue with the parse_appinfo function. The format of this changed on Steam's side and so the library had to be updated to accommodate this following commit https://github.com/ValvePython/steam/commit/abf65abff15385deb8ba05adaf4e312254fb435c.

Honestly I don't really understand what the changes here did but I can confirm it fixes the issue for me when running from source. Also from my armchair glance at the commit this should not have any breakages on Flatpak or require any sort of Python version bump. It is a fix but it doesn't seem to do anything too "magical" or extravegant that might break compatibility anywhere. My concern here is bumping from a minimum of 1.1.0 to a minimum of 1.4.4. In testing from source I could not see anything that has broken (yet, anyway) but I'm also not sure what the previous version of the library I was using was. I would guess that this is probably fine but the changes from 1.1.0 to 1.4.4 may want to be reviewed with more scrutiny first.

Currently the requirements.txt mandate steam>=1.1.0, however since this new version of the steam library fixes a critical bug, I set the version as >=1.4.4, forcing this newer version since without it, various pieces of functionality will not work without this version or higher.

Opening this as a PR for transparency on the research process for this fix, as well as discussion. Merging a 3 character change on its own is rarely worth a PR but the commentary around the change might be worth it :-)

Thanks!

DavidoTek commented 1 year ago

Thanks!

Looks like they changed the beginning "magic" of the file for some reason and the library wasn't able to detect the file as a vdf. Not sure why they would do this, maybe this is a new version. Looks like they changed 'DV\\x07 to (DV\\x07 and ( comes after ' in the ASCII table after all.

My concern here is bumping from a minimum of 1.1.0 to a minimum of 1.4.4.

It was using 1.3.0 before, 1.4.4 should be fine. I changed it to 1.1.0 as it ~is~was the minimum version required.

Opening this as a PR for transparency on the research process for this fix, as well as discussion. Merging a 3 character change on its own is rarely worth a PR but the commentary around the change might be worth it :-)

I think having a PR for discussions is the way to go. As you said, it will make the development of ProtonUp-Qt more transparent and as a side effect also provides an information base useful for other FOSS projects.

DavidoTek commented 1 year ago

Bumped the library version in a test Flatpak build: flatpak install --user https://dl.flathub.org/build-repo/123629/net.davidotek.pupgui2.flatpakref

Will merge soon and create a new release.