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

Lutris config files don't have name, slug or working dir #209

Closed
nihaals opened this issue May 21, 2023 · 26 comments
Closed

Lutris config files don't have name, slug or working dir #209

nihaals opened this issue May 21, 2023 · 26 comments
Labels
bug Something isn't working
Milestone

Comments

@nihaals
Copy link
Contributor

nihaals commented May 21, 2023

Ludusavi version

v0.18.1

Operating system

Linux (Steam Deck)

Installation method

Flatpak

Description

Using the Lutris root doesn't show the expected games in the preview.

Logs

For every game I have in Lutris I get this:

[2023-05-21T13:20:11.689Z] WARN [ludusavi::scan::launchers::lutris] Unable to parse Lutris game file: /home/deck/.var/app/net.lutris.Lutris/config/lutris/games/untitled-goose-game-1683316261.yml

Additional context

My Lutris configs look like this:

game:
  exe: /run/media/mmcblk0p1/Games/untitled-goose-game/game/Untitled.exe
  prefix: /run/media/mmcblk0p1/Games/untitled-goose-game/wine
system: {}
wine: {}

Which don't match the expected format:

/// https://github.com/lutris/lutris/blob/e4ae3d7193da777ebb370603a9e20c435f725300/docs/installers.rst
#[derive(serde::Deserialize)]
struct LutrisGame {
game: GameSection,
/// ID of the game itself.
game_slug: String,
/// Human-readable.
name: String,
}
#[derive(serde::Deserialize)]
struct GameSection {
prefix: Option<StrictPath>,
working_dir: StrictPath,
}

I'm using Lutris v0.5.13.

@nihaals nihaals added the bug Something isn't working label May 21, 2023
@mtkennerly
Copy link
Owner

Thanks for the report. Could you share (some or all of) the files you have? Unfortunately, I only had a small sample to use for the initial design, but I can definitely expand it.

@nihaals
Copy link
Contributor Author

nihaals commented May 21, 2023

I added my extra info as an edit since it didn't feel like it fit in with the form, wasn't expecting you to see it so fast :)

@nihaals
Copy link
Contributor Author

nihaals commented May 21, 2023

I looked at #164 and it's possible that some of the expected metadata is in the database (I haven't looked at it yet) and while the slug seems to only be used for logging at the moment, the name is actually used (which is actually in Lutris along with the "identifier" (which I assume is the same as the slug), so must just be stored elsewhere).

@nihaals
Copy link
Contributor Author

nihaals commented May 21, 2023

For additional context, all these games were installed through either "Install a Windows game from an executable" or "Add locally installed game". Maybe Lutris should be consistently setting the values in the config file?

Edit: Actually correction, that format only applies to games installed with "Add locally installed game". The games I've installed with "Install a Windows game from an executable" also fail to parse, but purely due to working_dir missing (I think it might only be set if the user explicitly adds a value for it, and if the user is using the default then it's not written).

@mtkennerly
Copy link
Owner

Ah, I see, so it fills in different amounts of metadata depending on how you add the games. That's unfortunate, but I can definitely improve it. Thanks for helping track this down!

The case where it's just missing working_dir should be easy enough. I can take the exe field and just use the parent folder of the executable,

For games missing the slug and name, that's a lot harder. Unless that info is stored in another file or the database, I don't think I can do much about that.

@nihaals
Copy link
Contributor Author

nihaals commented May 21, 2023

/data/lutris/pga.db does indeed include the name, slug, and in some cases the Wine prefix (although this seems to always be included in the YAML anyway). I think it might make sense to read from the database first and fallback to the YAML if it's not included. The DB also includes the path to the specific config file which means they can be tied together instead of needing to guess. The DB also includes a runner field which could help filter since I imagine Ludusavi should only be looking at wine games.

@mtkennerly
Copy link
Owner

That sounds promising. Would you be willing to share your pga.db? I'd like to test with a larger/realistic data set. If you'd rather share it privately, my email is [email protected].

The DB also includes a runner field which could help filter since I imagine Ludusavi should only be looking at wine games.

I'm not sure how often people use Lutris for native Linux games, but in theory, Ludusavi should be able to find those too.

@mtkennerly
Copy link
Owner

I just released v0.18.2 with the working_dir vs exe fix.

@nihaals
Copy link
Contributor Author

nihaals commented May 21, 2023

I've sent my pga.db.

I'm not sure how often people use Lutris for native Linux games, but in theory, Ludusavi should be able to find those too.

Agreed, although I assume prefix will be missing for those. Maybe it makes sense to log it so if someone runs into it not working it can more easily be traced to it being a non-Wine game (as Lutris also supports ROMs which are probably stored even more differently and might be an interesting future feature request).

@nihaals nihaals changed the title Lutris config files don't match expected format Lutris config files don't have name, slug or working dir May 24, 2023
@nioncode
Copy link

nioncode commented Aug 26, 2023

Any updates on this? My games also don't have a name in the yml, but have the correct name in the pga.db. It would be great if ludusavi could look up the name in the db, as currently my games are not detected.

For now, I'm just adding the game-slug and name fields to my yml file manually.

Edit: also created an issue in lutris. Maybe it makes more sense if lutris simply adds the necessary fields to the config instead of parsing pga.db in ludusavi.

@mtkennerly
Copy link
Owner

@nioncode No updates yet, but I do still have plans to look into this. I'll also keep an eye on the ticket you opened in the Lutris repository :)

@eskay993
Copy link

Sorry to resurrect this, but just wondering if you found a solution?

Grabbing the name from the db seems like a good option to me. I use it in various bash scripts I have to do al sorts of things. The games table has a configpath column with the name of the yml file (minus the .yml extension) so you could look up name quite easily from the db as a fallback.

Right now as a workaround, I am pulling the name from the db and injecting it into the yml file as part of ludusavi runner script I have before running ludusavi, which is a bit hacky as you can imagine.

I'm happy to help in any way I can with info (db locations, db structure, etc). I would do a PR but I'm not that familiar with rust unfortunately.

@mtkennerly
Copy link
Owner

Hey, @eskay993, I've been a bit busy recently, but I do still plan to work on this. I can definitely let you know when I have a prototype, if you'd be willing to test it out :)

@eskay993
Copy link

No worries at all and sure thing! Happy to test out any new builds :)

@mtkennerly
Copy link
Owner

I finally have a build ready for testing that checks both pga.db and games/*.yml. Any feedback would be great:

If you usually use Flatpak, just remember to pass the --config argument so it knows where to find your normal settings:

./ludusavi --config ~/.var/app/com.github.mtkennerly.ludusavi/config/ludusavi

@baptisteguidoux
Copy link

I finally have a build ready for testing that checks both pga.db and games/*.yml. Any feedback would be great:

* Build: [ludusavi-v0.23.0-post.18+f0a6767-linux.zip](https://github.com/user-attachments/files/15527849/ludusavi-v0.23.0-post.18%2Bf0a6767-linux.zip)

* [Changelog](https://github.com/mtkennerly/ludusavi/blob/f0a67673dc64a5a9976c656352a196db222ade9f/CHANGELOG.md#unreleased)

If you usually use Flatpak, just remember to pass the --config argument so it knows where to find your normal settings:

./ludusavi --config ~/.var/app/com.github.mtkennerly.ludusavi/config/ludusavi

I tested your latest build, it discovered all my 4 installed Lutris GOG games instead of just one as before.

@mtkennerly
Copy link
Owner

Thanks 👍 I'll try to make a new release within a week or so with this change.

@nihaals
Copy link
Contributor Author

nihaals commented Jun 20, 2024

Now I'm just very confused.

I just tried v0.24.1 and first I got an error about the Lutris DB not existing so I fixed my Lutris path. I then started hitting this log:

log::warn!("Unable to read Lutris game file: {:?}", file);

I found lutris/lutris@7af680d and I assume there's also a migration commit somewhere that moved data over, but I had my games/*.yml and runners/*.yml in my config directory but my pga.db in my data directory. I copied them all over to the data directory but still hit the log.

I can't check the trace logs since one preview causes all the log files to overflow (at least through the GUI) and I tried going through StrictPath to figure out if it was trying to read the correct YAML path but I still wasn't confident (the "Unable to read Lutris game file" logs had a raw e.g. game-name-1718840607 and the basis for all of them was None).

I checked my pga.db in case anything changed and the configpath format still matches the format used in the database used in tests (e.g. game-name-1718840607 with no file extension or more explicit path).

After looking at the tests in f0a6767, it looks like there isn't actually a test checking that the files referenced in the configpaths are being read and maybe there's a bug here? I don't actually see any logic doing games/{}.yml with the configpath but I might be missing something.

Actually I just realised, the DB in the tests only has rows (1) with directory set, but some of my games don't and so require reading the YAML, which likely also fails in the tests but wasn't caught as reading the spec wasn't actually required.

Here's the test DB with another row that matches some of mine:

pga.db.zip

And here's a games/windows-game-1683516079.yml that matches:

game:
  exe: /.../game.exe
  prefix: /.../wine-prefix
system: {}
wine: {}

This should make sure that the merging logic works, although this might flag the Lutris directory deprecation issue where we need to consider not only the games/*.yml being in the data directory but also the config directory (and I assume this is why previously I had Ludusavi set to use the config directory). I don't know if there's a good way of getting where the game specs are without just replicating the logic in lutris/lutris@7af680d which is probably trickier with Flatpaks and special casing it being a Flatpak might be worse than not supporting the old config directory?

Also I just checked and Lutris only updates the YAML and not the database if you change the Wine prefix in the UI so maybe we also need to test that we're prioritising the YAML data over the database data?

@mtkennerly
Copy link
Owner

I found lutris/lutris@7af680d

Ah, geez 😅 Thanks for finding this.

I can't check the trace logs since one preview causes all the log files to overflow (at least through the GUI)

Yeah, it helps to just scan one game via the CLI: RUST_LOG=ludusavi=trace ludusavi backup --preview "Lutris Game Title"

it looks like there isn't actually a test checking that the files referenced in the configpaths are being read and maybe there's a bug here? I don't actually see any logic doing games/{}.yml with the configpath but I might be missing something.

Good catch - read_spec here was assuming that it gets the full path, not just the bare file name:

match scan_db(root) {
Ok(db_games) => {
for (spec, db_pending) in db_games {
let spec_pending = read_spec(&spec);

I've pushed some fixes in the master branch:

  • 1abc826 - Treat configpath as a bare file name instead of a full path
  • 730acc7 - Always check all YAML files (instead of limiting it to only the YAML files identified in database configpaths) and prefer prefixes from spec over database

I'll look into the XDG_CONFIG_HOME/XDG_DATA_HOME split tomorrow.

@nihaals
Copy link
Contributor Author

nihaals commented Jun 20, 2024

Great! I was going to PR some tests today but it looks like everything I was thinking of is already covered. One thing did catch my eye:

Self {
name: db.name.or(spec.name),
slug: db.slug.or(spec.slug),
// Apparently, if you change the prefix in the Lutris GUI,
// Lutris updates the spec, but not the database,
// so we prefer the spec version.
prefix: spec.prefix.or(db.prefix),
platform: spec.platform.or(db.platform),
install_dir: db.install_dir.or(spec.install_dir),
}

Is it worth investigating which place is used for each field? I can't remember at the moment which ones are even changeable but do we know for sure that more of these shouldn't be prioritised from the spec? If not I should be able to check them today like with the prefix.

@mtkennerly
Copy link
Owner

Is it worth investigating which place is used for each field?

Definitely! That would be a big help if you don't mind.

@mtkennerly
Copy link
Owner

@nihaals Would you mind testing the fix for the pga.db path?

Linux build: ludusavi-v0.24.1-post.11+0ebb6d4-linux.zip

  • Lutris roots now have a field for the pga.db file path. If you leave it blank, it'll look for pga.db inside the root's main path as before.
  • Ludusavi can automatically detect/update these Lutris roots based on whether or not they contain games/*.yml:
    • ~/.config/lutris (will prompt to add ~/.local/share/lutris/pga.db)
    • ~/.local/share/lutris (no prompt)
    • ~/.var/app/net.lutris.Lutris/config/lutris (will prompt to add ~/.var/app/net.lutris.Lutris/data/lutris/pga.db)
    • ~/.var/app/net.lutris.Lutris/data/lutris (no prompt)

@nihaals
Copy link
Contributor Author

nihaals commented Jun 24, 2024

It works great when config and data are merged, but with them separate, it doesn't work. I think this might be outside of the Lutris scanning logic though:

  • Auto-detecting the root worked
  • I got TRACE [ludusavi::resource::config] Configured root: Lutris(Lutris { path: StrictPath { raw: "/home/deck/.var/app/net.lutris.Lutris/config/lutris", basis: None }, database: Some(StrictPath { raw: "/home/deck/.var/app/net.lutris.Lutris/data/lutris/pga.db", basis: None }) }) | interpreted: Ok("/home/deck/.var/app/net.lutris.Lutris/config/lutris") | exists: true | is dir: true
  • I got TRACE [ludusavi::resource::config] Expanded root: Lutris(Lutris { path: StrictPath { raw: "/home/deck/.var/app/net.lutris.Lutris/config/lutris", basis: None }, database: None }) | interpreted: Ok("/home/deck/.var/app/net.lutris.Lutris/config/lutris") | exists: true | is dir: true
  • I got DEBUG [ludusavi::scan::launchers] Scanning launcher info: Lutris(Lutris { path: StrictPath { raw: "/home/deck/.var/app/net.lutris.Lutris/config/lutris", basis: None }, database: None })
  • And then ERROR [ludusavi::scan::launchers::lutris] Failed to read database: NoDatabase

It looks like it might be from .glob() essentially erasing database since new() always sets it to None:

pub fn glob(&self) -> Vec<Self> {
self.path()
.glob()
.iter()
.cloned()
.map(|path| Root::new(path, self.store()))
.collect()
}


Re: #209 (comment), I checked all of them and made #359.

@mtkennerly
Copy link
Owner

mtkennerly commented Jun 25, 2024

It looks like it might be from .glob() essentially erasing database since new() always sets it to None:

Good catch 👍 Should be fixed now in 834a8ce.

Linux build: ludusavi-v0.24.1-post.15+834a8ce-linux.zip

@nihaals
Copy link
Contributor Author

nihaals commented Jun 25, 2024

It works great! 🎉

@mtkennerly
Copy link
Owner

Awesome, thanks for testing 🎉 I'll try to do a release soon after a couple of other fixes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants