Fix bypassing vaulting clumsy check with verb action. (#24977)

* Fix bypassing bonking with verb

* Revert "Fix bypassing bonking with verb"

This reverts commit efa0f0f5777b893bcee5a852994cfa1e3fda3e71.

* Properly refactored BonkSystem.

* Oh hey, this is redundant now

* Better solution

* Reduced default bonk chance from 75% to 50%

* Also do a little grammar fix

* Moved BonkChance from BonkableComponent to ClumsyComponent.

* Revert "Moved BonkChance from BonkableComponent to ClumsyComponent."

This reverts commit 0acbd9273f20ec478692603781adf15e06e5ed41.

* Another little grammar fix

* Matched default bonk doAfter length to default climb doAfter length

* Fixed duplicate popups

* Check CanVault with verb use too. Add granularity to ClimbingComponent and remove Leg/Foot requirement.

* Don't show verb if you can't climb

* Removed CanForceClimb

* byref record struct

Content.Shared/Climbing/Components/BonkableComponent.cs

@@ -15,7 +15,7 @@ public sealed partial class BonkableComponent : Component
     /// Chance of bonk triggering if the user is clumsy.
     /// </summary>
     [DataField("bonkClumsyChance")]
-    public float BonkClumsyChance = 0.75f;
+    public float BonkClumsyChance = 0.5f;
 
     /// <summary>
     /// Sound to play when bonking.
@@ -42,5 +42,5 @@ public sealed partial class BonkableComponent : Component
     /// How long it takes to bonk.
     /// </summary>
     [DataField("bonkDelay")]
-    public float BonkDelay = 0.8f;
+    public float BonkDelay = 1.5f;
 }

Content.Shared/Climbing/Components/ClimbingComponent.cs

@@ -4,9 +4,21 @@ using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom;
 
 namespace Content.Shared.Climbing.Components;
 
+/// <summary>
+/// Indicates that this entity is able to be placed on top of surfaces like tables.
+/// Does not by itself allow the entity to carry out the action of climbing, unless
+/// <see cref="CanClimb"/> is true. Use <see cref="CanForceClimb"/> to control whether
+/// the entity can force other entities onto surfaces.
+/// </summary>
 [RegisterComponent, NetworkedComponent, AutoGenerateComponentState, AutoGenerateComponentPause]
 public sealed partial class ClimbingComponent : Component
 {
+    /// <summary>
+    /// Whether the owner is able to climb onto things by their own action.
+    /// </summary>
+    [DataField, AutoNetworkedField]
+    public bool CanClimb = true;
+
     /// <summary>
     /// Whether the owner is climbing on a climbable entity.
     /// </summary>

Content.Shared/Climbing/Events/AttemptClimbEvent.cs

@@ -0,0 +1,7 @@
+namespace Content.Shared.Climbing.Events;
+
+[ByRefEvent]
+public record struct AttemptClimbEvent(EntityUid User, EntityUid Climber, EntityUid Climbable)
+{
+    public bool Cancelled;
+}

Content.Shared/Climbing/Systems/BonkSystem.cs

@@ -1,5 +1,6 @@
 using Content.Shared.CCVar;
 using Content.Shared.Climbing.Components;
+using Content.Shared.Climbing.Events;
 using Content.Shared.Damage;
 using Content.Shared.DoAfter;
 using Content.Shared.DragDrop;
@@ -9,7 +10,6 @@ using Content.Shared.Interaction;
 using Content.Shared.Interaction.Components;
 using Content.Shared.Popups;
 using Content.Shared.Stunnable;
-using Robust.Shared.Audio;
 using Robust.Shared.Audio.Systems;
 using Robust.Shared.Configuration;
 using Robust.Shared.Player;
@@ -30,42 +30,54 @@ public sealed partial class BonkSystem : EntitySystem
     public override void Initialize()
     {
         base.Initialize();
-        SubscribeLocalEvent<BonkableComponent, DragDropTargetEvent>(OnDragDrop);
         SubscribeLocalEvent<BonkableComponent, BonkDoAfterEvent>(OnBonkDoAfter);
+        SubscribeLocalEvent<BonkableComponent, AttemptClimbEvent>(OnAttemptClimb);
     }
 
-    private void OnBonkDoAfter(EntityUid uid, Components.BonkableComponent component, BonkDoAfterEvent args)
+    private void OnBonkDoAfter(EntityUid uid, BonkableComponent component, BonkDoAfterEvent args)
     {
-        if (args.Handled || args.Cancelled || args.Args.Target == null)
+        if (args.Handled || args.Cancelled || args.Args.Used == null)
             return;
 
-        TryBonk(args.Args.User, uid, component);
+        TryBonk(args.Args.Used.Value, uid, component, source: args.Args.User);
 
         args.Handled = true;
     }
 
 
-    public bool TryBonk(EntityUid user, EntityUid bonkableUid, Components.BonkableComponent? bonkableComponent = null)
+    public bool TryBonk(EntityUid user, EntityUid bonkableUid, BonkableComponent? bonkableComponent = null, EntityUid? source = null)
     {
         if (!Resolve(bonkableUid, ref bonkableComponent, false))
             return false;
 
-        if (!_cfg.GetCVar(CCVars.GameTableBonk))
-        {
-            // Not set to always bonk, try clumsy roll.
-            if (!_interactionSystem.TryRollClumsy(user, bonkableComponent.BonkClumsyChance))
-                return false;
-        }
-
         // BONK!
         var userName = Identity.Entity(user, EntityManager);
         var bonkableName = Identity.Entity(bonkableUid, EntityManager);
 
-        _popupSystem.PopupEntity(Loc.GetString("bonkable-success-message-others", ("user", userName), ("bonkable", bonkableName)), user, Filter.PvsExcept(user), true);
+        if (user == source)
+        {
+            // Non-local, non-bonking players
+            _popupSystem.PopupEntity(Loc.GetString("bonkable-success-message-others", ("user", userName), ("bonkable", bonkableName)), user, Filter.PvsExcept(user), true);
+            // Local, bonking player
+            _popupSystem.PopupClient(Loc.GetString("bonkable-success-message-user", ("user", userName), ("bonkable", bonkableName)), user, user);
+        }
+        else if (source != null)
+        {
+            // Local, non-bonking player (dragger)
+            _popupSystem.PopupClient(Loc.GetString("bonkable-success-message-others", ("user", userName), ("bonkable", bonkableName)), user, source.Value);
+            // Non-local, non-bonking players
+            _popupSystem.PopupEntity(Loc.GetString("bonkable-success-message-others", ("user", userName), ("bonkable", bonkableName)), user, Filter.Pvs(user).RemoveWhereAttachedEntity(e => e == user || e == source.Value), true);
+            // Non-local, bonking player
+            _popupSystem.PopupEntity(Loc.GetString("bonkable-success-message-user", ("user", userName), ("bonkable", bonkableName)), user, user);
+        }
 
-        _popupSystem.PopupEntity(Loc.GetString("bonkable-success-message-user", ("user", userName), ("bonkable", bonkableName)), user, user);
 
-        _audioSystem.PlayPvs(bonkableComponent.BonkSound, bonkableUid);
+
+        if (source != null)
+            _audioSystem.PlayPredicted(bonkableComponent.BonkSound, bonkableUid, source);
+        else
+            _audioSystem.PlayPvs(bonkableComponent.BonkSound, bonkableUid);
+
         _stunSystem.TryParalyze(user, TimeSpan.FromSeconds(bonkableComponent.BonkTime), true);
 
         if (bonkableComponent.BonkDamage is { } bonkDmg)
@@ -75,12 +87,22 @@ public sealed partial class BonkSystem : EntitySystem
 
     }
 
-    private void OnDragDrop(EntityUid uid, Components.BonkableComponent component, ref DragDropTargetEvent args)
+    private bool TryStartBonk(EntityUid uid, EntityUid user, EntityUid climber, BonkableComponent? bonkableComponent = null)
     {
-        if (args.Handled || !HasComp<ClumsyComponent>(args.Dragged) || !HasComp<HandsComponent>(args.User))
+        if (!Resolve(uid, ref bonkableComponent, false))
-            return;
+            return false;
 
-        var doAfterArgs = new DoAfterArgs(EntityManager, args.Dragged, component.BonkDelay, new BonkDoAfterEvent(), uid, target: uid)
+        if (!HasComp<ClumsyComponent>(climber) || !HasComp<HandsComponent>(user))
+            return false;
+
+        if (!_cfg.GetCVar(CCVars.GameTableBonk))
+        {
+            // Not set to always bonk, try clumsy roll.
+            if (!_interactionSystem.TryRollClumsy(climber, bonkableComponent.BonkClumsyChance))
+                return false;
+        }
+
+        var doAfterArgs = new DoAfterArgs(EntityManager, user, bonkableComponent.BonkDelay, new BonkDoAfterEvent(), uid, target: uid, used: climber)
         {
             BreakOnMove = true,
             BreakOnDamage = true
@@ -88,7 +110,16 @@ public sealed partial class BonkSystem : EntitySystem
 
         _doAfter.TryStartDoAfter(doAfterArgs);
 
-        args.Handled = true;
+        return true;
+    }
+
+    private void OnAttemptClimb(EntityUid uid, BonkableComponent component, AttemptClimbEvent args)
+    {
+        if (args.Cancelled || !HasComp<ClumsyComponent>(args.Climber) || !HasComp<HandsComponent>(args.User))
+            return;
+
+        if (TryStartBonk(uid, args.User, args.Climber, component))
+            args.Cancelled = true;
     }
 
     [Serializable, NetSerializable]

Content.Shared/Climbing/Systems/ClimbSystem.cs

@@ -1,6 +1,4 @@
 using Content.Shared.ActionBlocker;
-using Content.Shared.Body.Components;
-using Content.Shared.Body.Part;
 using Content.Shared.Body.Systems;
 using Content.Shared.Buckle.Components;
 using Content.Shared.Climbing.Components;
@@ -151,7 +149,6 @@ public sealed partial class ClimbSystem : VirtualController
         if (args.Handled)
             return;
 
-
         var canVault = args.User == args.Dragged
             ? CanVault(component, args.User, uid, out _)
             : CanVault(component, args.User, args.Dragged, uid, out _);
@@ -169,7 +166,7 @@ public sealed partial class ClimbSystem : VirtualController
         if (!args.CanAccess || !args.CanInteract || !_actionBlockerSystem.CanMove(args.User))
             return;
 
-        if (!TryComp(args.User, out ClimbingComponent? climbingComponent) || climbingComponent.IsClimbing)
+        if (!TryComp(args.User, out ClimbingComponent? climbingComponent) || climbingComponent.IsClimbing || !climbingComponent.CanClimb)
             return;
 
         // TODO VERBS ICON add a climbing icon?
@@ -198,14 +195,28 @@ public sealed partial class ClimbSystem : VirtualController
     {
         id = null;
 
-        if (!Resolve(climbable, ref comp) || !Resolve(entityToMove, ref climbing))
+        if (!Resolve(climbable, ref comp) || !Resolve(entityToMove, ref climbing, false))
             return false;
 
+        var canVault = user == entityToMove
+             ? CanVault(comp, user, climbable, out var reason)
+             : CanVault(comp, user, entityToMove, climbable, out reason);
+        if (!canVault)
+        {
+            _popupSystem.PopupClient(reason, user, user);
+            return false;
+        }
+
         // Note, IsClimbing does not mean a DoAfter is active, it means the target has already finished a DoAfter and
         // is currently on top of something..
         if (climbing.IsClimbing)
             return true;
 
+        var ev = new AttemptClimbEvent(user, entityToMove, climbable);
+        RaiseLocalEvent(climbable, ref ev);
+        if (ev.Cancelled)
+            return false;
+
         var args = new DoAfterArgs(EntityManager, user, comp.ClimbDelay, new ClimbDoAfterEvent(),
             entityToMove,
             target: climbable,
@@ -244,7 +255,7 @@ public sealed partial class ClimbSystem : VirtualController
         var (worldPos, worldRot) = _xformSystem.GetWorldPositionRotation(xform);
         var worldDirection = _xformSystem.GetWorldPosition(climbable) - worldPos;
         var distance = worldDirection.Length();
-        var parentRot = (worldRot - xform.LocalRotation);
+        var parentRot = worldRot - xform.LocalRotation;
         // Need direction relative to climber's parent.
         var localDirection = (-parentRot).RotateVec(worldDirection);
 
@@ -399,10 +410,8 @@ public sealed partial class ClimbSystem : VirtualController
             return false;
         }
 
-        if (!HasComp<ClimbingComponent>(user)
+        if (!TryComp<ClimbingComponent>(user, out var climbingComp)
-            || !TryComp(user, out BodyComponent? body)
+            || !climbingComp.CanClimb)
-            || !_bodySystem.BodyHasPartType(user, BodyPartType.Leg, body)
-            || !_bodySystem.BodyHasPartType(user, BodyPartType.Foot, body))
         {
             reason = Loc.GetString("comp-climbable-cant-climb");
             return false;
@@ -438,7 +447,7 @@ public sealed partial class ClimbSystem : VirtualController
 
         if (!HasComp<ClimbingComponent>(dragged))
         {
-            reason = Loc.GetString("comp-climbable-cant-climb");
+            reason = Loc.GetString("comp-climbable-target-cant-climb", ("moved-user", Identity.Entity(dragged, EntityManager)));
             return false;
         }
 

Resources/Locale/en-US/bonk/components/bonkable-component.ftl

@@ -1,2 +1,2 @@
-bonkable-success-message-others = { CAPITALIZE(THE($user)) } bonks { POSS-ADJ($user) } head against { $bonkable }
+bonkable-success-message-others = { CAPITALIZE(THE($user)) } bonks { POSS-ADJ($user) } head against { THE($bonkable) }
 bonkable-success-message-user = You bonk your head against { THE($bonkable) }

Resources/Locale/en-US/climbing/climbable-component.ftl

@@ -12,10 +12,10 @@ comp-climbable-user-climbs = You jump onto { THE($climbable) }!
 # Shown to others when $user climbs on $climbable
 comp-climbable-user-climbs-other  = { CAPITALIZE(THE($user)) } jumps onto { THE($climbable) }!
 
-# Shown to you when your character force someone to climb on $climbable
+# Shown to you when your character forces someone to climb on $climbable
-comp-climbable-user-climbs-force = You force { CAPITALIZE(THE($moved-user)) } onto { THE($climbable) }!
+comp-climbable-user-climbs-force = You force { THE($moved-user) } onto { THE($climbable) }!
 
-# Shown to others when someone force other $moved-user to climb on $climbable
+# Shown to others when someone forces other $moved-user to climb on $climbable
 comp-climbable-user-climbs-force-other = { CAPITALIZE(THE($user)) } forces { THE($moved-user) } onto { THE($climbable) }!
 
 # Shown to you when your character is far away from climbable
@@ -24,5 +24,8 @@ comp-climbable-cant-reach = You can't reach there!
 # Shown to you when your character can't interact with climbable for some reason
 comp-climbable-cant-interact = You can't do that!
 
-# Shown to you when your character can't climb
+# Shown to you when your character isn't able to climb by their own actions
 comp-climbable-cant-climb = You are incapable of climbing!
+
+# Shown to you when your character tries to force someone else who can't climb onto a climbable
+comp-climbable-target-cant-climb = { CAPITALIZE(THE($moved-user)) } can't go there!