From e746d566f9d309b3244ea9d17f8ce6502ec10de6 Mon Sep 17 00:00:00 2001 From: B_Kirill <153602297+B-Kirill@users.noreply.github.com> Date: Wed, 25 Mar 2026 19:57:38 +1000 Subject: [PATCH 1/2] Add deterministic disposal for textures in IResourceCache --- .../ResourceManagement/BaseResource.cs | 5 ++++ .../ResourceManagement/IResourceCache.cs | 4 +++ .../ResourceManagement/ResourceCache.cs | 24 ++++++++++++++- .../ResourceTypes/TextureResource.cs | 29 +++++++++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/Robust.Client/ResourceManagement/BaseResource.cs b/Robust.Client/ResourceManagement/BaseResource.cs index 4bc1b30fb8b..c7383b83718 100644 --- a/Robust.Client/ResourceManagement/BaseResource.cs +++ b/Robust.Client/ResourceManagement/BaseResource.cs @@ -15,6 +15,11 @@ public abstract class BaseResource : IDisposable /// public virtual ResPath? Fallback => null; + /// + /// Whether this resource type can be deterministically removed from the cache. + /// + public virtual bool CanBeRemoved => false; + /// /// Disposes this resource. /// diff --git a/Robust.Client/ResourceManagement/IResourceCache.cs b/Robust.Client/ResourceManagement/IResourceCache.cs index 16a2532c71d..ae7c9aab7f0 100644 --- a/Robust.Client/ResourceManagement/IResourceCache.cs +++ b/Robust.Client/ResourceManagement/IResourceCache.cs @@ -28,6 +28,10 @@ bool TryGetResource(ResPath path, [NotNullWhen(true)] out T? resource) bool TryGetResource(AudioStream stream, [NotNullWhen(true)] out AudioResource? resource); + bool TryRemoveResource(string path) where T : BaseResource, new(); + + bool TryRemoveResource(ResPath path) where T : BaseResource, new(); + void ReloadResource(string path) where T : BaseResource, new(); diff --git a/Robust.Client/ResourceManagement/ResourceCache.cs b/Robust.Client/ResourceManagement/ResourceCache.cs index cdcea4fccfe..cd30ee8f577 100644 --- a/Robust.Client/ResourceManagement/ResourceCache.cs +++ b/Robust.Client/ResourceManagement/ResourceCache.cs @@ -7,7 +7,6 @@ using Robust.Client.Audio; using Robust.Shared.ContentPack; using Robust.Shared.IoC; -using Robust.Shared.Log; using Robust.Shared.Utility; namespace Robust.Client.ResourceManagement; @@ -107,6 +106,29 @@ public bool TryGetResource(AudioStream stream, [NotNullWhen(true)] out AudioReso return true; } + public bool TryRemoveResource(string path) where T : BaseResource, new() + => TryRemoveResource(new ResPath(path)); + + public bool TryRemoveResource(ResPath path) where T : BaseResource, new() + { + var cache = GetTypeData(); + + if (!cache.Resources.TryGetValue(path, out var resource)) + return false; + + if (!resource.CanBeRemoved) + return false; + + if (resource.Fallback == path || (_fallbacks.TryGetValue(typeof(T), out var fallbackRes) && ReferenceEquals(fallbackRes, resource))) + return false; + + cache.Resources.Remove(path); + cache.NonExistent.Remove(path); + resource.Dispose(); + + return true; + } + public void ReloadResource(string path) where T : BaseResource, new() { ReloadResource(new ResPath(path)); diff --git a/Robust.Client/ResourceManagement/ResourceTypes/TextureResource.cs b/Robust.Client/ResourceManagement/ResourceTypes/TextureResource.cs index 8b5fb6f96df..8251089c32d 100644 --- a/Robust.Client/ResourceManagement/ResourceTypes/TextureResource.cs +++ b/Robust.Client/ResourceManagement/ResourceTypes/TextureResource.cs @@ -1,4 +1,5 @@ -using System.IO; +using System; +using System.IO; using System.Threading; using Robust.Client.Graphics; using Robust.Shared.ContentPack; @@ -16,9 +17,19 @@ namespace Robust.Client.ResourceManagement public sealed class TextureResource : BaseResource { private OwnedTexture _texture = default!; + private bool _disposed; + public override ResPath? Fallback => new("/Textures/noSprite.png"); + public override bool CanBeRemoved => true; - public Texture Texture => _texture; + public Texture Texture + { + get + { + ObjectDisposedException.ThrowIf(_disposed, this); + return _texture; + } + } public override void Load(IDependencyCollection dependencies, ResPath path) { @@ -106,12 +117,14 @@ internal void LoadFinish(IResourceCache cache, LoadStepData data) public override void Reload(IDependencyCollection dependencies, ResPath path, CancellationToken ct = default) { + ObjectDisposedException.ThrowIf(_disposed, this); + var data = new LoadStepData {Path = path}; LoadTextureParameters(dependencies.Resolve(), data); LoadPreTextureData(dependencies.Resolve(), data); - if (data.Image.Width == Texture.Width && data.Image.Height == Texture.Height) + if (data.Image.Width == _texture.Width && data.Image.Height == _texture.Height) { // Dimensions match, rewrite texture in place. _texture.SetSubImage(Vector2i.Zero, data.Image); @@ -127,6 +140,16 @@ public override void Reload(IDependencyCollection dependencies, ResPath path, Ca data.Image.Dispose(); } + public override void Dispose() + { + if (_disposed) + return; + + _texture?.Dispose(); + _disposed = true; + base.Dispose(); + } + internal sealed class LoadStepData { public ResPath Path = default!; From 171b8802a9a6070c5faf3107a051ead02b9352bf Mon Sep 17 00:00:00 2001 From: B_Kirill <153602297+B-Kirill@users.noreply.github.com> Date: Thu, 26 Mar 2026 22:03:57 +1000 Subject: [PATCH 2/2] Review --- .../ResourceManagement/BaseResource.cs | 12 +------- .../ResourceManagement/IBaseResource.cs | 19 ++++++++++++ .../ResourceManagement/IResourceCache.cs | 10 +++---- .../ResourceManagement/ResourceCache.cs | 29 +++++++++---------- .../ResourceTypes/RSIResource.cs | 4 +-- .../ResourceTypes/TextureResource.cs | 17 ++++++----- 6 files changed, 51 insertions(+), 40 deletions(-) create mode 100644 Robust.Client/ResourceManagement/IBaseResource.cs diff --git a/Robust.Client/ResourceManagement/BaseResource.cs b/Robust.Client/ResourceManagement/BaseResource.cs index c7383b83718..8d6314e074d 100644 --- a/Robust.Client/ResourceManagement/BaseResource.cs +++ b/Robust.Client/ResourceManagement/BaseResource.cs @@ -8,18 +8,8 @@ namespace Robust.Client.ResourceManagement; /// /// Base resource for the cache. /// -public abstract class BaseResource : IDisposable +public abstract class BaseResource : IBaseResource, IDisposable { - /// - /// Fallback resource path if this one does not exist. - /// - public virtual ResPath? Fallback => null; - - /// - /// Whether this resource type can be deterministically removed from the cache. - /// - public virtual bool CanBeRemoved => false; - /// /// Disposes this resource. /// diff --git a/Robust.Client/ResourceManagement/IBaseResource.cs b/Robust.Client/ResourceManagement/IBaseResource.cs new file mode 100644 index 00000000000..4e4e9574b2c --- /dev/null +++ b/Robust.Client/ResourceManagement/IBaseResource.cs @@ -0,0 +1,19 @@ +using Robust.Shared.Utility; + +namespace Robust.Client.ResourceManagement; + +/// +/// Defines type-level metadata for resource types. +/// +public interface IBaseResource +{ + /// + /// Whether resources of this type can be deterministically removed from the cache. + /// + static virtual bool CanBeRemoved => false; + + /// + /// The fallback path for this resource type (if any). + /// + static virtual ResPath? FallbackPath => null; +} diff --git a/Robust.Client/ResourceManagement/IResourceCache.cs b/Robust.Client/ResourceManagement/IResourceCache.cs index ae7c9aab7f0..bde0d63b29a 100644 --- a/Robust.Client/ResourceManagement/IResourceCache.cs +++ b/Robust.Client/ResourceManagement/IResourceCache.cs @@ -15,10 +15,10 @@ namespace Robust.Client.ResourceManagement; public interface IResourceCache : IResourceManager { T GetResource(string path, bool useFallback = true) - where T : BaseResource, new(); + where T : BaseResource, IBaseResource, new(); T GetResource(ResPath path, bool useFallback = true) - where T : BaseResource, new(); + where T : BaseResource, IBaseResource, new(); bool TryGetResource(string path, [NotNullWhen(true)] out T? resource) where T : BaseResource, new(); @@ -28,9 +28,9 @@ bool TryGetResource(ResPath path, [NotNullWhen(true)] out T? resource) bool TryGetResource(AudioStream stream, [NotNullWhen(true)] out AudioResource? resource); - bool TryRemoveResource(string path) where T : BaseResource, new(); + bool TryRemoveResource(string path) where T : BaseResource, IBaseResource, new(); - bool TryRemoveResource(ResPath path) where T : BaseResource, new(); + bool TryRemoveResource(ResPath path) where T : BaseResource, IBaseResource, new(); void ReloadResource(string path) where T : BaseResource, new(); @@ -45,7 +45,7 @@ void CacheResource(ResPath path, T resource) where T : BaseResource, new(); T GetFallback() - where T : BaseResource, new(); + where T : BaseResource, IBaseResource, new(); IEnumerable> GetAllResources() where T : BaseResource, new(); diff --git a/Robust.Client/ResourceManagement/ResourceCache.cs b/Robust.Client/ResourceManagement/ResourceCache.cs index cd30ee8f577..8b9d5b70615 100644 --- a/Robust.Client/ResourceManagement/ResourceCache.cs +++ b/Robust.Client/ResourceManagement/ResourceCache.cs @@ -19,12 +19,12 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt private readonly Dictionary _cachedResources = new(); private readonly Dictionary _fallbacks = new(); - public T GetResource(string path, bool useFallback = true) where T : BaseResource, new() + public T GetResource(string path, bool useFallback = true) where T : BaseResource, IBaseResource, new() { return GetResource(new ResPath(path), useFallback); } - public T GetResource(ResPath path, bool useFallback = true) where T : BaseResource, new() + public T GetResource(ResPath path, bool useFallback = true) where T : BaseResource, IBaseResource, new() { var cache = GetTypeData(); if (cache.Resources.TryGetValue(path, out var cached)) @@ -42,11 +42,11 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt } catch (Exception e) { - if (useFallback && resource.Fallback != null) + if (useFallback && T.FallbackPath != null) { Sawmill.Error( $"Exception while loading resource {typeof(T)} at '{path}', resorting to fallback.\n{Environment.StackTrace}\n{e}"); - return GetResource(resource.Fallback.Value, false); + return GetResource(T.FallbackPath.Value, false); } else { @@ -106,20 +106,20 @@ public bool TryGetResource(AudioStream stream, [NotNullWhen(true)] out AudioReso return true; } - public bool TryRemoveResource(string path) where T : BaseResource, new() + public bool TryRemoveResource(string path) where T : BaseResource, IBaseResource, new() => TryRemoveResource(new ResPath(path)); - public bool TryRemoveResource(ResPath path) where T : BaseResource, new() + public bool TryRemoveResource(ResPath path) where T : BaseResource, IBaseResource, new() { - var cache = GetTypeData(); + if (!T.CanBeRemoved) + throw new NotSupportedException($"Resource type '{typeof(T)}' does not support deterministic removal."); - if (!cache.Resources.TryGetValue(path, out var resource)) + if (T.FallbackPath == path) return false; - if (!resource.CanBeRemoved) - return false; + var cache = GetTypeData(); - if (resource.Fallback == path || (_fallbacks.TryGetValue(typeof(T), out var fallbackRes) && ReferenceEquals(fallbackRes, resource))) + if (!cache.Resources.TryGetValue(path, out var resource)) return false; cache.Resources.Remove(path); @@ -175,20 +175,19 @@ public bool TryGetResource(AudioStream stream, [NotNullWhen(true)] out AudioReso GetTypeData().Resources[path] = resource; } - public T GetFallback() where T : BaseResource, new() + public T GetFallback() where T : BaseResource, IBaseResource, new() { if (_fallbacks.TryGetValue(typeof(T), out var fallback)) { return (T) fallback; } - var res = new T(); - if (res.Fallback == null) + if (T.FallbackPath == null) { throw new InvalidOperationException($"Resource of type '{typeof(T)}' has no fallback."); } - fallback = GetResource(res.Fallback.Value, useFallback: false); + fallback = GetResource(T.FallbackPath.Value, useFallback: false); _fallbacks.Add(typeof(T), fallback); return (T) fallback; } diff --git a/Robust.Client/ResourceManagement/ResourceTypes/RSIResource.cs b/Robust.Client/ResourceManagement/ResourceTypes/RSIResource.cs index 0880f8e9518..5fd015da841 100644 --- a/Robust.Client/ResourceManagement/ResourceTypes/RSIResource.cs +++ b/Robust.Client/ResourceManagement/ResourceTypes/RSIResource.cs @@ -18,9 +18,9 @@ namespace Robust.Client.ResourceManagement /// Handles the loading code for RSI files. /// See for the RSI API itself. /// - public sealed class RSIResource : BaseResource + public sealed class RSIResource : BaseResource, IBaseResource { - public override ResPath? Fallback => new("/Textures/error.rsi"); + static ResPath? IBaseResource.FallbackPath => new("/Textures/error.rsi"); public RSI RSI { get; private set; } = default!; diff --git a/Robust.Client/ResourceManagement/ResourceTypes/TextureResource.cs b/Robust.Client/ResourceManagement/ResourceTypes/TextureResource.cs index 8251089c32d..8babbe6b9da 100644 --- a/Robust.Client/ResourceManagement/ResourceTypes/TextureResource.cs +++ b/Robust.Client/ResourceManagement/ResourceTypes/TextureResource.cs @@ -14,13 +14,14 @@ namespace Robust.Client.ResourceManagement { - public sealed class TextureResource : BaseResource + public sealed class TextureResource : BaseResource, IBaseResource { private OwnedTexture _texture = default!; private bool _disposed; + private readonly Lock _lock = new(); - public override ResPath? Fallback => new("/Textures/noSprite.png"); - public override bool CanBeRemoved => true; + static ResPath? IBaseResource.FallbackPath => new("/Textures/noSprite.png"); + static bool IBaseResource.CanBeRemoved => true; public Texture Texture { @@ -142,11 +143,13 @@ public override void Reload(IDependencyCollection dependencies, ResPath path, Ca public override void Dispose() { - if (_disposed) - return; + lock (_lock) + { + if (_disposed) return; + _disposed = true; + _texture?.Dispose(); + } - _texture?.Dispose(); - _disposed = true; base.Dispose(); }