Games List: Add checks for skipping invalid games #401
Conversation
Some games in the Lutris `pga.db` are missing data, in most cases this is a result of a game that we don't want to display on the games list. Add a method to the Games List dialog that will check for and skip games missing certain pieces of info, such as the game runner and install_dir. When querying the Lutris DB, it appears that only not-installed Steam games were missing a runner. As a result they were not being skipped by the Steam runner skip check, and we were getting a whole bunch of errors for trying to do operations on NoneType. Valid games for the Games List should have a runner and an install_dir if they are actually installed. So we skip if they don't meet this criteria, as well as continuing to skip if a game is a Steam game. We also add some more None-safe logic to our installed_at parsing. This also probably will never be missing but there is no harm in accounting for it being missing.
|
Thanks!
That would be a good addition if Lutris adds that. Good you kept that in mind! 😄
Makes sense. Good, we should be safe then in case that ever happens.
Okay. It's probably the simplest to introduce such changes gradually as you did. @DavidoTek Note to myself: Remember to include a note about the updated Python requirement in the release notes.
Looks good. I tested it with my (small) Lutris game library and everthing was displayed as expected. The code looks good as well. Off topic: I'm wondering, did Lutris Flatpak change it's config location? I got an error and noticed that my lutris config is not stored in Strange. According to |
|
Thanks!! :-)
It's using It changes Some Lutris Flatpak issues (flathub/net.lutris.Lutris#422) are also pointing to At least, this is what I'm getting from looking at the code, but I might be missing something (I'm overdue an iced latte to wake to get my brain in gear 😆) |
Oh, not sure how I missed that. I should have ready two more lines of code :)
Can you check if a ProtonUp-Qt/pupgui2/datastructures.py Lines 178 to 185 in fcf0eaa I tried reinstalling Lutris. Patching the path in - {.... 'config_dir': '~/.var/app/net.lutris.Lutris/config/lutris'},
+ {.... 'config_dir': '~/.var/app/net.lutris.Lutris/data/lutris'}, |
I checked and I only have a Perhaps we should, for the time being, add support for both directories, as This will likely also apply to the non-Flatpak Lutris installations as well (package manager, or AppImage if one exists as well). So we'd need to add whatever the XDG data path is as well. |
Fixes #397.
Overview
This PR fixes a crash in the Lutris Games List if some games have data as
Nonethat we expect to be defined.I covered this a lot in #397 but to reiterate briefly: When loading the games list, we get our list of
LutrisGames withlutrisutil#get_lutris_game_list. We filter out Steam games from this list. Then we extract data from eachLutrisGameand display it on the UI; the name, runner, install directory, and installation time, as well as some other data for tooltips.However some games can be missing this information. Lutris stores information about its games in an sqlite3 database,
pga.db, which we query. Some games in this DB may be missing a lot of information though. The example I encountered this with was by syncing my Lutris Steam list. It stored information about all my Steam games including ones that weren't installed, and the ones not installed had lots of empty information including an empty runner.This missing runner meant the check we do to skip Lutris games gotten from Steam with
lutrisutil#is_lutris_game_using_runnerwere not catching these games. Since the runner wasNoneand not'steam'then it was missing this.Solution
The fix I ended up going with was to create a method,
PupguiGameListDialog#is_valid_lutris_gameslist_game. We then use this in our list comprehension when storing our list of Lutris games.Stricter checking with
is_valid_lutris_gameslist_gameThe purpose of this function is to check if a
LutrisGameis valid to be displayed on the Games List. In 99.9999% of cases, this is essentially just ensuring a game is installed, as an entry in Lutris' DB does not guarantee that a game is installed. We have to try an infer this (this is even a problem in Lutris itself, where many a missing game may not always be marked as missing, and will simply fail to launch). Lutris does have a column in its DB to track if a game is installed but this is not very reliable from what I have observed from querying, and from what I recall in the past, which is why we don't simply check this.So now we have
is_valid_lutris_gameslist_gamethat will essentially perform a stricter check to make sure games we want to display on the Games List have all the data we need. Instead of working around withNonechecks all over the place, we can assume early when generating theself.gameslist, that if a game is missing arunnerand aninstall_dir, and if the runner is'steam', then we don't want to display it on the games list.A small benefit to this method is that if later on we decide we want other criteria, for example if Lutris started displaying Bottles or Heroic games for some reason, we can exclude those with an additional runner check. This keeps the list comprehension cleaner and also using this method name should make it a bit more obvious why the conditional check is in place.
Safer Logic for Parsing
LutrisGame#installed_atDespite what I said above, I decided that a safer check for
installed_atwasn't out of the question. Based on my extensive querying of my own Lutris DB in #397 (comment), a game that is installed and valid for our purposes (in other words, a game that passesis_valid_lutris_gameslist_game) should always haveinstalled_at. However in case this is ever missing, it is a less important factor in determining if a game is valid, so I kept it out ofis_valid_lutris_gameslist_gameand instead did a slight refactor of our logic for setting this.Instead of setting all the data when setting up the
QTableWidgetItem, we instead store the data in variables. Then we set the variables to different values based on whetherinstalled_atis Truthy (notNoneand not an empty string). If we have aninstalled_atthen we set it up as before. Otherwise, we set the text toself.tr('Unknown'), set the tooltip to "Install Date is Unknown", and then set the data to0. The data is used for sorting to know how to sort by a given column, so we simply set to0. here.After this check we set up the
QTableWidgetItemas before but we give it the variables defined above. This keeps our conditional logic out of the widget item setup at the expense of some extra lines.Minor Type Hinting Refactor
We recently updated our base Python version to 3.10, meaning we can finally start to use the type hinting in Python3.10 without all the imports or capitalizations (https://docs.python.org/3.10/library/typing.html). Just because I was working in this file I decided to update our type hintings in
pupgui2gamelistdialog.pyto use things likelistandtupleinstead ofListandTuple, meaning we can also remove those imports as well.We couldn't replace the
Callabletype hint, but you win some, you lose some 😄In my testing this isn't missing any games in the Lutris Games List, all the games I would expect to be there are present (there are games I know is missing, but they were displayed before, and Lutris also does not display them as missing, but the files do not exist anymore).
Let me know if you have any concerns about the implementation here or if we need to do any additional work in the
is_valid_lutris_gameslist_gamecheck. I know we had some discussion in #397 already but sometimes it's easier to look at an implementation and realise there are problems 😅Thanks!