fix some assets not reapplied correctly when playing in non-English and returning to title

docs/release-notes.md

@@ -11,6 +11,9 @@
 * For players:
   * Aggressive memory optimization (added in 3.9.2) is now disabled by default. The option reduces errors for a subset of players who use certain mods, but may cause crashes for farmhands in multiplayer. You can edit `smapi-internal/config.json` to enable it if you experience frequent `OutOfMemoryException` errors.
 
+* For mod authors:
+  * Fixed assets changed by a mod not reapplied if playing in non-English, the changes are only applicable after the save is loaded, the player returns to title and reloads a save, and the game reloads the target asset before the save is loaded.
+
 ## 3.9.4
 Released 07 March 2021 for Stardew Valley 1.5.4 or later.
 

src/SMAPI/Framework/ContentCoordinator.cs

@@ -207,11 +207,30 @@ namespace StardewModdingAPI.Framework
         /// <remarks>This is called after the player returns to the title screen, but before <see cref="Game1.CleanupReturningToTitle"/> runs.</remarks>
         public void OnReturningToTitleScreen()
         {
-            this.ContentManagerLock.InReadLock(() =>
-            {
-                foreach (IContentManager contentManager in this.ContentManagers)
-                    contentManager.OnReturningToTitleScreen();
-            });
+            // The game clears LocalizedContentManager.localizedAssetNames after returning to the title screen. That
+            // causes an inconsistency in the SMAPI asset cache, which leads to an edge case where assets already
+            // provided by mods via IAssetLoader when playing in non-English are ignored.
+            //
+            // For example, let's say a mod provides the 'Data\mail' asset through IAssetLoader when playing in
+            // Portuguese. Here's the normal load process after it's loaded:
+            //   1. The game requests Data\mail.
+            //   2. SMAPI sees that it's already cached, and calls LoadRaw to bypass asset interception.
+            //   3. LoadRaw sees that there's a localized key mapping, and gets the mapped key.
+            //   4. In this case "Data\mail" is mapped to "Data\mail" since it was loaded by a mod, so it loads that
+            //      asset.
+            //
+            // When the game clears localizedAssetNames, that process goes wrong in step 4:
+            //  3. LoadRaw sees that there's no localized key mapping *and* the locale is non-English, so it attempts
+            //     to load from the localized key format.
+            //  4. In this case that's 'Data\mail.pt-BR', so it successfully loads that asset.
+            //  5. Since we've bypassed asset interception at this point, it's loaded directly from the base content
+            //     manager without mod changes.
+            //
+            // To avoid issues, we just remove affected assets from the cache here so they'll be reloaded normally.
+            // Note that we *must* propagate changes here, otherwise when mods invalidate the cache later to reapply
+            // their changes, the assets won't be found in the cache so no changes will be propagated.
+            if (LocalizedContentManager.CurrentLanguageCode != LocalizedContentManager.LanguageCode.en)
+                this.InvalidateCache((contentManager, key, type) => contentManager is GameContentManager);
         }
 
         /// <summary>Get whether this asset is mapped to a mod folder.</summary>
@@ -275,7 +294,7 @@ namespace StardewModdingAPI.Framework
         public IEnumerable<string> InvalidateCache(Func<IAssetInfo, bool> predicate, bool dispose = false)
         {
             string locale = this.GetLocale();
-            return this.InvalidateCache((assetName, type) =>
+            return this.InvalidateCache((contentManager, assetName, type) =>
             {
                 IAssetInfo info = new AssetInfo(locale, assetName, type, this.MainContentManager.AssertAndNormalizeAssetName);
                 return predicate(info);
@@ -286,7 +305,7 @@ namespace StardewModdingAPI.Framework
         /// <param name="predicate">Matches the asset keys to invalidate.</param>
         /// <param name="dispose">Whether to dispose invalidated assets. This should only be <c>true</c> when they're being invalidated as part of a dispose, to avoid crashing the game.</param>
         /// <returns>Returns the invalidated asset names.</returns>
-        public IEnumerable<string> InvalidateCache(Func<string, Type, bool> predicate, bool dispose = false)
+        public IEnumerable<string> InvalidateCache(Func<IContentManager, string, Type, bool> predicate, bool dispose = false)
         {
             // invalidate cache & track removed assets
             IDictionary<string, Type> removedAssets = new Dictionary<string, Type>(StringComparer.OrdinalIgnoreCase);
@@ -295,7 +314,7 @@ namespace StardewModdingAPI.Framework
                 // cached assets
                 foreach (IContentManager contentManager in this.ContentManagers)
                 {
-                    foreach (var entry in contentManager.InvalidateCache(predicate, dispose))
+                    foreach (var entry in contentManager.InvalidateCache((key, type) => predicate(contentManager, key, type), dispose))
                     {
                         if (!removedAssets.ContainsKey(entry.Key))
                             removedAssets[entry.Key] = entry.Value.GetType();
@@ -313,7 +332,7 @@ namespace StardewModdingAPI.Framework
 
                         // get map path
                         string mapPath = this.MainContentManager.AssertAndNormalizeAssetName(location.mapPath.Value);
-                        if (!removedAssets.ContainsKey(mapPath) && predicate(mapPath, typeof(Map)))
+                        if (!removedAssets.ContainsKey(mapPath) && predicate(this.MainContentManager, mapPath, typeof(Map)))
                             removedAssets[mapPath] = typeof(Map);
                     }
                 }

src/SMAPI/Framework/ContentManagers/BaseContentManager.cs

@@ -121,9 +121,6 @@ namespace StardewModdingAPI.Framework.ContentManagers
         /// <inheritdoc />
         public virtual void OnLocaleChanged() { }
 
-        /// <inheritdoc />
-        public virtual void OnReturningToTitleScreen() { }
-
         /// <inheritdoc />
         [Pure]
         public string NormalizePathSeparators(string path)

src/SMAPI/Framework/ContentManagers/GameContentManager.cs

@@ -136,31 +136,6 @@ namespace StardewModdingAPI.Framework.ContentManagers
                 this.Monitor.Log($"Invalidated {invalidated.Length} asset names: {string.Join(", ", invalidated)} for locale change.");
         }
 
-        /// <inheritdoc />
-        public override void OnReturningToTitleScreen()
-        {
-            // The game clears LocalizedContentManager.localizedAssetNames after returning to the title screen. That
-            // causes an inconsistency in the SMAPI asset cache, which leads to an edge case where assets already
-            // provided by mods via IAssetLoader when playing in non-English are ignored.
-            //
-            // For example, let's say a mod provides the 'Data\mail' asset through IAssetLoader when playing in
-            // Portuguese. Here's the normal load process after it's loaded:
-            //   1. The game requests Data\mail.
-            //   2. SMAPI sees that it's already cached, and calls LoadRaw to bypass asset interception.
-            //   3. LoadRaw sees that there's a localized key mapping, and gets the mapped key.
-            //   4. In this case "Data\mail" is mapped to "Data\mail" since it was loaded by a mod, so it loads that
-            //      asset.
-            //
-            // When the game clears localizedAssetNames, that process goes wrong in step 4:
-            //  3. LoadRaw sees that there's no localized key mapping *and* the locale is non-English, so it attempts
-            //     to load from the localized key format.
-            //  4. In this case that's 'Data\mail.pt-BR', so it successfully loads that asset.
-            //  5. Since we've bypassed asset interception at this point, it's loaded directly from the base content
-            //     manager without mod changes.
-            if (LocalizedContentManager.CurrentLanguageCode != LocalizedContentManager.LanguageCode.en)
-                this.InvalidateCache((_, _) => true);
-        }
-
         /// <inheritdoc />
         public override LocalizedContentManager CreateTemporary()
         {

src/SMAPI/Framework/ContentManagers/IContentManager.cs

@@ -69,9 +69,5 @@ namespace StardewModdingAPI.Framework.ContentManagers
 
         /// <summary>Perform any cleanup needed when the locale changes.</summary>
         void OnLocaleChanged();
-
-        /// <summary>Clean up when the player is returning to the title screen.</summary>
-        /// <remarks>This is called after the player returns to the title screen, but before <see cref="Game1.CleanupReturningToTitle"/> runs.</remarks>
-        void OnReturningToTitleScreen();
     }
 }

src/SMAPI/Framework/ModHelpers/ContentHelper.cs

@@ -136,7 +136,7 @@ namespace StardewModdingAPI.Framework.ModHelpers
         public bool InvalidateCache<T>()
         {
             this.Monitor.Log($"Requested cache invalidation for all assets of type {typeof(T)}. This is an expensive operation and should be avoided if possible.", LogLevel.Trace);
-            return this.ContentCore.InvalidateCache((key, type) => typeof(T).IsAssignableFrom(type)).Any();
+            return this.ContentCore.InvalidateCache((contentManager, key, type) => typeof(T).IsAssignableFrom(type)).Any();
         }
 
         /// <inheritdoc />