Quest factory follows mod reloads; unknown saved quests kept; mod unit tests

manifest.json

@@ -8,6 +8,7 @@
   "sources": [
     "src/**/*.cs"
   ],
+  "test_sources": ["tests/*.cs"],
   "depends": ["characters_base>=1", "ui_base>=1"],
   "ui": [
     {

src/Quests/QuestFactory.cs

@@ -16,7 +16,8 @@ namespace Cthangover.Core.Quests
     /// definitions, on‑demand lookup by ID, and restoration of mutable state
     /// when the player loads a save file. The factory defers loading until
     /// the first access (via <see cref="Get"/> or <see cref="GetAll"/>) and
-    /// guards against re‑initialisation through the <c>isLoaded</c> flag.
+    /// rebuilds the definitions whenever the entity resolver hands out a new
+    /// data snapshot (a mod reload keeps this mod but may change the quests).
     /// Because quest definitions come from <see cref="QuestDefinition"/>
     /// objects collected by <see cref="ModManager.GetResolver{EntityResolver}()"/> through the
     /// DTO's <c>[EntityKind("quest")]</c> attribute, the factory remains
@@ -50,7 +51,8 @@ namespace Cthangover.Core.Quests
     public static QuestFactory Instance => instance.Value;
 
         private readonly IDictionary<string, QuestBase> dataByID = new Dictionary<string, QuestBase>();
-        private bool isLoaded;
+        private readonly List<QuestBase> unresolved = new();
+        private Dictionary<string, QuestDefinition> loadedDefinitions;
 		
         private QuestFactory()
         {
@@ -58,46 +60,45 @@ namespace Cthangover.Core.Quests
 
         /// <summary>
         /// Seeds the internal <c>dataByID</c> dictionary from mod JSON if it
-        /// has not been seeded yet. When <see cref="ModManager"/> is still
+        /// has not been seeded from the current data snapshot. When
-        /// mid‑initialisation, the call is silently deferred (logged at
+        /// <see cref="ModManager"/> is still mid‑initialisation, the call is
-        /// debug level) so that early code paths don't crash; the next
+        /// deferred so that early code paths don't crash; the next access
-        /// access will retry. Each <see cref="QuestDefinition"/> is
+        /// retries. <c>Entities.Collect</c> is cached by the entity resolver
-        /// converted into a fresh <see cref="QuestBase"/> with default
+        /// until the next mod reload, so a new dictionary means new data: the
-        /// status and an empty tag set.
+        /// quests are rebuilt from it, a quest that stays keeps its instance and
+        /// state, a new one starts with default status and an empty tag set.
         /// </summary>
         private void EnsureLoaded()
         {
-            if (isLoaded)
-                return;
-
             if (!ModManager.Instance.IsInitialized)
             {
                 GameLogger.Log("QUEST", "mods not initialized, quest loading deferred");
                 return;
             }
 
-            isLoaded = true;
+            Dictionary<string, QuestDefinition> definitions;
-
             try
             {
-                var definitions = Entities.Collect<QuestDefinition>();
+                definitions = Entities.Collect<QuestDefinition>();
-                if (definitions != null)
-                {
-                    foreach (var (id, def) in definitions)
-                    {
-                        var quest = new QuestBase
-                        {
-                            ID = def.ID,
-                            Name = def.Name,
-                            StatusToDescription = def.StatusToDescription ?? new Dictionary<int, string>(),
-                        };
-                        dataByID.Add(quest.ID, quest);
-                    }
-                }
             }
             catch (Exception ex)
             {
                 GameLogger.Log("QUEST", "failed to load quests from mods: " + ex.Message, LogLevel.Error);
+                return;
+            }
+
+            if (ReferenceEquals(definitions, loadedDefinitions))
+                return;
+            loadedDefinitions = definitions;
+
+            var previous = new Dictionary<string, QuestBase>(dataByID);
+            dataByID.Clear();
+            foreach (var def in definitions.Values)
+            {
+                var quest = previous.TryGetValue(def.ID, out var kept) ? kept : new QuestBase { ID = def.ID };
+                quest.Name = def.Name;
+                quest.StatusToDescription = def.StatusToDescription ?? new Dictionary<int, string>();
+                dataByID[quest.ID] = quest;
             }
 
             GameLogger.Log("QUEST", "loaded '" + dataByID.Count + "' quests");
@@ -127,6 +128,13 @@ namespace Cthangover.Core.Quests
         /// serialise the full quest state for the save file, and by the
         /// journal UI to populate the quest list window.
         /// </summary>
+        /// <summary>
+        /// Quests of the loaded save that no loaded mod defines (their mod left
+        /// the game). They take no part in the game, but the save writes them
+        /// back, so their progress returns with the mod. Cleared by a new game.
+        /// </summary>
+        public IReadOnlyList<QuestBase> Unresolved => unresolved;
+
         public List<QuestBase> GetAll()
         {
             EnsureLoaded();
@@ -140,8 +148,8 @@ namespace Cthangover.Core.Quests
         /// corresponding entry by ID, and overwrites its
         /// <see cref="QuestBase.Data"/> and <see cref="QuestBase.Status"/>
         /// properties. Entries whose ID is not registered (a removed mod or a
-        /// quest renamed between sessions) are skipped with a warning so that
+        /// quest renamed between sessions) go to <see cref="Unresolved"/> with
-        /// an ageing save cannot block loading. Reads from empty or <c>null</c>
+        /// a warning, so an ageing save neither blocks loading nor loses them. Reads from empty or <c>null</c>
         /// lists are silently ignored so that save files created before the
         /// quest system existed (or with no quest progress) do not break the
         /// pipeline. Publishes a single <see cref="QuestStateRestoredEvent"/>
@@ -152,13 +160,15 @@ namespace Cthangover.Core.Quests
         {
             EnsureLoaded();
 
+            unresolved.Clear();
             if(Lists.IsEmpty(quests))
                 return;
             foreach (var questFromSave in quests)
             {
                 if(!dataByID.TryGetValue(questFromSave.ID, out var quest))
                 {
-                    GameLogger.Log("QUEST", "quest '" + questFromSave.ID + "' from save is not registered, skipped", LogLevel.Warning);
+                    GameLogger.Log("QUEST", "quest '" + questFromSave.ID + "' from save is not registered; its state is kept in the save", LogLevel.Warning);
+                    unresolved.Add(questFromSave);
                     continue;
                 }
                 quest.Data   = questFromSave.Data;
@@ -183,6 +193,7 @@ namespace Cthangover.Core.Quests
         {
             EnsureLoaded();
 
+            unresolved.Clear();
             foreach (var quest in dataByID.Values)
             {
                 quest.Status = QuestStatus.NotStarted;

src/QuestsSaveable.cs

@@ -27,7 +27,9 @@ namespace Cthangover.Core.Quests
         /// <summary>Writes the full quest state list into the mod slot.</summary>
         public void OnSave(ModSlot slot)
         {
-            slot.Data[BlobKey] = new DataBlob(1, QuestFactory.Instance.GetAll());
+            var quests = QuestFactory.Instance.GetAll();
+            quests.AddRange(QuestFactory.Instance.Unresolved);
+            slot.Data[BlobKey] = new DataBlob(1, quests);
         }
 
         /// <summary>Restores quest state from the mod slot; an absent blob keeps fresh quest state.</summary>

tests/QuestsSaveableTests.cs

@@ -0,0 +1,48 @@
+namespace Cthangover.Mods.Quests.Tests
+{
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Text.Json;
+    using Cthangover.Core.Mods.Testing;
+    using Cthangover.Core.Quests;
+    using Cthangover.Core.Settings;
+
+    /// <summary>
+    /// The quests slot holds the state of every content mod's quests, so a
+    /// quest whose mod left the game is still in a loaded save: it must not
+    /// take part in the game, and the next save must write it back so its
+    /// progress returns with the mod.
+    /// </summary>
+    public class QuestsSaveableTests
+    {
+        private const string UnknownId = "__test_no_such_quest";
+
+        [ModTest("save/unknown_quest_survives_load_and_save")]
+        static void UnknownQuestSurvivesLoadAndSave(ModTestContext ctx)
+        {
+            var factory = QuestFactory.Instance;
+            var before = JsonSerializer.Serialize(factory.GetAll());
+            try
+            {
+                var unknown = new QuestBase { ID = UnknownId, Status = QuestStatus.Progress };
+                unknown.Data.Tags.Add("met");
+                var slot = new ModSlot();
+                slot.Data["Quests"] = new DataBlob(1, new List<QuestBase> { unknown });
+                var saveable = new QuestsSaveable();
+
+                ModAssert.True(saveable.OnLoad(slot, out _), "slot with an unknown quest loads");
+                ModAssert.False(factory.GetAll().Any(q => q.ID == UnknownId), "unknown quest takes no part in the game");
+
+                var saved = new ModSlot();
+                saveable.OnSave(saved);
+                var kept = saved.Data["Quests"].ReadJson<List<QuestBase>>().SingleOrDefault(q => q.ID == UnknownId);
+                ModAssert.NotNull(kept, "unknown quest written back");
+                ModAssert.True(kept.Status == QuestStatus.Progress && kept.Data.Tags.Contains("met"), "its status and tags kept");
+            }
+            finally
+            {
+                factory.SetAll(JsonSerializer.Deserialize<List<QuestBase>>(before));
+            }
+        }
+    }
+}