Fix entities getting stuck red (#28981)

diff --git a/Content.Client/Effects/ColorFlashEffectSystem.cs b/Content.Client/Effects/ColorFlashEffectSystem.cs
index af0bd4b600dd556a509e189ad6158508577ca8a0..0570b4951ecfbe99a9d2cf731893b75168c5332b 100644 (file)
--- a/Content.Client/Effects/ColorFlashEffectSystem.cs
+++ b/Content.Client/Effects/ColorFlashEffectSystem.cs
@@ -2,8 +2,10 @@ using Content.Shared.Effects;
 using Robust.Client.Animations;
 using Robust.Client.GameObjects;
 using Robust.Shared.Animations;
+using Robust.Shared.Collections;
 using Robust.Shared.Player;
 using Robust.Shared.Timing;
+using Robust.Shared.Utility;
 
 namespace Content.Client.Effects;
 
@@ -11,13 +13,13 @@ public sealed class ColorFlashEffectSystem : SharedColorFlashEffectSystem
 {
     [Dependency] private readonly IGameTiming _timing = default!;
     [Dependency] private readonly AnimationPlayerSystem _animation = default!;
-    [Dependency] private readonly IComponentFactory _factory = default!;
 
     /// <summary>
     /// It's a little on the long side but given we use multiple colours denoting what happened it makes it easier to register.
     /// </summary>
     private const float AnimationLength = 0.30f;
     private const string AnimationKey = "color-flash-effect";
+    private ValueList<EntityUid> _toRemove = new();
 
     public override void Initialize()
     {
@@ -44,8 +46,28 @@ public sealed class ColorFlashEffectSystem : SharedColorFlashEffectSystem
         {
             sprite.Color = component.Color;
         }
+    }
+
+    public override void Update(float frameTime)
+    {
+        base.Update(frameTime);
+
+        var query = AllEntityQuery<ColorFlashEffectComponent>();
+        _toRemove.Clear();
+
+        // Can't use deferred removal on animation completion or it will cause issues.
+        while (query.MoveNext(out var uid, out _))
+        {
+            if (_animation.HasRunningAnimation(uid, AnimationKey))
+                continue;
 
-        RemCompDeferred<ColorFlashEffectComponent>(uid);
+            _toRemove.Add(uid);
+        }
+
+        foreach (var ent in _toRemove)
+        {
+            RemComp<ColorFlashEffectComponent>(ent);
+        }
     }
 
     private Animation? GetDamageAnimation(EntityUid uid, Color color, SpriteComponent? sprite = null)
@@ -82,51 +104,31 @@ public sealed class ColorFlashEffectSystem : SharedColorFlashEffectSystem
         {
             var ent = GetEntity(nent);
 
-            if (Deleted(ent))
+            if (Deleted(ent) || !TryComp(ent, out SpriteComponent? sprite))
             {
                 continue;
             }
 
-            if (!TryComp(ent, out AnimationPlayerComponent? player))
-            {
-                player = (AnimationPlayerComponent) _factory.GetComponent(typeof(AnimationPlayerComponent));
-                player.Owner = ent;
-                player.NetSyncEnabled = false;
-                AddComp(ent, player);
-            }
-
-            // Need to stop the existing animation first to ensure the sprite color is fixed.
-            // Otherwise we might lerp to a red colour instead.
-            if (_animation.HasRunningAnimation(ent, player, AnimationKey))
-            {
-                _animation.Stop(ent, player, AnimationKey);
-            }
-
-            if (!TryComp<SpriteComponent>(ent, out var sprite))
-            {
-                continue;
-            }
-
-            if (TryComp<ColorFlashEffectComponent>(ent, out var effect))
+#if DEBUG
+            if (!TryComp(ent, out ColorFlashEffectComponent? comp))
             {
-                sprite.Color = effect.Color;
+                DebugTools.Assert(!_animation.HasRunningAnimation(ent, AnimationKey));
             }
+#endif
 
+            _animation.Stop(ent, AnimationKey);
             var animation = GetDamageAnimation(ent, color, sprite);
 
             if (animation == null)
-                continue;
-
-            if (!TryComp(ent, out ColorFlashEffectComponent? comp))
             {
-                comp = (ColorFlashEffectComponent) _factory.GetComponent(typeof(ColorFlashEffectComponent));
-                comp.Owner = ent;
-                comp.NetSyncEnabled = false;
-                AddComp(ent, comp);
+                continue;
             }
 
+            comp = EnsureComp<ColorFlashEffectComponent>(ent);
+            comp.NetSyncEnabled = false;
             comp.Color = sprite.Color;
-            _animation.Play((ent, player), animation, AnimationKey);
+
+            _animation.Play(ent, animation, AnimationKey);
         }
     }
 }

diff --git a/Content.Client/Gravity/FloatingVisualizerSystem.cs b/Content.Client/Gravity/FloatingVisualizerSystem.cs
index f489237d107b570dc6c764f6a045d2eb62b3b13c..a7cb3333b2734a76b37438e02a06f3f15f3906dd 100644 (file)
--- a/Content.Client/Gravity/FloatingVisualizerSystem.cs
+++ b/Content.Client/Gravity/FloatingVisualizerSystem.cs
@@ -57,6 +57,6 @@ public sealed class FloatingVisualizerSystem : SharedFloatingVisualizerSystem
         if (args.Key != component.AnimationKey)
             return;
 
-        FloatAnimation(uid, component.Offset, component.AnimationKey, component.AnimationTime, !component.CanFloat);
+        FloatAnimation(uid, component.Offset, component.AnimationKey, component.AnimationTime, stop: !component.CanFloat);
     }
 }