Closed sonic2kk closed 5 months ago
Thanks! :tada:
Sorry for the late response, I put it off over the new year. Decided it is time now after 1 month to do the review :smile:
Return the list of games in use by a Steam Runtime BasicCompatTool, meaning we can populate the games list in the CtInfo dialog Set the ct.no_games on the main menu, allowing us to accurately mark a runtime as used/unused.
That is very nice. Good to know that the runtime information is in extended.additional_dependencies
for each game.
This may have an impact on startup times
I took a look at this. Please note that I have only ~100 games and my processor is quite recent.
for dep in app_additional_dependencies.values():
(steamutil.py) should only be a few iterations, if any at all. The for loop is invoked for every game, that could bring some overhead but should be okay.
[game for game in steam_app_list if ctool_is_runtime_for_app(game, ct)]
(pupgui2.py) seems to be the more work intensive part as it will loop through every game and do the checks, regardless if it using a runtime or not. It is only run once per runtime.
I changed this function to take an actual BasicCompatTool object, from which we can get the tools internal name, and also this means we can pass the actual compat tool object to ctool_is_runtime_for_app
Ah, that's clever.
In update_steamapp_info, perform some Steam Linux Runtime/anticheat runtime checks using constants instead of magic AppIDs
Good, that also makes it more clear what we are actually doing.
Create a new Enum to track the runtime type
That's nice. That now also allows us to use RuntimeType
as a type hint for function parameters.
Testing
I did some rudimentary testing using my somewhat limited Steam library, it didn't seem to break anything. On the code-review side, it looks very good.
Some Dictionary Weirdness
That could be related to the fact that the initial state is actually a static variable. Could probably be fixed using the @dataclass
decorator for SteamApp
.
Overall Implementation
Actually, it seems fine to me. There are probably design decisions that could have been made differently, but I haven't noticed anything wrong with the implementation.
Future Work
That would be interesting to know.
No worries about the delay, I had a bunch of time and motivation over the December period but of course December+January is a hectic time. Thank you as always for your review! In particular:
That could be related to the fact that the initial state is actually a static variable. Could probably be fixed using the @dataclass decorator for SteamApp.
This part was bugging me for a while, so thank you for clearing it up :-)
FYI, I just discovered in v2.9.1, Anti-Cheat Runtimes do not set their game count correctly, so the game count is blank, despite correctly displaying the "No games" text.
This PR resolves the blank count issue that is present in v2.9.1, since we now can get a game list for Anti-Cheat Runtimes to get a count of. Just in case you or someone else notices it, but then notices it's fixed on main, just clarifying that this PR is what fixed it :-)
Supersedes #186. Implements #190.
Overview
This PR will display the games in use by a given anti-cheat runtime on its CtInfo dialog, and will correctly mark those tools as used/unused on the main menu.
(Note: The padding issues here are fixed in #329)
It also fixes a minor UI issue on the CtInfo dialog where the game list column headings and the total games used by a compatibility tool were not set if the compatibility tool type was not
CTType.CUSTOM
, allowing us to properly display the number of games on the label for anti-cheat runtimes and the correct column names.Double-clicking on an item in the games list to open the Steam Store page functionality still works here, since the games list is populated the same way as before, but
get_steam_game_list
can now actually return a filtered list for anti-cheat runtimes.Implementation
(See #186 and #190 for historical discussion on how Steam tracks per-game Proton anti-cheat runtime information.)
The core changes here were updating
SteamApp
s to be aware of what anti-cheat runtimes they're using, and updatingget_steam_game_list
(which is used to optionally filter a games list by games using a specific compatibility tool) to be able to filter on anti-cheat runtimes.This PR implements logic to detect which games are using which anti-cheat runtimes, by tracking it as a dictionary on the
SteamApp
object. We set this inupdate_steamapp_info
, where we check theadditional_dependencies
inappinfo.vdf
to see if a game is using the Proton EasyAntiCheat/BattlEye Runtime based on their AppIDs.Since these runtimes are not actually compatibility tools as such (and since most compatibility tools will not have AppIDs like runtimes do), we have to associate them differently. The approach I chose was to create a helper function,
ctool_is_runtime_for_app
. This takes a SteamApp and a BasicCompatTool, and checks if the internal name of the compatibility tool matches a compatibility tool in use by that SteamApp. So if a SteamApp is using EAC, and the internal name of the compatibility tool containseasyanticheat
(and if that compatibility tool type is a Steam Runtime, since we only want to match Steam Runtimes and don't want to risk matching names of other compat tool types), then we return true. This allows us to do two main things:ct.no_games
on the main menu, allowing us to accurately mark a runtime as used/unused.get_steam_game_list
(we don't make any extra calls) and filter on the result of this call, the only performance cost would be that of the list comprehension, which hopefully isn't too significant. On my PC running from source, I can't feel any performance hit, but I also have not measured to confirm.To allow getting a games list using
ctool_is_runtime_for_app
, we add this to our optional filtering inget_steam_game_list
. This function returns a list of all Steam apps filtered to be of typegame
, and can optionally filter based on which compatibility tool a game is using (i.e. return all games using GE-Proton8-25). This is what we use to populate the games list on the CtInfo dialog. However, we were filtering on the tool internal name. To allow more flexible checks, I changed this function to take an actualBasicCompatTool
object, from which we can get the tools internal name, and also this means we can pass the actual compat tool object toctool_is_runtime_for_app
. Doing this allows us to check the compat tool type.When these runtimes are marked as a dependency, it is distinct from when a tool will simply use EAC/battlEye. We use the dependency AppID to ensure we're tracking games that have specifically marked that they want the Proton anticheat runtime, meaning the developer has explicitly stated that they want to download the Proton runtime and not just general EAC/battlEye. So we aren't going to list all games which use EAC/battlEye, only the ones that have marked themselves as wanting the Proton runtime.
However, this does not mean all games are compatible. A game can say that yes, they want to download the runtime as a dependency when the game launches, but there is no guarantee that they will have implemented it correctly in-game. This
additional_dependency
simply means the game will tell Steam that the anticheat runtime will have to be downloaded before the game starts as it wants to use it. Whether it uses it correctly or even at all, that's not ProtonUp-Qt's problem :-)Finally, a couple of smaller refactors were made and implementation details as well. Mainly:
update_steamapp_info
, perform some Steam Linux Runtime/anticheat runtime checks using constants instead of magic AppIDsanticheat_runtimes
dictionary keys. This means if we want to track other runtimes in future (or other anti-cheat runtimes if they become available) we have a clean, extensible way of doing it.Testing
I strongly avoid buying games which use invasive anti-cheats like EAC and battlEye, and games which I do own that use it are unfortunate accidents or games that allow you to disable it (such as Halo MCC). Therefore my testing of this PR has been somewhat limited. However with the small handful of games I was able to test with did work, and since the battlEye runtime is still correctly displayed as unused, I believe this PR should work as intended :-)
Concerns
Some Dictionary Weirdness
For some reason, even though
SteamApp.anticheat_runtimes
should be initialised with both runtimes asFalse
, EAC was always marked asTrue
(but not battlEye). To fix it, I had to update it for the app inupdate_steamapp_info
, by settingapp.anticheat_runtimes = { RuntimeType.EAC: False, RuntimeType.BATTLEYE: False }
again. I don't know why I have to do this, since the other properties seem to work fine...Overall Implementation
This is kinda strange, but I don't feel like this is quite implemented in the best way? For some reason I have a feeling that something has to be different here, but I can't really figure out what. I slept on it before making this PR and couldn't come up with anything. So if you have any concerns about how this is implemented we can discuss better ways of doing it. Even though I feel like there should be, I don't know what that is or why I feel that way. I was a bit hesitant to open this PR, which is why I didn't open it yesterday when I closed #186.
Future Work
Lutris and Heroic allow you to toggle the EasyAntiCheat and BattlEye runtimes in their settings, but I have no idea how this works. You can toggle this globally or per-game as well. I think this is probably out-of-scope for this PR and what #190 was going for, these changes are specific to detecting which Steam games are using anti-cheat runtimes.
If Lutris/Heroic download these runtimes, we could try to detect and display those on a per-launcher basis and implement something like this PR for those launchers. But for now, this change is specific to Steam.
Phew, well, several months later, seems we finally made it, and at least have some kind of working proof-of-concept for this feature :partying_face:
Thanks!