Skip to content

Commit 4d0e12b

Browse files
LxKnsyclaude
andcommitted
Give saves a slot, and download into the core's own folder
Two things that between them made save sync a one-way trip. sync/negotiate only considers saves that carry a slot. We uploaded without one, so every save this plugin wrote landed on the server correctly and was then invisible to the sync layer - not offered to any device, not matched when reported back. Negotiate answered "Save exists on client but not on server" even for a byte-identical copy of what it was storing, so the local side always counted as new and won unconditionally. An empty save could quietly replace a good one; that is how I lost one while testing this. Measured against a server holding both kinds: 40 saves with a slot, 27 of them offered for download; 8 without one, none ever offered, to any device. The slot-less ones were exactly those written by clients that omit it. Uploading the same file with slot=autosave makes it show up as a download operation immediately. Other RomM clients use "autosave" for a game's live save whatever the platform, and the server keys saves by (rom_id, slot), so a different value here would split one game's save into two entries that never reconcile. The second one only surfaces once downloads happen at all. With sort_savefiles_enable, RetroArch keeps saves in a folder named after the running core. We resolved the path without a core name, which is harmless while a local save exists - 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, where RetroArch never looks. The game then started fresh on a save that was sitting right there, and that fresh save went back up on exit. The core 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 already exists its spelling wins, since RetroArch's own name for a core ("mGBA") is not always how Playnite spells it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent cdf05ad commit 4d0e12b

7 files changed

Lines changed: 177 additions & 9 deletions

File tree

RomM.Tests/FileSaveTargetTests.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ public void Reports_absent_when_no_save_has_been_written_yet()
3131
Assert.False(target.Exists);
3232
}
3333

34+
// sync/negotiate skips saves without a slot entirely: they upload fine and are then
35+
// invisible to every device, including the one that wrote them. "autosave" is what the
36+
// other RomM clients use for a game's live save.
37+
[Fact]
38+
public void Carries_a_slot_so_the_save_is_visible_to_negotiate()
39+
{
40+
Assert.Equal("autosave", new FileSaveTarget("retroarch", Path_("game.srm"), null).Slot);
41+
}
42+
3443
[Fact]
3544
public void Reports_the_file_it_found_rather_than_the_configured_path()
3645
{
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
using Playnite.SDK.Models;
2+
using RomM.Saves;
3+
using Xunit;
4+
5+
namespace RomM.Tests
6+
{
7+
public class RetroArchSaveHandlerTests
8+
{
9+
// Playnite names its built-in RetroArch profiles after the core, and RetroArch names the
10+
// per-core save folder the same way, so the profile name is the value we want verbatim.
11+
[Fact]
12+
public void Core_name_of_a_builtin_profile_is_its_name()
13+
{
14+
var profile = new BuiltInEmulatorProfile { Name = "mGBA" };
15+
16+
Assert.Equal("mGBA", RetroArchSaveHandler.ResolveCoreName(profile));
17+
}
18+
19+
// A custom profile only carries the core in its libretro argument, as a dll path.
20+
[Theory]
21+
[InlineData("-L \"cores\\mgba_libretro.dll\" \"{ImagePath}\"", "mgba")]
22+
[InlineData("-L cores\\snes9x_libretro.dll \"{ImagePath}\"", "snes9x")]
23+
[InlineData("-f -L \"D:\\RetroArch\\cores\\gambatte_libretro.dll\"", "gambatte")]
24+
public void Core_name_of_a_custom_profile_comes_from_the_libretro_argument(string args, string expected)
25+
{
26+
var profile = new CustomEmulatorProfile { Arguments = args };
27+
28+
Assert.Equal(expected, RetroArchSaveHandler.ResolveCoreName(profile));
29+
}
30+
31+
// No core name means the per-core folder is simply left out of the path, which is the
32+
// behaviour that existed before — not a reason to fail resolution.
33+
[Theory]
34+
[InlineData("\"{ImagePath}\"")]
35+
[InlineData("")]
36+
[InlineData(null)]
37+
public void Core_name_is_null_when_the_arguments_carry_none(string args)
38+
{
39+
Assert.Null(RetroArchSaveHandler.ResolveCoreName(new CustomEmulatorProfile { Arguments = args }));
40+
}
41+
42+
[Fact]
43+
public void Core_name_is_null_without_a_profile()
44+
{
45+
Assert.Null(RetroArchSaveHandler.ResolveCoreName(null));
46+
}
47+
48+
[Fact]
49+
public void Builtin_profile_without_a_name_yields_null()
50+
{
51+
Assert.Null(RetroArchSaveHandler.ResolveCoreName(new BuiltInEmulatorProfile { Name = " " }));
52+
}
53+
}
54+
}

Saves/FileSaveTarget.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ public FileSaveTarget(string emulatorTag, string writePath, string existingPath)
2929

3030
public override string EmulatorTag { get; }
3131

32+
public override string Slot => "autosave";
33+
3234
public override bool Exists => File.Exists(_readPath);
3335

3436
public override string FileName => Path.GetFileName(_readPath);

Saves/ISaveHandler.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ internal class SaveTargetRequest
3737

3838
public Emulator Emulator { get; set; }
3939

40+
/// <summary>
41+
/// The profile the game launches with, when one is set. Carries the detail that decides
42+
/// where some emulators file a save — for RetroArch, which core is running.
43+
/// </summary>
44+
public EmulatorProfile Profile { get; set; }
45+
4046
/// <summary>The ROM's path with Playnite's variables already expanded.</summary>
4147
public string ContentPath { get; set; }
4248

Saves/RetroArchSaveHandler.cs

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Collections.Generic;
44
using System.IO;
55
using System.Linq;
6+
using System.Text.RegularExpressions;
67

78
namespace RomM.Saves
89
{
@@ -50,19 +51,24 @@ public SaveTarget ResolveTarget(SaveTargetRequest request)
5051
: new Dictionary<string, string>();
5152

5253
var baseDir = request.Emulator.InstallDir;
54+
var saveRoot = RetroArchConfig.ResolveSaveBaseDirectory(cfg, request.ContentPath, baseDir);
5355

54-
var expectedPath = RetroArchConfig.ResolveSaveFilePath(cfg, request.ContentPath, null, baseDir);
56+
// With sort_savefiles_enable the save sits in a folder named after the running core.
57+
// Resolving without that name only matters once a download has to create the file:
58+
// it would land beside the core folders instead of inside the right one, where
59+
// RetroArch never looks, and the game would start over on a save that is present.
60+
var coreName = MatchExistingCoreFolder(saveRoot, ResolveCoreName(request.Profile));
61+
62+
var expectedPath = RetroArchConfig.ResolveSaveFilePath(cfg, request.ContentPath, coreName, baseDir);
5563
if (string.IsNullOrEmpty(expectedPath))
5664
return null;
5765

5866
// The configured path is where RetroArch *would* write. When nothing is there, the
59-
// save may still exist under a per-core or per-content subfolder we did not model,
60-
// so fall back to searching for it by ROM name before assuming there is none.
67+
// save may still exist under a subfolder we did not model, so fall back to
68+
// searching for it by ROM name before assuming there is none.
6169
var existing = File.Exists(expectedPath)
6270
? expectedPath
63-
: FindExistingSave(
64-
RetroArchConfig.ResolveSaveBaseDirectory(cfg, request.ContentPath, baseDir),
65-
Path.GetFileNameWithoutExtension(request.ContentPath));
71+
: FindExistingSave(saveRoot, Path.GetFileNameWithoutExtension(request.ContentPath));
6672

6773
return new FileSaveTarget(EmulatorTag, expectedPath, existing);
6874
}
@@ -73,6 +79,72 @@ public SaveTarget ResolveTarget(SaveTargetRequest request)
7379
}
7480
}
7581

82+
/// <summary>
83+
/// The core a profile runs, as far as it can be told from Playnite. Built-in RetroArch
84+
/// profiles are named after the core ("mGBA"); a custom profile carries it in the libretro
85+
/// argument (`-L "cores\mgba_libretro.dll"`). Null when neither yields anything, which
86+
/// leaves the per-core folder out of the path exactly as before.
87+
/// </summary>
88+
internal static string ResolveCoreName(EmulatorProfile profile)
89+
{
90+
var builtIn = profile as BuiltInEmulatorProfile;
91+
if (builtIn != null)
92+
return string.IsNullOrWhiteSpace(builtIn.Name) ? null : builtIn.Name.Trim();
93+
94+
var custom = profile as CustomEmulatorProfile;
95+
if (custom != null)
96+
return CoreFromArguments(custom.Arguments);
97+
98+
return null;
99+
}
100+
101+
private static readonly Regex LibretroArgument =
102+
new Regex(@"-L\s+""?(?<path>[^""\s]+)""?", RegexOptions.IgnoreCase);
103+
104+
private static string CoreFromArguments(string arguments)
105+
{
106+
if (string.IsNullOrEmpty(arguments))
107+
return null;
108+
109+
var match = LibretroArgument.Match(arguments);
110+
if (!match.Success)
111+
return null;
112+
113+
var name = Path.GetFileNameWithoutExtension(match.Groups["path"].Value);
114+
if (string.IsNullOrEmpty(name))
115+
return null;
116+
117+
if (name.EndsWith("_libretro", StringComparison.OrdinalIgnoreCase))
118+
name = name.Substring(0, name.Length - "_libretro".Length);
119+
120+
return name.Length == 0 ? null : name;
121+
}
122+
123+
/// <summary>
124+
/// RetroArch names the folder after the core's own display name, which is not always how
125+
/// Playnite spells it — a profile can yield "mgba" where the folder on disk is "mGBA".
126+
/// Where a matching folder already exists its spelling wins, so a download joins the saves
127+
/// RetroArch is already writing instead of creating a near-duplicate beside them.
128+
/// </summary>
129+
private static string MatchExistingCoreFolder(string saveRoot, string coreName)
130+
{
131+
if (string.IsNullOrEmpty(coreName) || string.IsNullOrEmpty(saveRoot) || !Directory.Exists(saveRoot))
132+
return coreName;
133+
134+
try
135+
{
136+
var match = Directory.EnumerateDirectories(saveRoot)
137+
.Select(Path.GetFileName)
138+
.FirstOrDefault(n => string.Equals(n, coreName, StringComparison.OrdinalIgnoreCase));
139+
140+
return match ?? coreName;
141+
}
142+
catch
143+
{
144+
return coreName;
145+
}
146+
}
147+
76148
private static string FindConfig(Emulator emulator)
77149
{
78150
if (!string.IsNullOrEmpty(emulator.InstallDir))

Saves/SaveSyncService.cs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ private RomMSyncNegotiateResponse Negotiate(string deviceId, int romId, SaveTarg
140140
{
141141
RomId = romId,
142142
FileName = target.FileName,
143-
Slot = null,
143+
Slot = target.Slot,
144144
Emulator = target.EmulatorTag,
145145
ContentHash = target.ContentHash(),
146146
UpdatedAt = target.UpdatedAtUtc,
@@ -254,6 +254,7 @@ private bool Upload(RomMSyncOperation op, string deviceId, int sessionId, SaveTa
254254
{
255255
var url = RomMUrl.Combine(Settings.RomMHost,
256256
$"api/saves?rom_id={op.RomId}&emulator={target.EmulatorTag}" +
257+
$"&slot={WebUtility.UrlEncode(target.Slot)}" +
257258
$"&device_id={WebUtility.UrlEncode(deviceId)}&session_id={sessionId}");
258259
response = HttpClientSingleton.Instance.PostAsync(url, content).GetAwaiter().GetResult();
259260
}
@@ -404,22 +405,37 @@ private SaveTarget ResolveTarget(Game game)
404405
{
405406
Game = game,
406407
Emulator = emulator,
408+
Profile = ResolveProfile(game, emulator),
407409
ContentPath = _romM.Playnite.ExpandGameVariables(game, contentPath),
408410
Logger = Logger,
409411
});
410412
}
411413

412414
private Emulator ResolveEmulator(Game game)
413415
{
414-
var action = game.GameActions?.FirstOrDefault(a => a.IsPlayAction && a.Type == GameActionType.Emulator)
415-
?? game.GameActions?.FirstOrDefault(a => a.Type == GameActionType.Emulator);
416+
var action = EmulatorAction(game);
416417

417418
if (action != null && action.EmulatorId != Guid.Empty)
418419
return _romM.Playnite.Database.Emulators?.FirstOrDefault(e => e.Id == action.EmulatorId);
419420

420421
return null;
421422
}
422423

424+
private static EmulatorProfile ResolveProfile(Game game, Emulator emulator)
425+
{
426+
var profileId = EmulatorAction(game)?.EmulatorProfileId;
427+
if (string.IsNullOrEmpty(profileId))
428+
return null;
429+
430+
return emulator.SelectableProfiles?.FirstOrDefault(p => p.Id == profileId);
431+
}
432+
433+
private static GameAction EmulatorAction(Game game)
434+
{
435+
return game.GameActions?.FirstOrDefault(a => a.IsPlayAction && a.Type == GameActionType.Emulator)
436+
?? game.GameActions?.FirstOrDefault(a => a.Type == GameActionType.Emulator);
437+
}
438+
423439
#endregion
424440

425441
#region HTTP helper

Saves/SaveTarget.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ internal abstract class SaveTarget
2323
/// </summary>
2424
public abstract string EmulatorTag { get; }
2525

26+
/// <summary>
27+
/// The slot the save is filed under. Not optional in practice: `sync/negotiate` only
28+
/// considers saves that carry one, so a slot-less upload lands on the server correctly and
29+
/// is then invisible to every device, including the one that wrote it. Other RomM clients
30+
/// use "autosave" for a game's live save regardless of platform, and matching that is what
31+
/// keeps the same save reconcilable across them.
32+
/// </summary>
33+
public abstract string Slot { get; }
34+
2635
/// <summary>Whether there is anything locally to report or upload yet.</summary>
2736
public abstract bool Exists { get; }
2837

0 commit comments

Comments
 (0)