Add save sync with the RomM server

[LxKnsy]

Supersedes #112, which @gantoine offered to close so this could be picked up. The commits from that branch are kept as they are rather than squashed, so its authorship stays intact — this builds on it rather than replacing it. Current main is merged in.

Save sync works end to end now. It didn't before: uploads landed on the server but were invisible to the sync layer, and downloads never fired.

What's here

From #112 — device registration, POST /sync/negotiate, applying the returned operations, session completion, most-recent-wins conflicts, retroarch.cfg save-path resolution, the settings toggle and the game-menu action.

Archives are hashed the way the server hashes them

SaveFileHash MD5'd raw bytes, which is right for a plain .srm and wrong for anything packed. The server's _compute_zip_hash digests each entry's content, pairs it with the entry name, sorts by name, joins as name:hash with newlines, and MD5s that string. Raw-byte MD5 over a zip can never agree with it, since zip bytes vary with entry order, compression and timestamps while the content does not.

Nothing uploads an archive yet, but every folder-based platform will (PS2 memory cards, Switch, PSP, GameCube), and the failure mode is quiet — a conflict reported on every sync for saves that are byte-identical. Also adds FolderAsZipHex / FoldersAsZipHex, producing the same digest straight from a folder without a temp archive. Entry names are normalised to /, because a name derived from a Windows path would otherwise diverge from what every other client computes.

Cross-checked against argosy-launcher's SaveArchiver.calculateZipHash (GPL-3.0, same licence) and its published vector is pinned as a test:

a.sav = 00 01 02, b.sav = FF FE  ->  fe72f8d850245659647bd6b5f3577a7a

Save location behind a handler abstraction

Per @ScottamDendar's point about this being RetroArch-specific. SaveSyncService now talks to a SaveTarget that answers what negotiate asks — does a local save exist, its hash, name, slot, time and size, what to upload, what applying a download means — without saying whether that is one file or a directory tree. FileSaveTarget covers the single-file case and uploads in place, matching what argosy-launcher sends for the same platforms.

Where the save lives is an ISaveHandler, picked from SaveHandlerRegistry by the emulator Playnite launches the game with. RetroArchSaveHandler holds what SaveSyncService used to. Adding an emulator is a handler plus one line in the registry. An emulator no handler recognises is skipped with a log line rather than falling through to a path that could overwrite an unrelated save.

Saves now carry a slot

sync/negotiate only considers saves that have one. We uploaded without, so everything this plugin wrote was invisible to the sync layer — never offered to any device, and not matched when reported back. Measured against a server holding both kinds: 40 saves with a slot, 27 offered for download; 8 without, none ever offered to anyone. Same save, same server, only the slot differing:

slot=autosave  ->  no_op,  "Content is identical"
slot=null      ->  upload, "Save exists on client but not on server"

So the local side always counted as new and won unconditionally — an empty save could quietly replace a good one. autosave matches what the other clients use for a game's live save; since the server keys on (rom_id, slot), a different value would split one game's save into two entries that never reconcile.

Downloads land where RetroArch actually reads

With sort_savefiles_enable, RetroArch keeps saves in a folder named after the running core. The path was resolved without a core name — harmless while a local save exists, because the recursive search finds it, but wrong the moment a download has to create the file. It landed beside the core folders rather than inside one, so the game started fresh on a save that was right there, and that fresh save went back up on exit.

The core now comes from the profile Playnite launches with: built-in RetroArch profiles are named after it, custom ones carry it in the libretro argument. Where a matching folder exists its spelling wins, since RetroArch's own name for a core (mGBA) isn't always how Playnite spells it.

The sorting options, checked against RetroArch itself

Two were modelled on what they sound like rather than what they do:

• sort_savefiles_by_content_enable keys on the directory the ROM sits in, not the ROM — and it wraps around the per-core folder, giving <saves>\<rom's folder>\<core>\<rom>.srm. We appended the ROM's name, and the core first.
• savefiles_in_content_dir wasn't read at all. It overrides savefile_directory rather than filling in for an empty one, so with both set RetroArch writes beside the ROM while we searched the configured tree — and the recursive fallback searched that same wrong tree.

Neither shows up with the defaults; both are one checkbox away. RetroArchConfigLayoutTests pins all six combinations.

Verified end to end

RomM 5.1.0, portable RetroArch + mGBA, a real GBA save:

• upload carries slot=autosave; exit is a clean no_op instead of a redundant re-upload
• delete the local save, relaunch → 1 downloaded, lands in saves\mGBA\, progress is there in-game
• same again with the core folder deleted entirely, i.e. the fresh-machine case → folder recreated, save restored

dotnet test RomM.Tests/RomM.Tests.csproj — 153 passing.

Not in scope here

Save states. SyncNegotiatePayload only carries saves, and state records have none of content_hash, slot, origin_device_id, device_syncs — so there's nothing for a client to reconcile against. Happy to do the Playnite side if negotiate grows to cover them.

Folder-based platforms (PS2, Switch, PSP, GameCube). The abstraction is there for them, but they want save_id from the API — better once rommapp/romm#3925 lands than growing their own extraction here.

A save is never offered back to the device that uploaded it, including with a slot — verified: offered as a download to another device, no operation for its origin. Probably deliberate, since the origin is assumed to still have the file; it only bites when the local file is gone, which is the restore-on-a-fresh-machine case. Not something a client can express through negotiate.

Happy to feature-flag this as @ScottamDendar suggested if you'd rather land it in smaller pieces. Fair warning that I'm doing this as a hobby alongside a full-time job, so I can't promise a fixed pace — the work is split so each commit stands on its own.

[ScottamDendar]

Do we want to feature flag this PR to get it reviewed and merged with the new hashing? To try and keep a short lived PR vs many dependencies? Im more of a Java dev, but c# is close enough, I could leave comments.

[LxKnsy]

Pushed the handler abstraction, and merged current main in — the branch was 13 commits behind, which also meant it still declared 0.7.0.

Then deployed the whole thing and ran it end to end against RomM 5.0.0 and again on 5.1.0, RetroArch + mGBA, one GBA game. Behaviour is identical on both. Writing it up in full since I couldn't find another end-to-end result for this yet.

What works

Device registration, negotiate, upload and session completion all run clean, no errors in extensions.log:

[SaveSync] Registered device '37b0ed7c-…' with RomM.
[SaveSync] Advance Wars: 0 uploaded, 0 downloaded, 0 conflicts, 0 failed.   (launch)
[SaveSync] Advance Wars: 1 uploaded, 0 downloaded, 0 conflicts, 0 failed.   (exit)

The uploaded save is byte-identical to the local file — 65536 bytes, MD5 2e91143b604d7a17fdc802bc11ad39d3 on both sides.

Path resolution held up in a config that could have broken it: portable RetroArch with savefile_directory = ":\saves" and sort_savefiles_enable = true, so mGBA writes to saves\mGBA\<rom>.srm. ResolveSaveFilePath is called with coreName: null and therefore predicts saves\<rom>.srm, but the recursive fallback finds the real file. Worth keeping that fallback.

Downloads never happen — and the main cause is on our side

I deleted the local .srm and relaunched, expecting the server copy back. The game started a fresh save instead, and on exit that empty save was uploaded over the good one.

I first assumed this was server-side. It mostly isn't. Two separate mechanisms, and the first one masked the second:

1. We upload slot-less saves, and negotiate ignores those entirely

SaveSyncService sends Slot = null in the negotiate payload and posts to api/saves without a slot parameter. Across my library:

saves WITH a slot : 40  ->  27 offered for download
saves WITHOUT one :  8  ->   0 offered, ever, to any device

The eight slot-less ones are exactly those written by clients that don't set a slot — this plugin, plus a script of mine. None of them is visible to sync/negotiate at all. Reporting one in the payload comes back as:

action: upload
reason: "Save exists on client but not on server"

…even when I report the exact stored state, same hash, size and timestamp. So every save this plugin creates is invisible to the sync layer, the local side is always classified as new, local wins unconditionally, and an empty save silently replaces a good one.

POST /api/saves accepts slot as an optional query parameter, so this looks fixable here. Verified directly: uploading a throwaway save with slot=autosave makes it show up as a download operation immediately, where the same upload without a slot never does. Argosy uses autosave for .srm files, so matching that seems like the way to stay compatible — but I'd rather ask than guess: is autosave the intended slot for a battery save, and should the plugin be setting it on both the upload and the negotiate payload? Happy to push that as a follow-up commit here.

2. A save is never offered back to the device that uploaded it

Separate from the above, and it survives the slot fix. In the same experiment, the slotted save is offered as a download to another device but produces no operation at all for the device it came from. device_syncs is [] on every save I've looked at.

That's probably deliberate — the origin device is assumed to still have the file. It only bites when the local file is gone, which is exactly the "restore my save on a fresh machine" case. The negotiate payload can only carry saves the client has, so there's no way for a client to say "I'm the origin and I no longer have this."

Not sure whether that's worth changing, or whether clients are simply expected to handle that case outside negotiate — happy to open something on rommapp/romm if it is.

Smaller observation

Every launch creates a session carrying operations for the whole library (27 in mine), and the client applies only those matching the current rom via the Where(o => o.RomId == romId) filter — which the comment there explains, and which is clearly the safe choice. CompleteSession then reports counts that don't account for the rest, and the abandoned sessions accumulate server-side. Not harmful in anything I saw, just noting it.

The abstraction, now in

SaveTarget / ISaveHandler / SaveHandlerRegistry, with the RetroArch logic moved out of SaveSyncService into RetroArchSaveHandler. No behaviour change — the run above is with it in place.

SaveSyncService now talks to a SaveTarget that answers what negotiate actually asks (does a local save exist, its hash, name, time and size, what to upload, what applying a download means) without saying whether that is one file or a directory tree. FileSaveTarget covers the single-file case and uploads in place, matching what argosy-launcher sends for the same platforms. Adding an emulator is a handler plus one line in the registry.

An emulator no handler recognises now resolves to nothing and the game is skipped with a log line, rather than falling through to a path that could overwrite an unrelated save.

Folder-based platforms are the obvious next step but want save_id from the API, so they're better off following rommapp/romm#3925 than growing their own extraction here.

[LxKnsy]

Do we want to feature flag this PR to get it reviewed and merged with the new hashing? To try and keep a short lived PR vs many dependencies? Im more of a Java dev, but c# is close enough, I could leave comments.

I'm totally fine with this.

[LxKnsy]

@gantoine — found why downloads never fire, and it's on our side rather than the server's.

SaveSyncService uploads with no slot (posts to api/saves without the parameter, and sends Slot = null in the negotiate payload). sync/negotiate ignores slot-less saves completely. Across my library on 5.1.0:

saves WITH a slot : 40  ->  27 offered for download
saves WITHOUT one :  8  ->   0 offered, ever, to any device

So every save this plugin uploads lands on the server correctly but is invisible to the sync layer. Reporting one back comes in as upload / "Save exists on client but not on server" even when the hash, size and timestamp match what's stored — which means local always wins and an empty save can silently replace a good one. That's how I lost a save while testing.

Verified the cause directly: the same upload with slot=autosave shows up as a download operation straight away; without it, never.

The fix looks like one line each on the upload URL and in RomMClientSaveState. Before I push it — is autosave the slot you'd want for a battery save? Argosy uses it for .srm, so it seemed like the value that keeps the two clients interoperable, but you know the server side better than I do.

Full traces and the rest of the end-to-end run are in the full report above.

[LxKnsy]

Three fixes pushed ✅ round trip works end to end now (5.1.0, RetroArch + mGBA, real save, including the fresh-machine case):

• slot — we uploaded without one, so nothing this plugin wrote was ever visible to sync/negotiate. Now autosave, matching the other clients.
• core folder — downloads landed beside the core folders instead of inside one, where RetroArch never looks, so the game started fresh on a save that was right there.
• save layout — sort_savefiles_by_content_enable keys on the ROM's folder rather than the ROM and wraps around the core folder; savefiles_in_content_dir wasn't read at all. Both checked against RetroArch itself.

@gantoine Two things from you whenever it suits:

1. CI hasn't run on this yet. think it needs your approve-and-run, first-time contributor.
2. Is extending sync/negotiate to cover states on the cards? The payload only carries saves today, and state records have none of content_hash, slot, origin_device_id, device_syncs ;so there's nothing for a client to reconcile against yet. Happy to do the Playnite side once there is.

[gantoine]

Is extending sync/negotiate to cover states on the cards

possibly? depends how necessary it is to get this working/polished.

[ScottamDendar]

Wow you've been churning on this. Sorry I haven't had time to help despite being the one that brought it up in the first place. I hope to have time to help review this tmrw

[LxKnsy]

Is extending sync/negotiate to cover states on the cards

possibly? depends how necessary it is to get this working/polished.

@gantoine Not blocking for this PR; saves work without it. But if states are meant to sync at all, the server is the cheaper place to put it, and here's why.

Argosy already syncs states, and for 16 emulators, not just RetroArch. It does it entirely over plain /api/states CRUD, which means it has to carry its own bookkeeping to make sense of them: a state cache, an ownership tracker, and tombstones; the StateCacheManager alone is ~1400 lines. The tombstones exist because without server support there's no way to tell a state that was deleted from one that was never there.

That's not a knock on Argosy. State records have none of content_hash, slot, origin_device_id or device_syncs, so there was nothing else to build on. But it does mean every client that wants states re-implements reconciliation independently and they'll disagree the moment two of them touch the same game, which is the thing RFC-0001 set out to stop.

So: no urgency from my side. Just that doing it in negotiate is one implementation instead of N, and it would let Argosy delete a chunk rather than have Playnite grow its own version of it.

[ScottamDendar]

The only thing Im concerned with is the ISaveHandler needing to be implemented per emulator. That feels clunky considering how many different emulators there are out there. Maybe this could be refined in a follow-up since this PR is working, but get the SaveHandling to go along with the "Emulator path mapping" settings in the "Configure Integration" window. So when you configure your emulator paths, you can also set anything necessary to get the saveHandling to work.

[ScottamDendar]

Nitpick (as in take it or leave it): Maybe move the ISaveHandler and the RetroHandler into its own Saves/Handler dir? If we expect multiple handlers to exist at some point.

[ScottamDendar]

I'll checkout the branch (hopefully tmrw but you never know) to assure it works on another setup