Refactor LightBehaviorSystem to remove obsolete code (#30890)

Content.Client/Light/Components/LightBehaviourComponent.cs

@@ -1,4 +1,4 @@
-using System.Linq;
+using System.Linq;
 using Content.Shared.Light.Components;
 using JetBrains.Annotations;
 using Robust.Client.Animations;
@@ -362,7 +362,7 @@ namespace Content.Client.Light.Components
         [Dependency] private readonly IEntityManager _entMan = default!;
         [Dependency] private readonly IRobustRandom _random = default!;
 
-        private const string KeyPrefix = nameof(LightBehaviourComponent);
+        public const string KeyPrefix = nameof(LightBehaviourComponent);
 
         public sealed class AnimationContainer
         {
@@ -387,7 +387,7 @@ namespace Content.Client.Light.Components
         public readonly List<AnimationContainer> Animations = new();
 
         [ViewVariables(VVAccess.ReadOnly)]
-        private Dictionary<string, object> _originalPropertyValues = new();
+        public Dictionary<string, object> OriginalPropertyValues = new();
 
         void ISerializationHooks.AfterDeserialization()
         {
@@ -404,148 +404,5 @@ namespace Content.Client.Light.Components
                 key++;
             }
         }
-
-        /// <summary>
-        /// If we disable all the light behaviours we want to be able to revert the light to its original state.
-        /// </summary>
-        private void CopyLightSettings(EntityUid uid, string property)
-        {
-            if (_entMan.TryGetComponent(uid, out PointLightComponent? light))
-            {
-                var propertyValue = AnimationHelper.GetAnimatableProperty(light, property);
-                if (propertyValue != null)
-                {
-                    _originalPropertyValues.Add(property, propertyValue);
-                }
-            }
-            else
-            {
-                Logger.Warning($"{_entMan.GetComponent<MetaDataComponent>(uid).EntityName} has a {nameof(LightBehaviourComponent)} but it has no {nameof(PointLightComponent)}! Check the prototype!");
-            }
-        }
-
-        /// <summary>
-        /// Start animating a light behaviour with the specified ID. If the specified ID is empty, it will start animating all light behaviour entries.
-        /// If specified light behaviours are already animating, calling this does nothing.
-        /// Multiple light behaviours can have the same ID.
-        /// </summary>
-        public void StartLightBehaviour(string id = "")
-        {
-            var uid = Owner;
-            if (!_entMan.TryGetComponent(uid, out AnimationPlayerComponent? animation))
-            {
-                return;
-            }
-
-            var animations = _entMan.System<AnimationPlayerSystem>();
-
-            foreach (var container in Animations)
-            {
-                if (container.LightBehaviour.ID == id || id == string.Empty)
-                {
-                    if (!animations.HasRunningAnimation(uid, animation, KeyPrefix + container.Key))
-                    {
-                        CopyLightSettings(uid, container.LightBehaviour.Property);
-                        container.LightBehaviour.UpdatePlaybackValues(container.Animation);
-                        animations.Play(uid, animation, container.Animation, KeyPrefix + container.Key);
-                    }
-                }
-            }
-        }
-
-        /// <summary>
-        /// If any light behaviour with the specified ID is animating, then stop it.
-        /// If no ID is specified then all light behaviours will be stopped.
-        /// Multiple light behaviours can have the same ID.
-        /// </summary>
-        /// <param name="id"></param>
-        /// <param name="removeBehaviour">Should the behaviour(s) also be removed permanently?</param>
-        /// <param name="resetToOriginalSettings">Should the light have its original settings applied?</param>
-        public void StopLightBehaviour(string id = "", bool removeBehaviour = false, bool resetToOriginalSettings = false)
-        {
-            var uid = Owner;
-            if (!_entMan.TryGetComponent(uid, out AnimationPlayerComponent? animation))
-            {
-                return;
-            }
-
-            var toRemove = new List<AnimationContainer>();
-            var animations = _entMan.System<AnimationPlayerSystem>();
-
-            foreach (var container in Animations)
-            {
-                if (container.LightBehaviour.ID == id || id == string.Empty)
-                {
-                    if (animations.HasRunningAnimation(uid, animation, KeyPrefix + container.Key))
-                    {
-                        animations.Stop(uid, animation, KeyPrefix + container.Key);
-                    }
-
-                    if (removeBehaviour)
-                    {
-                        toRemove.Add(container);
-                    }
-                }
-            }
-
-            foreach (var container in toRemove)
-            {
-                Animations.Remove(container);
-            }
-
-            if (resetToOriginalSettings && _entMan.TryGetComponent(uid, out PointLightComponent? light))
-            {
-                foreach (var (property, value) in _originalPropertyValues)
-                {
-                    AnimationHelper.SetAnimatableProperty(light, property, value);
-                }
-            }
-
-            _originalPropertyValues.Clear();
-        }
-
-        /// <summary>
-        /// Checks if at least one behaviour is running.
-        /// </summary>
-        /// <returns>Whether at least one behaviour is running, false if none is.</returns>
-        public bool HasRunningBehaviours()
-        {
-            var uid = Owner;
-            if (!_entMan.TryGetComponent(uid, out AnimationPlayerComponent? animation))
-            {
-                return false;
-            }
-
-            var animations = _entMan.System<AnimationPlayerSystem>();
-            return Animations.Any(container => animations.HasRunningAnimation(uid, animation, KeyPrefix + container.Key));
-        }
-
-        /// <summary>
-        /// Add a new light behaviour to the component and start it immediately unless otherwise specified.
-        /// </summary>
-        public void AddNewLightBehaviour(LightBehaviourAnimationTrack behaviour, bool playImmediately = true)
-        {
-            var key = 0;
-
-            while (Animations.Any(x => x.Key == key))
-            {
-                key++;
-            }
-
-            var animation = new Animation()
-            {
-                AnimationTracks = {behaviour}
-            };
-
-            behaviour.Initialize(Owner, _random, _entMan);
-
-            var container = new AnimationContainer(key, animation, behaviour);
-            Animations.Add(container);
-
-            if (playImmediately)
-            {
-                StartLightBehaviour(behaviour.ID);
-            }
-        }
     }
 }

Content.Client/Light/EntitySystems/ExpendableLightSystem.cs

@@ -11,6 +11,7 @@ public sealed class ExpendableLightSystem : VisualizerSystem<ExpendableLightComp
 {
     [Dependency] private readonly PointLightSystem _pointLightSystem = default!;
     [Dependency] private readonly SharedAudioSystem _audioSystem = default!;
+    [Dependency] private readonly LightBehaviorSystem _lightBehavior = default!;
 
     public override void Initialize()
     {
@@ -32,11 +33,11 @@ public sealed class ExpendableLightSystem : VisualizerSystem<ExpendableLightComp
         if (AppearanceSystem.TryGetData<string>(uid, ExpendableLightVisuals.Behavior, out var lightBehaviourID, args.Component)
         &&  TryComp<LightBehaviourComponent>(uid, out var lightBehaviour))
         {
-            lightBehaviour.StopLightBehaviour();
+            _lightBehavior.StopLightBehaviour((uid, lightBehaviour));
 
             if (!string.IsNullOrEmpty(lightBehaviourID))
             {
-                lightBehaviour.StartLightBehaviour(lightBehaviourID);
+                _lightBehavior.StartLightBehaviour((uid, lightBehaviour), lightBehaviourID);
             }
             else if (TryComp<PointLightComponent>(uid, out var light))
             {

Content.Client/Light/EntitySystems/LightBehaviorSystem.cs

@@ -1,7 +1,9 @@
 using System.Linq;
 using Content.Client.Light.Components;
 using Robust.Client.GameObjects;
+using Robust.Client.Animations;
 using Robust.Shared.Random;
+using Robust.Shared.Animations;
 
 namespace Content.Client.Light.EntitySystems;
 
@@ -36,23 +38,163 @@ public sealed class LightBehaviorSystem : EntitySystem
         }
     }
 
-    private void OnLightStartup(EntityUid uid, LightBehaviourComponent component, ComponentStartup args)
+    private void OnLightStartup(Entity<LightBehaviourComponent> entity, ref ComponentStartup args)
     {
         // TODO: Do NOT ensure component here. And use eventbus events instead...
-        EnsureComp<AnimationPlayerComponent>(uid);
+        EnsureComp<AnimationPlayerComponent>(entity);
 
-        foreach (var container in component.Animations)
+        foreach (var container in entity.Comp.Animations)
         {
-            container.LightBehaviour.Initialize(uid, _random, EntityManager);
+            container.LightBehaviour.Initialize(entity, _random, EntityManager);
         }
 
         // we need to initialize all behaviours before starting any
-        foreach (var container in component.Animations)
+        foreach (var container in entity.Comp.Animations)
         {
             if (container.LightBehaviour.Enabled)
             {
-                component.StartLightBehaviour(container.LightBehaviour.ID);
+                StartLightBehaviour(entity, container.LightBehaviour.ID);
+            }
+        }
+    }
+
+    /// <summary>
+    /// If we disable all the light behaviours we want to be able to revert the light to its original state.
+    /// </summary>
+    private void CopyLightSettings(Entity<LightBehaviourComponent> entity, string property)
+    {
+        if (EntityManager.TryGetComponent(entity, out PointLightComponent? light))
+        {
+            var propertyValue = AnimationHelper.GetAnimatableProperty(light, property);
+            if (propertyValue != null)
+            {
+                entity.Comp.OriginalPropertyValues.Add(property, propertyValue);
+            }
+        }
+        else
+        {
+            Log.Warning($"{EntityManager.GetComponent<MetaDataComponent>(entity).EntityName} has a {nameof(LightBehaviourComponent)} but it has no {nameof(PointLightComponent)}! Check the prototype!");
+        }
+    }
+
+    /// <summary>
+    /// Start animating a light behaviour with the specified ID. If the specified ID is empty, it will start animating all light behaviour entries.
+    /// If specified light behaviours are already animating, calling this does nothing.
+    /// Multiple light behaviours can have the same ID.
+    /// </summary>
+    public void StartLightBehaviour(Entity<LightBehaviourComponent> entity, string id = "")
+    {
+        if (!EntityManager.TryGetComponent(entity, out AnimationPlayerComponent? animation))
+        {
+            return;
+        }
+
+        foreach (var container in entity.Comp.Animations)
+        {
+            if (container.LightBehaviour.ID == id || id == string.Empty)
+            {
+                if (!_player.HasRunningAnimation(entity, animation, LightBehaviourComponent.KeyPrefix + container.Key))
+                {
+                    CopyLightSettings(entity, container.LightBehaviour.Property);
+                    container.LightBehaviour.UpdatePlaybackValues(container.Animation);
+                    _player.Play(entity, container.Animation, LightBehaviourComponent.KeyPrefix + container.Key);
                 }
             }
         }
     }
+
+    /// <summary>
+    /// If any light behaviour with the specified ID is animating, then stop it.
+    /// If no ID is specified then all light behaviours will be stopped.
+    /// Multiple light behaviours can have the same ID.
+    /// </summary>
+    /// <param name="id"></param>
+    /// <param name="removeBehaviour">Should the behaviour(s) also be removed permanently?</param>
+    /// <param name="resetToOriginalSettings">Should the light have its original settings applied?</param>
+    public void StopLightBehaviour(Entity<LightBehaviourComponent> entity, string id = "", bool removeBehaviour = false, bool resetToOriginalSettings = false)
+    {
+        if (!EntityManager.TryGetComponent(entity, out AnimationPlayerComponent? animation))
+        {
+            return;
+        }
+
+        var comp = entity.Comp;
+
+        var toRemove = new List<LightBehaviourComponent.AnimationContainer>();
+
+        foreach (var container in comp.Animations)
+        {
+            if (container.LightBehaviour.ID == id || id == string.Empty)
+            {
+                if (_player.HasRunningAnimation(entity, animation, LightBehaviourComponent.KeyPrefix + container.Key))
+                {
+                    _player.Stop(entity, animation, LightBehaviourComponent.KeyPrefix + container.Key);
+                }
+
+                if (removeBehaviour)
+                {
+                    toRemove.Add(container);
+                }
+            }
+        }
+
+        foreach (var container in toRemove)
+        {
+            comp.Animations.Remove(container);
+        }
+
+        if (resetToOriginalSettings && EntityManager.TryGetComponent(entity, out PointLightComponent? light))
+        {
+            foreach (var (property, value) in comp.OriginalPropertyValues)
+            {
+                AnimationHelper.SetAnimatableProperty(light, property, value);
+            }
+        }
+
+        comp.OriginalPropertyValues.Clear();
+    }
+
+    /// <summary>
+    /// Checks if at least one behaviour is running.
+    /// </summary>
+    /// <returns>Whether at least one behaviour is running, false if none is.</returns>
+    public bool HasRunningBehaviours(Entity<LightBehaviourComponent> entity)
+    {
+        //var uid = Owner;
+        if (!EntityManager.TryGetComponent(entity, out AnimationPlayerComponent? animation))
+        {
+            return false;
+        }
+
+        return entity.Comp.Animations.Any(container => _player.HasRunningAnimation(entity, animation, LightBehaviourComponent.KeyPrefix + container.Key));
+    }
+
+    /// <summary>
+    /// Add a new light behaviour to the component and start it immediately unless otherwise specified.
+    /// </summary>
+    public void AddNewLightBehaviour(Entity<LightBehaviourComponent> entity, LightBehaviourAnimationTrack behaviour, bool playImmediately = true)
+    {
+        var key = 0;
+        var comp = entity.Comp;
+
+        while (comp.Animations.Any(x => x.Key == key))
+        {
+            key++;
+        }
+
+        var animation = new Animation()
+        {
+            AnimationTracks = { behaviour }
+        };
+
+        behaviour.Initialize(entity.Owner, _random, EntityManager);
+
+        var container = new LightBehaviourComponent.AnimationContainer(key, animation, behaviour);
+        comp.Animations.Add(container);
+
+        if (playImmediately)
+        {
+            StartLightBehaviour(entity, behaviour.ID);
+        }
+    }
+}

Content.Client/Light/HandheldLightSystem.cs

@@ -3,15 +3,15 @@ using Content.Client.Light.Components;
 using Content.Shared.Light;
 using Content.Shared.Light.Components;
 using Content.Shared.Toggleable;
-using Robust.Client.Animations;
 using Robust.Client.GameObjects;
-using Robust.Shared.Animations;
+using Content.Client.Light.EntitySystems;
 
 namespace Content.Client.Light;
 
 public sealed class HandheldLightSystem : SharedHandheldLightSystem
 {
     [Dependency] private readonly SharedAppearanceSystem _appearance = default!;
+    [Dependency] private readonly LightBehaviorSystem _lightBehavior = default!;
 
     public override void Initialize()
     {
@@ -41,9 +41,9 @@ public sealed class HandheldLightSystem : SharedHandheldLightSystem
         if (TryComp<LightBehaviourComponent>(uid, out var lightBehaviour))
         {
             // Reset any running behaviour to reset the animated properties back to the original value, to avoid conflicts between resets
-            if (lightBehaviour.HasRunningBehaviours())
+            if (_lightBehavior.HasRunningBehaviours((uid, lightBehaviour)))
             {
-                lightBehaviour.StopLightBehaviour(resetToOriginalSettings: true);
+                _lightBehavior.StopLightBehaviour((uid, lightBehaviour), resetToOriginalSettings: true);
             }
 
             if (!enabled)
@@ -56,10 +56,10 @@ public sealed class HandheldLightSystem : SharedHandheldLightSystem
                 case HandheldLightPowerStates.FullPower:
                     break; // We just needed to reset all behaviours
                 case HandheldLightPowerStates.LowPower:
-                    lightBehaviour.StartLightBehaviour(component.RadiatingBehaviourId);
+                    _lightBehavior.StartLightBehaviour((uid, lightBehaviour), component.RadiatingBehaviourId);
                     break;
                 case HandheldLightPowerStates.Dying:
-                    lightBehaviour.StartLightBehaviour(component.BlinkingBehaviourId);
+                    _lightBehavior.StartLightBehaviour((uid, lightBehaviour), component.BlinkingBehaviourId);
                     break;
             }
         }