Resolve saves from the platform mapping, not only the play action - #139
Merged
Merged
Conversation
Save sync read the emulator solely from the game's Playnite play action. That action is a snapshot of the emulator mapping written once at import and never refreshed: the importer skips games that already exist, so a mapping later repointed at another emulator left every game it had imported still launching -- and resolving saves against -- the old one. Sync then gave up with a single message that named neither the emulator it looked at nor what to do about it (#138). The play action still leads, because a user who repoints it should have their saves follow it, but when it names an emulator no handler covers, the platform's own mapping -- which the settings screen presents as the thing that decides this -- now gets its turn before sync gives up. The mapping comes from the ROM sidecar, which every import rewrites, so it tracks the user's edits where the action does not. A play action can also name an emulator without naming a profile, and for RetroArch the profile identifies the core, which is a folder in the save path under sort_savefiles_enable. The profile is now taken from another candidate for the same emulator rather than resolving a path with the core folder missing, where a download would land somewhere RetroArch never reads. Stale actions are repaired rather than only worked around: an import repoints an existing game's action at its mapping's current emulator, but only while the action is still the one the plugin wrote. The sidecar records what was last applied; actions from before that record are recognised by the generated "Play in <emulator>" name. Where two enabled mappings share a platform, neither touches the actions, so the two cannot fight over every game. The one catch-all failure message becomes three -- no emulator set, an unsupported one named, or a supported one whose save path could not be worked out -- and the sidecar's path, read and write move into RomMGameData, replacing the four hand-rolled copies in RomM.cs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Quality pass over the previous commit, no behaviour intended beyond the one noted below. The resolver described its outcome twice: a problem enum beside the emulator and handler it had already stored. A null emulator means none was named and a null handler means none is supported, so the enum and the duplicate emulator name go, and the source of the pick becomes a bool. ResolveTarget takes an out reason rather than reaching into the caller's outcome to write one field of it. The importer's play action had two writers: RomMPlayAction.Build for a new game and a field-by-field copy of it for a refreshed one, which would have drifted the first time a field was added. Build now delegates to Apply, which both use. Matches and IsUnedited take the applied action whole instead of unpacking it into loose parameters, and the mapping lookup behind MappingFor is shared with the install path and the legacy game id through SettingsViewModel.MappingById. Three pieces of wasted work, all per ROM or on the pre-launch path: the sidecar was re-serialised to compare against what had just been parsed, where the text read off disk was already in hand; the emulator name was looked up by scanning the whole collection, eagerly, for a legacy branch that usually does not need it, and is now a keyed Get behind a thunk; and save sync read the sidecar on every game even when the play action already answered on its own, which it does whenever its emulator is supported and it names a profile. The one behaviour change: an install whose sidecar is missing or corrupt now logs one message and cancels, where it logged two and reached the same end. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gantoine
marked this pull request as ready for review
September 24, 2026 14:22
…k-and-stale-actions # Conflicts: # RomM.cs
Contributor
|
- Keep a user's per-game core, arguments and demoted actions when repointing play actions, and save each game once per import. - Clear a mapping's profile when its emulator changes, so a mismatched pair is never written; drop the importer's guard for it. - Share the controller's mapping skip rules via EmulatorMapping so the rival-mapping check can't drift. - Consume sync suppression whether or not KeepRomMSynced is on. - Log locked sidecars as unreadable rather than corrupted, write the JSON already produced, and reuse the parsed sha1 when loading sidecars. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- The mapping no longer stands in for a play action naming a different emulator: that emulator is what launches, so syncing another one's saves would download where it never reads. The mapping now only fills in when the action names no emulator, or lends a profile to that same emulator; the unsupported message names the mapping's emulator when save sync supports it. - Save sync ignores the sidecar's mapping on a platform two importable mappings cover, since it only records whichever import ran last. The rival check is shared as Settings.HasRivalMapping. - An action already matching the mapping is only recorded as plugin-applied if the plugin owned it before, so a user's matching choice is not repointed on a later mapping change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #138.
What was happening
Save sync resolved the emulator only from the game's Playnite play action:
That action is a snapshot written once at import and never refreshed —
ProcessDataskips games that already exist, touching only favourite and completion status. So a mapping later repointed at another emulator leaves every game it had imported still pointed at the old one.The reporter's screenshots show exactly that: their action is named
Play in Dolphin(only the importer produces that string), while their mapping today is RetroArch / Dolphin - GC/Wii / Nintendo GameCube. No handler covers Dolphin, so sync gave up. Note the action also means the game launches with the old emulator — save sync only made it visible.What this changes
The mapping fills gaps; it never overrides the launched emulator. The play action's emulator is what launches, so it is the save target. Syncing a different emulator's saves instead would download to a place the launched emulator never reads. The platform mapping is used only when the action names no emulator, or to supply a profile for that same emulator. It comes from the ROM sidecar (
{sha1}.json), which every import rewrites, and is skipped when two importable mappings cover the platform, because then the sidecar only records whichever import ran last. When a stale action names an unsupported emulator, the message names the mapping's emulator so the user knows where to point the action. The real fix is the refresh below. The lookup that install and uninstall already did by hand is nowIRomM.MappingFor(game).The profile comes along. An action can name an emulator without naming a profile, and for RetroArch the profile identifies the core — a folder in the save path under
sort_savefiles_enable. Resolving without it means reads limp along viaFindExistingSavebut a download writes where RetroArch never reads. The profile is now borrowed from another candidate for the same emulator id (never across emulators).Stale actions are repaired, not only worked around. An import repoints an existing game's action at its mapping's current emulator, but only while the action is still the plugin's:
RomMRomLocalnow recordsAppliedEmulatorID/AppliedEmulatorProfileID. Action still carries them → ours to repoint; differs → the user's, left alone.Play in <the emulator it points at>is one the importer wrote. The reporter's game gets repointed on their next library update, launching included.Three messages instead of one catch-all: no emulator set (and where to set one),
Save sync does not support Dolphin yet. Supported: retroarch., and "could not work out where X keeps this game's saves". A missing ROM path gets its own, and falling through to the mapping is logged.Shared sidecar helper.
Games/RomMGameData.cscentralises the path, read and write, replacing four hand-rolled copies inRomM.cs(install, uninstall, version menu, delete).Tests
SaveEmulatorResolverTests(action is authoritative, mapping only when no emulator is set, unsupported vs. none set, profile borrowing) andRomMPlayActionTests(ownership rules, unset-profile equivalence, the legacy name marker). Both new sources are linked intoRomM.Tests.csproj.Not verified locally
Draft because I could not compile or run the tests: no .NET SDK, msbuild or mono on this machine, and per
DEVELOPMENT.mdthe project only builds on Windows. The PR build is the first real check. The end-to-end path — a stale action being repointed on import, and a save syncing through the mapping fallback before that — still wants a run against a real Playnite library.🤖 Generated with Claude Code