Don't process paused MoverControllers (#39444)

* refactor: make MoverController use more queries

* perf: don't process paused MoverControllers

* perf: track active input movers via events

* Revert "place stored changeling identities next to each other (#39452)"

This reverts commit 9b5d2ff11b.

* perf: keep around the seen movers hashset

* fix: don't reintroduce wild wild west ordering

* style: use virtual method

Co-authored-by: slarticodefast <161409025+slarticodefast@users.noreply.github.com>

* docs: better ActiveInputMoverComponent motiviation

Co-authored-by: slarticodefast <161409025+slarticodefast@users.noreply.github.com>

* fix: pass through known comp

* fix: properly order relay movers for real

* perf: use proxy Transform() and inline it

Actually this might be a slight performance improvement since it avoids

the dictionary lookup until the case that its body status is on ground.

* style: switch an event handler to Entity<T>

* fix: just-in-case track for relay loops

* merg conflix

* borger

* whitespace moment

* whoops

* empty

---------

Co-authored-by: slarticodefast <161409025+slarticodefast@users.noreply.github.com>
Co-authored-by: Princess Cheeseballs <66055347+Pronana@users.noreply.github.com>
Co-authored-by: ArtisticRoomba <145879011+ArtisticRoomba@users.noreply.github.com>

Content.Server/Physics/Controllers/MoverController.cs

@@ -1,18 +1,16 @@
+using System.Diagnostics;
 using System.Numerics;
 using System.Runtime.CompilerServices;
 using Content.Server.Shuttles.Components;
 using Content.Server.Shuttles.Systems;
-using Content.Shared.Friction;
 using Content.Shared.Movement.Components;
 using Content.Shared.Movement.Systems;
 using Content.Shared.Shuttles.Components;
 using Content.Shared.Shuttles.Systems;
 using Prometheus;
-using Robust.Shared.Physics.Components;
 using Robust.Shared.Player;
-using DroneConsoleComponent = Content.Server.Shuttles.DroneConsoleComponent;
 using DependencyAttribute = Robust.Shared.IoC.DependencyAttribute;
-using Robust.Shared.Map.Components;
+using DroneConsoleComponent = Content.Server.Shuttles.DroneConsoleComponent;
 
 namespace Content.Server.Physics.Controllers;
 
@@ -20,20 +18,111 @@ public sealed class MoverController : SharedMoverController
 {
     private static readonly Gauge ActiveMoverGauge = Metrics.CreateGauge(
         "physics_active_mover_count",
-        "Active amount of InputMovers being processed by MoverController");
+        "Amount of ActiveInputMovers being processed by MoverController");
 
     [Dependency] private readonly ThrusterSystem _thruster = default!;
-    [Dependency] private readonly SharedTransformSystem _xformSystem = default!;
 
     private Dictionary<EntityUid, (ShuttleComponent, List<(EntityUid, PilotComponent, InputMoverComponent, TransformComponent)>)> _shuttlePilots = new();
 
+    private EntityQuery<ActiveInputMoverComponent> _activeQuery;
+    private EntityQuery<DroneConsoleComponent> _droneQuery;
+    private EntityQuery<ShuttleComponent> _shuttleQuery;
+
+    // Not needed for persistence; just used to save an alloc
+    private readonly HashSet<EntityUid> _seenMovers = [];
+    private readonly HashSet<EntityUid> _seenRelayMovers = [];
+    private readonly List<Entity<InputMoverComponent>> _moversToUpdate = [];
+
     public override void Initialize()
     {
         base.Initialize();
+
+        SubscribeLocalEvent<ActiveInputMoverComponent, EntityPausedEvent>(OnEntityPaused);
+        SubscribeLocalEvent<InputMoverComponent, EntityUnpausedEvent>(OnEntityUnpaused);
+
         SubscribeLocalEvent<RelayInputMoverComponent, PlayerAttachedEvent>(OnRelayPlayerAttached);
         SubscribeLocalEvent<RelayInputMoverComponent, PlayerDetachedEvent>(OnRelayPlayerDetached);
         SubscribeLocalEvent<InputMoverComponent, PlayerAttachedEvent>(OnPlayerAttached);
         SubscribeLocalEvent<InputMoverComponent, PlayerDetachedEvent>(OnPlayerDetached);
+
+        _activeQuery = GetEntityQuery<ActiveInputMoverComponent>();
+        _droneQuery = GetEntityQuery<DroneConsoleComponent>();
+        _shuttleQuery = GetEntityQuery<ShuttleComponent>();
+    }
+
+    private void OnEntityPaused(Entity<ActiveInputMoverComponent> ent, ref EntityPausedEvent args)
+    {
+        // Become unactive [sic] if we don't have PhysicsComp.IgnorePaused
+        if (PhysicsQuery.TryComp(ent, out var phys) && phys.IgnorePaused)
+            return;
+        RemCompDeferred<ActiveInputMoverComponent>(ent);
+    }
+
+    private void OnEntityUnpaused(Entity<InputMoverComponent> ent, ref EntityUnpausedEvent args)
+    {
+        UpdateMoverStatus((ent, ent.Comp));
+    }
+
+    protected override void OnMoverStartup(Entity<InputMoverComponent> ent, ref ComponentStartup args)
+    {
+        base.OnMoverStartup(ent, ref args);
+        UpdateMoverStatus((ent, ent.Comp));
+    }
+
+    protected override void OnTargetRelayShutdown(Entity<MovementRelayTargetComponent> ent, ref ComponentShutdown args)
+    {
+        base.OnTargetRelayShutdown(ent, ref args);
+        UpdateMoverStatus((ent, null, ent.Comp));
+    }
+
+    protected override void UpdateMoverStatus(Entity<InputMoverComponent?, MovementRelayTargetComponent?> ent)
+    {
+        // Track that we aren't in a loop of movement relayers
+        _seenMovers.Clear();
+        while (true)
+        {
+            if (!MoverQuery.Resolve(ent, ref ent.Comp1, logMissing: false))
+            {
+                RemCompDeferred<ActiveInputMoverComponent>(ent);
+                break;
+            }
+
+            var meta = MetaData(ent);
+            if (Terminating(ent, meta))
+                break;
+
+            ActiveInputMoverComponent? activeMover = null;
+            if (!meta.EntityPaused
+                || PhysicsQuery.TryComp(ent, out var phys) && phys.IgnorePaused)
+                activeMover = EnsureComp<ActiveInputMoverComponent>(ent);
+
+            // If we're a relay target, make sure our drivers are InputMovers
+            if (RelayTargetQuery.Resolve(ent, ref ent.Comp2, logMissing: false)
+                // In case we're called from ComponentShutdown:
+                && ent.Comp2.LifeStage <= ComponentLifeStage.Running
+                && Exists(ent.Comp2.Source)
+                && !_seenMovers.Contains(ent.Comp2.Source))
+            {
+                if (ent.Comp2.Source == ent.Owner)
+                {
+                    Log.Error($"Entity {ToPrettyString(ent)} is attempting to relay movement to itself!");
+                    break;
+                }
+
+                if (activeMover is not null)
+                    activeMover.RelayedFrom = ent.Comp2.Source;
+
+                ent = ent.Comp2.Source;
+                _seenMovers.Add(ent);
+                continue;
+            }
+
+            // No longer a well-defined relay target
+            if (activeMover is not null)
+                activeMover.RelayedFrom = null;
+
+            break;
+        }
     }
 
     private void OnRelayPlayerAttached(Entity<RelayInputMoverComponent> entity, ref PlayerAttachedEvent args)
@@ -63,48 +152,70 @@ public sealed class MoverController : SharedMoverController
         return true;
     }
 
-    private HashSet<EntityUid> _moverAdded = new();
-    private List<Entity<InputMoverComponent>> _movers = new();
-
-    private void InsertMover(Entity<InputMoverComponent> source)
-    {
-        if (TryComp(source, out MovementRelayTargetComponent? relay))
-        {
-            if (TryComp(relay.Source, out InputMoverComponent? relayMover))
-            {
-                InsertMover((relay.Source, relayMover));
-            }
-        }
-
-        // Already added
-        if (!_moverAdded.Add(source.Owner))
-            return;
-
-        _movers.Add(source);
-    }
-
     public override void UpdateBeforeSolve(bool prediction, float frameTime)
     {
         base.UpdateBeforeSolve(prediction, frameTime);
 
-        _moverAdded.Clear();
-        _movers.Clear();
-        var inputQueryEnumerator = AllEntityQuery<InputMoverComponent>();
+        // We use _seenMovers here as well as in UpdateMoverStatus—this means we
+        // cannot have any events get fired while we use it in this while loop.
+        _seenMovers.Clear();
+        _moversToUpdate.Clear();
 
-        // Need to order mob movement so that movers don't run before their relays.
-        while (inputQueryEnumerator.MoveNext(out var uid, out var mover))
+        // Don't use EntityQueryEnumerator because admin ghosts have to move on
+        // paused maps. Pausing movers is handled via ActiveInputMoverComponent.
+        var inputQueryEnumerator = AllEntityQuery<ActiveInputMoverComponent, InputMoverComponent>();
+        while (inputQueryEnumerator.MoveNext(out var uid, out var activeComp, out var moverComp))
         {
-            InsertMover((uid, mover));
+            _seenRelayMovers.Clear(); // O(1) if already empty
+            QueueRelaySources(activeComp.RelayedFrom);
+
+            // If it's already inserted, that's fine—that means it'll still be
+            // handled before its child movers
+            AddMover((uid, moverComp));
         }
 
-        foreach (var mover in _movers)
-        {
-            HandleMobMovement(mover, frameTime);
-        }
+        ActiveMoverGauge.Set(_moversToUpdate.Count);
 
-        ActiveMoverGauge.Set(_movers.Count);
+        foreach (var ent in _moversToUpdate)
+        {
+            HandleMobMovement(ent, frameTime);
+        }
 
         HandleShuttleMovement(frameTime);
+        return;
+
+        // When we insert a chain of relay sources we have to flip its ordering
+        // It's going to be extremely uncommon for a relay chain to be more than
+        // one entity so we just recurse as needed.
+        void QueueRelaySources(EntityUid? next)
+        {
+            // We only care if it's still a mover
+            if (!_activeQuery.TryComp(next, out var nextActive)
+                || !MoverQuery.TryComp(next, out var nextMover)
+                || !_seenRelayMovers.Add(next.Value))
+                return;
+
+            Debug.Assert(next.Value != nextActive.RelayedFrom);
+
+            // While it is (as of writing) currently true that this recursion
+            // should always terminate due to RelayedFrom always being written
+            // in a way that tracks if it's made a loop, we still take the extra
+            // memory (and small time cost) of making sure via _seenRelayMovers.
+            QueueRelaySources(nextActive.RelayedFrom);
+            AddMover((next.Value, nextMover));
+        }
+
+        // Track inserts so we have ~ O(1) inserts without duplicates. Hopefully
+        // it doesn't matter that both _seenMovers and _moversToUpdate are never
+        // trimmed? They should be pretty memory light anyway, and in general
+        // it'll be rare for there to be a decrease in movers.
+        void AddMover(Entity<InputMoverComponent> entity)
+        {
+            if (!_seenMovers.Add(entity))
+                return;
+
+            _moversToUpdate.Add(entity);
+        }
     }
 
     public (Vector2 Strafe, float Rotation, float Brakes) GetPilotVelocityInput(PilotComponent component)
@@ -152,7 +263,7 @@ public sealed class MoverController : SharedMoverController
 
     protected override void HandleShuttleInput(EntityUid uid, ShuttleButtons button, ushort subTick, bool state)
     {
-        if (!TryComp<PilotComponent>(uid, out var pilot) || pilot.Console == null)
+        if (!PilotQuery.TryComp(uid, out var pilot) || pilot.Console == null)
             return;
 
         ResetSubtick(pilot);
@@ -263,27 +374,25 @@ public sealed class MoverController : SharedMoverController
 
         // We just mark off their movement and the shuttle itself does its own movement
         var activePilotQuery = EntityQueryEnumerator<PilotComponent, InputMoverComponent>();
-        var shuttleQuery = GetEntityQuery<ShuttleComponent>();
         while (activePilotQuery.MoveNext(out var uid, out var pilot, out var mover))
         {
             var consoleEnt = pilot.Console;
 
             // TODO: This is terrible. Just make a new mover and also make it remote piloting + device networks
-            if (TryComp<DroneConsoleComponent>(consoleEnt, out var cargoConsole))
-            {
+            if (_droneQuery.TryComp(consoleEnt, out var cargoConsole))
                 consoleEnt = cargoConsole.Entity;
-            }
 
-            if (!TryComp(consoleEnt, out TransformComponent? xform)) continue;
+            if (!XformQuery.TryComp(consoleEnt, out var xform))
+                continue;
 
             var gridId = xform.GridUid;
             // This tries to see if the grid is a shuttle and if the console should work.
-            if (!TryComp<MapGridComponent>(gridId, out var _) ||
-                !shuttleQuery.TryGetComponent(gridId, out var shuttleComponent) ||
+            if (!MapGridQuery.HasComp(gridId) ||
+                !_shuttleQuery.TryGetComponent(gridId, out var shuttleComponent) ||
                 !shuttleComponent.Enabled)
                 continue;
 
-            if (!newPilots.TryGetValue(gridId!.Value, out var pilots))
+            if (!newPilots.TryGetValue(gridId.Value, out var pilots))
             {
                 pilots = (shuttleComponent, new List<(EntityUid, PilotComponent, InputMoverComponent, TransformComponent)>());
                 newPilots[gridId.Value] = pilots;
@@ -305,13 +414,12 @@ public sealed class MoverController : SharedMoverController
 
         // Collate all of the linear / angular velocites for a shuttle
         // then do the movement input once for it.
-        var xformQuery = GetEntityQuery<TransformComponent>();
         foreach (var (shuttleUid, (shuttle, pilots)) in _shuttlePilots)
         {
-            if (Paused(shuttleUid) || CanPilot(shuttleUid) || !TryComp<PhysicsComponent>(shuttleUid, out var body))
+            if (Paused(shuttleUid) || CanPilot(shuttleUid) || !PhysicsQuery.TryComp(shuttleUid, out var body))
                 continue;
 
-            var shuttleNorthAngle = _xformSystem.GetWorldRotation(shuttleUid, xformQuery);
+            var shuttleNorthAngle = TransformSystem.GetWorldRotation(shuttleUid, XformQuery);
 
             // Collate movement linear and angular inputs together
             var linearInput = Vector2.Zero;
@@ -321,7 +429,7 @@ public sealed class MoverController : SharedMoverController
             var brakeCount = 0;
             var angularCount = 0;
 
-            foreach (var (pilotUid, pilot, _, consoleXform) in pilots)
+            foreach (var (_, pilot, _, consoleXform) in pilots)
             {
                 var (strafe, rotation, brakes) = GetPilotVelocityInput(pilot);
 
@@ -571,9 +679,9 @@ public sealed class MoverController : SharedMoverController
 
     private bool CanPilot(EntityUid shuttleUid)
     {
-        return TryComp<FTLComponent>(shuttleUid, out var ftl)
+        return FTLQuery.TryComp(shuttleUid, out var ftl)
         && (ftl.State & (FTLState.Starting | FTLState.Travelling | FTLState.Arriving)) != 0x0
-            || HasComp<PreventPilotComponent>(shuttleUid);
+            || PreventPilotQuery.HasComp(shuttleUid);
     }
 
 }

Content.Shared/ActionBlocker/ActionBlockerSystem.cs

@@ -30,15 +30,11 @@ namespace Content.Shared.ActionBlocker
             base.Initialize();
 
             _complexInteractionQuery = GetEntityQuery<ComplexInteractionComponent>();
-
-            SubscribeLocalEvent<InputMoverComponent, ComponentStartup>(OnMoverStartup);
-        }
-
-        private void OnMoverStartup(EntityUid uid, InputMoverComponent component, ComponentStartup args)
-        {
-            UpdateCanMove(uid, component);
         }
 
+        // These two methods should probably both live in SharedMoverController
+        // but they're called in a million places and I'm not doing that
+        // refactor right now.
         public bool CanMove(EntityUid uid, InputMoverComponent? component = null)
         {
             return Resolve(uid, ref component, false) && component.CanMove;

Content.Shared/Changeling/Systems/SharedChangelingIdentitySystem.cs

@@ -23,7 +23,6 @@ public abstract class SharedChangelingIdentitySystem : EntitySystem
     [Dependency] private readonly SharedPvsOverrideSystem _pvsOverrideSystem = default!;
 
     public MapId? PausedMapId;
-    private int _numberOfStoredIdentities = 0; // TODO: remove this
 
     public override void Initialize()
     {
@@ -99,11 +98,7 @@ public abstract class SharedChangelingIdentitySystem : EntitySystem
             return null;
 
         EnsurePausedMap();
-        // TODO: Setting the spawn location is a shitty bandaid to prevent admins from crashing our servers.
-        // Movercontrollers and mob collisions are currently being calculated even for paused entities.
-        // Spawning all of them in the same spot causes severe performance problems.
-        // Cryopods and Polymorph have the same problem.
-        var clone = Spawn(speciesPrototype.Prototype, new MapCoordinates(new Vector2(2 * _numberOfStoredIdentities++, 0), PausedMapId!.Value));
+        var clone = Spawn(speciesPrototype.Prototype, new MapCoordinates(Vector2.Zero, PausedMapId!.Value));
 
         var storedIdentity = EnsureComp<ChangelingStoredIdentityComponent>(clone);
         storedIdentity.OriginalEntity = target; // TODO: network this once we have WeakEntityReference or the autonetworking source gen is fixed

Content.Shared/Movement/Components/ActiveInputMoverComponent.cs

@@ -0,0 +1,27 @@
+using Content.Shared.Movement.Systems;
+
+namespace Content.Shared.Movement.Components;
+
+/// <summary>
+/// Marker component for entities that are being processed by MoverController.
+/// </summary>
+/// <remarks>
+/// The idea here is to keep track via event subscriptions which mover
+/// controllers actually need to be processed. Instead of having this be a
+/// boolean field on the <see cref="InputMoverComponent"/>, we instead track it
+/// as a separate component which is much faster to query all at once.
+/// </remarks>
+/// <seealso cref="InputMoverComponent"/>
+/// <seealso cref="SharedMoverController.UpdateMoverStatus"/>
+[RegisterComponent, Access(typeof(SharedMoverController))]
+public sealed partial class ActiveInputMoverComponent : Component
+{
+    /// <summary>
+    /// Cached version of <see cref="MovementRelayTargetComponent.Source"/>.
+    /// </summary>
+    /// <remarks>
+    /// This <i>must not</i> form a loop of EntityUids.
+    /// </remarks>
+    [DataField, ViewVariables]
+    public EntityUid? RelayedFrom;
+};

Content.Shared/Movement/Systems/SharedMoverController.Relay.cs

@@ -1,4 +1,3 @@
-using Content.Shared.ActionBlocker;
 using Content.Shared.Movement.Components;
 
 namespace Content.Shared.Movement.Systems;
@@ -61,6 +60,7 @@ public abstract partial class SharedMoverController
         Dirty(uid, component);
         Dirty(relayEntity, targetComp);
         _blocker.UpdateCanMove(uid);
+        UpdateMoverStatus((relayEntity, null, targetComp));
     }
 
     private void OnRelayShutdown(Entity<RelayInputMoverComponent> entity, ref ComponentShutdown args)
@@ -80,7 +80,7 @@ public abstract partial class SharedMoverController
         _blocker.UpdateCanMove(entity.Owner);
     }
 
-    private void OnTargetRelayShutdown(Entity<MovementRelayTargetComponent> entity, ref ComponentShutdown args)
+    protected virtual void OnTargetRelayShutdown(Entity<MovementRelayTargetComponent> entity, ref ComponentShutdown args)
     {
         PhysicsSystem.UpdateIsPredicted(entity.Owner);
         PhysicsSystem.UpdateIsPredicted(entity.Comp.Source);
@@ -91,4 +91,6 @@ public abstract partial class SharedMoverController
         if (TryComp(entity.Comp.Source, out RelayInputMoverComponent? relay) && relay.LifeStage <= ComponentLifeStage.Running)
             RemComp(entity.Comp.Source, relay);
     }
+
+    protected virtual void UpdateMoverStatus(Entity<InputMoverComponent?, MovementRelayTargetComponent?> ent) { }
 }

Content.Shared/Movement/Systems/SharedMoverController.cs

@@ -9,6 +9,7 @@ using Content.Shared.Maps;
 using Content.Shared.Mobs.Systems;
 using Content.Shared.Movement.Components;
 using Content.Shared.Movement.Events;
+using Content.Shared.Shuttles.Components;
 using Content.Shared.Tag;
 using Robust.Shared.Audio;
 using Robust.Shared.Audio.Systems;
@@ -48,6 +49,7 @@ public abstract partial class SharedMoverController : VirtualController
 
     protected EntityQuery<CanMoveInAirComponent> CanMoveInAirQuery;
     protected EntityQuery<FootstepModifierComponent> FootstepModifierQuery;
+    protected EntityQuery<FTLComponent> FTLQuery;
     protected EntityQuery<InputMoverComponent> MoverQuery;
     protected EntityQuery<MapComponent> MapQuery;
     protected EntityQuery<MapGridComponent> MapGridQuery;
@@ -56,6 +58,8 @@ public abstract partial class SharedMoverController : VirtualController
     protected EntityQuery<MovementSpeedModifierComponent> ModifierQuery;
     protected EntityQuery<NoRotateOnMoveComponent> NoRotateQuery;
     protected EntityQuery<PhysicsComponent> PhysicsQuery;
+    protected EntityQuery<PilotComponent> PilotQuery;
+    protected EntityQuery<PreventPilotComponent> PreventPilotQuery;
     protected EntityQuery<RelayInputMoverComponent> RelayQuery;
     protected EntityQuery<PullableComponent> PullableQuery;
     protected EntityQuery<TransformComponent> XformQuery;
@@ -92,8 +96,12 @@ public abstract partial class SharedMoverController : VirtualController
         FootstepModifierQuery = GetEntityQuery<FootstepModifierComponent>();
         MapGridQuery = GetEntityQuery<MapGridComponent>();
         MapQuery = GetEntityQuery<MapComponent>();
+        FTLQuery = GetEntityQuery<FTLComponent>();
+        PilotQuery = GetEntityQuery<PilotComponent>();
+        PreventPilotQuery = GetEntityQuery<PreventPilotComponent>();
 
         SubscribeLocalEvent<MovementSpeedModifierComponent, TileFrictionEvent>(OnTileFriction);
+        SubscribeLocalEvent<InputMoverComponent, ComponentStartup>(OnMoverStartup);
 
         InitializeInput();
         InitializeRelay();
@@ -103,6 +111,11 @@ public abstract partial class SharedMoverController : VirtualController
         Subs.CVar(_configManager, CCVars.OffgridFriction, value => _offGridDamping = value, true);
     }
 
+    protected virtual void OnMoverStartup(Entity<InputMoverComponent> ent, ref ComponentStartup args)
+    {
+       _blocker.UpdateCanMove(ent, ent.Comp);
+    }
+
     public override void Shutdown()
     {
         base.Shutdown();
@@ -469,9 +482,9 @@ public abstract partial class SharedMoverController : VirtualController
             // Only allow pushing off of anchored things that have collision.
             if (otherCollider.BodyType != BodyType.Static ||
                 !otherCollider.CanCollide ||
-                ((collider.CollisionMask & otherCollider.CollisionLayer) == 0 &&
-                (otherCollider.CollisionMask & collider.CollisionLayer) == 0) ||
-                (TryComp(otherEntity, out PullableComponent? pullable) && pullable.BeingPulled))
+                (collider.CollisionMask & otherCollider.CollisionLayer) == 0 &&
+                (otherCollider.CollisionMask & collider.CollisionLayer) == 0 ||
+                PullableQuery.TryComp(otherEntity, out var pullable) && pullable.BeingPulled)
             {
                 continue;
             }
@@ -621,7 +634,7 @@ public abstract partial class SharedMoverController : VirtualController
 
     private void OnTileFriction(Entity<MovementSpeedModifierComponent> ent, ref TileFrictionEvent args)
     {
-        if (!TryComp<PhysicsComponent>(ent, out var physicsComponent) || !XformQuery.TryComp(ent, out var xform))
+        if (!PhysicsQuery.TryComp(ent, out var physicsComponent))
             return;
 
         if (physicsComponent.BodyStatus != BodyStatus.OnGround || _gravity.IsWeightless(ent.Owner))