Resolve saves from the platform mapping, not only the play action

Games/RomMGameData.cs

@@ -0,0 +1,81 @@
+using Newtonsoft.Json;
+using Playnite.SDK;
+using RomM.Models.RomM.Rom;
+using System;
+using System.IO;
+
+namespace RomM.Games
+{
+    /// <summary>
+    /// The per-ROM sidecar ("{sha1}.json" under the plugin's data folder): the download
+    /// descriptors for every revision, the emulator mapping the ROM was imported under, and what
+    /// the importer last wrote onto the game's play action.
+    ///
+    /// Install, uninstall, the version menu and save sync all need it, and each used to carry its
+    /// own copy of the path building, the id parsing and the try/catch around a corrupt file.
+    /// </summary>
+    internal static class RomMGameData
+    {
+        public static string PathFor(string romDataPath, string sha1) =>
+            Path.Combine(romDataPath ?? string.Empty, $"{sha1}.json");
+
+        /// <summary>
+        /// The sidecar for a game id, or null when the id is malformed, the file is missing or its
+        /// contents cannot be read. What a missing sidecar means is the caller's business.
+        /// </summary>
+        public static RomMRomLocal Load(string romDataPath, string gameId, ILogger logger, string gameName = null)
+        {
+            if (!RomMGameId.TryParse(gameId, out int _, out string sha1))
+            {
+                logger?.Error($"{gameName ?? gameId} GameID is malformed!");
+                return null;
+            }
+
+            return LoadBySha1(romDataPath, sha1, logger, out string _, gameName);
+        }
+
+        /// <param name="rawJson">
+        /// The file's text exactly as read, or null when there was none. A caller about to rewrite
+        /// the sidecar compares against this to decide whether anything changed, rather than
+        /// re-serialising what it has just parsed once per ROM.
+        /// </param>
+        public static RomMRomLocal LoadBySha1(string romDataPath, string sha1, ILogger logger, out string rawJson, string gameName = null)
+        {
+            rawJson = null;
+
+            var path = PathFor(romDataPath, sha1);
+            if (!File.Exists(path))
+                return null;
+
+            try
+            {
+                var text = File.ReadAllText(path);
+                var data = JsonConvert.DeserializeObject<RomMRomLocal>(text);
+                rawJson = text;
+                return data;
+            }
+            catch (IOException ex)
+            {
+                // Save sync reads the sidecar off the UI thread while an import may be rewriting it;
+                // that is a transient sharing violation, not a corrupt file.
+                logger?.Warn(ex, $"{gameName ?? sha1} ROM data file could not be read.");
+                return null;
+            }
+            catch (Exception ex)
+            {
+                logger?.Error(ex, $"{gameName ?? sha1} ROM data file is corrupted!");
+                return null;
+            }
+        }
+
+        public static void Save(string romDataPath, string sha1, RomMRomLocal data) =>
+            SaveJson(romDataPath, sha1, JsonConvert.SerializeObject(data));
+
+        /// <summary>
+        /// For a caller that has already serialised the sidecar to compare it with what was on
+        /// disk, so it is not serialised a second time just to be written.
+        /// </summary>
+        public static void SaveJson(string romDataPath, string sha1, string json) =>
+            File.WriteAllText(PathFor(romDataPath, sha1), json);
+    }
+}

Games/RomMGameInfo.Plugin.cs

@@ -20,7 +20,7 @@ public EmulatorMapping Mapping
         {
             get
             {
-                return Settings.SettingsViewModel.Instance.Mappings.FirstOrDefault(m => m.MappingId == MappingId);
+                return Settings.SettingsViewModel.Instance.MappingById(MappingId);
             }
         }
 

Games/RomMImport.cs

@@ -25,6 +25,12 @@ internal class RomMImport
         Dictionary<string, Game> _legacyGames;         // unmigrated "!0..." games, keyed by GameId
         Dictionary<string, Guid> _completionStatusMap;
         List<int> _favourites;
+        // Two enabled mappings on one platform both walk the same ROMs, and each pass would repoint
+        // the play actions the other just wrote. Neither is more right than the other, so when that
+        // is the setup the actions are left exactly as they are.
+        bool _platformHasRivalMapping;
+        // The emulator and profile this import writes onto play actions.
+        readonly AppliedPlayAction _mapped;
 
         public RomMImport(RomM plugin, LibraryImportGamesArgs args, EmulatorMapping mapping, List<RomMRom> roms, List<int> favourites)
         {
@@ -61,6 +67,12 @@ public RomMImport(RomM plugin, LibraryImportGamesArgs args, EmulatorMapping mapp
 
             _completionStatusMap = plugin.Playnite.Database.CompletionStatuses.ToDictionary(cs => cs.Name, cs => cs.Id);
             _favourites = favourites;
+
+            // Only mappings the import controller will actually run count: one it skips as
+            // misconfigured writes nothing, so it cannot fight this one over the actions.
+            _platformHasRivalMapping = plugin.Settings?.HasRivalMapping(mapping) == true;
+
+            _mapped = new AppliedPlayAction(mapping.EmulatorId, mapping.EmulatorProfileId);
         }
 
         // Builds the per-ROM download descriptor via the shared factory (see RomMRevisionFactory).
@@ -125,38 +137,54 @@ public List<Game> ProcessData()
                         }
                     }
 
-                    // Save Game ROM data to file
-                    SaveGameData(ROM);
+                    // What the sidecar held before this import overwrites it: the record of the
+                    // play action the plugin last wrote, which decides whether that action is
+                    // still ours to repoint at the mapping's current emulator.
+                    var previous = RomMGameData.LoadBySha1(_plugin.ROMDataPath, ROM.SHA1, _plugin.Logger,
+                                                          out string previousJson, ROM.Name);
 
                     // Skip full import if ROM has already been imported
                     Guid statusID = Guid.Empty;
                     if (_existingGames.TryGetValue(gameID, out var existingGame))
                     {
+                        bool changed = RefreshPlayAction(existingGame, previous, out var applied);
+
                         // Sync user data
                         if (_plugin.Settings.KeepRomMSynced)
                         {
                             statusID = DetermineCompletionStatus(ROM);
                             existingGame.Favorite = _favourites.Exists(f => f == ROM.Id);
                             if (statusID != Guid.Empty)
                                 existingGame.CompletionStatusId = statusID;
-
-                            // This is our own write of the server's values; don't let OnItemUpdated echo it back.
-                            _plugin.SuppressSync(existingGame.Id);
-                            _plugin.Playnite.Database.Games.Update(existingGame);
+                            changed = true;
                         }
 
+                        if (changed)
+                            SaveOwnWrite(existingGame);
+
+                        // Save Game ROM data to file
+                        SaveGameData(ROM, previous, previousJson, applied);
                         importedGameIds.Add(gameID);
                         continue;
                     }
 
                     // If keep deleted games is enabled and a deleted game gets re-added back to the
                     // server under a new romMId, update the existing playnite entry instead.
-                    if (_plugin.Settings.KeepDeletedGames && UpdatedDeletedGame(ROM))
+                    if (_plugin.Settings.KeepDeletedGames && UpdatedDeletedGame(ROM, out var adoptedGame))
                     {
+                        // The adopted entry is an existing game re-keyed under the new id, so its
+                        // action is refreshed on the same terms as any other existing game's.
+                        if (RefreshPlayAction(adoptedGame, previous, out var applied))
+                            SaveOwnWrite(adoptedGame);
+
+                        SaveGameData(ROM, previous, previousJson, applied);
                         importedGameIds.Add(gameID);
                         continue;
                     }
 
+                    // A new game gets the mapping's emulator outright, below.
+                    SaveGameData(ROM, previous, previousJson, _mapped);
+
                     var importedGame = ImportGame(ROM, statusID);
                     if (importedGame != null)
                     {
@@ -235,14 +263,7 @@ private Game ImportGame(RomMRom ROM, Guid StatusID)
             metadata.InstallSize = ROM.FileSizeBytes;
             metadata.GameActions = new List<GameAction>
             {
-                new GameAction
-                {
-                    Name = $"Play in {_mapping.Emulator.Name}",
-                    Type = GameActionType.Emulator,
-                    EmulatorId = _mapping.EmulatorId,
-                    EmulatorProfileId = _mapping.EmulatorProfileId,
-                    IsPlayAction = true,
-                },
+                RomMPlayAction.Build(_mapping.Emulator.Name, _mapped.EmulatorId, _mapped.ProfileId),
                 new GameAction
                 {
                     Type = GameActionType.URL,
@@ -351,7 +372,7 @@ private bool UpdatedOldGameID(RomMRom ROM)
             return false;
         }
 
-        private bool UpdatedDeletedGame(RomMRom ROM)
+        private bool UpdatedDeletedGame(RomMRom ROM, out Game adopted)
         {
             // A game with the same SHA1 but a different romMId means RomM deleted and re-added it; adopt
             // the existing entry under the new id.
@@ -364,22 +385,26 @@ private bool UpdatedDeletedGame(RomMRom ROM)
 
                 _existingGames.Remove(oldId);
                 _existingGames[newId] = oldgame;
+                adopted = oldgame;
                 return true;
             }
 
+            adopted = null;
             return false;
         }
 
         private MainSibling CheckForMainSibling(RomMRom ROM)
             => RomMSiblings.ClassifyMain(ROM, _romById);
 
-        private void SaveGameData(RomMRom ROM)
+        private void SaveGameData(RomMRom ROM, RomMRomLocal previous, string previousJson, AppliedPlayAction applied)
         {
             RomMRomLocal toSave = new RomMRomLocal
             {
                 Name = ROM.Name,
                 SHA1 = ROM.SHA1,
                 MappingID = _mapping.MappingId,
+                AppliedEmulatorID = applied.EmulatorId,
+                AppliedEmulatorProfileID = applied.ProfileId,
                 ROMVersions = new List<RomMRevision>()
             };
 
@@ -413,30 +438,79 @@ private void SaveGameData(RomMRom ROM)
             }
 
             // Carry over the user's previously selected version and only rewrite when something changed.
-            string sidecarPath = $"{_plugin.ROMDataPath}{ROM.SHA1}.json";
-            string existingJson = null;
-            if (File.Exists(sidecarPath))
+            foreach (var revision in previous?.ROMVersions ?? new List<RomMRevision>())
             {
-                try
-                {
-                    existingJson = File.ReadAllText(sidecarPath);
-                    var localROM = JsonConvert.DeserializeObject<RomMRomLocal>(existingJson);
-                    foreach (var revision in localROM?.ROMVersions ?? new List<RomMRevision>())
-                    {
-                        var matchedRevision = toSave.ROMVersions.FirstOrDefault(x => x.Id == revision.Id);
-                        if (matchedRevision != null)
-                            matchedRevision.IsSelected = revision.IsSelected;
-                    }
-                }
-                catch (Exception)
-                {
-                    _plugin.Logger.Error($"{ROM.Name} GameID is malformed or {ROM.SHA1} json file is corrupted!");
-                }
+                var matchedRevision = toSave.ROMVersions.FirstOrDefault(x => x.Id == revision.Id);
+                if (matchedRevision != null)
+                    matchedRevision.IsSelected = revision.IsSelected;
             }
 
             string json = JsonConvert.SerializeObject(toSave);
-            if (json != existingJson)
-                File.WriteAllText(sidecarPath, json);
+            if (json != previousJson)
+                RomMGameData.SaveJson(_plugin.ROMDataPath, ROM.SHA1, json);
+        }
+
+        // Saves a change the importer made to a game; OnItemUpdated must not echo it back to RomM.
+        private void SaveOwnWrite(Game game)
+        {
+            _plugin.SuppressSync(game.Id);
+            _plugin.Playnite.Database.Games.Update(game);
+        }
+
+        /// <summary>
+        /// Brings an already-imported game's play action back in step with its mapping, and reports
+        /// the action the sidecar should now record. Returns whether the action was changed; the
+        /// caller saves the game, so a repoint and a user-data sync share one write.
+        ///
+        /// Existing games are otherwise skipped wholesale by the importer, so a mapping repointed
+        /// at another emulator left every game it had imported launching -- and resolving its saves
+        /// against -- the old one. Only an action the plugin still owns is rewritten; once the user
+        /// has repointed it themselves it is theirs, and the sidecar keeps remembering what we last
+        /// wrote so that stays true across imports.
+        /// </summary>
+        private bool RefreshPlayAction(Game game, RomMRomLocal previous, out AppliedPlayAction result)
+        {
+            result = new AppliedPlayAction(previous?.AppliedEmulatorID ?? Guid.Empty,
+                                           previous?.AppliedEmulatorProfileID);
+            var applied = result;
+
+            // No action to keep in step.
+            var action = RomMPlayAction.Find(game?.GameActions);
+            if (action == null || _platformHasRivalMapping)
+                return false;
+
+            Func<string> actionEmulatorName = () =>
+                _plugin.Playnite.Database.Emulators?.Get(action.EmulatorId)?.Name;
+
+            // Already in step. It only becomes ours to record if it was ours before: a user who set
+            // this emulator themselves and then pointed the mapping at the same one still owns it,
+            // and must not have it repointed when the mapping moves on.
+            if (RomMPlayAction.Matches(action, _mapped))
+            {
+                if (RomMPlayAction.IsUnedited(action, applied, actionEmulatorName))
+                    result = _mapped;
+                return false;
+            }
+
+            // A sidecar from before the plugin recorded what it applied leaves only the generated
+            // name to go on, and that name records which emulator the importer chose, not which
+            // profile. On the mapping's own emulator a different profile is as likely a core the
+            // user picked for this one game as a mapping edit, so it is left alone.
+            if (applied.EmulatorId == Guid.Empty && action.EmulatorId == _mapped.EmulatorId)
+                return false;
+
+            if (!RomMPlayAction.IsUnedited(action, applied, actionEmulatorName))
+            {
+                _plugin.Logger.Info($"[Importer] Leaving {game.Name}'s play action pointed at " +
+                                    $"{actionEmulatorName() ?? "<Unknown>"}: it is no longer the one the plugin wrote.");
+                return false;
+            }
+
+            RomMPlayAction.Apply(action, _mapping.Emulator.Name, _mapped.EmulatorId, _mapped.ProfileId);
+            _plugin.Logger.Info($"[Importer] Repointed {game.Name}'s play action at {_mapping.Emulator.Name} " +
+                                $"to follow the {_mapping.MappingName} mapping.");
+            result = _mapped;
+            return true;
         }
 
         private Guid DetermineCompletionStatus(RomMRom ROM)

Games/RomMImportController.cs

@@ -59,7 +59,7 @@ public List<Game> Import(LibraryImportGamesArgs args)
                     break;
 
                 // A mapping with no emulator/profile is genuinely unconfigured — skip quietly.
-                if (mapping.Emulator == null || mapping.EmulatorProfile == null)
+                if (!mapping.HasEmulatorProfile)
                 {
                     Logger.Warn($"[Import Controller] Emulator {mapping.MappingId} is misconfigured, skipping.");
                     continue;
@@ -70,7 +70,7 @@ public List<Game> Import(LibraryImportGamesArgs args)
                 // is carried over. Give the user an actionable message instead of a cryptic
                 // "-1 not found". The <= 0 check covers both the unset RomMPlatformId (-1) and the
                 // empty default RomMPlatform (Id 0).
-                if (mapping.RomMPlatformId <= 0 || mapping.RomMPlatform == null || mapping.RomMPlatform.Id <= 0)
+                if (!mapping.HasRomMPlatform)
                 {
                     var name = !string.IsNullOrWhiteSpace(mapping.MappingName)
                         ? mapping.MappingName