Closed sonic2kk closed 1 year ago
Sorry for the delayed reply. I did some testing with Lutris and took a look at the code.
Instead of building a string, we now loop through all the files in the Lutris game config directory (~/.config/lutris/games) and when we find a filename that matches either the slug or the installer_slug, we set the name of the config file to that, and then we break. The rest of the logic is the same after that.
That seems like a solid method for doing so. I found two exceptions where the method could pose a problem:
grand-theft-auto-v
for both grand-theft-auto-v-epic-games-launcher
and grand-theft-auto-v-rockstar-games-launc
.I'm not sure how likely these cases are and if they even matter (the dialog is currently only used for rough information and won't display the launcher anyway).
I'm wondering:
<installer_slug>-<installed_at>.yml
or if installer_slug not available, <slug>-<installed_at>.yml
? In this case we could look for this two files (and maybe if not found loop through all files looking for installed_at)...We can also leave it as you proposed, it's definitively better than the current solution and see if it causes a problem and then do some adjustments later. I would like to hear you opinion on this.
Lutris seems very... temperamental about how it saves the compatibility tool name
I think it handles its own compatibility tools different than externaly installed ones. Not sure about that though.
Should we be stricter about matching the filename? I wasn't sure if we should go to the effort of checking if the installed_at timestamp was in the filename too.
Is it possible to install a game in Lutris multiple times (same game and launcher)? If not, it won't be an issue at all.
As always, thanks!! 🥳
When the user has thousands of games, the loop could cause timing problems on slower computers
Lutris itself has this issue (takes ~10 seconds to open up) as I have around 1.5k games that it detects (across Steam, GOG, Origin and manually added games on the Lutris website). However in ~/.config/lutris
, there are only 392 files. I could definitely forsee a slowdown when the user gets into the several thousand file territory.
Are there any Python functions we could use to speed this up? I know there is a filter
method but I am not sure if it provides a significant speed bump over the for loop the code currently uses. We could also use next
on a list comprehension I guess? But list comprehensions are not always faster.
I'm not sure, I think trying to find the file instead of building a string is more reliable but I'm not sure how to make it more performant. Definitely a concern though!
If a user has installed the same game from two launchers, the slug will be the same ...
Very good point. Maybe we could solve this by reversing the order of the check, e.g. prioritizing the installer_slug
with str(self.installer_slug) or str(self.slug)
? Because:
Is the filename always
<installer_slug>-<installed_at>.yml
or ifinstaller_slug
not available,<slug>-<installed_at>.yml?
In this case we could look for this two files (and maybe if not found loop through all files looking for installed_at)...
I believe this is correct, and I'll use two examples for this:
blizzard-battlenet-standard-timestamp.yml
-- This is its installer_slug
field in the yml file - However, it also has a regular slug (in the yml called game_slug
) of battlenet
. Inside of Lutris, this game_slug
as its labelled in the yaml, is called the "Identifier" in the UI (see below). For Battlenet, this Identifier field is battlenet
. In other words, Lutris will first look for the installer slug to set the filename (since Battlenets filename is blizzard-battlenet-standard
, but it's "Identifier" in the Lutris UI is battlenet
). But if there is no installer slug (like in the case of the manually added Bad Piggies), it will fall back to using the Identifier as the name.
I hope that made sense :sweat_smile: Since Lutris handles it in the order of installer slug and then slug, I think I should update the logic to do this as well when looking for the filename. That should resolve the issue of two of the same game installed from different sources, since the installer slug will have the slug for e.g. the Epic Games Store first before it has one without any e.g. for the Steam release.
In theory it should be possible to detect a game using the install timestamp as it is unlikely(/impossible?) that two games are installed simultaneously. Is this timestamp always present?
Timestamps actually can be the same. I'm fairly sure this is for when Lutris detects installed Steam games, which may not be something we need to worry about too much, but I wonder if it could happen for other detected / imported games i.e. imported GOG games? I sadly don't know :disappointed:
Here's a sample screenshot from my ~/.config/games
folder, you can see quite a few have the same timestamp (and they all appear to be games I own on Steam from skimming furtherr down)
I think it handles its own compatibility tools different than externaly installed ones. Not sure about that though.
Maybe, the issue I was running into was that I would have to try and save 3-4 times before it actually saved the compatibility tool correctly. Sometimes it would save the wrong one, or a blank one, or None
, and one time it saved a compatibility tool I didn't even have installed. I'm fairly sure it's a lutris-git
issue and to be honest, probably not something we need to worry about on the ProtonUp-Qt end -- PUPQT just reads the compat tool, if Lutris writes it incorrectly that's not our business :sweat_smile:
Is it possible to install a game in Lutris multiple times (same game and launcher)? If not, it won't be an issue at all.
It might be,, but it still might not be a problem.
When you right click on a game in Lutris, you can select the option "Install Another Version". This only works for games which have installers. When you try to install a game with Lutris, it will show a dialog with available installers, since there could be several. But since it only works for games with installers, each of these installers would have a different installer slug, making it a unique game entry in PUPQT.
However, when you choose "Install Another Version", I think that might replace the existing installation. Meaning you can install a new version, but it'll replace the old one. Meaning you couldn't have, say, two Battlenet installations at the same time. But I am not positive on this.
I am not sure if the "Install Another Version" only shows unique entries either. That is, say you use Installer A to install Battlenet, and then once installed you choose "Install Another Version". I'm not sure if Lutris will still show options A and B to install with.
I think It will still show all option as my installer slug is blizzard-battlenet-standard
and when I choose "Install Another Version," it shows me a version labelled "Standard":
In this case, I'm not sure if it would overwrite an existing yml, or create a new one with the same name but a different timestamp. Lutris might just remove the old yml and overwrite it with the new install -- So even though it says "Install Another Version", it replaces your existing one.
Since there are a lot of unknowns with the last one, I will test with checking the installed timestamp and if it doesn't break the matching (which it shouldn't), I think there is no harm in including it.
So the main takeaways for now are to change the order of precedence with the slugs, prioritising installer_slug
and falling back to slug
which matches more closely with how Lutris works.
The second thing is to try and match on the installed_at timestamps, just to be safe, assuming it doesn't cause too many issues for the file matching.
I did a few tests and compared how long it took each method of looping through and getting the Lutris game yaml. The tl;dr is that the current for loop is the fastest in my tests. Of course this isn't really "scientific", but it gives a rough idea. And to be honest, on my hardware each method is fast enough.
fn = ''
for lutris_cfg in os.listdir(os.path.join(os.path.expanduser(lutris_config_dir), 'games')):
if str(self.installer_slug) in lutris_cfg or self.slug in lutris_cfg:
fn = lutris_cfg
break
The current for loop to get thee matching game config yml takes around 0.18ms on average to complete, with the longest being 0.38ms.
I tried implementing a list comprehension to replace the for
loop, and it actually took longer.
fn = [lutris_cfg for lutris_cfg in os.listdir(os.path.join(os.path.expanduser(lutris_config_dir), 'games')) if str(self.installer_slug) in lutris_cfg or self.slug in lutris_cfg]
fn = '' if len(fn) <= 0 else fn[0]
Took on average about 0.24ms, with the longest being 0.51ms.
I also tried using next
and a generator, and that took even longer:
fn = next(lutris_cfg for lutris_cfg in os.listdir(os.path.join(os.path.expanduser(lutris_config_dir), 'games')) if str(self.installer_slug) in lutris_cfg or self.slug in lutris_cfg) or ''
Took on average about 0.30ms, with the longest being 0.40ms.
Again, this isn't really scientific, but the existing for loop always seemed to come out by just slightly faster.
I think the best way to speed this up would be to avoid calling os.listdir
on each call to get_game_config
-- But I'm not sure how we could cleanly do this. I considered the idea of passing it as an argument, but get_game_config
might need a lot of changes around the codebase for that, and plus each time it's called we'd need to get the lutris_config_dir
.
So I'm not sure of the best way to go about optimizing this, but if you have any suggestions I'd be happy to experiment with/implement them :smile:
I did a few tests and compared how long it took each method of looping through and getting the Lutris game yaml.
I also did some testing on a slower machine:
Listing around 3500 files, doing two 'x' in y
and printing them took around 50ms, so maybe I was too concerned, the impact seems minimal.
I think we can leave it as is.
Maybe we could solve this by reversing the order of the check, e.g. prioritizing the installer_slug with str(self.installer_slug) or str(self.slug)?
That should work.
I don't think str(self.installer_slug) in lutris_cfg or self.slug in lutris_cfg
is enough as it does the matching per entry, not per list.
We need to do two loops, on for str(self.installer_slug) in lutris_cfg
and one for self.slug in lutris_cfg
if no match on installer_slug
was found. (see code below)
Timestamps actually can be the same. I'm fairly sure this is for when Lutris detects installed Steam games
Right, didn't think of that. Good to know.
Maybe, the issue I was running into was that I would have to try and save 3-4 times before it actually saved the compatibility tool correctly [...] I'm fairly sure it's a lutris-git issue and to be honest,
Yeah, seems a bit like an issue with the git version.
Since there are a lot of unknowns with the last one, I will test with checking the installed timestamp and if it doesn't break the matching (which it shouldn't), I think there is no harm in including it.
Okay. The code I included only checks the timestamp if the installer_slug matches. If the the game slug matches, it doesn't check it. We can change that behavior though.
def get_game_config(self):
lutris_config_dir = self.install_loc.get('config_dir')
if not lutris_config_dir:
print('no config dir')
return {}
# search a *.yml game configuration file that contains either the install_slug+installed_at or, if not found, the game slug
fn = ''
for game_cfg_file in os.listdir(os.path.join(os.path.expanduser(lutris_config_dir), 'games')):
if str(self.installer_slug) in game_cfg_file and str(self.installed_at) in game_cfg_file:
fn = game_cfg_file
break
if fn == '':
for game_cfg_file in os.listdir(os.path.join(os.path.expanduser(lutris_config_dir), 'games')):
if self.slug in game_cfg_file:
fn = game_cfg_file
break
lutris_game_cfg = os.path.join(os.path.expanduser(lutris_config_dir), 'games', fn)
if not os.path.exists(lutris_game_cfg):
return {}
with open(lutris_game_cfg, 'r') as f:
return yaml.safe_load(f)
Thanks! Tested the code on my end too and it seems to work, and on my PC visually it has 0 impact on load times for the info window - It's still virtually instantaneous :tada:
Since I don't have as many Lutris games in use as other users might, I can't say if this is 100% foolproof just yet. In my testing though it matches my games correctly. If there are complaints though they can be addressed as they come in.
(Also, I did actually try playing around with matching the installed timestamp, and I didn't have much luck. This solution works much better though :smile:)
Perfect!
Since I don't have as many Lutris games in use as other users might, I can't say if this is 100% foolproof just yet. In my testing though it matches my games correctly.
Works fine in my test too. I too think:
If there are complaints though they can be addressed as they come.
Thanks you, will merge! :partying_face:
I was originally going to open an issue for this, but I decided to investigate to try and fix this issue instead. This PR solves the issue I mentioned in #137 with being unable to test Lutris games.
Overview
The Lutris compatibility tools list has a couple of issues:
installer_slug
isNone
and so a game file won't be found, since the logic tries to build a file string for the Lutris game yml config (so a filename could end up beingNone-34496832234.yml
This PR addresses both of these problems.
Solution
Instead of building a string, we now loop through all the files in the Lutris game config directory (
~/.config/lutris/games
) and when we find a filename that matches either theslug
or theinstaller_slug
, we set the name of the config file to that, and then we break. The rest of the logic is the same after that.For the compat tool count issue, we use a list comprehension to generate the list of Lutris games with a Wine runner that match the
ctool.displayname
. and then set thetxtNumGamesUsingTool
to the length of the generated list.Concerns
I have a couple of concerns here:
installed_at
timestamp was in the filename too. I think maybe just checking on the slug is enough? :slightly_smiling_face:As usual, all feedback is welcomed here :smile:
Thanks!