Simplify DoAfterArgs behavior for movement and distance checks (#25226)

* Merge BreakOnWeightlessMove and BreakOnMove. Provide different theshold for weightless movement.

* Adjust WeightlessMovementThresholds. Put a thing I forgot to put in the doafterargs.

* Make DoAfterArgs only use OnMove to determine whether to check for
movement and MoveThreshold to determine the threshold regardless of
weightlessness. Gave DistanceThreshold a default value which will always
be checked now.

* Fix issue introduced by merge.

* Use interaction system for determining whether a distance is within range

* Fix incorrect doafter args introduced by previous merge.
Forgor to commit these.

* Exorcise ghost.

The execution system should have been deleted when I merged previously.
For a reason I cannot comprehend it came back, but only the execution
system.

* Exorcise ghost Pt. 2

* Allow for movement check to be overriden in zero g and adjust doafter args where needed.

You can now override checking for movement in zero g with the BreakOnWeightlessMove bool. By default it will check.
The following doafters were made to ignore the movement check in zero g:
- Healing yourself with healing items,
- Removing embedded projectiles,
- Using tools like welders and crowbars

* Adjust distance for cuffing/uncuffing to work. Make injections not break on weightless movement.

* Fix evil incorrect and uneeded comments

Content.Shared/Burial/BurialSystem.cs

@@ -45,8 +45,7 @@ public sealed class BurialSystem : EntitySystem
         {
             var doAfterEventArgs = new DoAfterArgs(EntityManager, args.User, component.DigDelay / shovel.SpeedModifier, new GraveDiggingDoAfterEvent(), uid, target: args.Target, used: uid)
             {
-                BreakOnTargetMove = true,
-                BreakOnUserMove = true,
+                BreakOnMove = true,
                 BreakOnDamage = true,
                 NeedHand = true,
                 BreakOnHandChange = true
@@ -159,8 +158,7 @@ public sealed class BurialSystem : EntitySystem
         var doAfterEventArgs = new DoAfterArgs(EntityManager, args.Entity, component.DigDelay / component.DigOutByHandModifier, new GraveDiggingDoAfterEvent(), uid, target: uid)
         {
             NeedHand = false,
-            BreakOnUserMove = true,
-            BreakOnTargetMove = false,
+            BreakOnMove = true,
             BreakOnHandChange = false,
             BreakOnDamage = false
         };

Content.Shared/Climbing/Systems/BonkSystem.cs

@@ -82,8 +82,7 @@ public sealed partial class BonkSystem : EntitySystem
 
         var doAfterArgs = new DoAfterArgs(EntityManager, args.Dragged, component.BonkDelay, new BonkDoAfterEvent(), uid, target: uid)
         {
-            BreakOnTargetMove = true,
-            BreakOnUserMove = true,
+            BreakOnMove = true,
             BreakOnDamage = true
         };
 

Content.Shared/Climbing/Systems/ClimbSystem.cs

@@ -211,8 +211,7 @@ public sealed partial class ClimbSystem : VirtualController
             target: climbable,
             used: entityToMove)
         {
-            BreakOnTargetMove = true,
-            BreakOnUserMove = true,
+            BreakOnMove = true,
             BreakOnDamage = true
         };
 

Content.Shared/Clothing/EntitySystems/ToggleableClothingSystem.cs

@@ -100,7 +100,7 @@ public sealed class ToggleableClothingSystem : EntitySystem
         var args = new DoAfterArgs(EntityManager, user, time, new ToggleClothingDoAfterEvent(), item, wearer, item)
         {
             BreakOnDamage = true,
-            BreakOnTargetMove = true,
+            BreakOnMove = true,
             // This should just re-use the BUI range checks & cancel the do after if the BUI closes. But that is all
             // server-side at the moment.
             // TODO BUI REFACTOR.

Content.Shared/Cuffs/SharedCuffableSystem.cs

@@ -479,10 +479,11 @@ namespace Content.Shared.Cuffs
 
             var doAfterEventArgs = new DoAfterArgs(EntityManager, user, cuffTime, new AddCuffDoAfterEvent(), handcuff, target, handcuff)
             {
-                BreakOnTargetMove = true,
-                BreakOnUserMove = true,
+                BreakOnMove = true,
+                BreakOnWeightlessMove = false,
                 BreakOnDamage = true,
-                NeedHand = true
+                NeedHand = true,
+                DistanceThreshold = 0.3f
             };
 
             if (!_doAfter.TryStartDoAfter(doAfterEventArgs))
@@ -574,11 +575,12 @@ namespace Content.Shared.Cuffs
 
             var doAfterEventArgs = new DoAfterArgs(EntityManager, user, uncuffTime, new UnCuffDoAfterEvent(), target, target, cuffsToRemove)
             {
-                BreakOnUserMove = true,
-                BreakOnTargetMove = true,
+                BreakOnMove = true,
+                BreakOnWeightlessMove = false,
                 BreakOnDamage = true,
                 NeedHand = true,
                 RequireCanInteract = false, // Trust in UncuffAttemptEvent
+                DistanceThreshold = 0.3f
             };
 
             if (!_doAfter.TryStartDoAfter(doAfterEventArgs))

Content.Shared/Devour/SharedDevourSystem.cs

@@ -57,8 +57,7 @@ public abstract class SharedDevourSystem : EntitySystem
 
                     _doAfterSystem.TryStartDoAfter(new DoAfterArgs(EntityManager, uid, component.DevourTime, new DevourDoAfterEvent(), uid, target: target, used: uid)
                     {
-                        BreakOnTargetMove = true,
-                        BreakOnUserMove = true,
+                        BreakOnMove = true,
                     });
                     break;
                 default:
@@ -76,8 +75,7 @@ public abstract class SharedDevourSystem : EntitySystem
 
         _doAfterSystem.TryStartDoAfter(new DoAfterArgs(EntityManager, uid, component.StructureDevourTime, new DevourDoAfterEvent(), uid, target: target, used: uid)
         {
-            BreakOnTargetMove = true,
-            BreakOnUserMove = true,
+            BreakOnMove = true,
         });
     }
 }

Content.Shared/DoAfter/DoAfter.cs

@@ -82,12 +82,6 @@ public sealed partial class DoAfter
     {
         Index = index;
 
-        if (args.Target == null)
-        {
-            DebugTools.Assert(!args.BreakOnTargetMove);
-            args.BreakOnTargetMove = false;
-        }
-
         Args = args;
         StartTime = startTime;
     }

Content.Shared/DoAfter/DoAfterArgs.cs

@@ -92,29 +92,23 @@ public sealed partial class DoAfterArgs
     public bool BreakOnHandChange = true;
 
     /// <summary>
-    ///     If do_after stops when the user moves
+    ///     If do_after stops when the user or target moves
     /// </summary>
-    [DataField("breakOnUserMove")]
-    public bool BreakOnUserMove;
+    [DataField]
+    public bool BreakOnMove;
 
     /// <summary>
-    ///     If this is true then any movement, even when weightless, will break the doafter.
-    ///     When there is no gravity, BreakOnUserMove is ignored. If it is false to begin with nothing will change.
+    ///     Whether to break on movement when the user is weightless.
+    ///     This does nothing if <see cref="BreakOnMove"/> is false.
     /// </summary>
-    [DataField("breakOnWeightlessMove")]
-    public bool BreakOnWeightlessMove;
-
-    /// <summary>
-    ///     If do_after stops when the target moves (if there is a target)
-    /// </summary>
-    [DataField("breakOnTargetMove")]
-    public bool BreakOnTargetMove;
+    [DataField]
+    public bool BreakOnWeightlessMove = true;
 
     /// <summary>
     ///     Threshold for user and target movement
     /// </summary>
     [DataField("movementThreshold")]
-    public float MovementThreshold = 0.1f;
+    public float MovementThreshold = 0.3f;
 
     /// <summary>
     ///     Threshold for distance user from the used OR target entities.
@@ -250,9 +244,8 @@ public sealed partial class DoAfterArgs
         Broadcast = other.Broadcast;
         NeedHand = other.NeedHand;
         BreakOnHandChange = other.BreakOnHandChange;
-        BreakOnUserMove = other.BreakOnUserMove;
+        BreakOnMove = other.BreakOnMove;
         BreakOnWeightlessMove = other.BreakOnWeightlessMove;
-        BreakOnTargetMove = other.BreakOnTargetMove;
         MovementThreshold = other.MovementThreshold;
         DistanceThreshold = other.DistanceThreshold;
         BreakOnDamage = other.BreakOnDamage;

Content.Shared/DoAfter/SharedDoAfterSystem.Update.cs

@@ -1,5 +1,7 @@
 using Content.Shared.Gravity;
 using Content.Shared.Hands.Components;
+using Content.Shared.Interaction;
+using Content.Shared.Physics;
 using Robust.Shared.Utility;
 
 namespace Content.Shared.DoAfter;
@@ -8,6 +10,7 @@ public abstract partial class SharedDoAfterSystem : EntitySystem
 {
     [Dependency] private readonly IDynamicTypeFactory _factory = default!;
     [Dependency] private readonly SharedGravitySystem _gravity = default!;
+    [Dependency] private readonly SharedInteractionSystem _interaction = default!;
 
     private DoAfter[] _doAfters = Array.Empty<DoAfter>();
 
@@ -163,24 +166,53 @@ public abstract partial class SharedDoAfterSystem : EntitySystem
             return true;
 
         // TODO: Re-use existing xform query for these calculations.
-        // when there is no gravity you will be drifting 99% of the time making many doafters impossible
-        // so this just ignores your movement if you are weightless (unless the doafter sets BreakOnWeightlessMove then moving will still break it)
-        if (args.BreakOnUserMove
-            && !userXform.Coordinates.InRange(EntityManager, _transform, doAfter.UserPosition, args.MovementThreshold)
-            && (args.BreakOnWeightlessMove || !_gravity.IsWeightless(args.User, xform: userXform)))
-            return true;
-
-        if (args.BreakOnTargetMove)
+        if (args.BreakOnMove && !(!args.BreakOnWeightlessMove && _gravity.IsWeightless(args.User, xform: userXform)))
         {
-            DebugTools.Assert(targetXform != null, "Break on move is true, but no target specified?");
-            if (targetXform != null && targetXform.Coordinates.TryDistance(EntityManager, userXform.Coordinates, out var distance))
+            // Whether the user has moved too much from their original position.
+            if (!userXform.Coordinates.InRange(EntityManager, _transform, doAfter.UserPosition, args.MovementThreshold))
+                return true;
+
+            // Whether the distance between the user and target(if any) has changed too much.
+            if (targetXform != null &&
+                targetXform.Coordinates.TryDistance(EntityManager, userXform.Coordinates, out var distance))
             {
-                // once the target moves too far from you the do after breaks
                 if (Math.Abs(distance - doAfter.TargetDistance) > args.MovementThreshold)
                     return true;
             }
         }
 
+        // Whether the user and the target are too far apart.
+        if (args.Target != null)
+        {
+            if (args.DistanceThreshold != null)
+            {
+                if (!_interaction.InRangeUnobstructed(args.User, args.Target.Value, args.DistanceThreshold.Value))
+                    return true;
+            }
+            else
+            {
+                if (!_interaction.InRangeUnobstructed(args.User, args.Target.Value))
+                    return true;
+            }
+        }
+
+        // Whether the distance between the tool and the user has grown too much.
+        if (args.Used != null)
+        {
+            if (args.DistanceThreshold != null)
+            {
+                if (!_interaction.InRangeUnobstructed(args.User,
+                        args.Used.Value,
+                        args.DistanceThreshold.Value))
+                    return true;
+            }
+            else
+            {
+                if (!_interaction.InRangeUnobstructed(args.User,args.Used.Value))
+                    return true;
+            }
+        }
+
         if (args.AttemptFrequency == AttemptFrequency.EveryTick && !TryAttemptEvent(doAfter))
             return true;
 
@@ -199,23 +231,6 @@ public abstract partial class SharedDoAfterSystem : EntitySystem
         if (args.RequireCanInteract && !_actionBlocker.CanInteract(args.User, args.Target))
             return true;
 
-        if (args.DistanceThreshold != null)
-        {
-            if (targetXform != null
-                && !args.User.Equals(args.Target)
-                && !userXform.Coordinates.InRange(EntityManager, _transform, targetXform.Coordinates,
-                    args.DistanceThreshold.Value))
-            {
-                return true;
-            }
-
-            if (usedXform != null
-                && !userXform.Coordinates.InRange(EntityManager, _transform, usedXform.Coordinates,
-                    args.DistanceThreshold.Value))
-            {
-                return true;
-            }
-        }
 
         return false;
     }

Content.Shared/DoAfter/SharedDoAfterSystem.cs

@@ -214,12 +214,11 @@ public abstract partial class SharedDoAfterSystem : EntitySystem
         args.NetUser = GetNetEntity(args.User);
         args.NetEventTarget = GetNetEntity(args.EventTarget);
 
-        if (args.BreakOnUserMove || args.BreakOnTargetMove)
+        if (args.BreakOnMove)
             doAfter.UserPosition = Transform(args.User).Coordinates;
 
-        if (args.Target != null && args.BreakOnTargetMove)
+        if (args.Target != null && args.BreakOnMove)
         {
-            // Target should never be null if the bool is set.
             var targetPosition = Transform(args.Target.Value).Coordinates;
             doAfter.UserPosition.TryDistance(EntityManager, targetPosition, out doAfter.TargetDistance);
         }

Content.Shared/Inventory/InventorySystem.Equip.cs

@@ -168,8 +168,7 @@ public abstract partial class InventorySystem
             {
                 BlockDuplicate = true,
                 BreakOnHandChange = true,
-                BreakOnUserMove = true,
-                BreakOnTargetMove = true,
+                BreakOnMove = true,
                 CancelDuplicate = true,
                 RequireCanInteract = true,
                 NeedHand = true
@@ -414,8 +413,7 @@ public abstract partial class InventorySystem
             {
                 BlockDuplicate = true,
                 BreakOnHandChange = true,
-                BreakOnUserMove = true,
-                BreakOnTargetMove = true,
+                BreakOnMove = true,
                 CancelDuplicate = true,
                 RequireCanInteract = true,
                 NeedHand = true

Content.Shared/Lock/LockSystem.cs

@@ -111,7 +111,7 @@ public sealed class LockSystem : EntitySystem
             return _doAfter.TryStartDoAfter(
                 new DoAfterArgs(EntityManager, user, lockComp.LockTime, new LockDoAfter(), uid, uid)
                 {
-                    BreakOnDamage = true, BreakOnTargetMove = true, BreakOnUserMove = true, RequireCanInteract = true,
+                    BreakOnDamage = true, BreakOnMove = true, RequireCanInteract = true,
                     NeedHand = true
                 });
         }
@@ -187,7 +187,7 @@ public sealed class LockSystem : EntitySystem
             return _doAfter.TryStartDoAfter(
                 new DoAfterArgs(EntityManager, user, lockComp.LockTime, new UnlockDoAfter(), uid, uid)
                 {
-                    BreakOnDamage = true, BreakOnTargetMove = true, BreakOnUserMove = true, RequireCanInteract = true,
+                    BreakOnDamage = true, BreakOnMove = true, RequireCanInteract = true,
                     NeedHand = true
                 });
         }

Content.Shared/Mech/EntitySystems/SharedMechSystem.cs

@@ -434,7 +434,7 @@ public abstract class SharedMechSystem : EntitySystem
 
         var doAfterEventArgs = new DoAfterArgs(EntityManager, args.Dragged, component.EntryDelay, new MechEntryEvent(), uid, target: uid)
         {
-            BreakOnUserMove = true,
+            BreakOnMove = true,
         };
 
         _doAfter.TryStartDoAfter(doAfterEventArgs);

Content.Shared/Projectiles/SharedProjectileSystem.cs

@@ -53,10 +53,7 @@ public abstract partial class SharedProjectileSystem : EntitySystem
         args.Handled = true;
 
         _doAfter.TryStartDoAfter(new DoAfterArgs(EntityManager, args.User, component.RemovalTime.Value,
-            new RemoveEmbeddedProjectileEvent(), eventTarget: uid, target: uid)
-        {
-            DistanceThreshold = SharedInteractionSystem.InteractionRange,
-        });
+            new RemoveEmbeddedProjectileEvent(), eventTarget: uid, target: uid));
     }
 
     private void OnEmbedRemove(EntityUid uid, EmbeddableProjectileComponent component, RemoveEmbeddedProjectileEvent args)

Content.Shared/Prying/Systems/PryingSystem.cs

@@ -136,8 +136,7 @@ public sealed class PryingSystem : EntitySystem
         var doAfterArgs = new DoAfterArgs(EntityManager, user, TimeSpan.FromSeconds(modEv.BaseTime * modEv.PryTimeModifier / toolModifier), new DoorPryDoAfterEvent(), target, target, tool)
         {
             BreakOnDamage = true,
-            BreakOnUserMove = true,
-            BreakOnWeightlessMove = true,
+            BreakOnMove = true,
         };
 
         if (tool != null)

Content.Shared/RCD/Systems/RCDSystem.cs

@@ -104,8 +104,7 @@ public sealed class RCDSystem : EntitySystem
             BreakOnDamage = true,
             NeedHand = true,
             BreakOnHandChange = true,
-            BreakOnUserMove = true,
-            BreakOnTargetMove = args.Target != null,
+            BreakOnMove = true,
             AttemptFrequency = AttemptFrequency.EveryTick
         };
 

Content.Shared/RatKing/SharedRatKingSystem.cs

@@ -119,7 +119,7 @@ public abstract class SharedRatKingSystem : EntitySystem
                 {
                     BlockDuplicate = true,
                     BreakOnDamage = true,
-                    BreakOnUserMove = true,
+                    BreakOnMove = true,
                     DistanceThreshold = 2f
                 });
             }

Content.Shared/Research/Systems/SharedResearchStealerSystem.cs

@@ -46,8 +46,8 @@ public abstract class SharedResearchStealerSystem : EntitySystem
         var doAfterArgs = new DoAfterArgs(EntityManager, uid, comp.Delay, new ResearchStealDoAfterEvent(), target: target, used: uid, eventTarget: uid)
         {
             BreakOnDamage = true,
-            BreakOnUserMove = true,
-            MovementThreshold = 0.5f
+            BreakOnMove = true,
+            MovementThreshold = 0.5f,
         };
 
         _doAfter.TryStartDoAfter(doAfterArgs);

Content.Shared/Salvage/Fulton/SharedFultonSystem.cs

@@ -153,8 +153,7 @@ public abstract partial class SharedFultonSystem : EntitySystem
             {
                 CancelDuplicate = true,
                 MovementThreshold = 0.5f,
-                BreakOnUserMove = true,
-                BreakOnTargetMove = true,
+                BreakOnMove = true,
                 Broadcast = true,
                 NeedHand = true,
             });

Content.Shared/Sericulture/SericultureSystem.cs

@@ -61,7 +61,7 @@ public abstract partial class SharedSericultureSystem : EntitySystem
 
         var doAfter = new DoAfterArgs(EntityManager, uid, comp.ProductionLength, new SericultureDoAfterEvent(), uid)
         { // I'm not sure if more things should be put here, but imo ideally it should probably be set in the component/YAML. Not sure if this is currently possible.
-            BreakOnUserMove = true,
+            BreakOnMove = true,
             BlockDuplicate = true,
             BreakOnDamage = true,
             CancelDuplicate = true,

Content.Shared/Species/Systems/ReformSystem.cs

@@ -69,7 +69,7 @@ public sealed partial class ReformSystem : EntitySystem
         // Create a doafter & start it
         var doAfter = new DoAfterArgs(EntityManager, uid, comp.ReformTime, new ReformDoAfterEvent(), uid)
         {
-            BreakOnUserMove = true,
+            BreakOnMove = true,
             BlockDuplicate = true,
             BreakOnDamage = true,
             CancelDuplicate = true,
@@ -89,7 +89,7 @@ public sealed partial class ReformSystem : EntitySystem
             return;
 
         // Spawn a new entity
-        // This is, to an extent, taken from polymorph. I don't use polymorph for various reasons- most notably that this is permanent. 
+        // This is, to an extent, taken from polymorph. I don't use polymorph for various reasons- most notably that this is permanent.
         var child = Spawn(comp.ReformPrototype, Transform(uid).Coordinates);
 
         // This transfers the mind to the new entity
@@ -105,7 +105,7 @@ public sealed partial class ReformSystem : EntitySystem
         _actionsSystem.RemoveAction(uid, comp.ActionEntity); // Zombies can't reform
     }
 
-    public sealed partial class ReformEvent : InstantActionEvent { } 
+    public sealed partial class ReformEvent : InstantActionEvent { }
     
     [Serializable, NetSerializable]
     public sealed partial class ReformDoAfterEvent : SimpleDoAfterEvent { }

Content.Shared/SprayPainter/SharedSprayPainterSystem.cs

@@ -134,8 +134,7 @@ public abstract class SharedSprayPainterSystem : EntitySystem
 
         var doAfterEventArgs = new DoAfterArgs(EntityManager, args.User, painter.AirlockSprayTime, new SprayPainterDoorDoAfterEvent(sprite, style.Department), args.Used, target: ent, used: args.Used)
         {
-            BreakOnTargetMove = true,
-            BreakOnUserMove = true,
+            BreakOnMove = true,
             BreakOnDamage = true,
             NeedHand = true
         };

Content.Shared/Storage/EntitySystems/DumpableSystem.cs

@@ -133,8 +133,7 @@ public sealed class DumpableSystem : EntitySystem
 
         _doAfterSystem.TryStartDoAfter(new DoAfterArgs(EntityManager, userUid, delay, new DumpableDoAfterEvent(), storageUid, target: targetUid, used: storageUid)
         {
-            BreakOnTargetMove = true,
-            BreakOnUserMove = true,
+            BreakOnMove = true,
             NeedHand = true
         });
     }

Content.Shared/Storage/EntitySystems/SharedStorageSystem.cs

@@ -289,7 +289,7 @@ public abstract class SharedStorageSystem : EntitySystem
                 var doAfterArgs = new DoAfterArgs(EntityManager, args.User, delay, new AreaPickupDoAfterEvent(GetNetEntityList(validStorables)), uid, target: uid)
                 {
                     BreakOnDamage = true,
-                    BreakOnUserMove = true,
+                    BreakOnMove = true,
                     NeedHand = true
                 };
 

Content.Shared/Tools/Systems/SharedToolSystem.cs

@@ -124,8 +124,8 @@ public abstract partial class SharedToolSystem : EntitySystem
         var doAfterArgs = new DoAfterArgs(EntityManager, user, delay / toolComponent.SpeedModifier, toolEvent, tool, target: target, used: tool)
         {
             BreakOnDamage = true,
-            BreakOnTargetMove = true,
-            BreakOnUserMove = true,
+            BreakOnMove = true,
+            BreakOnWeightlessMove = false,
             NeedHand = tool != user,
             AttemptFrequency = IsWelder(tool) ? AttemptFrequency.EveryTick : AttemptFrequency.Never
         };

Content.Shared/VendingMachines/SharedVendingMachineSystem.Restock.cs

@@ -70,8 +70,7 @@ public abstract partial class SharedVendingMachineSystem
         var doAfterArgs = new DoAfterArgs(EntityManager, args.User, (float) component.RestockDelay.TotalSeconds, new RestockDoAfterEvent(), target,
             target: target, used: uid)
         {
-            BreakOnTargetMove = true,
-            BreakOnUserMove = true,
+            BreakOnMove = true,
             BreakOnDamage = true,
             NeedHand = true
         };

Content.Shared/Weapons/Ranged/Systems/SharedGunSystem.Ballistic.cs

@@ -74,8 +74,7 @@ public abstract partial class SharedGunSystem
 
         _doAfter.TryStartDoAfter(new DoAfterArgs(EntityManager, args.User, component.FillDelay, new AmmoFillDoAfterEvent(), used: uid, target: args.Target, eventTarget: uid)
         {
-            BreakOnTargetMove = true,
-            BreakOnUserMove = true,
+            BreakOnMove = true,
             BreakOnDamage = false,
             NeedHand = true
         });