Fix pulling mispredicts (#22941)

* Fix pulling mispredicts

* Make behaviour consistent

* Good ol terminating entities

Content.Server/Hands/Systems/HandsSystem.cs

@@ -1,25 +1,18 @@
-using System.Linq;
 using System.Numerics;
-using Content.Server.Popups;
 using Content.Server.Pulling;
 using Content.Server.Stack;
-using Content.Server.Storage.EntitySystems;
 using Content.Server.Stunnable;
 using Content.Shared.ActionBlocker;
 using Content.Shared.Body.Part;
 using Content.Shared.CombatMode;
 using Content.Shared.Explosion;
-using Content.Shared.Hands;
 using Content.Shared.Hands.Components;
 using Content.Shared.Hands.EntitySystems;
 using Content.Shared.Input;
-using Content.Shared.Inventory;
 using Content.Shared.Physics.Pull;
 using Content.Shared.Pulling.Components;
 using Content.Shared.Stacks;
-using Content.Shared.Storage;
 using Content.Shared.Throwing;
-using Robust.Shared.Containers;
 using Robust.Shared.GameStates;
 using Robust.Shared.Input.Binding;
 using Robust.Shared.Map;
@@ -30,16 +23,12 @@ namespace Content.Server.Hands.Systems
 {
     public sealed class HandsSystem : SharedHandsSystem
     {
-        [Dependency] private readonly InventorySystem _inventorySystem = default!;
         [Dependency] private readonly StackSystem _stackSystem = default!;
         [Dependency] private readonly HandVirtualItemSystem _virtualItemSystem = default!;
         [Dependency] private readonly ActionBlockerSystem _actionBlockerSystem = default!;
-        [Dependency] private readonly SharedHandVirtualItemSystem _virtualSystem = default!;
-        [Dependency] private readonly PopupSystem _popupSystem = default!;
         [Dependency] private readonly SharedHandsSystem _handsSystem = default!;
         [Dependency] private readonly PullingSystem _pullingSystem = default!;
         [Dependency] private readonly ThrowingSystem _throwingSystem = default!;
-        [Dependency] private readonly StorageSystem _storageSystem = default!;
 
         public override void Initialize()
         {
@@ -98,14 +87,6 @@ namespace Content.Server.Hands.Systems
             args.Handled = true; // no shove/stun.
         }
 
-        protected override void HandleEntityRemoved(EntityUid uid, HandsComponent hands, EntRemovedFromContainerMessage args)
-        {
-            base.HandleEntityRemoved(uid, hands, args);
-
-            if (!Deleted(args.Entity) && TryComp(args.Entity, out HandVirtualItemComponent? @virtual))
-                _virtualSystem.Delete((args.Entity, @virtual), uid);
-        }
-
         private void HandleBodyPartAdded(EntityUid uid, HandsComponent component, ref BodyPartAddedEvent args)
         {
             if (args.Part.PartType != BodyPartType.Hand)

Content.Shared/Hands/EntitySystems/SharedHandsSystem.Drop.cs

@@ -25,6 +25,9 @@ public abstract partial class SharedHandsSystem
 
         var didUnequip = new DidUnequipHandEvent(uid, args.Entity, hand);
         RaiseLocalEvent(uid, didUnequip);
+
+        if (TryComp(args.Entity, out HandVirtualItemComponent? @virtual))
+            _virtualSystem.Delete((args.Entity, @virtual), uid);
     }
 
     /// <summary>
@@ -100,8 +103,14 @@ public abstract partial class SharedHandsSystem
         var entity = hand.HeldEntity!.Value;
         DoDrop(uid, hand, doDropInteraction: doDropInteraction, handsComp);
 
-        var userXform = Transform(uid);
+        if (TerminatingOrDeleted(entity))
+            return true;
+
         var itemXform = Transform(entity);
+        if (itemXform.MapUid == null)
+            return true;
+
+        var userXform = Transform(uid);
         var isInContainer = ContainerSystem.IsEntityInContainer(uid);
 
         if (targetDropLocation == null || isInContainer)

Content.Shared/Hands/EntitySystems/SharedHandsSystem.cs

@@ -20,6 +20,7 @@ public abstract partial class SharedHandsSystem
     [Dependency] private readonly SharedItemSystem _items = default!;
     [Dependency] private readonly SharedStorageSystem _storage = default!;
     [Dependency] protected readonly SharedTransformSystem TransformSystem = default!;
+    [Dependency] private readonly SharedHandVirtualItemSystem _virtualSystem = default!;
 
     protected event Action<Entity<HandsComponent>?>? OnHandSetActive;
 

Content.Shared/Hands/SharedHandVirtualItemSystem.cs

@@ -11,6 +11,7 @@ public abstract class SharedHandVirtualItemSystem : EntitySystem
 {
     [Dependency] private readonly INetManager _net = default!;
     [Dependency] private readonly SharedHandsSystem _hands = default!;
+    [Dependency] private readonly SharedTransformSystem _transform = default!;
 
     public override void Initialize()
     {
@@ -82,14 +83,16 @@ public abstract class SharedHandVirtualItemSystem : EntitySystem
     /// </summary>
     public void Delete(Entity<HandVirtualItemComponent> item, EntityUid user)
     {
-        if (_net.IsClient)
-            return;
-
         var userEv = new VirtualItemDeletedEvent(item.Comp.BlockingEntity, user);
         RaiseLocalEvent(user, userEv);
         var targEv = new VirtualItemDeletedEvent(item.Comp.BlockingEntity, user);
         RaiseLocalEvent(item.Comp.BlockingEntity, targEv);
 
+        if (TerminatingOrDeleted(item))
+            return;
+
+        _transform.DetachParentToNull(item, Transform(item));
+        if (_net.IsServer)
             QueueDel(item);
     }
 }

Content.Shared/Pulling/Components/PullableComponent.cs

@@ -5,7 +5,7 @@ using Robust.Shared.Serialization;
 namespace Content.Shared.Pulling.Components
 {
     // Before you try to add another type than SharedPullingStateManagementSystem, consider the can of worms you may be opening!
-    [NetworkedComponent()]
+    [NetworkedComponent, AutoGenerateComponentState]
     [Access(typeof(SharedPullingStateManagementSystem))]
     [RegisterComponent]
     public sealed partial class SharedPullableComponent : Component
@@ -13,11 +13,13 @@ namespace Content.Shared.Pulling.Components
         /// <summary>
         /// The current entity pulling this component.
         /// </summary>
+        [DataField, AutoNetworkedField]
         public EntityUid? Puller { get; set; }
 
         /// <summary>
         /// The pull joint.
         /// </summary>
+        [DataField, AutoNetworkedField]
         public string? PullJointId { get; set; }
 
         public bool BeingPulled => Puller != null;
@@ -40,17 +42,6 @@ namespace Content.Shared.Pulling.Components
         public bool PrevFixedRotation;
     }
 
-    [Serializable, NetSerializable]
-    public sealed class PullableComponentState : ComponentState
-    {
-        public readonly NetEntity? Puller;
-
-        public PullableComponentState(NetEntity? puller)
-        {
-            Puller = puller;
-        }
-    }
-
     /// <summary>
     /// Raised when a request is made to stop pulling an entity.
     /// </summary>

Content.Shared/Pulling/Components/SharedPullerComponent.cs

@@ -1,6 +1,8 @@
-namespace Content.Shared.Pulling.Components
+using Robust.Shared.GameStates;
+
+namespace Content.Shared.Pulling.Components
 {
-    [RegisterComponent]
+    [RegisterComponent, NetworkedComponent, AutoGenerateComponentState]
     [Access(typeof(SharedPullingStateManagementSystem))]
     public sealed partial class SharedPullerComponent : Component
     {
@@ -9,7 +11,7 @@
 
         public float SprintSpeedModifier => Pulling == default ? 1.0f : 0.95f;
 
-        [ViewVariables]
+        [DataField, AutoNetworkedField]
         public EntityUid? Pulling { get; set; }
 
         /// <summary>

Content.Shared/Pulling/Systems/SharedPullingStateManagementSystem.cs

@@ -1,7 +1,6 @@
 using Content.Shared.Physics.Pull;
 using Content.Shared.Pulling.Components;
 using JetBrains.Annotations;
-using Robust.Shared.GameStates;
 using Robust.Shared.Map;
 using Robust.Shared.Physics;
 using Robust.Shared.Physics.Components;
@@ -26,43 +25,6 @@ namespace Content.Shared.Pulling
             base.Initialize();
 
             SubscribeLocalEvent<SharedPullableComponent, ComponentShutdown>(OnShutdown);
-            SubscribeLocalEvent<SharedPullableComponent, ComponentGetState>(OnGetState);
-            SubscribeLocalEvent<SharedPullableComponent, ComponentHandleState>(OnHandleState);
-        }
-
-        private void OnGetState(EntityUid uid, SharedPullableComponent component, ref ComponentGetState args)
-        {
-            args.State = new PullableComponentState(GetNetEntity(component.Puller));
-        }
-
-        private void OnHandleState(EntityUid uid, SharedPullableComponent component, ref ComponentHandleState args)
-        {
-            if (args.Current is not PullableComponentState state)
-                return;
-
-            var puller = EnsureEntity<SharedPullableComponent>(state.Puller, uid);
-
-            if (!puller.HasValue)
-            {
-                ForceDisconnectPullable(component);
-                return;
-            }
-
-            if (component.Puller == puller)
-            {
-                // don't disconnect and reconnect a puller for no reason
-                return;
-            }
-
-            if (!TryComp<SharedPullerComponent>(puller, out var comp))
-            {
-                Log.Error($"Pullable state for entity {ToPrettyString(uid)} had invalid puller entity {ToPrettyString(puller.Value)}");
-                // ensure it disconnects from any different puller, still
-                ForceDisconnectPullable(component);
-                return;
-            }
-
-            ForceRelationship(comp, component);
         }
 
         private void OnShutdown(EntityUid uid, SharedPullableComponent component, ComponentShutdown args)
@@ -111,6 +73,9 @@ namespace Content.Shared.Pulling
 
         public void ForceRelationship(SharedPullerComponent? puller, SharedPullableComponent? pullable)
         {
+            if (_timing.ApplyingState)
+                return;
+            ;
             if (pullable != null && puller != null && (puller.Pulling == pullable.Owner))
             {
                 // Already done
@@ -187,6 +152,9 @@ namespace Content.Shared.Pulling
 
         public void ForceSetMovingTo(SharedPullableComponent pullable, EntityCoordinates? movingTo)
         {
+            if (_timing.ApplyingState)
+                return;
+
             if (pullable.MovingTo == movingTo)
             {
                 return;
@@ -200,6 +168,7 @@ namespace Content.Shared.Pulling
             }
 
             pullable.MovingTo = movingTo;
+            Dirty(pullable);
 
             if (movingTo == null)
             {

Content.Shared/Pulling/Systems/SharedPullingSystem.Actions.cs

@@ -12,6 +12,8 @@ using Robust.Shared.Map;
 using Robust.Shared.Physics;
 using Robust.Shared.Physics.Components;
 using Robust.Shared.Physics.Systems;
+using Robust.Shared.Timing;
+using Robust.Shared.Utility;
 
 namespace Content.Shared.Pulling
 {
@@ -23,6 +25,7 @@ namespace Content.Shared.Pulling
         [Dependency] private readonly SharedInteractionSystem _interaction = default!;
         [Dependency] private readonly SharedPhysicsSystem _physics = default!;
         [Dependency] private readonly ISharedAdminLogManager _adminLogger = default!;
+        [Dependency] private readonly IGameTiming _timing = default!;
 
         public bool CanPull(EntityUid puller, EntityUid pulled)
         {
@@ -90,6 +93,9 @@ namespace Content.Shared.Pulling
 
         public bool TryStopPull(SharedPullableComponent pullable, EntityUid? user = null)
         {
+            if (_timing.ApplyingState)
+                return false;
+
             if (!pullable.BeingPulled)
             {
                 return false;
@@ -127,6 +133,9 @@ namespace Content.Shared.Pulling
         // The main "start pulling" function.
         public bool TryStartPull(SharedPullerComponent puller, SharedPullableComponent pullable)
         {
+            if (_timing.ApplyingState)
+                return false;
+
             if (puller.Pulling == pullable.Owner)
                 return true;