Lutris Games List Crashes if Game Has No Runner, Install Location, or Installation Time
sonic2kk opened this issue · 3 comments
Describe the bug
What the problem is: When viewing the Games List for Lutris, if there is a game with no runner, it will crash because we attempt to run capitalize() on the runner name to display it. Even if we try to skip games that have game.runner == None or infer a game's runner based on the Lutris config file (if a Lutris config has game.appid then we can assume it's a Steam game), we run into problems where we're missing other information like game.install_dir and game.installed_at.
What causes the problem: Valid games should always have a runner, but Lutris will store uninstalled Steam games in its sqlite DB. So if you link your Steam library to Lutris, uninstalled Steam games will be there but they will be missing a runner, install_dir, installed_at, and I believe a slug.
When did this start: I am not sure if Lutris always stored Steam games like this. Attempting to sync my games list always caused Lutris to crash, but at the same time I don't remember it missing any games. So I think this might be new behaviour that it stores these games in its DB.
Possible solutions:
- One solution might be to filter the games to show on the games list even further. If a game doesn't have a runner, and install location, OR an install time, then we can skip them. My justification for skipping them is that games missing all of this information are most probably games we don't want to display anyway, as they are likely not installed (like in this case). We can make a method in
PupguiGameListDialogto do this, and we can call it when we're generating the Lutris game list:
ProtonUp-Qt/pupgui2/pupgui2gamelistdialog.py
Line 183 in db33194
In this case I have found the issue of all these missing fields only being applicable for not-installed Steam games that are in my library, but it could theoretically happen for any other game using any runner. That said, in my head it's equally valid to assume any game from any runner is invalid if it's missing all of this data (invalid meaning, not relevant to display on the Game List). - Another solution is to be more "None-safe" and display labels missing this data as "Unknown", but I am slightly less in favour of this. If a game is missing so much information, like runner and install location, it might be better to assume that it is either invalid or not installed. In other words, in Solution 1 we filter out games missing all this data, which means we effectively consider games missing all this data as not valid to display on the Games List. Sure, we could just set the labels as "Unknown", but I don't know. I think at least right now we're better off simply skipping these games, and we could revisit this if users start reporting that games which they expect to be present in the Games List are missing.
I have already prototyped a patch based on Solution 1, creating a PupguiGameListDialog#is_valid_lutris_gameslist_game method that returns False if game.runner, game.install_dir, OR game.installed_at are Falsey, as well as if the current runner is steam. In my mind, based on the issue as I have encountered it now, a game missing a runner shouldn't be displayed on the Games List. Then, if a game doesn't have a known install location, we probably want to skip it too. Installation time is one I'm a bit on the fence about. if it's missing it'll cause a crash right now but we could easily fix this by doing a check for game.installed_at == None and displaying "Unknown" on this column (and skipping the tooltip building logic). We already do a check to display the install_dir as "Unknown" if it's Falsey but maybe it's best to skip it altogether. Implementation detail is probably something we can discuss separately 😄
I'm opening this issue to get a bit of discussion going on the problem, to make sure I've put across what the problem is properly as it took me a little while to figure out the issue, and also to discuss the solution in case there's a better approach.
But regardless of where we land I'm happy to be the one to tackle this :-)
In short, uninstalled Steam games can be in the Lutris DB file, and these are missing a bunch of information. Even though we filter out Steam games, they're missing a runner to do this filtering. We can make some assumptions to add this runner, but even then these games are missing a lot of information. And while this issue I have found is isolated to not-installed Steam games being missing this data, games from other runners could also be missing this information for any other reason, but it is likely just as relevant to ignore those games. If any game is missing this data we probably don't want to display it on the Games List, it just happens that for the purposes of this issue, I have only found this with uninstalled Steam games.
To Reproduce
- Take note of the Lutris sqlite DB content and size first (i.e. at
~/.local/share/lutris/pga.db). For context, mine was around 1mb. - Connect your Steam Account to Lutris
- Hover over the "Steam" source on the left and click on the "Refresh" button that appears
- Sync can take a while, if Lutris crashes, try again
- Inspect the Lutris sqlite DB (i.e. at
~/.local/share/lutris/pga.db). My DB size increased by around 1.2mb, and now includes uninstalled Steam games - Open ProtonUp-Qt and try to open the Games List
- There will be a crash for trying to run
.capitalize()on
Expected behavior
Avoid this crash by handling games that are missing this data.
Screenshots
The following backtrace is shown in the error dialog:
Desktop (please complete the following information):
- Platform: PC
- System: Arch Linux
- Version: db33194
- How did you install ProtonUp-Qt?: Running from source with
python3 -m pupgui2
Additional context
Not sure if this affects the CtInfo dialog. I think it probably wouldn't since it should rely on a runner being present in the first place and would skip if no runner is present? At least I haven't ran into any issues with the CtInfo dialog.
Terminal output
The following backtrace is printed, and also shows up in the error dialog.
Traceback (most recent call last):
File "/home/emma/Programming/ProtonUp-Qt/pupgui2/pupgui2.py", line 362, in btn_show_game_list_clicked
gl_dialog = PupguiGameListDialog(install_directory(), self.ui)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/emma/Programming/ProtonUp-Qt/pupgui2/pupgui2gamelistdialog.py", line 41, in __init__
self.setup_ui()
File "/home/emma/Programming/ProtonUp-Qt/pupgui2/pupgui2gamelistdialog.py", line 53, in setup_ui
self.setup_lutris_list_ui()
File "/home/emma/Programming/ProtonUp-Qt/pupgui2/pupgui2gamelistdialog.py", line 94, in setup_lutris_list_ui
self.update_game_list_lutris()
File "/home/emma/Programming/ProtonUp-Qt/pupgui2/pupgui2gamelistdialog.py", line 196, in update_game_list_lutris
runner_item = QTableWidgetItem(game.runner.capitalize())
^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'capitalize'I noticed I left a comment in in PupguiGameListDialog#update_game_list_lutris where I said some Lutris games may be missing an install directory. It seems like it also already has an game.install_dir or self.tr('Unknown') check, too:
https://github.com/davidotek/ProtonUp-Qt/blob/main/pupgui2/pupgui2gamelistdialog.py#L215
So maybe the only other problematic element was installed_at, which if it is None, will fall over here:
https://github.com/davidotek/ProtonUp-Qt/blob/main/pupgui2/pupgui2gamelistdialog.py#L219-L226
It falls over when trying to perform all of those date operations and splits on None, and would probably crash on the int cast as well (int(None) is not valid).
So we could go with just skipping of game.runner is Falsey (None or and empty string), and then doing a None-safe appraoch to handle the dates by making it all Unknown (and giving the item data a value of 0; item data is needed for sorting).
Or we could remove the self.tr('Unknown') for the install dir, and filter out games that are missing a runner, an install location, or install time.
So if you link your Steam library to Lutris, uninstalled Steam games will be there but they will be missing a runner, install_dir, installed_at, and I believe a slug.
We should always ensure that the variables in LutrisGame (and all other types for that matter) have the correct data type or expect None (i.e. runner: Union[str|None]).
I guess Python could take a lession from Rust and introduce a proper "Option" type any dynamic typing. That would eliminate this Null/None problem. We had that multiple times now 😄
One solution might be to filter the games to show on the games list even further. [...] My justification for skipping them is that games missing all of this information are most probably games we don't want to display anyway, as they are likely not installed
I noticed I left a comment in in PupguiGameListDialog#update_game_list_lutris where I said some Lutris games may be missing an install directory. It seems like it also already has an game.install_dir or self.tr('Unknown') check, too:
If any game is missing this data we probably don't want to display it on the Games List, it just happens that for the purposes of this issue, I have only found this with uninstalled Steam games.
Sounds reasonable. We probably should also filter out games with no install directory then.
If a game is actually installed and properly configured, it should have all of the properties. Not 100% sure about slug and installed_at though, is it possible to manually add a game without them?
I guess Python could take a lession from Rust and introduce a proper "Option" type any dynamic typing.
Funny, I've been looking into Rust the last couple of days out of interest 😄
Sounds reasonable. We probably should also filter out games with no install directory then.
If a game is actually installed and properly configured, it should have all of the properties
Yup, that's what I was trying to get at but put more clearly. At least that's my assumption, a "correct" game or at least correct in terms of being valid to display on the Games List, should contain at minimum a runner and an install_dir. A Lutris game that is actually installed should not be missing an install_dir because even if it is missing that in the sqlite db, we try to infer it.
Past sonic2kk noted in lutrisutil that a manually-added game can be missing an install_dir as this is usually only set for games added with an installer script. But ProtonUp-Qt infers it from either a Lutris config file's working_dir property, or the EXE base dir.
If it is still missing after this, we set install_dir to an empty string.
Relevant part of the code:
ProtonUp-Qt/pupgui2/lutrisutil.py
Lines 34 to 46 in 0eecd90
Not 100% sure about
slugandinstalled_atthough, is it possible to manually add a game without them?
Games cannot be missing a slug as far as I can tell. At least, zero games in my Lutris DB are missing a slug. They can be missing an installer_slug (the slug for the installer script, i.e. World of Warcraft might have something like wow-bnet-installer but the game slug would be world-of-warcraft -- Entirely made up example btw, no idea if these are the actual slugs).
I did a bit of digging on installed_at and it appears that any valid installed game, including manually added ones, should always have an installed_at.
I did a quick-and-dirty query of the DB: SELECT * FROM games WHERE runner NOT NULL AND runner != 'steam' AND runner != '' and all these games had an installed_at.
Games with installed_at IS NULL were exclusively games with either the steam runner (which we don't display on the Lutris Games List), a NULL runner (seems to be all not-installed Steam games as per this issue), or two games with an empty string runner (notably, neither of these games are installed, so they are also invalid).
I used the following query to check information about games with no installed_at: SELECT name, runner, installed_at FROM games WHERE installed_at IS NULL - Two empty runners, a number of steam runners, but the vast majority were NULL runners.
So it seems to me that, excluding Steam games (which we don't display on the Games List), installed_at should always be set if a game is actually installed and has a valid runner. In other words all games that meet our criteria to display on the Games List. When a game has a valid runner and install_dir, it will also have a valid installed_at.
SELECT name, runner, directory, installed_at FROM games WHERE runner NOT NULL AND runner != 'steam' AND runner != '' AND directory NOT NULL- This is the query-equivalent of the filtering we do. All of the games in my Lutris DB file have aninstalled_atwith this query.SELECT name, runner, directory, installed_at FROM games WHERE runner NOT NULL AND runner != 'steam' AND runner != '' AND directory NOT NULL AND installed_at IS NULLreturns 0 rows.
- While we could potentially filter out games in our query in
lutrisutil#get_lutris_game_list, perhaps it's better to have a "complete" games list and then filter out on a case-by-case basis.
To check the slugs, I used the following: SELECT slug, name, runner, directory, installed_at FROM games WHERE slug IS NULL and this returned zero records. I ran a couple of other queries and I am pretty confident all games should have a slug:
SELECT slug, name, runner, directory, installed_at FROM games WHERE slug NOT NULLreturned 1120 records.SELECT slug, name, runner, directory, installed_at FROM gamesreturned the exact same 1120 records.
So in short, all games should have a slug, and all games with a non-blank, non-null runner that is also not steam, should have an installed_at. If we wanted to be super cautious, though, we could always do a check first and display Unknown if it really isn't set :-)
Reference for all queries in a friendly code block with comments. For transparency I used "DB Browser for SQLite", I found it a bit cumbersome to use but I had it installed probably from a long time ago 😄
Slug Queries
-- Check games without slug (for me, returns zero records)
SELECT slug, name, runner, directory, installed_at
FROM games
WHERE slug IS NULL
-- First query gets all games with non-null slug, second query just fetches all games
-- Both queries should return the same amount of rows
SELECT slug, name, runner, directory, installed_at
FROM games
WHERE slug NOT NULL
SELECT slug, name, runner, directory, installed_at
FROM gamesinstalled_at queries
-- Fetch all games with a valid runner for the Games List
-- That is, a runner which is not null, which is not an empty string, and which is not Steam
-- All of these records should have a defined installed_at
SELECT *
FROM games
WHERE runner NOT NULL
AND runner != 'steam'
AND runner != ''
-- Same as above but with installed_at clause, this returns zero records
SELECT *
FROM games
WHERE runner NOT NULL
AND runner != 'steam'
AND runner != ''
AND installed_at IS NULL
-- Check all games with a null installed_at, these are all games with runners that we skip on the games list
-- It's the inverse of the above; all of the runners here match the criteria that we exclude in the above query
-- In other words, for all games with a NULL installed_at, the runner is either: null, steam, or an empty string
SELECT name, runner, installed_at
FROM games
WHERE installed_at IS NULL
-- Query-equivalent of the filtering that we will do on the Games List
-- This gets all games with a valid runner and a valid install_dir
-- All of these records have an installed_at, so games with valid runners and install_dirs should also always have installed_at
SELECT name, runner, directory, installed_at
FROM games
WHERE runner NOT NULL
AND runner != 'steam'
AND runner != ''
AND directory NOT NULL
-- To further illustrate the above, get all records with a missing installed_at but where the runner and install directory are valid
-- This should return zero records
SELECT name, runner, directory, installed_at
FROM games
WHERE runner NOT NULL
AND runner != 'steam'
AND runner != ''
AND directory NOT NULL
AND installed_at IS NULL