Appearance Weakrefs

OpenDreamRuntime/AtomManager.cs

@@ -36,6 +36,8 @@ public sealed class AtomManager {
     private ServerVerbSystem VerbSystem => _verbSystem ??= _entitySystemManager.GetEntitySystem<ServerVerbSystem>();
     private ServerAppearanceSystem? _appearanceSystem;
     private ServerVerbSystem? _verbSystem;
+    private DMISpriteSystem DMISpriteSystem => _dmiSpriteSystem ??= _entitySystemManager.GetEntitySystem<DMISpriteSystem>();
+    private DMISpriteSystem? _dmiSpriteSystem;
 
     // ReSharper disable ForCanBeConvertedToForeach (the collections could be added to)
     public IEnumerable<DreamObjectAtom> EnumerateAtoms(TreeEntry? filterType = null) {
@@ -191,7 +193,7 @@ public EntityUid CreateMovableEntity(DreamObjectMovable movable) {
         var entity = _entityManager.SpawnEntity(null, new MapCoordinates(0, 0, MapId.Nullspace));
 
         DMISpriteComponent sprite = _entityManager.AddComponent<DMISpriteComponent>(entity);
-        sprite.SetAppearance(GetAppearanceFromDefinition(movable.ObjectDefinition));
+        DMISpriteSystem.SetSpriteAppearance(new(entity, sprite), GetAppearanceFromDefinition(movable.ObjectDefinition));
 
         _entityToAtom.Add(entity, movable);
         return entity;
@@ -473,7 +475,6 @@ public bool TryGetAppearance(DreamObject atom, [NotNullWhen(true)] out IconAppea
     public void UpdateAppearance(DreamObject atom, Action<IconAppearance> update) {
         var appearance = MustGetAppearance(atom);
         appearance = (appearance != null) ? new(appearance) : new(); // Clone the appearance
-
         update(appearance);
         SetAtomAppearance(atom, appearance);
     }
@@ -482,12 +483,20 @@ public void SetAtomAppearance(DreamObject atom, IconAppearance appearance) {
         if (atom is DreamObjectTurf turf) {
             _dreamMapManager.SetTurfAppearance(turf, appearance);
         } else if (atom is DreamObjectMovable movable) {
-            movable.SpriteComponent.SetAppearance(appearance);
+            DMISpriteSystem.SetSpriteAppearance(new(movable.Entity, movable.SpriteComponent), appearance);
         } else if (atom is DreamObjectImage image) {
             image.Appearance = appearance;
         }
     }
 
+    public void SetMovableScreenLoc(DreamObjectMovable movable, ScreenLocation screenLocation) {
+        DMISpriteSystem.SetSpriteScreenLocation(new(movable.Entity, movable.SpriteComponent), screenLocation);
+    }
+
+    public void SetSpriteAppearance(Entity<DMISpriteComponent> ent, IconAppearance appearance) {
+        DMISpriteSystem.SetSpriteAppearance(ent, appearance);
+    }
+
     public void AnimateAppearance(DreamObject atom, TimeSpan duration, AnimationEasing easing, int loop, AnimationFlags flags, int delay, bool chainAnim, Action<IconAppearance> animate) {
         if (atom is not DreamObjectMovable movable)
             return; //Animating non-movables is unimplemented TODO: should handle images and maybe filters
@@ -497,7 +506,7 @@ public void AnimateAppearance(DreamObject atom, TimeSpan duration, AnimationEasi
         animate(appearance);
 
         // Don't send the updated appearance to clients, they will animate it
-        movable.SpriteComponent.SetAppearance(appearance, dirty: false);
+        DMISpriteSystem.SetSpriteAppearance(new(movable.Entity, movable.SpriteComponent), appearance, dirty: false);
 
         NetEntity ent = _entityManager.GetNetEntity(movable.Entity);
 
@@ -591,6 +600,7 @@ public IconAppearance GetAppearanceFromDefinition(DreamObjectDefinition def) {
         }
 
         _definitionAppearanceCache.Add(def, appearance);
+        AppearanceSystem.IncreaseAppearanceRefCount(appearance);
         return appearance;
     }
 

OpenDreamRuntime/Objects/Types/DreamObjectImage.cs

@@ -6,7 +6,8 @@
 namespace OpenDreamRuntime.Objects.Types;
 
 public sealed class DreamObjectImage : DreamObject {
-    public IconAppearance? Appearance;
+    private IconAppearance? _appearance = null;
+    public IconAppearance? Appearance { get => _appearance; set => SetAppearance(value); }
 
     private DreamObject? _loc;
     private DreamList _overlays;
@@ -42,13 +43,17 @@ public override void Initialize(DreamProcArguments args) {
         base.Initialize(args);
 
         DreamValue icon = args.GetArgument(0);
-        if (icon.IsNull || !AtomManager.TryCreateAppearanceFrom(icon, out Appearance)) {
+        IconAppearance? iconAppearance = null;
+        if (icon.IsNull || !AtomManager.TryCreateAppearanceFrom(icon, out iconAppearance)) {
             // Use a default appearance, but log a warning about it if icon wasn't null
             Appearance = new(AtomManager.GetAppearanceFromDefinition(ObjectDefinition));
             if (!icon.IsNull)
                 Logger.GetSawmill("opendream.image")
                     .Warning($"Attempted to create an /image from {icon}. This is invalid and a default image was created instead.");
         }
+        if(iconAppearance is not null)
+            Appearance = iconAppearance;
+
 
         int argIndex = 1;
         DreamValue loc = args.GetArgument(1);
@@ -111,7 +116,7 @@ protected override void SetVar(string varName, DreamValue value) {
                 Appearance = newAppearance;
                 if(_entity != EntityUid.Invalid) {
                     DMISpriteComponent sprite = EntityManager.GetComponent<DMISpriteComponent>(_entity);
-                    sprite.SetAppearance(Appearance!);
+                    AtomManager.SetSpriteAppearance(new(_entity, sprite), Appearance);
                 }
                 break;
             case "loc":
@@ -209,7 +214,7 @@ protected override void SetVar(string varName, DreamValue value) {
                     AtomManager.SetAppearanceVar(Appearance!, varName, value);
                     if(_entity != EntityUid.Invalid) {
                         DMISpriteComponent sprite = EntityManager.GetComponent<DMISpriteComponent>(_entity);
-                        sprite.SetAppearance(Appearance!);
+                        AtomManager.SetSpriteAppearance(new(_entity, sprite), Appearance);
                     }
                     break;
                 }
@@ -231,12 +236,22 @@ public EntityUid GetEntity() {
         if(_entity == EntityUid.Invalid) {
             _entity = EntityManager.SpawnEntity(null, new MapCoordinates(0, 0, MapId.Nullspace));
             DMISpriteComponent sprite = EntityManager.AddComponent<DMISpriteComponent>(_entity);
-            sprite.SetAppearance(Appearance!);
+            AtomManager.SetSpriteAppearance(new(_entity, sprite), Appearance);
         }
         return _entity;
     }
 
+    public void SetAppearance(IconAppearance? appearance) {
+        if(_appearance is not null)
+            AppearanceSystem!.DecreaseAppearanceRefCount(_appearance);
+        if(appearance is not null)
+            AppearanceSystem!.IncreaseAppearanceRefCount(appearance);
+        _appearance = appearance;
+    }
+
     protected override void HandleDeletion() {
+        if(_appearance is not null)
+            AppearanceSystem!.DecreaseAppearanceRefCount(_appearance);
         if(_entity != EntityUid.Invalid) {
             EntityManager.DeleteEntity(_entity);
         }

OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs

@@ -23,13 +23,7 @@ public class DreamObjectMovable : DreamObjectAtom {
     private string? ScreenLoc {
         get => _screenLoc;
         set {
-            _screenLoc = value;
-            if (!EntityManager.TryGetComponent<DMISpriteComponent>(Entity, out var sprite))
-                return;
-
-            sprite.ScreenLocation = !string.IsNullOrEmpty(value) ?
-                                        new ScreenLocation(value) :
-                                        new ScreenLocation(0, 0, 0, 0);
+            AtomManager.SetMovableScreenLoc(this, !string.IsNullOrEmpty(value) ? new ScreenLocation(value) : new ScreenLocation(0, 0, 0, 0));
         }
     }
 

OpenDreamRuntime/Objects/Types/DreamObjectTurf.cs

@@ -4,7 +4,9 @@ public sealed class DreamObjectTurf : DreamObjectAtom {
     public readonly int X, Y, Z;
     public readonly IDreamMapManager.Cell Cell;
     public readonly TurfContentsList Contents;
-    public int AppearanceId;
+    public int AppearanceId { get => _appearanceId; set => SetAppearanceId(value); }
+
+    private int _appearanceId = -1;
 
     public DreamObjectTurf(DreamObjectDefinition objectDefinition, int x, int y, int z, IDreamMapManager.Cell cell) : base(objectDefinition) {
         X = x;
@@ -63,4 +65,15 @@ protected override void SetVar(string varName, DreamValue value) {
                 break;
         }
     }
+
+    public void SetAppearanceId(int appearanceId) {
+        if (_appearanceId != -1) {
+            AppearanceSystem!.DecreaseAppearanceRefCount(_appearanceId);
+        }
+
+        _appearanceId = appearanceId;
+
+        AppearanceSystem!.IncreaseAppearanceRefCount(_appearanceId);
+
+    }
 }

OpenDreamRuntime/Rendering/DMISpriteComponent.cs

@@ -1,27 +1,14 @@
 using OpenDreamShared.Dream;
 using OpenDreamShared.Rendering;
 
-namespace OpenDreamRuntime.Rendering {
-    [RegisterComponent]
-    public sealed partial class DMISpriteComponent : SharedDMISpriteComponent {
-        [ViewVariables]
-        public ScreenLocation? ScreenLocation {
-            get => _screenLocation;
-            set {
-                _screenLocation = value;
-                Dirty();
-            }
-        }
-        private ScreenLocation? _screenLocation;
-
-        [ViewVariables] public IconAppearance? Appearance { get; private set; }
-
-        public void SetAppearance(IconAppearance? appearance, bool dirty = true) {
-            Appearance = appearance;
-
-            if (dirty) {
-                Dirty();
-            }
-        }
-    }
+namespace OpenDreamRuntime.Rendering;
+[RegisterComponent]
+public sealed partial class DMISpriteComponent : SharedDMISpriteComponent {
+    [ViewVariables]
+    [Access(typeof(DMISpriteSystem))]
+    public ScreenLocation ScreenLocation;
+
+    [Access(typeof(DMISpriteSystem))]
+    [ViewVariables] public IconAppearance? Appearance;
 }
+

OpenDreamRuntime/Rendering/DMISpriteSystem.cs

@@ -1,4 +1,5 @@
-using OpenDreamShared.Rendering;
+using OpenDreamShared.Dream;
+using OpenDreamShared.Rendering;
 using Robust.Shared.GameStates;
 
 namespace OpenDreamRuntime.Rendering;
@@ -18,4 +19,21 @@ private void GetComponentState(EntityUid uid, DMISpriteComponent component, ref
 
         args.State = new SharedDMISpriteComponent.DMISpriteComponentState(appearanceId, component.ScreenLocation);
     }
+
+    public void SetSpriteAppearance(Entity<DMISpriteComponent> ent, IconAppearance appearance, bool dirty = true) {
+        DMISpriteComponent component = ent.Comp;
+        _appearance.IncreaseAppearanceRefCount(appearance);
+        if(component.Appearance is not null)
+            _appearance.DecreaseAppearanceRefCount(component.Appearance);
+
+        component.Appearance = appearance;
+        if(dirty)
+            Dirty(ent, component);
+    }
+
+    public void SetSpriteScreenLocation(Entity<DMISpriteComponent> ent, ScreenLocation screenLocation) {
+        DMISpriteComponent component = ent.Comp;
+        component.ScreenLocation = screenLocation;
+        Dirty(ent, component);
+    }
 }

OpenDreamRuntime/Rendering/ServerAppearanceSystem.cs

@@ -10,6 +10,7 @@ namespace OpenDreamRuntime.Rendering;
 public sealed class ServerAppearanceSystem : SharedAppearanceSystem {
     private readonly Dictionary<IconAppearance, int> _appearanceToId = new();
     private readonly Dictionary<int, IconAppearance> _idToAppearance = new();
+    private readonly Dictionary<IconAppearance, int> _appearanceRefCounts = new();
     private int _appearanceIdCounter;
 
     /// <summary>
@@ -37,6 +38,47 @@ private void OnPlayerStatusChanged(object? sender, SessionStatusEventArgs e) {
         }
     }
 
+    public void IncreaseAppearanceRefCount(IconAppearance appearance) {
+        lock (_lock) {
+            if (!_appearanceRefCounts.TryGetValue(appearance, out int count)) {
+                count = 0;
+            }
+            _appearanceRefCounts[appearance] = count + 1;
+        }
+    }
+    public void IncreaseAppearanceRefCount(int appearanceId) {
+        if (!_idToAppearance.TryGetValue(appearanceId, out IconAppearance? appearance)) {
+            return;
+        }
+
+        IncreaseAppearanceRefCount(appearance);
+    }
+
+    public void DecreaseAppearanceRefCount(IconAppearance appearance) {
+        lock (_lock) {
+            if (!_appearanceRefCounts.TryGetValue(appearance, out int count)) {
+                throw new InvalidOperationException("Appearance ref count is already 0");
+            }
+
+            if (count == 1) {
+                _appearanceRefCounts.Remove(appearance);
+                _appearanceToId.Remove(appearance);
+                if(_appearanceToId.TryGetValue(appearance, out int id))
+                    _idToAppearance.Remove(id);
+                RaiseNetworkEvent(new RemoveAppearanceEvent(id));
+                //let the GC sort out the rest
+            } else {
+                _appearanceRefCounts[appearance] = count - 1;
+            }
+        }
+    }
+    public void DecreaseAppearanceRefCount(int appearanceId) {
+        if (!_idToAppearance.TryGetValue(appearanceId, out IconAppearance? appearance)) {
+            return;
+        }
+
+        DecreaseAppearanceRefCount(appearance);
+    }
     public int AddAppearance(IconAppearance appearance) {
         lock (_lock) {
             if (!_appearanceToId.TryGetValue(appearance, out int appearanceId)) {
@@ -45,7 +87,6 @@ public int AddAppearance(IconAppearance appearance) {
                 _idToAppearance.Add(appearanceId, appearance);
                 RaiseNetworkEvent(new NewAppearanceEvent(appearanceId, appearance));
             }
-
             return appearanceId;
         }
     }

OpenDreamShared/Rendering/SharedAppearanceSystem.cs

@@ -17,6 +17,10 @@ public sealed class NewAppearanceEvent(int appearanceId, IconAppearance appearan
         public int AppearanceId { get; } = appearanceId;
         public IconAppearance Appearance { get; } = appearance;
     }
+    [Serializable, NetSerializable]
+    public sealed class RemoveAppearanceEvent(int appearanceId) : EntityEventArgs {
+        public int AppearanceId { get; } = appearanceId;
+    }
 
     [Serializable, NetSerializable]
     public sealed class AnimationEvent(NetEntity entity, int targetAppearanceId, TimeSpan duration, AnimationEasing easing, int loop, AnimationFlags flags, int delay, bool chainAnim)