-
Notifications
You must be signed in to change notification settings - Fork 62
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
Upgrade GameFinder to 4.5.0 #2653
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a long-ish comment here.
Unfortunately, my browser crashed, and I lost all the text.
Here's a short summary from what I remember. [compared to original text anyway]
System.InvalidOperationException: GOG locator metadata is not a valid ulong
at NexusMods.Games.FileHashes.FileHashesService.TryGetGameVersionDefinition(GameInstallation installation, IEnumerable`1 locatorMetadata, ReadOnly& versionDefinition) in /home/sewer/Project/NexusMods.App/src/NexusMods.Games.FileHashes/FileHashesService.cs:line 303
at NexusMods.Games.FileHashes.FileHashesService.TryGetGameVersion(GameInstallation installation, IEnumerable`1 locatorMetadata, String& version) in /home/sewer/Project/NexusMods.App/src/NexusMods.Games.FileHashes/FileHashesService.cs:line 284
at NexusMods.App.UI.Controls.GameWidget.GameWidgetViewModel.<>c__DisplayClass1_0.<<-ctor>b__11>d.MoveNext() in /home/sewer/Project/NexusMods.App/src/NexusMods.App.UI/Controls/GameWidget/GameWidgetViewModel.cs:line 49
(On every App boot)
It seems we're missing a BuildID here.
I've not worked with GameFinder code here, but a quick debug session later, it seems, that we're pulling BuildID from Heroic's own metadata, and they don't store the BuildID. Presumably because it comes from installer, and not GOG's Content-System. You can only get the BuildID from game files post install.
[Note: BuildID is 58211149695218771
per goggame-1453375253.info
for latest Linux GOG release. This is a proper unique BuildID, not some copied ID from a non-Linux release.]
Opinions would differ here I imagine, but I would consider fetching this data from the store launcher (in this case Heroic) to be the 'correct' thing here. Ideally, long term (in my opinion), we should be editing the launchers' store metadata when we do an in-App game version switch.
This is to prevent the store or the user from accidentally restoring the files for a different game version; for example when user clicks 'Verify Game Files' in their launcher; or risking the launcher auto-replacing files, e.g. if they launch game from launcher.
Per @halgari comment though, it doesn't seem like we're going to be indexing Native GOG Linux SDV for now, as that's not served by GOG's Content-System, and we're not quite equipped to handle that.
Without a game-hashes DB entry you can't manage the game, so I would recommend against closing
Given that from a casual user's point of view, the change in this PR is they get an error on every boot if Native Linux SDV is installed via Heroic, I'm not sure if we should merge unless we could also get the game manage-able for release. It's a complicated call.
Edit: (Forgot to mention)
I would honestly start by sending a message over to the GOG folks to see if they're willing to put linux game builds on the Content-System. They seem to be clearly, properly packaging and cataloguing Linux releases, as in, they all have proper BuildIDs, proper launch settings (playTasks) etc. for the Linux installers; not just for this game either.
They may say no, as there may not be any plans to ship a native Galaxy client, but always worth a shot tbh.
It's kind of very valuable, because they don't offer easily accessible game version downgrades with their installers.
Otherwise, we'd probably want to do some sort of PR to Heroic to find the BuildID.
In SDV's case, I'm not sure why they're wrapping the installers in same scripts/stuff as the builds prebundled with Wine; maybe part of their automated installer bundle process. SDV is completely native, the game files can be used out the box, no wrappers or scripts needed. In fact, the wrapper script basically does nothing.
The code is fine. I've checked the extra commit yesterday after regaining power. If I recall, GameFinder yesterday was returning folder to where the installer put its stuff in, but the actual game files are in a subfolder. (They made the wrapper script). This might cause us to deploy stuff to wrong folder, maybe not be able to boot game either. I'll need to see and test that. I haven't been emotionally there this morning, so it may take a short while. Sorry about the other change (reverting TreeDataGrid), could be more on time. I lost power, and by the time it was back, I went to sleep right after. |
GameFinder Path returns as:
Which has following structure:
Actual assets are in With changes from
And a host of associated errors, due to the path returned from GameFinder pointing to Unmanaging the game, removed all of the game files, keeping only folders behind. I'll test fixing the path in a sec (with a quick hardcode hack); but I ran into a blocking bug; where the App gets confused between a GOG game installation in Wine and Heroic GOG installation. I'll investigate, throw up a ticket. Then clean my data and test with proper folder path. |
@Sewer56 I've updated the code to use the |
Ya, that works for me. I just ran into another bug where I have a GOG Stardew install in Wine. The item that shows up in the Manage Games page is the Heroic Install. You go to game page, it picks the Wine install location. 😅 Edit: Anyway, time to test with subpath. |
I had to wipe datastore, it seems that somewhere, we cache the install location in MMDB. If you manage game without the patch, apply the patch, then manage the game again, then the Outside of that, things mostly work. [CC. @Al12rs , the Diagnostic Emitters think this is the Windows version of SDV 1.6.15, probably because it was closest match] But you can install mods and run the game: But unmanaging the game, deletes it. ( |
CC @Al12rs in case the reason I tagged you isn't clear. |
We changed the diagnostics to get the game version using this code, remember?
If you check the logs I expect you should have a warning saying that the game version was not recognized, which would result in all the game files ending up in Overrides (which means that unmanaging the game discards those files, same as creating a new empty loadout).
I agree this isn't great. Currently the alternative is not allowing users to manage the game at all, which isn't great either. Before the new game version system, we used to store an initial state of the game, we assumed that whatever we got was the valid game state. When unmanaging the game we would restore to the initial state. With the new system everything we don't recognize ends up in Overrides. I think we should consider doing something different in case we don't have any game version in our DB, rather than putting the game files in overrides.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR itself is good.
Still got some issues, but they're unrelated to PR. See discussion.
@Al12rs I agree on the above points, w.r.t. unknown versions. What I ran into actually, was something different and unexpected, hence the ping.
That's the thing. I thought that would be the behaviour, but it turns out the actual behaviour is a bit different Only the files specific to the Linux version are in overrides. Although the version in
More specifically, the Windows version of 1.6.15. I think somewhere along the road the App decided to match the install to the closest set that is indexed; or similar. Edit: Forgot to mention. As per above. When I unmanage the game, it doesn't only delete items in 'overrides'; which I expected to be the case. It deletes the whole game. Can do a code dive on this, but you may or may not have an idea already. |
Everything you said I expected except for the last part:
The app will try to match the game files to known versions and will pick the closest match. |
Files will be deleted either way if unrecognized (overrides), and solving it properly for unknown versions will take some time, so we can leave it as is for the release, given there's not time to do this right in a week. We will need to solve this sooner rather than later though long term. We don't want users' optional files (DLCs) to be wiped on unmanage; which I imagine will be a more common variant of this problem. |
I agree, made a ticket: #2692 |
Fixes #2401