From c5be446701d4e24a03d8464e9b080ce74d158223 Mon Sep 17 00:00:00 2001
From: Jesse Plamondon-Willard <Pathoschild@users.noreply.github.com>
Date: Wed, 6 Jan 2021 00:29:39 -0500
Subject: rework vanilla tilesheet checking to avoid keeping a copy of the
 vanilla maps in memory

---
 src/SMAPI/Framework/Content/TilesheetReference.cs  | 33 +++++++++++++++++
 src/SMAPI/Framework/ContentCoordinator.cs          | 43 ++++++++++++++++------
 .../ContentManagers/GameContentManager.cs          | 32 ++++++----------
 3 files changed, 76 insertions(+), 32 deletions(-)
 create mode 100644 src/SMAPI/Framework/Content/TilesheetReference.cs

(limited to 'src/SMAPI')

diff --git a/src/SMAPI/Framework/Content/TilesheetReference.cs b/src/SMAPI/Framework/Content/TilesheetReference.cs
new file mode 100644
index 00000000..2ea38430
--- /dev/null
+++ b/src/SMAPI/Framework/Content/TilesheetReference.cs
@@ -0,0 +1,33 @@
+namespace StardewModdingAPI.Framework.Content
+{
+    /// <summary>Basic metadata about a vanilla tilesheet.</summary>
+    internal class TilesheetReference
+    {
+        /*********
+        ** Accessors
+        *********/
+        /// <summary>The tilesheet's index in the list.</summary>
+        public readonly int Index;
+
+        /// <summary>The tilesheet's unique ID in the map.</summary>
+        public readonly string Id;
+
+        /// <summary>The asset path for the tilesheet texture.</summary>
+        public readonly string ImageSource;
+
+
+        /*********
+        ** Public methods
+        *********/
+        /// <summary>Construct an instance.</summary>
+        /// <param name="index">The tilesheet's index in the list.</param>
+        /// <param name="id">The tilesheet's unique ID in the map.</param>
+        /// <param name="imageSource">The asset path for the tilesheet texture.</param>
+        public TilesheetReference(int index, string id, string imageSource)
+        {
+            this.Index = index;
+            this.Id = id;
+            this.ImageSource = imageSource;
+        }
+    }
+}
diff --git a/src/SMAPI/Framework/ContentCoordinator.cs b/src/SMAPI/Framework/ContentCoordinator.cs
index 3d5bb29d..27fb3dbb 100644
--- a/src/SMAPI/Framework/ContentCoordinator.cs
+++ b/src/SMAPI/Framework/ContentCoordinator.cs
@@ -54,6 +54,9 @@ namespace StardewModdingAPI.Framework
         /// <remarks>The game may adds content managers in asynchronous threads (e.g. when populating the load screen).</remarks>
         private readonly ReaderWriterLockSlim ContentManagerLock = new ReaderWriterLockSlim();
 
+        /// <summary>A cache of ordered tilesheet IDs used by vanilla maps.</summary>
+        private readonly IDictionary<string, TilesheetReference[]> VanillaTilesheets = new Dictionary<string, TilesheetReference[]>(StringComparer.OrdinalIgnoreCase);
+
         /// <summary>An unmodified content manager which doesn't intercept assets, used to compare asset data.</summary>
         private readonly LocalizedContentManager VanillaContentManager;
 
@@ -293,21 +296,21 @@ namespace StardewModdingAPI.Framework
             });
         }
 
-        /// <summary>Get a vanilla asset without interception.</summary>
-        /// <typeparam name="T">The type of asset to load.</typeparam>
+        /// <summary>Get the tilesheet ID order used by the unmodified version of a map asset.</summary>
         /// <param name="assetName">The asset path relative to the loader root directory, not including the <c>.xnb</c> extension.</param>
-        public bool TryLoadVanillaAsset<T>(string assetName, out T asset)
+        public TilesheetReference[] GetVanillaTilesheetIds(string assetName)
         {
-            try
+            if (!this.VanillaTilesheets.TryGetValue(assetName, out TilesheetReference[] tilesheets))
             {
-                asset = this.VanillaContentManager.Load<T>(assetName);
-                return true;
-            }
-            catch
-            {
-                asset = default;
-                return false;
+                tilesheets = this.TryLoadVanillaAsset(assetName, out Map map)
+                    ? map.TileSheets.Select((sheet, index) => new TilesheetReference(index, sheet.Id, sheet.ImageSource)).ToArray()
+                    : null;
+
+                this.VanillaTilesheets[assetName] = tilesheets;
+                this.VanillaContentManager.Unload();
             }
+
+            return tilesheets ?? new TilesheetReference[0];
         }
 
         /// <summary>Dispose held resources.</summary>
@@ -341,5 +344,23 @@ namespace StardewModdingAPI.Framework
                 this.ContentManagers.Remove(contentManager)
             );
         }
+
+        /// <summary>Get a vanilla asset without interception.</summary>
+        /// <typeparam name="T">The type of asset to load.</typeparam>
+        /// <param name="assetName">The asset path relative to the loader root directory, not including the <c>.xnb</c> extension.</param>
+        /// <param name="asset">The loaded asset data.</param>
+        private bool TryLoadVanillaAsset<T>(string assetName, out T asset)
+        {
+            try
+            {
+                asset = this.VanillaContentManager.Load<T>(assetName);
+                return true;
+            }
+            catch
+            {
+                asset = default;
+                return false;
+            }
+        }
     }
 }
diff --git a/src/SMAPI/Framework/ContentManagers/GameContentManager.cs b/src/SMAPI/Framework/ContentManagers/GameContentManager.cs
index 424d6ff3..540475f1 100644
--- a/src/SMAPI/Framework/ContentManagers/GameContentManager.cs
+++ b/src/SMAPI/Framework/ContentManagers/GameContentManager.cs
@@ -11,7 +11,6 @@ using StardewModdingAPI.Framework.Reflection;
 using StardewModdingAPI.Framework.Utilities;
 using StardewValley;
 using xTile;
-using xTile.Tiles;
 
 namespace StardewModdingAPI.Framework.ContentManagers
 {
@@ -398,14 +397,13 @@ namespace StardewModdingAPI.Framework.ContentManagers
             }
 
             // when replacing a map, the vanilla tilesheets must have the same order and IDs
-            if (data is Map loadedMap && this.Coordinator.TryLoadVanillaAsset(info.AssetName, out Map vanillaMap))
+            if (data is Map loadedMap)
             {
-                for (int i = 0; i < vanillaMap.TileSheets.Count; i++)
+                TilesheetReference[] vanillaTilesheetRefs = this.Coordinator.GetVanillaTilesheetIds(info.AssetName);
+                foreach (TilesheetReference vanillaSheet in vanillaTilesheetRefs)
                 {
-                    // check for match
-                    TileSheet vanillaSheet = vanillaMap.TileSheets[i];
-                    bool found = this.TryFindTilesheet(loadedMap, vanillaSheet.Id, out int loadedIndex, out TileSheet loadedSheet);
-                    if (found && loadedIndex == i)
+                    // skip if match
+                    if (loadedMap.TileSheets.Count > vanillaSheet.Index && loadedMap.TileSheets[vanillaSheet.Index].Id == vanillaSheet.Id)
                         continue;
 
                     // handle mismatch
@@ -414,9 +412,9 @@ namespace StardewModdingAPI.Framework.ContentManagers
                         // This is temporary: mods shouldn't do this for any vanilla map, but these are the ones we know will crash. Showing a warning for others instead gives modders time to update their mods, while still simplifying troubleshooting.
                         bool isFarmMap = info.AssetNameEquals("Maps/Farm") || info.AssetNameEquals("Maps/Farm_Combat") || info.AssetNameEquals("Maps/Farm_Fishing") || info.AssetNameEquals("Maps/Farm_Foraging") || info.AssetNameEquals("Maps/Farm_FourCorners") || info.AssetNameEquals("Maps/Farm_Island") || info.AssetNameEquals("Maps/Farm_Mining");
 
-
-                        string reason = found
-                            ? $"mod reordered the original tilesheets, which {(isFarmMap ? "would cause a crash" : "often causes crashes")}.\n\nTechnical details for mod author:\nExpected order [{string.Join(", ", vanillaMap.TileSheets.Select(p => $"'{p.ImageSource}' (id: {p.Id})"))}], but found tilesheet '{vanillaSheet.Id}' at index {loadedIndex} instead of {i}. Make sure custom tilesheet IDs are prefixed with 'z_' to avoid reordering tilesheets."
+                        int loadedIndex = this.TryFindTilesheet(loadedMap, vanillaSheet.Id);
+                        string reason = loadedIndex != -1
+                            ? $"mod reordered the original tilesheets, which {(isFarmMap ? "would cause a crash" : "often causes crashes")}.\n\nTechnical details for mod author:\nExpected order [{string.Join(", ", vanillaTilesheetRefs.Select(p => $"'{p.ImageSource}' (id: {p.Id})"))}], but found tilesheet '{vanillaSheet.Id}' at index {loadedIndex} instead of {vanillaSheet.Index}. Make sure custom tilesheet IDs are prefixed with 'z_' to avoid reordering tilesheets."
                             : $"mod has no tilesheet with ID '{vanillaSheet.Id}'. Map replacements must keep the original tilesheets to avoid errors or crashes.";
 
                         SCore.DeprecationManager.PlaceholderWarn("3.8.2", DeprecationLevel.PendingRemoval);
@@ -436,23 +434,15 @@ namespace StardewModdingAPI.Framework.ContentManagers
         /// <summary>Find a map tilesheet by ID.</summary>
         /// <param name="map">The map whose tilesheets to search.</param>
         /// <param name="id">The tilesheet ID to match.</param>
-        /// <param name="index">The matched tilesheet index, if any.</param>
-        /// <param name="tilesheet">The matched tilesheet, if any.</param>
-        private bool TryFindTilesheet(Map map, string id, out int index, out TileSheet tilesheet)
+        private int TryFindTilesheet(Map map, string id)
         {
             for (int i = 0; i < map.TileSheets.Count; i++)
             {
                 if (map.TileSheets[i].Id == id)
-                {
-                    index = i;
-                    tilesheet = map.TileSheets[i];
-                    return true;
-                }
+                    return i;
             }
 
-            index = -1;
-            tilesheet = null;
-            return false;
+            return -1;
         }
     }
 }
-- 
cgit