Make status effect networking not use TryAddStatusEffect (#28766)

* Make status effect networking not use `TryAddStatusEffect`

* a

Content.Shared/StatusEffect/StatusEffectsSystem.cs

@@ -14,6 +14,7 @@ namespace Content.Shared.StatusEffect
         [Dependency] private readonly IComponentFactory _componentFactory = default!;
         [Dependency] private readonly IGameTiming _gameTiming = default!;
         [Dependency] private readonly AlertsSystem _alertsSystem = default!;
+        private List<EntityUid> _toRemove = new();
 
         public override void Initialize()
         {
@@ -32,17 +33,27 @@ namespace Content.Shared.StatusEffect
 
             var curTime = _gameTiming.CurTime;
             var enumerator = EntityQueryEnumerator<ActiveStatusEffectsComponent, StatusEffectsComponent>();
+            _toRemove.Clear();
 
             while (enumerator.MoveNext(out var uid, out _, out var status))
             {
-                foreach (var state in status.ActiveEffects.ToArray())
+                if (status.ActiveEffects.Count == 0)
+                {
+                    // This shouldn't happen, but just in case something sneaks through
+                    _toRemove.Add(uid);
+                    continue;
+                }
+
+                foreach (var state in status.ActiveEffects)
                 {
-                    // if we're past the end point of the effect
                     if (curTime > state.Value.Cooldown.Item2)
-                    {
                         TryRemoveStatusEffect(uid, state.Key, status);
                 }
             }
+
+            foreach (var uid in _toRemove)
+            {
+                RemComp<ActiveStatusEffectsComponent>(uid);
             }
         }
 
@@ -62,29 +73,21 @@ namespace Content.Shared.StatusEffect
             component.AllowedEffects.AddRange(state.AllowedEffects);
 
             // Remove non-existent effects.
-            foreach (var effect in component.ActiveEffects.Keys)
+            foreach (var key in component.ActiveEffects.Keys)
             {
-                if (!state.ActiveEffects.ContainsKey(effect))
+                if (!state.ActiveEffects.ContainsKey(key))
-                {
+                    component.ActiveEffects.Remove(key);
-                    TryRemoveStatusEffect(uid, effect, component, remComp: false);
-                }
             }
 
             foreach (var (key, effect) in state.ActiveEffects)
-            {
-                // don't bother with anything if we already have it
-                if (component.ActiveEffects.ContainsKey(key))
             {
                 component.ActiveEffects[key] = new(effect);
-                    continue;
             }
 
-                var time = effect.Cooldown.Item2 - effect.Cooldown.Item1;
+            if (component.ActiveEffects.Count == 0)
-
+                RemComp<ActiveStatusEffectsComponent>(uid);
-                TryAddStatusEffect(uid, key, time, true, component, effect.Cooldown.Item1);
+            else
-                component.ActiveEffects[key].RelevantComponent = effect.RelevantComponent;
+                EnsureComp<ActiveStatusEffectsComponent>(uid);
-                // state handling should not add networked components, that is handled separately by the client game state manager.
-            }
         }
 
         private void OnRejuvenate(EntityUid uid, StatusEffectsComponent component, RejuvenateEvent args)
@@ -109,18 +112,16 @@ namespace Content.Shared.StatusEffect
             if (!Resolve(uid, ref status, false))
                 return false;
 
-            if (TryAddStatusEffect(uid, key, time, refresh, status))
+            if (!TryAddStatusEffect(uid, key, time, refresh, status))
-            {
-                // If they already have the comp, we just won't bother updating anything.
-                if (!EntityManager.HasComponent<T>(uid))
-                {
-                    var comp = EntityManager.AddComponent<T>(uid);
-                    status.ActiveEffects[key].RelevantComponent = _componentFactory.GetComponentName(comp.GetType());
-                }
-                return true;
-            }
-
                 return false;
+
+            if (HasComp<T>(uid))
+                return true;
+
+            EntityManager.AddComponent<T>(uid);
+            status.ActiveEffects[key].RelevantComponent = _componentFactory.GetComponentName<T>();
+            return true;
+
         }
 
         public bool TryAddStatusEffect(EntityUid uid, string key, TimeSpan time, bool refresh, string component,
@@ -162,8 +163,12 @@ namespace Content.Shared.StatusEffect
         ///     If the effect already exists, it will simply replace the cooldown with the new one given.
         ///     If you want special 'effect merging' behavior, do it your own damn self!
         /// </remarks>
-        public bool TryAddStatusEffect(EntityUid uid, string key, TimeSpan time, bool refresh,
+        public bool TryAddStatusEffect(EntityUid uid,
-            StatusEffectsComponent? status = null, TimeSpan? startTime = null)
+            string key,
+            TimeSpan time,
+            bool refresh,
+            StatusEffectsComponent? status = null,
+            TimeSpan? startTime = null)
         {
             if (!Resolve(uid, ref status, false))
                 return false;
@@ -334,8 +339,7 @@ namespace Content.Shared.StatusEffect
         /// <param name="uid">The entity to check on.</param>
         /// <param name="key">The status effect ID to check for</param>
         /// <param name="status">The status effect component, should you already have it.</param>
-        public bool CanApplyEffect(EntityUid uid, string key,
+        public bool CanApplyEffect(EntityUid uid, string key, StatusEffectsComponent? status = null)
-            StatusEffectsComponent? status = null)
         {
             // don't log since stuff calling this prolly doesn't care if we don't actually have it
             if (!Resolve(uid, ref status, false))