rework asset editor/loader tracking so they're affected by load order

docs/release-notes.md

@@ -19,9 +19,11 @@
 
 * For modders:
   * Asset propagation for player sprites now affects other players' sprites, and updates recolor maps (e.g. sleeves).
+  * Reworked the order that asset editors/loaders are called between multiple mods to support some framework mods like Content Patcher and Json Assets. Note that the order is undefined and should not be depended on.
   * Removed invalid-schedule validation which had false positives.
   * Fixed `helper.Data.Read/WriteGlobalData` using the `Saves` folder. The installer will move existing folders to the appdata folder.
   * Fixed dialogue asset changes not correctly propagated until the next day.
+  * Fixed issue where a mod which implemented `IAssetEditor`/`IAssetLoader` on its entry class could then remove itself from the editor/loader list.
 
 * For SMAPI/tool developers:
   * Added internal performance monitoring (thanks to Drachenkätzchen!). This is disabled by default in the current version, but can be enabled using the `performance` console command.

src/SMAPI/Framework/ContentCoordinator.cs

@@ -65,10 +65,10 @@ namespace StardewModdingAPI.Framework
         public LocalizedContentManager.LanguageCode Language => this.MainContentManager.Language;
 
         /// <summary>Interceptors which provide the initial versions of matching assets.</summary>
-        public IDictionary<IModMetadata, IList<IAssetLoader>> Loaders { get; } = new Dictionary<IModMetadata, IList<IAssetLoader>>();
+        public IList<ModLinked<IAssetLoader>> Loaders { get; } = new List<ModLinked<IAssetLoader>>();
 
         /// <summary>Interceptors which edit matching assets after they're loaded.</summary>
-        public IDictionary<IModMetadata, IList<IAssetEditor>> Editors { get; } = new Dictionary<IModMetadata, IList<IAssetEditor>>();
+        public IList<ModLinked<IAssetEditor>> Editors { get; } = new List<ModLinked<IAssetEditor>>();
 
         /// <summary>The absolute path to the <see cref="ContentManager.RootDirectory"/>.</summary>
         public string FullRootDirectory { get; }

src/SMAPI/Framework/ContentManagers/GameContentManager.cs

@@ -21,10 +21,10 @@ namespace StardewModdingAPI.Framework.ContentManagers
         private readonly ContextHash<string> AssetsBeingLoaded = new ContextHash<string>();
 
         /// <summary>Interceptors which provide the initial versions of matching assets.</summary>
-        private IDictionary<IModMetadata, IList<IAssetLoader>> Loaders => this.Coordinator.Loaders;
+        private IList<ModLinked<IAssetLoader>> Loaders => this.Coordinator.Loaders;
 
         /// <summary>Interceptors which edit matching assets after they're loaded.</summary>
-        private IDictionary<IModMetadata, IList<IAssetEditor>> Editors => this.Coordinator.Editors;
+        private IList<ModLinked<IAssetEditor>> Editors => this.Coordinator.Editors;
 
         /// <summary>A lookup which indicates whether the asset is localizable (i.e. the filename contains the locale), if previously loaded.</summary>
         private readonly IDictionary<string, bool> IsLocalizableLookup;
@@ -278,16 +278,16 @@ namespace StardewModdingAPI.Framework.ContentManagers
         private IAssetData ApplyLoader<T>(IAssetInfo info)
         {
             // find matching loaders
-            var loaders = this.GetInterceptors(this.Loaders)
+            var loaders = this.Loaders
                 .Where(entry =>
                 {
                     try
                     {
-                        return entry.Value.CanLoad<T>(info);
+                        return entry.Data.CanLoad<T>(info);
                     }
                     catch (Exception ex)
                     {
-                        entry.Key.LogAsMod($"Mod failed when checking whether it could load asset '{info.AssetName}', and will be ignored. Error details:\n{ex.GetLogSummary()}", LogLevel.Error);
+                        entry.Mod.LogAsMod($"Mod failed when checking whether it could load asset '{info.AssetName}', and will be ignored. Error details:\n{ex.GetLogSummary()}", LogLevel.Error);
                         return false;
                     }
                 })
@@ -298,14 +298,14 @@ namespace StardewModdingAPI.Framework.ContentManagers
                 return null;
             if (loaders.Length > 1)
             {
-                string[] loaderNames = loaders.Select(p => p.Key.DisplayName).ToArray();
+                string[] loaderNames = loaders.Select(p => p.Mod.DisplayName).ToArray();
                 this.Monitor.Log($"Multiple mods want to provide the '{info.AssetName}' asset ({string.Join(", ", loaderNames)}), but an asset can't be loaded multiple times. SMAPI will use the default asset instead; uninstall one of the mods to fix this. (Message for modders: you should usually use {typeof(IAssetEditor)} instead to avoid conflicts.)", LogLevel.Warn);
                 return null;
             }
 
             // fetch asset from loader
-            IModMetadata mod = loaders[0].Key;
+            IModMetadata mod = loaders[0].Mod;
-            IAssetLoader loader = loaders[0].Value;
+            IAssetLoader loader = loaders[0].Data;
             T data;
             try
             {
@@ -338,11 +338,11 @@ namespace StardewModdingAPI.Framework.ContentManagers
             IAssetData GetNewData(object data) => new AssetDataForObject(info, data, this.AssertAndNormalizeAssetName);
 
             // edit asset
-            foreach (var entry in this.GetInterceptors(this.Editors))
+            foreach (var entry in this.Editors)
             {
                 // check for match
-                IModMetadata mod = entry.Key;
+                IModMetadata mod = entry.Mod;
-                IAssetEditor editor = entry.Value;
+                IAssetEditor editor = entry.Data;
                 try
                 {
                     if (!editor.CanEdit<T>(info))
@@ -382,19 +382,5 @@ namespace StardewModdingAPI.Framework.ContentManagers
             // return result
             return asset;
         }
-
-        /// <summary>Get all registered interceptors from a list.</summary>
-        private IEnumerable<KeyValuePair<IModMetadata, T>> GetInterceptors<T>(IDictionary<IModMetadata, IList<T>> entries)
-        {
-            foreach (var entry in entries)
-            {
-                IModMetadata mod = entry.Key;
-                IList<T> interceptors = entry.Value;
-
-                // registered editors
-                foreach (T interceptor in interceptors)
-                    yield return new KeyValuePair<IModMetadata, T>(mod, interceptor);
-            }
-        }
     }
 }

src/SMAPI/Framework/ModLinked.cs

@@ -0,0 +1,29 @@
+namespace StardewModdingAPI.Framework
+{
+    /// <summary>A generic tuple which links something to a mod.</summary>
+    /// <typeparam name="T">The interceptor type.</typeparam>
+    internal class ModLinked<T>
+    {
+        /*********
+        ** Accessors
+        *********/
+        /// <summary>The mod metadata.</summary>
+        public IModMetadata Mod { get; }
+
+        /// <summary>The instance linked to the mod.</summary>
+        public T Data { get; }
+
+
+        /*********
+        ** Public methods
+        *********/
+        /// <summary>Construct an instance.</summary>
+        /// <param name="mod">The mod metadata.</param>
+        /// <param name="data">The instance linked to the mod.</param>
+        public ModLinked(IModMetadata mod, T data)
+        {
+            this.Mod = mod;
+            this.Data = data;
+        }
+    }
+}

src/SMAPI/Framework/SCore.cs

@@ -807,13 +807,13 @@ namespace StardewModdingAPI.Framework
                 {
                     // ReSharper disable SuspiciousTypeConversion.Global
                     if (metadata.Mod is IAssetEditor editor)
-                        helper.ObservableAssetEditors.Add(editor);
+                        this.ContentCore.Editors.Add(new ModLinked<IAssetEditor>(metadata, editor));
                     if (metadata.Mod is IAssetLoader loader)
-                        helper.ObservableAssetLoaders.Add(loader);
+                        this.ContentCore.Loaders.Add(new ModLinked<IAssetLoader>(metadata, loader));
                     // ReSharper restore SuspiciousTypeConversion.Global
 
-                    this.ContentCore.Editors[metadata] = helper.ObservableAssetEditors;
+                    helper.ObservableAssetEditors.CollectionChanged += (sender, e) => this.OnInterceptorsChanged(metadata, e.NewItems?.Cast<IAssetEditor>(), e.OldItems?.Cast<IAssetEditor>(), this.ContentCore.Editors);
-                    this.ContentCore.Loaders[metadata] = helper.ObservableAssetLoaders;
+                    helper.ObservableAssetLoaders.CollectionChanged += (sender, e) => this.OnInterceptorsChanged(metadata, e.NewItems?.Cast<IAssetLoader>(), e.OldItems?.Cast<IAssetLoader>(), this.ContentCore.Loaders);
                 }
 
                 // call entry method
@@ -862,6 +862,24 @@ namespace StardewModdingAPI.Framework
             this.ModRegistry.AreAllModsInitialized = true;
         }
 
+        /// <summary>Handle a mod adding or removing asset interceptors.</summary>
+        /// <typeparam name="T">The asset interceptor type (one of <see cref="IAssetEditor"/> or <see cref="IAssetLoader"/>).</typeparam>
+        /// <param name="mod">The mod metadata.</param>
+        /// <param name="added">The interceptors that were added.</param>
+        /// <param name="removed">The interceptors that were removed.</param>
+        /// <param name="list">The list to update.</param>
+        private void OnInterceptorsChanged<T>(IModMetadata mod, IEnumerable<T> added, IEnumerable<T> removed, IList<ModLinked<T>> list)
+        {
+            foreach (T interceptor in added ?? new T[0])
+                list.Add(new ModLinked<T>(mod, interceptor));
+
+            foreach (T interceptor in removed ?? new T[0])
+            {
+                foreach (ModLinked<T> entry in list.Where(p => p.Mod == mod && object.ReferenceEquals(p.Data, interceptor)).ToArray())
+                    list.Remove(entry);
+            }
+        }
+
         /// <summary>Load a given mod.</summary>
         /// <param name="mod">The mod to load.</param>
         /// <param name="mods">The mods being loaded.</param>