From 822cc71619cd173a67de241843cf1679cfc1904d Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Thu, 28 Jan 2021 19:51:30 -0500 Subject: fix error running 'install on Windows.bat' for one user --- docs/release-notes.md | 4 ++++ src/SMAPI.Installer/assets/windows-install.bat | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index dabdada1..bb5998f8 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -7,6 +7,10 @@ * Migrated to Harmony 2.0 (see [_migrate to Harmony 2.0_](https://stardewvalleywiki.com/Modding:Migrate_to_Harmony_2.0) for more info). --> +## Upcoming release +* For players: + * Fixed error running `install on Windows.bat` in very rare cases. + ## 3.9.1 Released 25 January 2021 for Stardew Valley 1.5.4 or later. diff --git a/src/SMAPI.Installer/assets/windows-install.bat b/src/SMAPI.Installer/assets/windows-install.bat index d02dd4c6..2cc54e80 100644 --- a/src/SMAPI.Installer/assets/windows-install.bat +++ b/src/SMAPI.Installer/assets/windows-install.bat @@ -4,5 +4,5 @@ if not errorlevel 1 ( echo Oops! It looks like you're running the installer from inside a zip file. Make sure you unzip the download first. pause ) else ( - start /WAIT /B internal/windows-install.exe + start /WAIT /B ./internal/windows-install.exe ) -- cgit From b2a6933efb0719b48034eff8c29b5f12beb00248 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Thu, 28 Jan 2021 21:21:18 -0500 Subject: fix mod type defaulted incorrectly in SMAPI toolkit --- docs/release-notes.md | 3 +++ src/SMAPI.Toolkit/Framework/ModScanning/ModScanner.cs | 15 ++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index bb5998f8..2e99277f 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -11,6 +11,9 @@ * For players: * Fixed error running `install on Windows.bat` in very rare cases. +* For modders: + * Fixed SMAPI toolkit defaulting the mod type to SMAPI if its `manifest.json` has neither `EntryDll` nor `ContentPackFor`. This only affects external tools, since SMAPI itself validates those fields separately. + ## 3.9.1 Released 25 January 2021 for Stardew Valley 1.5.4 or later. diff --git a/src/SMAPI.Toolkit/Framework/ModScanning/ModScanner.cs b/src/SMAPI.Toolkit/Framework/ModScanning/ModScanner.cs index 86a97016..fd206d9d 100644 --- a/src/SMAPI.Toolkit/Framework/ModScanning/ModScanner.cs +++ b/src/SMAPI.Toolkit/Framework/ModScanning/ModScanner.cs @@ -177,12 +177,17 @@ namespace StardewModdingAPI.Toolkit.Framework.ModScanning } // get mod type - ModType type = ModType.Invalid; - if (manifest != null) + ModType type; { - type = !string.IsNullOrWhiteSpace(manifest.ContentPackFor?.UniqueID) - ? ModType.ContentPack - : ModType.Smapi; + bool isContentPack = !string.IsNullOrWhiteSpace(manifest?.ContentPackFor?.UniqueID); + bool isSmapi = !string.IsNullOrWhiteSpace(manifest?.EntryDll); + + if (isContentPack == isSmapi) + type = ModType.Invalid; + else if (isContentPack) + type = ModType.ContentPack; + else + type = ModType.Smapi; } // build result -- cgit From 7e8f4518764a86e7d3589ae75235b1d3d4462f8b Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sun, 31 Jan 2021 15:37:00 -0500 Subject: add experimental 'aggressive memory optimization' flag (#757) --- docs/release-notes.md | 1 + src/SMAPI/Framework/ContentCoordinator.cs | 18 +++++++++++---- .../ContentManagers/BaseContentManager.cs | 26 ++++++++++++++++----- .../ContentManagers/GameContentManager.cs | 5 ++-- .../Framework/ContentManagers/ModContentManager.cs | 5 ++-- src/SMAPI/Framework/Logging/LogManager.cs | 26 +++++++++++++-------- src/SMAPI/Framework/Models/SConfig.cs | 6 ++++- src/SMAPI/Framework/SCore.cs | 4 ++-- src/SMAPI/Metadata/CoreAssetPropagator.cs | 27 ++++++++++++++++++---- src/SMAPI/SMAPI.config.json | 6 +++++ 10 files changed, 92 insertions(+), 32 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 2e99277f..e54fd24b 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -9,6 +9,7 @@ ## Upcoming release * For players: + * Added _aggressive memory optimization_ option. This is experimental and disabled by default; you can enable it in `smapi-internal/config.json` if you experience `OutOfMemoryException` crashes. * Fixed error running `install on Windows.bat` in very rare cases. * For modders: diff --git a/src/SMAPI/Framework/ContentCoordinator.cs b/src/SMAPI/Framework/ContentCoordinator.cs index 77dd6c72..81265fa2 100644 --- a/src/SMAPI/Framework/ContentCoordinator.cs +++ b/src/SMAPI/Framework/ContentCoordinator.cs @@ -26,6 +26,9 @@ namespace StardewModdingAPI.Framework /// An asset key prefix for assets from SMAPI mod folders. private readonly string ManagedPrefix = "SMAPI"; + /// Whether to enable more aggressive memory optimizations. + private readonly bool AggressiveMemoryOptimizations; + /// Encapsulates monitoring and logging. private readonly IMonitor Monitor; @@ -91,8 +94,10 @@ namespace StardewModdingAPI.Framework /// Simplifies access to private code. /// Encapsulates SMAPI's JSON file parsing. /// A callback to invoke the first time *any* game content manager loads an asset. - public ContentCoordinator(IServiceProvider serviceProvider, string rootDirectory, CultureInfo currentCulture, IMonitor monitor, Reflector reflection, JsonHelper jsonHelper, Action onLoadingFirstAsset) + /// Whether to enable more aggressive memory optimizations. + public ContentCoordinator(IServiceProvider serviceProvider, string rootDirectory, CultureInfo currentCulture, IMonitor monitor, Reflector reflection, JsonHelper jsonHelper, Action onLoadingFirstAsset, bool aggressiveMemoryOptimizations) { + this.AggressiveMemoryOptimizations = aggressiveMemoryOptimizations; this.Monitor = monitor ?? throw new ArgumentNullException(nameof(monitor)); this.Reflection = reflection; this.JsonHelper = jsonHelper; @@ -108,11 +113,12 @@ namespace StardewModdingAPI.Framework monitor: monitor, reflection: reflection, onDisposing: this.OnDisposing, - onLoadingFirstAsset: onLoadingFirstAsset + onLoadingFirstAsset: onLoadingFirstAsset, + aggressiveMemoryOptimizations: aggressiveMemoryOptimizations ) ); this.VanillaContentManager = new LocalizedContentManager(serviceProvider, rootDirectory); - this.CoreAssets = new CoreAssetPropagator(this.MainContentManager.AssertAndNormalizeAssetName, reflection); + this.CoreAssets = new CoreAssetPropagator(this.MainContentManager.AssertAndNormalizeAssetName, reflection, aggressiveMemoryOptimizations); } /// Get a new content manager which handles reading files from the game content folder with support for interception. @@ -130,7 +136,8 @@ namespace StardewModdingAPI.Framework monitor: this.Monitor, reflection: this.Reflection, onDisposing: this.OnDisposing, - onLoadingFirstAsset: this.OnLoadingFirstAsset + onLoadingFirstAsset: this.OnLoadingFirstAsset, + aggressiveMemoryOptimizations: this.AggressiveMemoryOptimizations ); this.ContentManagers.Add(manager); return manager; @@ -157,7 +164,8 @@ namespace StardewModdingAPI.Framework monitor: this.Monitor, reflection: this.Reflection, jsonHelper: this.JsonHelper, - onDisposing: this.OnDisposing + onDisposing: this.OnDisposing, + aggressiveMemoryOptimizations: this.AggressiveMemoryOptimizations ); this.ContentManagers.Add(manager); return manager; diff --git a/src/SMAPI/Framework/ContentManagers/BaseContentManager.cs b/src/SMAPI/Framework/ContentManagers/BaseContentManager.cs index 92264f8c..709c624e 100644 --- a/src/SMAPI/Framework/ContentManagers/BaseContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/BaseContentManager.cs @@ -11,6 +11,7 @@ using StardewModdingAPI.Framework.Content; using StardewModdingAPI.Framework.Exceptions; using StardewModdingAPI.Framework.Reflection; using StardewValley; +using xTile; namespace StardewModdingAPI.Framework.ContentManagers { @@ -29,6 +30,9 @@ namespace StardewModdingAPI.Framework.ContentManagers /// Encapsulates monitoring and logging. protected readonly IMonitor Monitor; + /// Whether to enable more aggressive memory optimizations. + protected readonly bool AggressiveMemoryOptimizations; + /// Whether the content coordinator has been disposed. private bool IsDisposed; @@ -75,7 +79,8 @@ namespace StardewModdingAPI.Framework.ContentManagers /// Simplifies access to private code. /// A callback to invoke when the content manager is being disposed. /// Whether this content manager handles managed asset keys (e.g. to load assets from a mod folder). - protected BaseContentManager(string name, IServiceProvider serviceProvider, string rootDirectory, CultureInfo currentCulture, ContentCoordinator coordinator, IMonitor monitor, Reflector reflection, Action onDisposing, bool isNamespaced) + /// Whether to enable more aggressive memory optimizations. + protected BaseContentManager(string name, IServiceProvider serviceProvider, string rootDirectory, CultureInfo currentCulture, ContentCoordinator coordinator, IMonitor monitor, Reflector reflection, Action onDisposing, bool isNamespaced, bool aggressiveMemoryOptimizations) : base(serviceProvider, rootDirectory, currentCulture) { // init @@ -85,6 +90,7 @@ namespace StardewModdingAPI.Framework.ContentManagers this.Monitor = monitor ?? throw new ArgumentNullException(nameof(monitor)); this.OnDisposing = onDisposing; this.IsNamespaced = isNamespaced; + this.AggressiveMemoryOptimizations = aggressiveMemoryOptimizations; // get asset data this.LanguageCodes = this.GetKeyLocales().ToDictionary(p => p.Value, p => p.Key, StringComparer.OrdinalIgnoreCase); @@ -198,14 +204,22 @@ namespace StardewModdingAPI.Framework.ContentManagers { this.ParseCacheKey(key, out string assetName, out _); - if (removeAssets.ContainsKey(assetName)) - return true; - if (predicate(assetName, asset.GetType())) + // check if asset should be removed + bool remove = removeAssets.ContainsKey(assetName); + if (!remove && predicate(assetName, asset.GetType())) { removeAssets[assetName] = asset; - return true; + remove = true; } - return false; + + // dispose if safe + if (remove && this.AggressiveMemoryOptimizations) + { + if (asset is Map map) + map.DisposeTileSheets(Game1.mapDisplayDevice); + } + + return remove; }, dispose); return removeAssets; diff --git a/src/SMAPI/Framework/ContentManagers/GameContentManager.cs b/src/SMAPI/Framework/ContentManagers/GameContentManager.cs index 665c019b..f8ee575f 100644 --- a/src/SMAPI/Framework/ContentManagers/GameContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/GameContentManager.cs @@ -52,8 +52,9 @@ namespace StardewModdingAPI.Framework.ContentManagers /// Simplifies access to private code. /// A callback to invoke when the content manager is being disposed. /// A callback to invoke the first time *any* game content manager loads an asset. - public GameContentManager(string name, IServiceProvider serviceProvider, string rootDirectory, CultureInfo currentCulture, ContentCoordinator coordinator, IMonitor monitor, Reflector reflection, Action onDisposing, Action onLoadingFirstAsset) - : base(name, serviceProvider, rootDirectory, currentCulture, coordinator, monitor, reflection, onDisposing, isNamespaced: false) + /// Whether to enable more aggressive memory optimizations. + public GameContentManager(string name, IServiceProvider serviceProvider, string rootDirectory, CultureInfo currentCulture, ContentCoordinator coordinator, IMonitor monitor, Reflector reflection, Action onDisposing, Action onLoadingFirstAsset, bool aggressiveMemoryOptimizations) + : base(name, serviceProvider, rootDirectory, currentCulture, coordinator, monitor, reflection, onDisposing, isNamespaced: false, aggressiveMemoryOptimizations: aggressiveMemoryOptimizations) { this.OnLoadingFirstAsset = onLoadingFirstAsset; } diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index 1456d3c1..284c1f37 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -50,8 +50,9 @@ namespace StardewModdingAPI.Framework.ContentManagers /// Simplifies access to private code. /// Encapsulates SMAPI's JSON file parsing. /// A callback to invoke when the content manager is being disposed. - public ModContentManager(string name, IContentManager gameContentManager, IServiceProvider serviceProvider, string modName, string rootDirectory, CultureInfo currentCulture, ContentCoordinator coordinator, IMonitor monitor, Reflector reflection, JsonHelper jsonHelper, Action onDisposing) - : base(name, serviceProvider, rootDirectory, currentCulture, coordinator, monitor, reflection, onDisposing, isNamespaced: true) + /// Whether to enable more aggressive memory optimizations. + public ModContentManager(string name, IContentManager gameContentManager, IServiceProvider serviceProvider, string modName, string rootDirectory, CultureInfo currentCulture, ContentCoordinator coordinator, IMonitor monitor, Reflector reflection, JsonHelper jsonHelper, Action onDisposing, bool aggressiveMemoryOptimizations) + : base(name, serviceProvider, rootDirectory, currentCulture, coordinator, monitor, reflection, onDisposing, isNamespaced: true, aggressiveMemoryOptimizations: aggressiveMemoryOptimizations) { this.GameContentManager = gameContentManager; this.JsonHelper = jsonHelper; diff --git a/src/SMAPI/Framework/Logging/LogManager.cs b/src/SMAPI/Framework/Logging/LogManager.cs index ff00cff7..5f191873 100644 --- a/src/SMAPI/Framework/Logging/LogManager.cs +++ b/src/SMAPI/Framework/Logging/LogManager.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Text.RegularExpressions; using System.Threading; using StardewModdingAPI.Framework.Commands; +using StardewModdingAPI.Framework.Models; using StardewModdingAPI.Framework.ModLoading; using StardewModdingAPI.Internal.ConsoleWriting; using StardewModdingAPI.Toolkit.Framework.ModData; @@ -284,19 +285,24 @@ namespace StardewModdingAPI.Framework.Logging } /// Log details for settings that don't match the default. - /// Whether to enable full console output for developers. - /// Whether to check for newer versions of SMAPI and mods on startup. - /// Whether to rewrite mods for compatibility. - public void LogSettingsHeader(bool isDeveloperMode, bool checkForUpdates, bool rewriteMods) + /// The settings to log. + public void LogSettingsHeader(SConfig settings) { - if (isDeveloperMode) - this.Monitor.Log("You have SMAPI for developers, so the console will be much more verbose. You can disable developer mode by installing the non-developer version of SMAPI.", LogLevel.Info); - if (!checkForUpdates) - this.Monitor.Log("You configured SMAPI to not check for updates. Running an old version of SMAPI is not recommended. You can enable update checks by reinstalling SMAPI.", LogLevel.Warn); - if (!rewriteMods) - this.Monitor.Log("You configured SMAPI to not rewrite broken mods. Many older mods may fail to load. You can undo this by reinstalling SMAPI.", LogLevel.Warn); + // developer mode + if (settings.DeveloperMode) + this.Monitor.Log("You enabled developer mode, so the console will be much more verbose. You can disable it by installing the non-developer version of SMAPI.", LogLevel.Info); + + // warnings + if (!settings.CheckForUpdates) + this.Monitor.Log("You disabled update checks, so you won't be notified of new SMAPI or mod updates. Running an old version of SMAPI is not recommended. You can undo this by reinstalling SMAPI.", LogLevel.Warn); + if (settings.AggressiveMemoryOptimizations) + this.Monitor.Log("You enabled aggressive memory optimizations. This is an experimental option which may cause errors or crashes. You can undo this by reinstalling SMAPI.", LogLevel.Warn); + if (!settings.RewriteMods) + this.Monitor.Log("You disabled rewriting broken mods, so many older mods may fail to load. You can undo this by reinstalling SMAPI.", LogLevel.Info); if (!this.Monitor.WriteToConsole) this.Monitor.Log("Writing to the terminal is disabled because the --no-terminal argument was received. This usually means launching the terminal failed.", LogLevel.Warn); + + // verbose logging this.Monitor.VerboseLog("Verbose logging enabled."); } diff --git a/src/SMAPI/Framework/Models/SConfig.cs b/src/SMAPI/Framework/Models/SConfig.cs index dea08717..382ae41f 100644 --- a/src/SMAPI/Framework/Models/SConfig.cs +++ b/src/SMAPI/Framework/Models/SConfig.cs @@ -21,7 +21,8 @@ namespace StardewModdingAPI.Framework.Models [nameof(WebApiBaseUrl)] = "https://smapi.io/api/", [nameof(VerboseLogging)] = false, [nameof(LogNetworkTraffic)] = false, - [nameof(RewriteMods)] = true + [nameof(RewriteMods)] = true, + [nameof(AggressiveMemoryOptimizations)] = false }; /// The default values for , to log changes if different. @@ -60,6 +61,9 @@ namespace StardewModdingAPI.Framework.Models /// Whether SMAPI should rewrite mods for compatibility. public bool RewriteMods { get; set; } = (bool)SConfig.DefaultValues[nameof(SConfig.RewriteMods)]; + /// Whether to enable more aggressive memory optimizations. + public bool AggressiveMemoryOptimizations { get; set; } = (bool)SConfig.DefaultValues[nameof(SConfig.AggressiveMemoryOptimizations)]; + /// Whether SMAPI should log network traffic. Best combined with , which includes network metadata. public bool LogNetworkTraffic { get; set; } diff --git a/src/SMAPI/Framework/SCore.cs b/src/SMAPI/Framework/SCore.cs index cd094ff4..0ae69f0f 100644 --- a/src/SMAPI/Framework/SCore.cs +++ b/src/SMAPI/Framework/SCore.cs @@ -277,7 +277,7 @@ namespace StardewModdingAPI.Framework // log basic info this.LogManager.HandleMarkerFiles(); - this.LogManager.LogSettingsHeader(this.Settings.DeveloperMode, this.Settings.CheckForUpdates, this.Settings.RewriteMods); + this.LogManager.LogSettingsHeader(this.Settings); // set window titles this.SetWindowTitles( @@ -1149,7 +1149,7 @@ namespace StardewModdingAPI.Framework // Game1._temporaryContent initializing from SGame constructor if (this.ContentCore == null) { - this.ContentCore = new ContentCoordinator(serviceProvider, rootDirectory, Thread.CurrentThread.CurrentUICulture, this.Monitor, this.Reflection, this.Toolkit.JsonHelper, this.InitializeBeforeFirstAssetLoaded); + this.ContentCore = new ContentCoordinator(serviceProvider, rootDirectory, Thread.CurrentThread.CurrentUICulture, this.Monitor, this.Reflection, this.Toolkit.JsonHelper, this.InitializeBeforeFirstAssetLoaded, this.Settings.AggressiveMemoryOptimizations); if (this.ContentCore.Language != this.Translator.LocaleEnum) this.Translator.SetLocale(this.ContentCore.GetLocale(), this.ContentCore.Language); diff --git a/src/SMAPI/Metadata/CoreAssetPropagator.cs b/src/SMAPI/Metadata/CoreAssetPropagator.cs index 063804e0..829aa451 100644 --- a/src/SMAPI/Metadata/CoreAssetPropagator.cs +++ b/src/SMAPI/Metadata/CoreAssetPropagator.cs @@ -29,6 +29,9 @@ namespace StardewModdingAPI.Metadata /********* ** Fields *********/ + /// Whether to enable more aggressive memory optimizations. + private readonly bool AggressiveMemoryOptimizations; + /// Normalizes an asset key to match the cache key and assert that it's valid. private readonly Func AssertAndNormalizeAssetName; @@ -55,10 +58,12 @@ namespace StardewModdingAPI.Metadata /// Initialize the core asset data. /// Normalizes an asset key to match the cache key and assert that it's valid. /// Simplifies access to private code. - public CoreAssetPropagator(Func assertAndNormalizeAssetName, Reflector reflection) + /// Whether to enable more aggressive memory optimizations. + public CoreAssetPropagator(Func assertAndNormalizeAssetName, Reflector reflection, bool aggressiveMemoryOptimizations) { this.AssertAndNormalizeAssetName = assertAndNormalizeAssetName; this.Reflection = reflection; + this.AggressiveMemoryOptimizations = aggressiveMemoryOptimizations; } /// Reload one of the game's core assets (if applicable). @@ -582,7 +587,7 @@ namespace StardewModdingAPI.Metadata titleMenu.aboutButton.texture = texture; titleMenu.languageButton.texture = texture; foreach (ClickableTextureComponent button in titleMenu.buttons) - button.texture = titleMenu.titleButtonsTexture; + button.texture = texture; foreach (TemporaryAnimatedSprite bird in titleMenu.birds) bird.texture = texture; @@ -785,6 +790,9 @@ namespace StardewModdingAPI.Metadata /// The location whose map to reload. private void ReloadMap(GameLocation location) { + if (this.AggressiveMemoryOptimizations) + location.map.DisposeTileSheets(Game1.mapDisplayDevice); + // reload map location.interiorDoors.Clear(); // prevent errors when doors try to update tiles which no longer exist location.reloadMap(); @@ -843,7 +851,7 @@ namespace StardewModdingAPI.Metadata // update sprite foreach (var target in characters) { - target.Npc.Sprite.spriteTexture = content.Load(target.Key); + target.Npc.Sprite.spriteTexture = this.DisposeIfNeeded(target.Npc.Sprite.spriteTexture, content.Load(target.Key)); propagated[target.Key] = true; } } @@ -881,7 +889,7 @@ namespace StardewModdingAPI.Metadata // update portrait foreach (var target in characters) { - target.Npc.Portrait = content.Load(target.Key); + target.Npc.Portrait = this.DisposeIfNeeded(target.Npc.Portrait, content.Load(target.Key)); propagated[target.Key] = true; } } @@ -1146,5 +1154,16 @@ namespace StardewModdingAPI.Metadata { return this.GetSegments(path).Length; } + + /// Dispose a texture if are enabled and it's different from the new instance. + /// The previous texture to dispose. + /// The new texture being loaded. + private Texture2D DisposeIfNeeded(Texture2D oldTexture, Texture2D newTexture) + { + if (this.AggressiveMemoryOptimizations && oldTexture != null && !oldTexture.IsDisposed && !object.ReferenceEquals(oldTexture, newTexture)) + oldTexture.Dispose(); + + return newTexture; + } } } diff --git a/src/SMAPI/SMAPI.config.json b/src/SMAPI/SMAPI.config.json index 7a710f14..6a485cbd 100644 --- a/src/SMAPI/SMAPI.config.json +++ b/src/SMAPI/SMAPI.config.json @@ -39,6 +39,12 @@ copy all the settings, or you may cause bugs due to overridden changes in future */ "RewriteMods": true, + /** + * Whether to enable more aggressive memory optimizations. + * THIS IS EXPERIMENTAL AND MAY CAUSE ERRORS OR CRASHES. + */ + "AggressiveMemoryOptimizations": true, + /** * Whether to add a section to the 'mod issues' list for mods which directly use potentially * sensitive .NET APIs like file or shell access. Note that many mods do this legitimately as -- cgit From 423f2352af9d0e9815daf4ba3ba33134f587ce47 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sun, 31 Jan 2021 22:08:03 -0500 Subject: rework aggressive memory optimization to minimize mod impact (#757) --- src/SMAPI/Framework/ContentCoordinator.cs | 17 +++++- .../GameContentManagerForAssetPropagation.cs | 47 ++++++++++++++++ src/SMAPI/Metadata/CoreAssetPropagator.cs | 63 ++++++++++++++-------- 3 files changed, 103 insertions(+), 24 deletions(-) create mode 100644 src/SMAPI/Framework/ContentManagers/GameContentManagerForAssetPropagation.cs diff --git a/src/SMAPI/Framework/ContentCoordinator.cs b/src/SMAPI/Framework/ContentCoordinator.cs index 81265fa2..6a7385c3 100644 --- a/src/SMAPI/Framework/ContentCoordinator.cs +++ b/src/SMAPI/Framework/ContentCoordinator.cs @@ -117,8 +117,21 @@ namespace StardewModdingAPI.Framework aggressiveMemoryOptimizations: aggressiveMemoryOptimizations ) ); + var contentManagerForAssetPropagation = new GameContentManagerForAssetPropagation( + name: nameof(GameContentManagerForAssetPropagation), + serviceProvider: serviceProvider, + rootDirectory: rootDirectory, + currentCulture: currentCulture, + coordinator: this, + monitor: monitor, + reflection: reflection, + onDisposing: this.OnDisposing, + onLoadingFirstAsset: onLoadingFirstAsset, + aggressiveMemoryOptimizations: aggressiveMemoryOptimizations + ); + this.ContentManagers.Add(contentManagerForAssetPropagation); this.VanillaContentManager = new LocalizedContentManager(serviceProvider, rootDirectory); - this.CoreAssets = new CoreAssetPropagator(this.MainContentManager.AssertAndNormalizeAssetName, reflection, aggressiveMemoryOptimizations); + this.CoreAssets = new CoreAssetPropagator(this.MainContentManager, contentManagerForAssetPropagation, reflection, aggressiveMemoryOptimizations); } /// Get a new content manager which handles reading files from the game content folder with support for interception. @@ -298,7 +311,7 @@ namespace StardewModdingAPI.Framework // reload core game assets if (removedAssets.Any()) { - IDictionary propagated = this.CoreAssets.Propagate(this.MainContentManager, removedAssets.ToDictionary(p => p.Key, p => p.Value)); // use an intercepted content manager + IDictionary propagated = this.CoreAssets.Propagate(removedAssets.ToDictionary(p => p.Key, p => p.Value)); // use an intercepted content manager this.Monitor.Log($"Invalidated {removedAssets.Count} asset names ({string.Join(", ", removedAssets.Keys.OrderBy(p => p, StringComparer.OrdinalIgnoreCase))}); propagated {propagated.Count(p => p.Value)} core assets.", LogLevel.Trace); } else diff --git a/src/SMAPI/Framework/ContentManagers/GameContentManagerForAssetPropagation.cs b/src/SMAPI/Framework/ContentManagers/GameContentManagerForAssetPropagation.cs new file mode 100644 index 00000000..cbbebf02 --- /dev/null +++ b/src/SMAPI/Framework/ContentManagers/GameContentManagerForAssetPropagation.cs @@ -0,0 +1,47 @@ +using System; +using System.Globalization; +using Microsoft.Xna.Framework.Graphics; +using StardewModdingAPI.Framework.Reflection; +using StardewValley; + +namespace StardewModdingAPI.Framework.ContentManagers +{ + /// An extension of specifically optimized for asset propagation. + /// This avoids sharing an asset cache with or mods, so that assets can be safely disposed when the vanilla game no longer references them. + internal class GameContentManagerForAssetPropagation : GameContentManager + { + /********* + ** Fields + *********/ + /// A unique value used in to identify assets loaded through this instance. + private readonly string Tag = $"Pathoschild.SMAPI/LoadedBy:{nameof(GameContentManagerForAssetPropagation)}"; + + + /********* + ** Public methods + *********/ + /// + public GameContentManagerForAssetPropagation(string name, IServiceProvider serviceProvider, string rootDirectory, CultureInfo currentCulture, ContentCoordinator coordinator, IMonitor monitor, Reflector reflection, Action onDisposing, Action onLoadingFirstAsset, bool aggressiveMemoryOptimizations) + : base(name, serviceProvider, rootDirectory, currentCulture, coordinator, monitor, reflection, onDisposing, onLoadingFirstAsset, aggressiveMemoryOptimizations) { } + + /// + public override T Load(string assetName, LanguageCode language, bool useCache) + { + T data = base.Load(assetName, language, useCache); + + if (data is Texture2D texture) + texture.Tag = this.Tag; + + return data; + } + + /// Get whether a texture was loaded by this content manager. + /// The texture to check. + public bool IsReponsibleFor(Texture2D texture) + { + return + texture?.Tag is string tag + && tag.Contains(this.Tag); + } + } +} diff --git a/src/SMAPI/Metadata/CoreAssetPropagator.cs b/src/SMAPI/Metadata/CoreAssetPropagator.cs index 829aa451..426fc3f6 100644 --- a/src/SMAPI/Metadata/CoreAssetPropagator.cs +++ b/src/SMAPI/Metadata/CoreAssetPropagator.cs @@ -5,6 +5,7 @@ using System.IO; using System.Linq; using Microsoft.Xna.Framework.Graphics; using Netcode; +using StardewModdingAPI.Framework.ContentManagers; using StardewModdingAPI.Framework.Reflection; using StardewModdingAPI.Toolkit.Utilities; using StardewValley; @@ -29,6 +30,12 @@ namespace StardewModdingAPI.Metadata /********* ** Fields *********/ + /// The main content manager through which to reload assets. + private readonly LocalizedContentManager MainContentManager; + + /// An internal content manager used only for asset propagation. See remarks on . + private readonly GameContentManagerForAssetPropagation DisposableContentManager; + /// Whether to enable more aggressive memory optimizations. private readonly bool AggressiveMemoryOptimizations; @@ -56,21 +63,24 @@ namespace StardewModdingAPI.Metadata ** Public methods *********/ /// Initialize the core asset data. - /// Normalizes an asset key to match the cache key and assert that it's valid. + /// The main content manager through which to reload assets. + /// An internal content manager used only for asset propagation. /// Simplifies access to private code. /// Whether to enable more aggressive memory optimizations. - public CoreAssetPropagator(Func assertAndNormalizeAssetName, Reflector reflection, bool aggressiveMemoryOptimizations) + public CoreAssetPropagator(LocalizedContentManager mainContent, GameContentManagerForAssetPropagation disposableContent, Reflector reflection, bool aggressiveMemoryOptimizations) { - this.AssertAndNormalizeAssetName = assertAndNormalizeAssetName; + this.MainContentManager = mainContent; + this.DisposableContentManager = disposableContent; this.Reflection = reflection; this.AggressiveMemoryOptimizations = aggressiveMemoryOptimizations; + + this.AssertAndNormalizeAssetName = disposableContent.AssertAndNormalizeAssetName; } /// Reload one of the game's core assets (if applicable). - /// The content manager through which to reload the asset. /// The asset keys and types to reload. /// Returns a lookup of asset names to whether they've been propagated. - public IDictionary Propagate(LocalizedContentManager content, IDictionary assets) + public IDictionary Propagate(IDictionary assets) { // group into optimized lists var buckets = assets.GroupBy(p => @@ -91,16 +101,16 @@ namespace StardewModdingAPI.Metadata switch (bucket.Key) { case AssetBucket.Sprite: - this.ReloadNpcSprites(content, bucket.Select(p => p.Key), propagated); + this.ReloadNpcSprites(bucket.Select(p => p.Key), propagated); break; case AssetBucket.Portrait: - this.ReloadNpcPortraits(content, bucket.Select(p => p.Key), propagated); + this.ReloadNpcPortraits(bucket.Select(p => p.Key), propagated); break; default: foreach (var entry in bucket) - propagated[entry.Key] = this.PropagateOther(content, entry.Key, entry.Value); + propagated[entry.Key] = this.PropagateOther(entry.Key, entry.Value); break; } } @@ -112,13 +122,13 @@ namespace StardewModdingAPI.Metadata ** Private methods *********/ /// Reload one of the game's core assets (if applicable). - /// The content manager through which to reload the asset. /// The asset key to reload. /// The asset type to reload. /// Returns whether an asset was loaded. The return value may be true or false, or a non-null value for true. [SuppressMessage("ReSharper", "StringLiteralTypo", Justification = "These deliberately match the asset names.")] - private bool PropagateOther(LocalizedContentManager content, string key, Type type) + private bool PropagateOther(string key, Type type) { + var content = this.MainContentManager; key = this.AssertAndNormalizeAssetName(key); /**** @@ -830,10 +840,9 @@ namespace StardewModdingAPI.Metadata } /// Reload the sprites for matching NPCs. - /// The content manager through which to reload the asset. /// The asset keys to reload. /// The asset keys which have been propagated. - private void ReloadNpcSprites(LocalizedContentManager content, IEnumerable keys, IDictionary propagated) + private void ReloadNpcSprites(IEnumerable keys, IDictionary propagated) { // get NPCs HashSet lookup = new HashSet(keys, StringComparer.OrdinalIgnoreCase); @@ -851,16 +860,15 @@ namespace StardewModdingAPI.Metadata // update sprite foreach (var target in characters) { - target.Npc.Sprite.spriteTexture = this.DisposeIfNeeded(target.Npc.Sprite.spriteTexture, content.Load(target.Key)); + target.Npc.Sprite.spriteTexture = this.LoadAndDisposeIfNeeded(target.Npc.Sprite.spriteTexture, target.Key); propagated[target.Key] = true; } } /// Reload the portraits for matching NPCs. - /// The content manager through which to reload the asset. /// The asset key to reload. /// The asset keys which have been propagated. - private void ReloadNpcPortraits(LocalizedContentManager content, IEnumerable keys, IDictionary propagated) + private void ReloadNpcPortraits(IEnumerable keys, IDictionary propagated) { // get NPCs HashSet lookup = new HashSet(keys, StringComparer.OrdinalIgnoreCase); @@ -889,7 +897,7 @@ namespace StardewModdingAPI.Metadata // update portrait foreach (var target in characters) { - target.Npc.Portrait = this.DisposeIfNeeded(target.Npc.Portrait, content.Load(target.Key)); + target.Npc.Portrait = this.LoadAndDisposeIfNeeded(target.Npc.Portrait, target.Key); propagated[target.Key] = true; } } @@ -1155,15 +1163,26 @@ namespace StardewModdingAPI.Metadata return this.GetSegments(path).Length; } - /// Dispose a texture if are enabled and it's different from the new instance. + /// Load a texture, and dispose the old one if is enabled and it's different from the new instance. /// The previous texture to dispose. - /// The new texture being loaded. - private Texture2D DisposeIfNeeded(Texture2D oldTexture, Texture2D newTexture) + /// The asset key to load. + private Texture2D LoadAndDisposeIfNeeded(Texture2D oldTexture, string key) { - if (this.AggressiveMemoryOptimizations && oldTexture != null && !oldTexture.IsDisposed && !object.ReferenceEquals(oldTexture, newTexture)) - oldTexture.Dispose(); + // if aggressive memory optimizations are enabled, load the asset from the disposable + // content manager and dispose the old instance if needed. + if (this.AggressiveMemoryOptimizations) + { + GameContentManagerForAssetPropagation content = this.DisposableContentManager; + + Texture2D newTexture = content.Load(key); + if (oldTexture?.IsDisposed == false && !object.ReferenceEquals(oldTexture, newTexture) && content.IsReponsibleFor(oldTexture)) + oldTexture.Dispose(); + + return newTexture; + } - return newTexture; + // else just (re)load it from the main content manager + return this.MainContentManager.Load(key); } } } -- cgit From e81e07594ca4863f9feb94c882b59ba7cc00a0ae Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sun, 31 Jan 2021 22:12:36 -0500 Subject: extend aggressive memory optimization to a few more common textures (#757) --- src/SMAPI/Metadata/CoreAssetPropagator.cs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/SMAPI/Metadata/CoreAssetPropagator.cs b/src/SMAPI/Metadata/CoreAssetPropagator.cs index 426fc3f6..e2a28c62 100644 --- a/src/SMAPI/Metadata/CoreAssetPropagator.cs +++ b/src/SMAPI/Metadata/CoreAssetPropagator.cs @@ -178,14 +178,19 @@ namespace StardewModdingAPI.Metadata ** Buildings ****/ case "buildings\\houses": // Farm - reflection.GetField(typeof(Farm), nameof(Farm.houseTextures)).SetValue(content.Load(key)); - return true; + { + var field = reflection.GetField(typeof(Farm), nameof(Farm.houseTextures)); + field.SetValue( + this.LoadAndDisposeIfNeeded(field.GetValue(), key) + ); + return true; + } /**** ** Content\Characters\Farmer ****/ case "characters\\farmer\\accessories": // Game1.LoadContent - FarmerRenderer.accessoriesTexture = content.Load(key); + FarmerRenderer.accessoriesTexture = this.LoadAndDisposeIfNeeded(FarmerRenderer.accessoriesTexture, key); return true; case "characters\\farmer\\farmer_base": // Farmer @@ -195,19 +200,19 @@ namespace StardewModdingAPI.Metadata return this.ReloadPlayerSprites(key); case "characters\\farmer\\hairstyles": // Game1.LoadContent - FarmerRenderer.hairStylesTexture = content.Load(key); + FarmerRenderer.hairStylesTexture = this.LoadAndDisposeIfNeeded(FarmerRenderer.hairStylesTexture, key); return true; case "characters\\farmer\\hats": // Game1.LoadContent - FarmerRenderer.hatsTexture = content.Load(key); + FarmerRenderer.hatsTexture = this.LoadAndDisposeIfNeeded(FarmerRenderer.hatsTexture, key); return true; case "characters\\farmer\\pants": // Game1.LoadContent - FarmerRenderer.pantsTexture = content.Load(key); + FarmerRenderer.pantsTexture = this.LoadAndDisposeIfNeeded(FarmerRenderer.pantsTexture, key); return true; case "characters\\farmer\\shirts": // Game1.LoadContent - FarmerRenderer.shirtsTexture = content.Load(key); + FarmerRenderer.shirtsTexture = this.LoadAndDisposeIfNeeded(FarmerRenderer.shirtsTexture, key); return true; /**** -- cgit From 54e7b5b846dfd542af3c8a904a57fc5ccc44ecb5 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Wed, 3 Feb 2021 20:24:25 -0500 Subject: enable aggressive memory optimizations by default (#757) The new approach should be safe, and no errors were reported so far by alpha testers. --- docs/release-notes.md | 4 ++-- src/SMAPI.Mods.ErrorHandler/ModEntry.cs | 2 +- src/SMAPI/Framework/Logging/LogManager.cs | 2 -- src/SMAPI/Framework/Models/SConfig.cs | 2 +- src/SMAPI/SMAPI.config.json | 2 +- 5 files changed, 5 insertions(+), 7 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index e54fd24b..9cea9fa9 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -9,11 +9,11 @@ ## Upcoming release * For players: - * Added _aggressive memory optimization_ option. This is experimental and disabled by default; you can enable it in `smapi-internal/config.json` if you experience `OutOfMemoryException` crashes. + * Added more aggressive memory optimization which should eliminate many cases of `OutOfMemoryException` crashes. * Fixed error running `install on Windows.bat` in very rare cases. * For modders: - * Fixed SMAPI toolkit defaulting the mod type to SMAPI if its `manifest.json` has neither `EntryDll` nor `ContentPackFor`. This only affects external tools, since SMAPI itself validates those fields separately. + * Fixed SMAPI toolkit defaulting the mod type incorrectly if a mod's `manifest.json` has neither `EntryDll` nor `ContentPackFor`. This only affects external tools, since SMAPI itself validates those fields separately. ## 3.9.1 Released 25 January 2021 for Stardew Valley 1.5.4 or later. diff --git a/src/SMAPI.Mods.ErrorHandler/ModEntry.cs b/src/SMAPI.Mods.ErrorHandler/ModEntry.cs index 2f6f1939..f543814e 100644 --- a/src/SMAPI.Mods.ErrorHandler/ModEntry.cs +++ b/src/SMAPI.Mods.ErrorHandler/ModEntry.cs @@ -30,7 +30,7 @@ namespace StardewModdingAPI.Mods.ErrorHandler LogManager logManager = core.GetType().GetField("LogManager", BindingFlags.Instance | BindingFlags.NonPublic)?.GetValue(core) as LogManager; if (logManager == null) { - this.Monitor.Log($"Can't access SMAPI's internal log manager. Error-handling patches won't be applied.", LogLevel.Error); + this.Monitor.Log("Can't access SMAPI's internal log manager. Error-handling patches won't be applied.", LogLevel.Error); return; } diff --git a/src/SMAPI/Framework/Logging/LogManager.cs b/src/SMAPI/Framework/Logging/LogManager.cs index 5f191873..2c7be399 100644 --- a/src/SMAPI/Framework/Logging/LogManager.cs +++ b/src/SMAPI/Framework/Logging/LogManager.cs @@ -295,8 +295,6 @@ namespace StardewModdingAPI.Framework.Logging // warnings if (!settings.CheckForUpdates) this.Monitor.Log("You disabled update checks, so you won't be notified of new SMAPI or mod updates. Running an old version of SMAPI is not recommended. You can undo this by reinstalling SMAPI.", LogLevel.Warn); - if (settings.AggressiveMemoryOptimizations) - this.Monitor.Log("You enabled aggressive memory optimizations. This is an experimental option which may cause errors or crashes. You can undo this by reinstalling SMAPI.", LogLevel.Warn); if (!settings.RewriteMods) this.Monitor.Log("You disabled rewriting broken mods, so many older mods may fail to load. You can undo this by reinstalling SMAPI.", LogLevel.Info); if (!this.Monitor.WriteToConsole) diff --git a/src/SMAPI/Framework/Models/SConfig.cs b/src/SMAPI/Framework/Models/SConfig.cs index 382ae41f..4a80e34c 100644 --- a/src/SMAPI/Framework/Models/SConfig.cs +++ b/src/SMAPI/Framework/Models/SConfig.cs @@ -22,7 +22,7 @@ namespace StardewModdingAPI.Framework.Models [nameof(VerboseLogging)] = false, [nameof(LogNetworkTraffic)] = false, [nameof(RewriteMods)] = true, - [nameof(AggressiveMemoryOptimizations)] = false + [nameof(AggressiveMemoryOptimizations)] = true }; /// The default values for , to log changes if different. diff --git a/src/SMAPI/SMAPI.config.json b/src/SMAPI/SMAPI.config.json index 6a485cbd..a9e6f389 100644 --- a/src/SMAPI/SMAPI.config.json +++ b/src/SMAPI/SMAPI.config.json @@ -41,7 +41,7 @@ copy all the settings, or you may cause bugs due to overridden changes in future /** * Whether to enable more aggressive memory optimizations. - * THIS IS EXPERIMENTAL AND MAY CAUSE ERRORS OR CRASHES. + * You can try disabling this if you get ObjectDisposedException errors. */ "AggressiveMemoryOptimizations": true, -- cgit From efec87065780426db15c51a7e68315ff488a89c0 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sat, 6 Feb 2021 13:03:37 -0500 Subject: fix edge case in non-English asset cache after returning to title screen --- docs/release-notes.md | 1 + src/SMAPI/Framework/ContentCoordinator.cs | 26 ++++++++++++++++++++++++++ src/SMAPI/Framework/SCore.cs | 1 + 3 files changed, 28 insertions(+) diff --git a/docs/release-notes.md b/docs/release-notes.md index 9cea9fa9..9f4ea50a 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -14,6 +14,7 @@ * For modders: * Fixed SMAPI toolkit defaulting the mod type incorrectly if a mod's `manifest.json` has neither `EntryDll` nor `ContentPackFor`. This only affects external tools, since SMAPI itself validates those fields separately. + * Fixed edge case when playing in non-English where translatable assets loaded via `IAssetLoader` would no longer be applied after returning to the title screen unless manually invalidated from the cache. ## 3.9.1 Released 25 January 2021 for Stardew Valley 1.5.4 or later. diff --git a/src/SMAPI/Framework/ContentCoordinator.cs b/src/SMAPI/Framework/ContentCoordinator.cs index 6a7385c3..b7c15526 100644 --- a/src/SMAPI/Framework/ContentCoordinator.cs +++ b/src/SMAPI/Framework/ContentCoordinator.cs @@ -203,6 +203,32 @@ namespace StardewModdingAPI.Framework }); } + /// Clean up when the player is returning to the title screen. + /// This is called after the player returns to the title screen, but before runs. + public 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. + + this.InvalidateCache(asset => true); + } + /// Get whether this asset is mapped to a mod folder. /// The asset key. public bool IsManagedAssetKey(string key) diff --git a/src/SMAPI/Framework/SCore.cs b/src/SMAPI/Framework/SCore.cs index 0ae69f0f..2d783eb2 100644 --- a/src/SMAPI/Framework/SCore.cs +++ b/src/SMAPI/Framework/SCore.cs @@ -1118,6 +1118,7 @@ namespace StardewModdingAPI.Framework { // perform cleanup this.Multiplayer.CleanupOnMultiplayerExit(); + this.ContentCore.OnReturningToTitleScreen(); this.JustReturnedToTitle = true; } -- cgit From 5ea871fee59e4a7ed7280d1678a76ed8019337b4 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sat, 6 Feb 2021 19:46:34 -0500 Subject: update schema for Content Patcher 1.20 --- docs/release-notes.md | 3 +++ src/SMAPI.Web/wwwroot/schemas/content-patcher.json | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 9f4ea50a..cb54ee98 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -16,6 +16,9 @@ * Fixed SMAPI toolkit defaulting the mod type incorrectly if a mod's `manifest.json` has neither `EntryDll` nor `ContentPackFor`. This only affects external tools, since SMAPI itself validates those fields separately. * Fixed edge case when playing in non-English where translatable assets loaded via `IAssetLoader` would no longer be applied after returning to the title screen unless manually invalidated from the cache. +* For the web UI: + * Updated the JSON validator/schema for Content Patcher 1.20. + ## 3.9.1 Released 25 January 2021 for Stardew Valley 1.5.4 or later. diff --git a/src/SMAPI.Web/wwwroot/schemas/content-patcher.json b/src/SMAPI.Web/wwwroot/schemas/content-patcher.json index 92149f4d..21514979 100644 --- a/src/SMAPI.Web/wwwroot/schemas/content-patcher.json +++ b/src/SMAPI.Web/wwwroot/schemas/content-patcher.json @@ -11,9 +11,9 @@ "title": "Format version", "description": "The format version. You should always use the latest version to enable the latest features and avoid obsolete behavior.", "type": "string", - "const": "1.19.0", + "const": "1.20.0", "@errorMessages": { - "const": "Incorrect value '@value'. This should be set to the latest format version, currently '1.19.0'." + "const": "Incorrect value '@value'. This should be set to the latest format version, currently '1.20.0'." } }, "ConfigSchema": { -- cgit From 97d3501e20c9b97dc85cfca32ccea4f55c9d61ff Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sat, 6 Feb 2021 20:47:04 -0500 Subject: improve ErrorHandler's error handling if it can't access log manager --- src/SMAPI.Mods.ErrorHandler/ModEntry.cs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/SMAPI.Mods.ErrorHandler/ModEntry.cs b/src/SMAPI.Mods.ErrorHandler/ModEntry.cs index f543814e..e4bcc5bc 100644 --- a/src/SMAPI.Mods.ErrorHandler/ModEntry.cs +++ b/src/SMAPI.Mods.ErrorHandler/ModEntry.cs @@ -26,21 +26,15 @@ namespace StardewModdingAPI.Mods.ErrorHandler public override void Entry(IModHelper helper) { // get SMAPI core types - SCore core = SCore.Instance; - LogManager logManager = core.GetType().GetField("LogManager", BindingFlags.Instance | BindingFlags.NonPublic)?.GetValue(core) as LogManager; - if (logManager == null) - { - this.Monitor.Log("Can't access SMAPI's internal log manager. Error-handling patches won't be applied.", LogLevel.Error); - return; - } + IMonitor monitorForGame = this.GetMonitorForGame(); // apply patches new GamePatcher(this.Monitor).Apply( - new EventErrorPatch(logManager.MonitorForGame), - new DialogueErrorPatch(logManager.MonitorForGame, this.Helper.Reflection), + new EventErrorPatch(monitorForGame), + new DialogueErrorPatch(monitorForGame, this.Helper.Reflection), new ObjectErrorPatch(), new LoadErrorPatch(this.Monitor, this.OnSaveContentRemoved), - new ScheduleErrorPatch(logManager.MonitorForGame), + new ScheduleErrorPatch(monitorForGame), new UtilityErrorPatches() ); @@ -61,7 +55,7 @@ namespace StardewModdingAPI.Mods.ErrorHandler /// The method invoked when a save is loaded. /// The event sender. /// The event arguments. - public void OnSaveLoaded(object sender, SaveLoadedEventArgs e) + private void OnSaveLoaded(object sender, SaveLoadedEventArgs e) { // show in-game warning for removed save content if (this.IsSaveContentRemoved) @@ -70,5 +64,16 @@ namespace StardewModdingAPI.Mods.ErrorHandler Game1.addHUDMessage(new HUDMessage(this.Helper.Translation.Get("warn.invalid-content-removed"), HUDMessage.error_type)); } } + + /// Get the monitor with which to log game errors. + private IMonitor GetMonitorForGame() + { + SCore core = SCore.Instance; + LogManager logManager = core.GetType().GetField("LogManager", BindingFlags.Instance | BindingFlags.NonPublic)?.GetValue(core) as LogManager; + if (logManager == null) + this.Monitor.Log("Can't access SMAPI's internal log manager. Some game errors may be reported as being from Error Handler.", LogLevel.Error); + + return logManager?.MonitorForGame ?? this.Monitor; + } } } -- cgit From 67c52af72d6d0c4fddd3a07b159cb44b5c277599 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sat, 6 Feb 2021 21:12:01 -0500 Subject: add early detection of disposed assets in error handler mod --- docs/release-notes.md | 3 ++ src/SMAPI.Mods.ErrorHandler/ModEntry.cs | 1 + .../Patches/SpriteBatchValidationPatches.cs | 63 ++++++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 src/SMAPI.Mods.ErrorHandler/Patches/SpriteBatchValidationPatches.cs diff --git a/docs/release-notes.md b/docs/release-notes.md index cb54ee98..9d95dacb 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -16,6 +16,9 @@ * Fixed SMAPI toolkit defaulting the mod type incorrectly if a mod's `manifest.json` has neither `EntryDll` nor `ContentPackFor`. This only affects external tools, since SMAPI itself validates those fields separately. * Fixed edge case when playing in non-English where translatable assets loaded via `IAssetLoader` would no longer be applied after returning to the title screen unless manually invalidated from the cache. +* For the ErrorHandler mod: + * Added early detection of disposed textures so the crash stack trace shows the actual code which used them. + * For the web UI: * Updated the JSON validator/schema for Content Patcher 1.20. diff --git a/src/SMAPI.Mods.ErrorHandler/ModEntry.cs b/src/SMAPI.Mods.ErrorHandler/ModEntry.cs index e4bcc5bc..87f531fe 100644 --- a/src/SMAPI.Mods.ErrorHandler/ModEntry.cs +++ b/src/SMAPI.Mods.ErrorHandler/ModEntry.cs @@ -35,6 +35,7 @@ namespace StardewModdingAPI.Mods.ErrorHandler new ObjectErrorPatch(), new LoadErrorPatch(this.Monitor, this.OnSaveContentRemoved), new ScheduleErrorPatch(monitorForGame), + new SpriteBatchValidationPatches(), new UtilityErrorPatches() ); diff --git a/src/SMAPI.Mods.ErrorHandler/Patches/SpriteBatchValidationPatches.cs b/src/SMAPI.Mods.ErrorHandler/Patches/SpriteBatchValidationPatches.cs new file mode 100644 index 00000000..0211cfb1 --- /dev/null +++ b/src/SMAPI.Mods.ErrorHandler/Patches/SpriteBatchValidationPatches.cs @@ -0,0 +1,63 @@ +#if HARMONY_2 +using HarmonyLib; +#else +using Harmony; +#endif +using System; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Xna.Framework.Graphics; +using StardewModdingAPI.Framework.Patching; + +namespace StardewModdingAPI.Mods.ErrorHandler.Patches +{ + /// Harmony patch for to validate textures earlier. + /// Patch methods must be static for Harmony to work correctly. See the Harmony documentation before renaming patch arguments. + [SuppressMessage("ReSharper", "InconsistentNaming", Justification = "Argument names are defined by Harmony and methods are named for clarity.")] + [SuppressMessage("ReSharper", "IdentifierTypo", Justification = "Argument names are defined by Harmony and methods are named for clarity.")] + internal class SpriteBatchValidationPatches : IHarmonyPatch + { + /********* + ** Accessors + *********/ + /// + public string Name => nameof(SpriteBatchValidationPatches); + + + /********* + ** Public methods + *********/ + /// +#if HARMONY_2 + public void Apply(Harmony harmony) +#else + public void Apply(HarmonyInstance harmony) +#endif + { + harmony.Patch( +#if SMAPI_FOR_WINDOWS + original: AccessTools.Method(typeof(SpriteBatch), "InternalDraw"), +#else + original: AccessTools.Method(typeof(SpriteBatch), "CheckValid", new[] { typeof(Texture2D) }), +#endif + postfix: new HarmonyMethod(this.GetType(), nameof(SpriteBatchValidationPatches.After_SpriteBatch_CheckValid)) + ); + } + + + /********* + ** Private methods + *********/ +#if SMAPI_FOR_WINDOWS + /// The method to call instead of . + /// The texture to validate. +#else + /// The method to call instead of . + /// The texture to validate. +#endif + private static void After_SpriteBatch_CheckValid(Texture2D texture) + { + if (texture?.IsDisposed == true) + throw new ObjectDisposedException("Cannot draw this texture because it's disposed."); + } + } +} -- cgit From 5173ddf53571703c3895cd5f2d45891489d96ff9 Mon Sep 17 00:00:00 2001 From: Jesse