Add BreakOnDropItem, update do afters, remove unnecessary declarations (#30361)

* Add BreakOnDropItem, update do afters, remove unnecessary declarations

* bola

* Changed my mind about the nuke

* gennies too

* Make the comments more clear.

* Sorry for the trailing commas

* Revert "Sorry for the trailing commas"

This reverts commit e60fd9a30977393df3344948e6d5c0ce035723cd.

---------

Co-authored-by: plykiya <plykiya@protonmail.com>

Content.Server/Botany/Systems/BotanySwabSystem.cs

@@ -48,7 +48,7 @@ public sealed class BotanySwabSystem : EntitySystem
         {
             Broadcast = true,
             BreakOnMove = true,
-            NeedHand = true
+            NeedHand = true,
         });
     }
 

Content.Server/Construction/ConstructionSystem.Interactions.cs

@@ -287,7 +287,7 @@ namespace Content.Server.Construction
                         {
                             BreakOnDamage = false,
                             BreakOnMove = true,
-                            NeedHand = true
+                            NeedHand = true,
                         };
 
                         var started  = _doAfterSystem.TryStartDoAfter(doAfterEventArgs);

Content.Server/Disposal/Unit/EntitySystems/DisposalUnitSystem.cs

@@ -488,7 +488,7 @@ public sealed class DisposalUnitSystem : SharedDisposalUnitSystem
         {
             BreakOnDamage = true,
             BreakOnMove = true,
-            NeedHand = false
+            NeedHand = false,
         };
 
         _doAfterSystem.TryStartDoAfter(doAfterArgs);

Content.Server/Ensnaring/EnsnareableSystem.Ensnaring.cs

@@ -128,7 +128,7 @@ public sealed partial class EnsnareableSystem
     /// <param name="component">The ensnaring component</param>
     public void TryFree(EntityUid target, EntityUid user, EntityUid ensnare, EnsnaringComponent component)
     {
-        //Don't do anything if they don't have the ensnareable component.
+        // Don't do anything if they don't have the ensnareable component.
         if (!HasComp<EnsnareableComponent>(target))
             return;
 
@@ -140,7 +140,7 @@ public sealed partial class EnsnareableSystem
             BreakOnMove = breakOnMove,
             BreakOnDamage = false,
             NeedHand = true,
-            BlockDuplicate = true,
+            BreakOnDropItem = false,
         };
 
         if (!_doAfter.TryStartDoAfter(doAfterEventArgs))

Content.Server/Forensics/Systems/ForensicsSystem.cs

@@ -159,7 +159,6 @@ namespace Content.Server.Forensics
                 var cleanDelay = cleanForensicsEntity.Comp.CleanDelay;
                 var doAfterArgs = new DoAfterArgs(EntityManager, user, cleanDelay, new CleanForensicsDoAfterEvent(), cleanForensicsEntity, target: target, used: cleanForensicsEntity)
                 {
-                    BreakOnHandChange = true,
                     NeedHand = true,
                     BreakOnDamage = true,
                     BreakOnMove = true,

Content.Server/Kitchen/EntitySystems/KitchenSpikeSystem.cs

@@ -259,7 +259,8 @@ namespace Content.Server.Kitchen.EntitySystems
             {
                 BreakOnDamage = true,
                 BreakOnMove = true,
-                NeedHand = true
+                NeedHand = true,
+                BreakOnDropItem = false,
             };
 
             _doAfter.TryStartDoAfter(doAfterArgs);

Content.Server/Kitchen/EntitySystems/SharpSystem.cs

@@ -77,7 +77,7 @@ public sealed class SharpSystem : EntitySystem
             {
                 BreakOnDamage = true,
                 BreakOnMove = true,
-                NeedHand = true
+                NeedHand = true,
             };
         _doAfterSystem.TryStartDoAfter(doAfter);
         return true;

Content.Server/MagicMirror/MagicMirrorSystem.cs

@@ -85,8 +85,7 @@ public sealed class MagicMirrorSystem : SharedMagicMirrorSystem
             DistanceThreshold = SharedInteractionSystem.InteractionRange,
             BreakOnDamage = true,
             BreakOnMove = true,
-            BreakOnHandChange = false,
+            NeedHand = true,
-            NeedHand = true
         },
             out var doAfterId);
 
@@ -166,7 +165,6 @@ public sealed class MagicMirrorSystem : SharedMagicMirrorSystem
         {
             BreakOnDamage = true,
             BreakOnMove = true,
-            BreakOnHandChange = false,
             NeedHand = true
         },
             out var doAfterId);
@@ -245,7 +243,6 @@ public sealed class MagicMirrorSystem : SharedMagicMirrorSystem
         {
             DistanceThreshold = SharedInteractionSystem.InteractionRange,
             BreakOnDamage = true,
-            BreakOnHandChange = false,
             NeedHand = true
         },
             out var doAfterId);
@@ -324,8 +321,7 @@ public sealed class MagicMirrorSystem : SharedMagicMirrorSystem
         {
             BreakOnDamage = true,
             BreakOnMove = true,
-            BreakOnHandChange = false,
+            NeedHand = true,
-            NeedHand = true
         },
             out var doAfterId);
 

Content.Server/Medical/BiomassReclaimer/BiomassReclaimerSystem.cs

@@ -169,7 +169,7 @@ namespace Content.Server.Medical.BiomassReclaimer
             _doAfterSystem.TryStartDoAfter(new DoAfterArgs(EntityManager, args.User, delay, new ReclaimerDoAfterEvent(), reclaimer, target: args.Target, used: args.Used)
             {
                 NeedHand = true,
-                BreakOnMove = true
+                BreakOnMove = true,
             });
         }
 

Content.Server/Medical/DefibrillatorSystem.cs

@@ -119,8 +119,6 @@ public sealed class DefibrillatorSystem : EntitySystem
         return _doAfter.TryStartDoAfter(new DoAfterArgs(EntityManager, user, component.DoAfterDuration, new DefibrillatorZapDoAfterEvent(),
             uid, target, uid)
             {
-                BlockDuplicate = true,
-                BreakOnHandChange = true,
                 NeedHand = true,
                 BreakOnMove = !component.AllowDoAfterMovement
             });

Content.Server/Medical/HealthAnalyzerSystem.cs

@@ -89,7 +89,7 @@ public sealed class HealthAnalyzerSystem : EntitySystem
         _doAfterSystem.TryStartDoAfter(new DoAfterArgs(EntityManager, args.User, uid.Comp.ScanDelay, new HealthAnalyzerDoAfterEvent(), uid, target: args.Target, used: uid)
         {
             NeedHand = true,
-            BreakOnMove = true
+            BreakOnMove = true,
         });
 
         var msg = Loc.GetString("health-analyzer-popup-scan-target", ("user", Identity.Entity(args.User, EntityManager)));

Content.Server/Nuke/NukeSystem.cs

@@ -590,7 +590,7 @@ public sealed class NukeSystem : EntitySystem
         {
             BreakOnDamage = true,
             BreakOnMove = true,
-            NeedHand = true
+            NeedHand = true,
         };
 
         if (!_doAfter.TryStartDoAfter(doAfter))

Content.Server/Power/Generator/PortableGeneratorSystem.cs

@@ -73,8 +73,10 @@ public sealed class PortableGeneratorSystem : SharedPortableGeneratorSystem
 
         _doAfter.TryStartDoAfter(new DoAfterArgs(EntityManager, user, component.StartTime, new GeneratorStartedEvent(), uid, uid)
         {
-            BreakOnDamage = true, BreakOnMove = true, RequireCanInteract = true,
+            BreakOnDamage = true,
-            NeedHand = true
+            BreakOnMove = true,
+            NeedHand = true,
+            BreakOnDropItem = false,
         });
     }
 

Content.Server/Resist/ResistLockerSystem.cs

@@ -54,7 +54,7 @@ public sealed class ResistLockerSystem : EntitySystem
         {
             BreakOnMove = true,
             BreakOnDamage = true,
-            NeedHand = false //No hands 'cause we be kickin'
+            NeedHand = false, //No hands 'cause we be kickin'
         };
 
         resistLockerComponent.IsResisting = true;

Content.Server/SprayPainter/SprayPainterSystem.cs

@@ -57,10 +57,9 @@ public sealed class SprayPainterSystem : SharedSprayPainterSystem
         {
             BreakOnMove = true,
             BreakOnDamage = true,
-            CancelDuplicate = true,
             // multiple pipes can be sprayed at once just not the same one
             DuplicateCondition = DuplicateConditions.SameTarget,
-            NeedHand = true
+            NeedHand = true,
         };
 
         args.Handled = DoAfter.TryStartDoAfter(doAfterEventArgs);

Content.Server/Sticky/Systems/StickySystem.cs

@@ -95,7 +95,7 @@ public sealed class StickySystem : EntitySystem
             _doAfterSystem.TryStartDoAfter(new DoAfterArgs(EntityManager, user, delay, new StickyDoAfterEvent(), uid, target: target, used: uid)
             {
                 BreakOnMove = true,
-                NeedHand = true
+                NeedHand = true,
             });
         }
         else
@@ -149,7 +149,7 @@ public sealed class StickySystem : EntitySystem
             _doAfterSystem.TryStartDoAfter(new DoAfterArgs(EntityManager, user, delay, new StickyDoAfterEvent(), uid, target: uid)
             {
                 BreakOnMove = true,
-                NeedHand = true
+                NeedHand = true,
             });
         }
         else

Content.Shared/Burial/BurialSystem.cs

@@ -49,7 +49,6 @@ public sealed class BurialSystem : EntitySystem
                 BreakOnMove = true,
                 BreakOnDamage = true,
                 NeedHand = true,
-                BreakOnHandChange = true
             };
 
             if (component.Stream == null)

Content.Shared/DoAfter/DoAfterArgs.cs

@@ -19,14 +19,14 @@ public sealed partial class DoAfterArgs
     /// <summary>
     ///     How long does the do_after require to complete
     /// </summary>
-    [DataField("delay", required: true)]
+    [DataField(required: true)]
     public TimeSpan Delay;
 
     /// <summary>
     ///     Applicable target (if relevant)
     /// </summary>
     [NonSerialized]
-    [DataField("target")]
+    [DataField]
     public EntityUid? Target;
 
     public NetEntity? NetTarget;
@@ -50,7 +50,7 @@ public sealed partial class DoAfterArgs
     /// <summary>
     ///     The event that will get raised when the DoAfter has finished. If null, this will simply raise a <see cref="SimpleDoAfterEvent"/>
     /// </summary>
-    [DataField("event", required: true)]
+    [DataField(required: true)]
     public DoAfterEvent Event = default!;
 
     /// <summary>
@@ -64,7 +64,7 @@ public sealed partial class DoAfterArgs
     ///     Entity which will receive the directed event. If null, no directed event will be raised.
     /// </summary>
     [NonSerialized]
-    [DataField("eventTarget")]
+    [DataField]
     public EntityUid? EventTarget;
 
     public NetEntity? NetEventTarget;
@@ -72,7 +72,7 @@ public sealed partial class DoAfterArgs
     /// <summary>
     /// Should the DoAfter event broadcast? If this is false, then <see cref="EventTarget"/> should be a valid entity.
     /// </summary>
-    [DataField("broadcast")]
+    [DataField]
     public bool Broadcast;
     #endregion
 
@@ -81,16 +81,24 @@ public sealed partial class DoAfterArgs
     /// <summary>
     ///     Whether or not this do after requires the user to have hands.
     /// </summary>
-    [DataField("needHand")]
+    [DataField]
     public bool NeedHand;
 
     /// <summary>
     ///     Whether we need to keep our active hand as is (i.e. can't change hand or change item). This also covers
     ///     requiring the hand to be free (if applicable). This does nothing if <see cref="NeedHand"/> is false.
     /// </summary>
-    [DataField("breakOnHandChange")]
+    [DataField]
     public bool BreakOnHandChange = true;
 
+    /// <summary>
+    ///     Whether the do-after should get interrupted if we drop the
+    ///     active item we started the do-after with
+    ///     This does nothing if <see cref="NeedHand"/> is false.
+    /// </summary>
+    [DataField]
+    public bool BreakOnDropItem = true;
+
     /// <summary>
     ///     If do_after stops when the user or target moves
     /// </summary>
@@ -107,31 +115,31 @@ public sealed partial class DoAfterArgs
     /// <summary>
     ///     Threshold for user and target movement
     /// </summary>
-    [DataField("movementThreshold")]
+    [DataField]
     public float MovementThreshold = 0.3f;
 
     /// <summary>
     ///     Threshold for distance user from the used OR target entities.
     /// </summary>
-    [DataField("distanceThreshold")]
+    [DataField]
     public float? DistanceThreshold;
 
     /// <summary>
     ///     Whether damage will cancel the DoAfter. See also <see cref="DamageThreshold"/>.
     /// </summary>
-    [DataField("breakOnDamage")]
+    [DataField]
     public bool BreakOnDamage;
 
     /// <summary>
     ///     Threshold for user damage. This damage has to be dealt in a single event, not over time.
     /// </summary>
-    [DataField("damageThreshold")]
+    [DataField]
     public FixedPoint2 DamageThreshold = 1;
 
     /// <summary>
     ///     If true, this DoAfter will be canceled if the user can no longer interact with the target.
     /// </summary>
-    [DataField("requireCanInteract")]
+    [DataField]
     public bool RequireCanInteract = true;
     #endregion
 
@@ -143,7 +151,7 @@ public sealed partial class DoAfterArgs
     ///     Note that this will block even if the duplicate is cancelled because either DoAfter had
     ///     <see cref="CancelDuplicate"/> enabled.
     /// </remarks>
-    [DataField("blockDuplicate")]
+    [DataField]
     public bool BlockDuplicate = true;
 
     //TODO: User pref to not cancel on second use on specific doafters
@@ -151,7 +159,7 @@ public sealed partial class DoAfterArgs
     ///     If true, this will cancel any duplicate DoAfters when attempting to add a new DoAfter. See also
     ///     <see cref="DuplicateConditions"/>.
     /// </summary>
-    [DataField("cancelDuplicate")]
+    [DataField]
     public bool CancelDuplicate = true;
 
     /// <summary>
@@ -162,7 +170,7 @@ public sealed partial class DoAfterArgs
     ///     Note that both DoAfters may have their own conditions, and they will be considered duplicated if either set
     ///     of conditions is satisfied.
     /// </remarks>
-    [DataField("duplicateCondition")]
+    [DataField]
     public DuplicateConditions DuplicateCondition = DuplicateConditions.All;
     #endregion
 
@@ -244,6 +252,7 @@ public sealed partial class DoAfterArgs
         Broadcast = other.Broadcast;
         NeedHand = other.NeedHand;
         BreakOnHandChange = other.BreakOnHandChange;
+        BreakOnDropItem = other.BreakOnDropItem;
         BreakOnMove = other.BreakOnMove;
         BreakOnWeightlessMove = other.BreakOnWeightlessMove;
         MovementThreshold = other.MovementThreshold;

Content.Shared/DoAfter/SharedDoAfterSystem.Update.cs

@@ -1,5 +1,6 @@
 using Content.Shared.Gravity;
 using Content.Shared.Hands.Components;
+using Content.Shared.Hands.EntitySystems;
 using Content.Shared.Interaction;
 using Content.Shared.Physics;
 using Robust.Shared.Utility;
@@ -11,6 +12,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!;
+    [Dependency] private readonly SharedHandsSystem _hands = default!;
 
     private DoAfter[] _doAfters = Array.Empty<DoAfter>();
 
@@ -217,16 +219,22 @@ public abstract partial class SharedDoAfterSystem : EntitySystem
         if (args.AttemptFrequency == AttemptFrequency.EveryTick && !TryAttemptEvent(doAfter))
             return true;
 
+        // Check if the do-after requires hands to perform at first
+        // For example, you need hands to strip clothes off of someone
+        // This does not mean their hand needs to be empty.
         if (args.NeedHand)
         {
             if (!handsQuery.TryGetComponent(args.User, out var hands) || hands.Count == 0)
                 return true;
 
-            if (args.BreakOnHandChange && (hands.ActiveHand?.Name != doAfter.InitialHand
+            // If an item was in the user's hand to begin with,
-                                           || hands.ActiveHandEntity != doAfter.InitialItem))
+            // check if the user is no longer holding the item.
-            {
+            if (args.BreakOnDropItem && !_hands.IsHolding((args.User, hands), doAfter.InitialItem))
+                    return true;
+
+            // If the user changes which hand is active at all, interrupt the do-after
+            if (args.BreakOnHandChange && hands.ActiveHand?.Name != doAfter.InitialHand)
                 return true;
-            }
         }
 
         if (args.RequireCanInteract && !_actionBlocker.CanInteract(args.User, args.Target))

Content.Shared/DoAfter/SharedDoAfterSystem.cs

@@ -227,7 +227,7 @@ public abstract partial class SharedDoAfterSystem : EntitySystem
 
         // For this we need to stay on the same hand slot and need the same item in that hand slot
         // (or if there is no item there we need to keep it free).
-        if (args.NeedHand && args.BreakOnHandChange)
+        if (args.NeedHand && (args.BreakOnHandChange || args.BreakOnDropItem))
         {
             if (!TryComp(args.User, out HandsComponent? handsComponent))
                 return false;

Content.Shared/Inventory/InventorySystem.Equip.cs

@@ -169,12 +169,8 @@ public abstract partial class InventorySystem
                 target,
                 itemUid)
             {
-                BlockDuplicate = true,
-                BreakOnHandChange = true,
                 BreakOnMove = true,
-                CancelDuplicate = true,
+                NeedHand = true,
-                RequireCanInteract = true,
-                NeedHand = true
             };
 
             _doAfter.TryStartDoAfter(args);
@@ -420,12 +416,8 @@ public abstract partial class InventorySystem
                 target,
                 removedItem.Value)
             {
-                BlockDuplicate = true,
-                BreakOnHandChange = true,
                 BreakOnMove = true,
-                CancelDuplicate = true,
+                NeedHand = true,
-                RequireCanInteract = true,
-                NeedHand = true
             };
 
             _doAfter.TryStartDoAfter(args);

Content.Shared/Lock/LockSystem.cs

@@ -124,8 +124,10 @@ public sealed class LockSystem : EntitySystem
             return _doAfter.TryStartDoAfter(
                 new DoAfterArgs(EntityManager, user, lockComp.LockTime, new LockDoAfter(), uid, uid)
                 {
-                    BreakOnDamage = true, BreakOnMove = true, RequireCanInteract = true,
+                    BreakOnDamage = true,
-                    NeedHand = true
+                    BreakOnMove = true,
+                    NeedHand = true,
+                    BreakOnDropItem = false,
                 });
         }
 
@@ -200,8 +202,10 @@ public sealed class LockSystem : EntitySystem
             return _doAfter.TryStartDoAfter(
                 new DoAfterArgs(EntityManager, user, lockComp.LockTime, new UnlockDoAfter(), uid, uid)
                 {
-                    BreakOnDamage = true, BreakOnMove = true, RequireCanInteract = true,
+                    BreakOnDamage = true,
-                    NeedHand = true
+                    BreakOnMove = true,
+                    NeedHand = true,
+                    BreakOnDropItem = false,
                 });
         }
 

Content.Shared/Magic/SpellbookSystem.cs

@@ -88,7 +88,7 @@ public sealed class SpellbookSystem : EntitySystem
         {
             BreakOnMove = true,
             BreakOnDamage = true,
-            NeedHand = true //What, are you going to read with your eyes only??
+            NeedHand = true, //What, are you going to read with your eyes only??
         };
 
         _doAfter.TryStartDoAfter(doAfterEventArgs);

Content.Shared/Salvage/Fulton/SharedFultonSystem.cs

@@ -153,7 +153,6 @@ public abstract partial class SharedFultonSystem : EntitySystem
         _doAfter.TryStartDoAfter(
             new DoAfterArgs(EntityManager, args.User, component.ApplyFultonDuration, ev, args.Target, args.Target, args.Used)
             {
-                CancelDuplicate = true,
                 MovementThreshold = 0.5f,
                 BreakOnMove = true,
                 Broadcast = true,

Content.Shared/SprayPainter/SharedSprayPainterSystem.cs

@@ -133,7 +133,7 @@ public abstract class SharedSprayPainterSystem : EntitySystem
         {
             BreakOnMove = true,
             BreakOnDamage = true,
-            NeedHand = true
+            NeedHand = true,
         };
         if (!DoAfter.TryStartDoAfter(doAfterEventArgs, out var id))
             return;

Content.Shared/Storage/EntitySystems/DumpableSystem.cs

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

Content.Shared/Storage/EntitySystems/SharedStorageSystem.cs

@@ -450,7 +450,7 @@ public abstract class SharedStorageSystem : EntitySystem
                 {
                     BreakOnDamage = true,
                     BreakOnMove = true,
-                    NeedHand = true
+                    NeedHand = true,
                 };
 
                 _doAfterSystem.TryStartDoAfter(doAfterArgs);

Content.Shared/VendingMachines/SharedVendingMachineSystem.Restock.cs

@@ -72,7 +72,7 @@ public abstract partial class SharedVendingMachineSystem
         {
             BreakOnMove = true,
             BreakOnDamage = true,
-            NeedHand = true
+            NeedHand = true,
         };
 
         if (!_doAfter.TryStartDoAfter(doAfterArgs))

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

@@ -81,7 +81,7 @@ public abstract partial class SharedGunSystem
         {
             BreakOnMove = true,
             BreakOnDamage = false,
-            NeedHand = true
+            NeedHand = true,
         });
     }