From 5171829ecc8acd8dc0e3292bd3c2c7893a148c8f Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Thu, 24 Aug 2017 21:48:56 -0400 Subject: restructure content manager to better handle asset disposal (#352) --- .../Framework/ContentManagerShim.cs | 50 ++++++++++++++++ src/StardewModdingAPI/Framework/SContentManager.cs | 69 ++++++++++++++++------ src/StardewModdingAPI/Framework/SGame.cs | 14 ++--- 3 files changed, 108 insertions(+), 25 deletions(-) create mode 100644 src/StardewModdingAPI/Framework/ContentManagerShim.cs (limited to 'src/StardewModdingAPI/Framework') diff --git a/src/StardewModdingAPI/Framework/ContentManagerShim.cs b/src/StardewModdingAPI/Framework/ContentManagerShim.cs new file mode 100644 index 00000000..d46f23a3 --- /dev/null +++ b/src/StardewModdingAPI/Framework/ContentManagerShim.cs @@ -0,0 +1,50 @@ +using StardewValley; + +namespace StardewModdingAPI.Framework +{ + /// A minimal content manager which defers to SMAPI's main content manager. + internal class ContentManagerShim : LocalizedContentManager + { + /********* + ** Properties + *********/ + /// SMAPI's underlying content manager. + private readonly SContentManager ContentManager; + + + /********* + ** Accessors + *********/ + /// The content manager's name for logs (if any). + public string Name { get; } + + + /********* + ** Public methods + *********/ + /// Construct an instance. + /// SMAPI's underlying content manager. + /// The content manager's name for logs (if any). + public ContentManagerShim(SContentManager contentManager, string name) + : base(contentManager.ServiceProvider, contentManager.RootDirectory, contentManager.CurrentCulture, contentManager.LanguageCodeOverride) + { + this.ContentManager = contentManager; + this.Name = name; + } + + /// Load an asset that has been processed by the content pipeline. + /// The type of asset to load. + /// The asset path relative to the loader root directory, not including the .xnb extension. + public override T Load(string assetName) + { + return this.ContentManager.LoadFor(assetName, this); + } + + /// Dispose held resources. + /// Whether the content manager is disposing (rather than finalising). + protected override void Dispose(bool disposing) + { + this.ContentManager.DisposeFor(this); + } + } +} diff --git a/src/StardewModdingAPI/Framework/SContentManager.cs b/src/StardewModdingAPI/Framework/SContentManager.cs index 25775291..4c4eee58 100644 --- a/src/StardewModdingAPI/Framework/SContentManager.cs +++ b/src/StardewModdingAPI/Framework/SContentManager.cs @@ -48,6 +48,9 @@ namespace StardewModdingAPI.Framework /// The assets currently being intercepted by instances. This is used to prevent infinite loops when a loader loads a new asset. private readonly ContextHash AssetsBeingLoaded = new ContextHash(); + /// A lookup of the content managers which loaded each asset. + private readonly IDictionary> AssetLoaders = new Dictionary>(); + /********* ** Accessors @@ -98,7 +101,6 @@ namespace StardewModdingAPI.Framework // get asset data this.CoreAssets = new CoreAssets(this.NormaliseAssetName); this.KeyLocales = this.GetKeyLocales(reflection); - } /// Normalise path separators in a file path. For asset keys, see instead. @@ -134,12 +136,24 @@ namespace StardewModdingAPI.Framework /// The type of asset to load. /// The asset path relative to the loader root directory, not including the .xnb extension. public override T Load(string assetName) + { + return this.LoadFor(assetName, this); + } + + /// Load an asset that has been processed by the content pipeline. + /// The type of asset to load. + /// The asset path relative to the loader root directory, not including the .xnb extension. + /// The content manager instance for which to load the asset. + public T LoadFor(string assetName, ContentManager instance) { assetName = this.NormaliseAssetName(assetName); // skip if already loaded if (this.IsNormalisedKeyLoaded(assetName)) + { + this.TrackAssetLoader(assetName, instance); return base.Load(assetName); + } // load asset T data; @@ -162,6 +176,7 @@ namespace StardewModdingAPI.Framework // update cache & return data this.Cache[assetName] = data; + this.TrackAssetLoader(assetName, instance); return data; } @@ -172,8 +187,8 @@ namespace StardewModdingAPI.Framework public void Inject(string assetName, T value) { assetName = this.NormaliseAssetName(assetName); - this.Cache[assetName] = value; + this.TrackAssetLoader(assetName, this); } /// Get the current content locale. @@ -229,8 +244,9 @@ namespace StardewModdingAPI.Framework /// Purge matched assets from the cache. /// Matches the asset keys to invalidate. + /// Whether to dispose invalidated assets. This should only be true when they're being invalidated as part of a dispose, to avoid crashing the game. /// Returns whether any cache entries were invalidated. - public bool InvalidateCache(Func predicate) + public bool InvalidateCache(Func predicate, bool dispose = true) { // find matching asset keys HashSet purgeCacheKeys = new HashSet(StringComparer.InvariantCultureIgnoreCase); @@ -246,9 +262,14 @@ namespace StardewModdingAPI.Framework } } - // purge from cache + // purge assets foreach (string key in purgeCacheKeys) + { + if (dispose && this.Cache[key] is IDisposable disposable) + disposable.Dispose(); this.Cache.Remove(key); + this.AssetLoaders.Remove(key); + } // reload core game assets int reloaded = 0; @@ -268,6 +289,19 @@ namespace StardewModdingAPI.Framework return false; } + /// Dispose assets for the given content manager shim. + /// The content manager whose assets to dispose. + internal void DisposeFor(ContentManagerShim shim) + { + this.Monitor.Log($"Content manager '{shim.Name}' disposed, disposing assets that aren't needed by any other asset loader.", LogLevel.Trace); + HashSet keys = new HashSet( + from entry in this.AssetLoaders + where entry.Value.Count == 1 && entry.Value.First() == shim + select entry.Key + ); + this.InvalidateCache((key, type) => keys.Contains(key)); + } + /********* ** Private methods @@ -280,6 +314,16 @@ namespace StardewModdingAPI.Framework || this.Cache.ContainsKey($"{normalisedAssetName}.{this.GetKeyLocale.Invoke()}"); // translated asset } + /// Track that a content manager loaded an asset. + /// The asset key that was loaded. + /// The content manager that loaded the asset. + private void TrackAssetLoader(string key, ContentManager manager) + { + if (!this.AssetLoaders.TryGetValue(key, out HashSet hash)) + hash = this.AssetLoaders[key] = new HashSet(); + hash.Add(manager); + } + /// Get the locale codes (like ja-JP) used in asset keys. /// Simplifies access to private game code. private IDictionary GetKeyLocales(Reflector reflection) @@ -463,23 +507,12 @@ namespace StardewModdingAPI.Framework } } - /// Dispose all game resources. + /// Dispose held resources. /// Whether the content manager is disposing (rather than finalising). protected override void Dispose(bool disposing) { - if (!disposing) - return; - - // Clear cache & reload all assets. While that may seem perverse during disposal, it's - // necessary due to limitations in the way SMAPI currently intercepts content assets. - // - // The game uses multiple content managers while SMAPI needs one and only one. The game - // only disposes some of its content managers when returning to title, which means SMAPI - // can't know which assets are meant to be disposed. Here we remove current assets from - // the cache, but don't dispose them to avoid crashing any code that still references - // them. The garbage collector will eventually clean up any unused assets. - this.Monitor.Log("Content manager disposed, resetting cache.", LogLevel.Trace); - this.InvalidateCache((key, type) => true); + this.Monitor.Log("Disposing SMAPI's main content manager. It will no longer be usable after this point.", LogLevel.Trace); + base.Dispose(disposing); } } } diff --git a/src/StardewModdingAPI/Framework/SGame.cs b/src/StardewModdingAPI/Framework/SGame.cs index 2bfbc06a..76c106d7 100644 --- a/src/StardewModdingAPI/Framework/SGame.cs +++ b/src/StardewModdingAPI/Framework/SGame.cs @@ -38,9 +38,6 @@ namespace StardewModdingAPI.Framework /// Encapsulates monitoring and logging. private readonly IMonitor Monitor; - /// SMAPI's content manager. - private readonly SContentManager SContentManager; - /// The maximum number of consecutive attempts SMAPI should make to recover from a draw error. private readonly Countdown DrawCrashTimer = new Countdown(60); // 60 ticks = roughly one second @@ -177,6 +174,9 @@ namespace StardewModdingAPI.Framework /********* ** Accessors *********/ + /// SMAPI's content manager. + public SContentManager SContentManager { get; } + /// Whether SMAPI should log more information about the game context. public bool VerboseLogging { get; set; } @@ -201,9 +201,9 @@ namespace StardewModdingAPI.Framework // override content manager this.Monitor?.Log("Overriding content manager...", LogLevel.Trace); this.SContentManager = new SContentManager(this.Content.ServiceProvider, this.Content.RootDirectory, Thread.CurrentThread.CurrentUICulture, null, this.Monitor); - this.Content = this.SContentManager; - Game1.content = this.SContentManager; - reflection.GetPrivateField(typeof(Game1), "_temporaryContent").SetValue(null); // regenerate value with new content manager + this.Content = new ContentManagerShim(this.SContentManager, "SGame.Content"); + Game1.content = new ContentManagerShim(this.SContentManager, "Game1.content"); + reflection.GetPrivateField(typeof(Game1), "_temporaryContent").SetValue(new ContentManagerShim(this.SContentManager, "Game1._temporaryContent")); // regenerate value with new content manager } /**** @@ -226,7 +226,7 @@ namespace StardewModdingAPI.Framework throw new InvalidOperationException("SMAPI uses a single content manager internally. You can't get a new content manager with a different service provider."); if (rootDirectory != this.Content.RootDirectory) throw new InvalidOperationException($"SMAPI uses a single content manager internally. You can't get a new content manager with a different root directory (current is {this.Content.RootDirectory}, requested {rootDirectory})."); - return this.SContentManager; + return new ContentManagerShim(this.SContentManager, "(generated instance)"); } /// The method called when the game is updating its state. This happens roughly 60 times per second. -- cgit