Physics Assert in SharedMoverController (#37970)

* Physics asserts and Xenoarch fixes

* Fix blocking asserts

* Alright ready for the test fails

* Fix whitespace issues

* Fix whitespace

* Okay fix whitespace issues for real

* Fix test fails

* Temp fix

* Fix

* Whitespace

* Added a big ass comment

* Right

* A

* Should work

* Debug performance

* Mothership

* fix test fails real

* push

* fix

---------

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

Content.IntegrationTests/Tests/Buckle/BuckleTest.cs

@@ -31,6 +31,8 @@ namespace Content.IntegrationTests.Tests.Buckle
   - type: Hands
   - type: ComplexInteraction
   - type: InputMover
+  - type: Physics
+    bodyType: KinematicController
   - type: Body
     prototype: Human
   - type: StandingState

Content.Shared/Blocking/BlockingSystem.cs

@@ -245,7 +245,7 @@ public sealed partial class BlockingSystem : EntitySystem
         if (TryComp<BlockingUserComponent>(user, out var blockingUserComponent) && TryComp<PhysicsComponent>(user, out var physicsComponent))
         {
             if (xform.Anchored)
-                _transformSystem.Unanchor(user, xform);
+                _transformSystem.Unanchor(user, xform, false);
 
             _actionsSystem.SetToggled(component.BlockingToggleActionEntity, false);
             _fixtureSystem.DestroyFixture(user, BlockingComponent.BlockFixtureID, body: physicsComponent);

Content.Shared/Friction/TileFrictionController.cs

@@ -1,7 +1,9 @@
 using System.Numerics;
 using Content.Shared.CCVar;
 using Content.Shared.Gravity;
+using Content.Shared.Interaction.Components;
 using Content.Shared.Interaction.Events;
+using Content.Shared.Movement.Components;
 using Content.Shared.Movement.Events;
 using Content.Shared.Movement.Pulling.Components;
 using Content.Shared.Movement.Systems;
@@ -14,6 +16,7 @@ using Robust.Shared.Physics.Components;
 using Robust.Shared.Physics.Controllers;
 using Robust.Shared.Physics.Dynamics;
 using Robust.Shared.Physics.Systems;
+using Robust.Shared.Utility;
 
 namespace Content.Shared.Friction
 {
@@ -26,11 +29,14 @@ namespace Content.Shared.Friction
         [Dependency] private readonly SharedMapSystem _map = default!;
 
         private EntityQuery<TileFrictionModifierComponent> _frictionQuery;
-        private EntityQuery<TransformComponent> _xformQuery;
         private EntityQuery<PullerComponent> _pullerQuery;
         private EntityQuery<PullableComponent> _pullableQuery;
         private EntityQuery<MapGridComponent> _gridQuery;
 
+        // For debug purposes only
+        private EntityQuery<InputMoverComponent> _moverQuery;
+        private EntityQuery<BlockMovementComponent> _blockMoverQuery;
+
         private float _frictionModifier;
         private float _minDamping;
         private float _airDamping;
@@ -45,10 +51,11 @@ namespace Content.Shared.Friction
             Subs.CVar(_configManager, CCVars.AirFriction, value => _airDamping = value, true);
             Subs.CVar(_configManager, CCVars.OffgridFriction, value => _offGridDamping = value, true);
             _frictionQuery = GetEntityQuery<TileFrictionModifierComponent>();
-            _xformQuery = GetEntityQuery<TransformComponent>();
             _pullerQuery = GetEntityQuery<PullerComponent>();
             _pullableQuery = GetEntityQuery<PullableComponent>();
             _gridQuery = GetEntityQuery<MapGridComponent>();
+            _moverQuery = GetEntityQuery<InputMoverComponent>();
+            _blockMoverQuery = GetEntityQuery<BlockMovementComponent>();
         }
 
         public override void UpdateBeforeSolve(bool prediction, float frameTime)
@@ -107,7 +114,16 @@ namespace Content.Shared.Friction
                 PhysicsSystem.SetAngularDamping(uid, body, friction);
 
                 if (body.BodyType != BodyType.KinematicController)
+                {
+                    /*
+                     * Extra catch for input movers that may be temporarily unable to move for whatever reason.
+                     * Block movement shouldn't be added and removed frivolously so it should be reliable to use this
+                     * as a check for brains and such which have input mover purely for ghosting behavior.
+                     */
+                    DebugTools.Assert(!_moverQuery.HasComp(uid) || _blockMoverQuery.HasComp(uid),
+                        $"Input mover: {ToPrettyString(uid)} in TileFrictionController is not the correct BodyType, BodyType found: {body.BodyType}, expected: KinematicController.");
                     continue;
+                }
 
                 // Physics engine doesn't apply damping to Kinematic Controllers so we have to do it here.
                 // BEWARE YE TRAVELLER:

Content.Shared/Interaction/Components/BlockMovementComponent.cs

@@ -8,6 +8,15 @@ namespace Content.Shared.Interaction.Components;
 [RegisterComponent, NetworkedComponent]
 public sealed partial class BlockMovementComponent : Component
 {
+    /// <summary>
+    /// Blocks generic interactions such as container insertion, pick up, drop and such.
+    /// </summary>
     [DataField]
     public bool BlockInteraction = true;
+
+    /// <summary>
+    /// Blocks being able to use entities.
+    /// </summary>
+    [DataField]
+    public bool BlockUse = true;
 }

Content.Shared/Interaction/SharedInteractionSystem.Blocking.cs

@@ -17,10 +17,10 @@ public partial class SharedInteractionSystem
     private void InitializeBlocking()
     {
         SubscribeLocalEvent<BlockMovementComponent, UpdateCanMoveEvent>(OnMoveAttempt);
-        SubscribeLocalEvent<BlockMovementComponent, UseAttemptEvent>(CancelEvent);
+        SubscribeLocalEvent<BlockMovementComponent, UseAttemptEvent>(CancelUseEvent);
         SubscribeLocalEvent<BlockMovementComponent, InteractionAttemptEvent>(CancelInteractEvent);
-        SubscribeLocalEvent<BlockMovementComponent, DropAttemptEvent>(CancelEvent);
+        SubscribeLocalEvent<BlockMovementComponent, DropAttemptEvent>(CancellableInteractEvent);
-        SubscribeLocalEvent<BlockMovementComponent, PickupAttemptEvent>(CancelEvent);
+        SubscribeLocalEvent<BlockMovementComponent, PickupAttemptEvent>(CancellableInteractEvent);
         SubscribeLocalEvent<BlockMovementComponent, ChangeDirectionAttemptEvent>(CancelEvent);
 
         SubscribeLocalEvent<BlockMovementComponent, ComponentStartup>(OnBlockingStartup);
@@ -33,6 +33,12 @@ public partial class SharedInteractionSystem
             args.Cancelled = true;
     }
 
+    private void CancelUseEvent(Entity<BlockMovementComponent> ent, ref UseAttemptEvent args)
+    {
+        if (ent.Comp.BlockUse)
+            args.Cancel();
+    }
+
     private void OnMoveAttempt(EntityUid uid, BlockMovementComponent component, UpdateCanMoveEvent args)
     {
         // If we're relaying then don't cancel.
@@ -42,6 +48,12 @@ public partial class SharedInteractionSystem
         args.Cancel(); // no more scurrying around
     }
 
+    private void CancellableInteractEvent(EntityUid uid, BlockMovementComponent component, CancellableEntityEventArgs args)
+    {
+        if (component.BlockInteraction)
+            args.Cancel();
+    }
+
     private void CancelEvent(EntityUid uid, BlockMovementComponent component, CancellableEntityEventArgs args)
     {
         args.Cancel();

Content.Shared/Movement/Systems/SharedMoverController.cs

@@ -22,6 +22,7 @@ using Robust.Shared.Map.Components;
 using Robust.Shared.Physics;
 using Robust.Shared.Physics.Components;
 using Robust.Shared.Physics.Controllers;
+using Robust.Shared.Physics.Events;
 using Robust.Shared.Prototypes;
 using Robust.Shared.Timing;
 using Robust.Shared.Utility;
@@ -107,6 +108,8 @@ public abstract partial class SharedMoverController : VirtualController
 
         SubscribeLocalEvent<MovementSpeedModifierComponent, TileFrictionEvent>(OnTileFriction);
         SubscribeLocalEvent<InputMoverComponent, ComponentStartup>(OnMoverStartup);
+        SubscribeLocalEvent<InputMoverComponent, PhysicsBodyTypeChangedEvent>(OnPhysicsBodyChanged);
+        SubscribeLocalEvent<InputMoverComponent, UpdateCanMoveEvent>(OnCanMove);
 
         InitializeInput();
         InitializeRelay();
@@ -213,6 +216,21 @@ public abstract partial class SharedMoverController : VirtualController
             return;
         }
 
+        /*
+         * This assert is here because any entity using inputs to move should be a Kinematic Controller.
+         * Kinematic Controllers are not built to use the entirety of the Physics engine by intention and
+         * setting an input mover to Dynamic will cause the Physics engine to occasionally throw asserts.
+         * In addition, SharedMoverController applies its own forms of fake impulses and friction outside
+         * Physics simulation, which will cause issues for Dynamic bodies (Such as Friction being applied twice).
+         * Kinematic bodies have even less Physics options and as such aren't suitable for a player, especially
+         * when we move to Box2D v3 where there will be more support for players updating outside of simulation.
+         * Lastly, static bodies can't move so they shouldn't be updated. If a static body makes it here we're
+         * doing unnecessary calculations.
+         * Only a Kinematic Controller should be making it to this point.
+         */
+        DebugTools.Assert(physicsComponent.BodyType == BodyType.KinematicController,
+            $"Input mover: {ToPrettyString(uid)} in HandleMobMovement is not the correct BodyType, BodyType found: {physicsComponent.BodyType}, expected: KinematicController.");
+
         // If the body is in air but isn't weightless then it can't move
         var weightless = _gravity.IsWeightless(uid);
         var inAirHelpless = false;
@@ -355,8 +373,7 @@ public abstract partial class SharedMoverController : VirtualController
             if (!weightless && MobMoverQuery.TryGetComponent(uid, out var mobMover) &&
                 TryGetSound(weightless, uid, mover, mobMover, xform, out var sound, tileDef: tileDef))
             {
-                var soundModifier = mover.Sprinting ? InputMoverComponent.SprintingSoundModifier
+                var soundModifier = mover.Sprinting ? InputMoverComponent.SprintingSoundModifier : InputMoverComponent.WalkingSoundModifier;
-                    : InputMoverComponent.WalkingSoundModifier;
 
                 var audioParams = sound.Params
                     .WithVolume(sound.Params.Volume + soundModifier)
@@ -661,4 +678,16 @@ public abstract partial class SharedMoverController : VirtualController
         else
             args.Modifier *= ent.Comp.BaseFriction;
     }
+
+    private void OnPhysicsBodyChanged(Entity<InputMoverComponent> entity, ref PhysicsBodyTypeChangedEvent args)
+    {
+        _blocker.UpdateCanMove(entity);
+    }
+
+    private void OnCanMove(Entity<InputMoverComponent> entity, ref UpdateCanMoveEvent args)
+    {
+        // If we don't have a physics component, or have a static body type then we can't move.
+        if (!PhysicsQuery.TryComp(entity, out var body) || body.BodyType == BodyType.Static)
+            args.Cancel();
+    }
 }

Resources/Prototypes/Entities/Mobs/Player/admin_ghost.yml

@@ -44,7 +44,6 @@
   - type: CombatMode
   - type: Physics
     ignorePaused: true
-    bodyType: Kinematic
   - type: Access
     groups:
     - AllAccess

Resources/Prototypes/Entities/Mobs/Player/mothershipcore.yml

@@ -5,9 +5,9 @@
   description: A sentient machine that can produce Xenoborgs. Without this the Xenoborgs are doomed.
   components:
   - type: InputMover # needs this to pilot the mothership
-  - type: MovementSpeedModifier
+  - type: BlockMovement # They should never be moving!
-    baseWalkSpeed : 0 # shouldn't move
+    blockInteraction: false
-    baseSprintSpeed : 0 # shouldn't move
+    blockUse: false
   - type: Appearance
   - type: WiresVisuals
   - type: Damageable

Resources/Prototypes/Entities/Mobs/Player/narsie.yml

@@ -86,7 +86,6 @@
     - Syndicate
     globalReceive: true
   - type: Physics
-    bodyType: Dynamic
     bodyStatus: InAir
   - type: CanMoveInAir
   # singulose components

Resources/Prototypes/Entities/Mobs/Player/ratvar.yml

@@ -82,7 +82,6 @@
     - Syndicate
     globalReceive: true
   - type: Physics
-    bodyType: Dynamic
     bodyStatus: InAir
   - type: CanMoveInAir
   - type: EventHorizon

Resources/Prototypes/Entities/Objects/Fun/immovable_rod.yml

@@ -83,6 +83,8 @@
     damage:
       types:
         Blunt: 190
+  - type: Physics
+    bodyType: KinematicController
   - type: InputMover
   - type: MovementSpeedModifier
     baseWeightlessAcceleration: 5

Resources/Prototypes/Entities/Objects/Specific/Xenoarchaeology/item_xenoartifacts.yml

@@ -20,7 +20,7 @@
       map: [ "enum.ArtifactsVisualLayers.ActivationEffect" ]
       visible: false
   - type: Physics
-    bodyType: Dynamic
+    bodyType: KinematicController
   - type: CollisionWake
     enabled: false
   - type: InteractionOutline

Resources/Prototypes/Entities/Objects/Specific/Xenoarchaeology/structure_xenoartifacts.yml

@@ -19,6 +19,8 @@
     - state: artifact-activation
       map: [ "enum.ArtifactsVisualLayers.ActivationEffect" ]
       visible: false
+  - type: Physics
+    bodyType: KinematicController
   - type: RandomArtifactSprite
     maxSprite: 36
   - type: RandomSprite

Resources/Prototypes/Entities/Objects/Specific/Xenoarchaeology/xenoartifacts.yml

@@ -29,7 +29,7 @@
   - type: Damageable
   - type: Actions
   - type: Physics
-    bodyType: Dynamic
+    bodyType: KinematicController
   - type: MovementSpeedModifier
     baseWalkSpeed: 0.25
     baseSprintSpeed: 0.5