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

steamutil: Force SteamApp game_name to string #439

Merged
merged 3 commits into from
Aug 10, 2024

Conversation

sonic2kk
Copy link
Contributor

@sonic2kk sonic2kk commented Aug 2, 2024

Overview

This PR casts our a.game_name assignment in steamutil#update_steamapp_info to always return a string. This fixes an error coming back Error updating SteamApp info from appinfo.vdf: argument of type 'int' is not iterable when a.game_name is not a string. With the new appinfo.vdf format, it seems name can be stored as an integer in appinfo.vdf now if the game's name in the library only contains numbers.

If we try to parse this game's name, it'll be stored as an integer, and we will run into the mentioned error when we try to perform string operations on it. The one causing the error mentioned is our elif 'Steamworks' in a.game_name check inside of the update_steamapp_info function.

The error does not present appinfo.vdf from being parsed and only the problematic game that has its a.game_name as an integer gets skipped. This causes the game to be missing from our apps list. With this PR we force a.game_name's assignment to be a string, which allows ProtonUp-Qt to parse the games properly.

This PR also makes a couple of type-hinting improvements, including removing the deprecated List type from this function.

To emphasise, I don't think this is a regression on our part of a problem caused by the steam or vdf dependency, I think this is a change on Valve's side.

main @ f2573d5

image

This PR

image

Background

I noticed when working on a PR that I was seeing a new error on ProtonUp-Qt startup: Error updating SteamApp info from appinfo.vdf: argument of type 'int' is not iterable. I tracked this back to being present since the steam and vdf dependency update (e8197bc). I can't test before that because parsing appinfo.vdf doesn't work at all.

After troubleshooting I tracked this error down the a.game_name assignment. When we try to assign a.game_name in steamutil#update_steamapp_info, our app_appinfo_common.get('name', '') assignment to retrieve the game name from appinfo.vdf is not guaranteed to return a string. This causes us to hit an exception when we try to check if an app in the loop is a Steamworks app with 'if Steamworks' in a.game_name check which is an Iterative operation. So if a.game_name = 21, then we'll hit this exception.

The case where app_appinfo_common.get('name', '') is not guaranteed to return a string seems to be new. This was not happening in the old appinfo.vdf format and seems to be a result of the Steam appinfo.vdf format changing.

Games that can reproduce this problem are ones which only have numbers as their title in the library, such as Twenty One which is named "Twenty One" on the store page, but the actual app name in the library is "21". The name of this app is 21 in appinfo.vdf, inspecting the value that ProtonUp-Qt gets with a debugger.

Solution

The solution is just to wrap our app_appinfo_common.get in a str() cast.

We will pretty much always expect the SteamApp game_name to be a string, I don't think it makes sense to account for it as an integer. For our purposes we will always want it to be represented as a string. Even though the appinfo.vdf does store the name as an integer, it used to store it as a string, which is why this problem never came up before. I have owned the game that caused this bug and had it installed for a while, and it previously showed up in ProtonUp-Qt before the appinfo.vdf format change. This could be an oversight by Valve or an intentional change, but either way I think casting to a string allows us to be on the safe side 😄


Not much else to say on this one! appinfo.vdf can store game names as integers now and we weren't accounting for it, causing an exception and resulting in the game being missing from our app list, so the game would not be displayed in ProtonUp-Qt. But I wanted to give background because this is an edge-case. It is rare for games that people are likely to have installed, play often, and care about and so would want to interact with in ProtonUp-Qt that would only have numbers in their name. Even from searching SteamDB for a bit I only found 1 (pun intended) and it isn't even available anymore.

Thanks!

P.S. - Ironically, Twenty One, the game that caused the issue for me, does not work and has never worked with Proton 😄

@sonic2kk
Copy link
Contributor Author

sonic2kk commented Aug 2, 2024

On the note of testing; We'd probably want some kind of static appinfo.vdf in our test folder to parse that would be stripped down and allow us to read it in and test other operations that rely on this (getting the Steam compatibility tool list comes to mind). It's probably too much for this PR which is why I skipped it, but something for the future!

@DavidoTek
Copy link
Owner

Thanks!

it seems name can be stored as an integer in appinfo.vdf now if the game's name in the library only contains numbers
I think this is a change on Valve's side.

Interesting. I wonder why they made that decision. The memory savings of a few bytes is nice, but it needs to be converted to a string anyway for displying it. I don't think the 1 byte saving in case of 21 is worth the trouble 😄

We will pretty much always expect the SteamApp game_name to be a string, I don't think it makes sense to account for it as an integer.
This could be an oversight by Valve or an intentional change, but either way I think casting to a string allows us to be on the safe side 😄

Okay. I haven't yet encountered this problem (I don't think I own a game which has a numeric name), but it seems to be a good idea to be on the safe side. I agree that we should only store strings in game_name, allowing for other data types would make it too complex.

P.S. - Ironically, Twenty One, the game that caused the issue for me, does not work and has never worked with Proton 😄

Still good you noticed that issue!

On the note of testing; We'd probably want some kind of static appinfo.vdf in our test folder to parse that would be stripped down and allow us to read it in and test other operations that rely on this (getting the Steam compatibility tool list comes to mind). It's probably too much for this PR which is why I skipped it, but something for the future!

That's a good idea. Ideally, we have one of each vdf revision.

@DavidoTek DavidoTek merged commit ec242d6 into DavidoTek:main Aug 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants