Make CuffableComponent and CuffableSystem not Crash (Hopefully) (#39123)

* This system is ancient

* Destroy that API

* Address reviews

* Destroy merge conflicts from orbit

* seems to work fine

---------

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

Content.Client/Inventory/StrippableBoundUserInterface.cs

@@ -190,7 +190,7 @@ namespace Content.Client.Inventory
             if (EntMan.TryGetComponent<VirtualItemComponent>(heldEntity, out var virt))
             {
                 button.Blocked = true;
-                if (EntMan.TryGetComponent<CuffableComponent>(Owner, out var cuff) && _cuffable.GetAllCuffs(cuff).Contains(virt.BlockingEntity))
+                if (_cuffable.TryGetAllCuffs(Owner, out var cuffs) && cuffs.Contains(virt.BlockingEntity))
                     button.BlockedRect.MouseFilter = MouseFilterMode.Ignore;
             }
 

Content.Server/Cuffs/CuffableSystem.cs

@@ -15,7 +15,7 @@ namespace Content.Server.Cuffs
             SubscribeLocalEvent<CuffableComponent, ComponentGetState>(OnCuffableGetState);
         }
 
-        private void OnCuffableGetState(EntityUid uid, CuffableComponent component, ref ComponentGetState args)
+        private void OnCuffableGetState(Entity<CuffableComponent> entity, ref ComponentGetState args)
         {
             // there are 2 approaches i can think of to handle the handcuff overlay on players
             // 1 - make the current RSI the handcuff type that's currently active. all handcuffs on the player will appear the same.
@@ -23,12 +23,12 @@ namespace Content.Server.Cuffs
             // approach #2 would be more difficult/time consuming to do and the payoff doesn't make it worth it.
             // right now we're doing approach #1.
             HandcuffComponent? cuffs = null;
-            if (component.CuffedHandCount > 0)
+            if (TryGetLastCuff((entity, entity.Comp), out var cuff))
-                TryComp(component.LastAddedCuffs, out cuffs);
+                TryComp(cuff, out cuffs);
-            args.State = new CuffableComponentState(component.CuffedHandCount,
+            args.State = new CuffableComponentState(entity.Comp.CuffedHandCount,
-                component.CanStillInteract,
+                entity.Comp.CanStillInteract,
                 cuffs?.CuffedRSI,
-                $"{cuffs?.BodyIconState}-{component.CuffedHandCount}",
+                $"{cuffs?.BodyIconState}-{entity.Comp.CuffedHandCount}",
                 cuffs?.Color);
             // the iconstate is formatted as blah-2, blah-4, blah-6, etc.
             // the number corresponds to how many hands are cuffed.

Content.Shared/Cuffs/Components/CuffableComponent.cs

@@ -24,12 +24,6 @@ public sealed partial class CuffableComponent : Component
     [ViewVariables]
     public int CuffedHandCount => Container.ContainedEntities.Count * 2;
 
-    /// <summary>
-    /// The last pair of cuffs that was added to this entity.
-    /// </summary>
-    [ViewVariables]
-    public EntityUid LastAddedCuffs => Container.ContainedEntities[^1];
-
     /// <summary>
     ///     Container of various handcuffs currently applied to the entity.
     /// </summary>

Content.Shared/Cuffs/SharedCuffableSystem.cs

@@ -1,3 +1,4 @@
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using Content.Shared.ActionBlocker;
 using Content.Shared.Administration.Components;
@@ -260,7 +261,7 @@ namespace Content.Shared.Cuffs
         {
             if (args.Handled)
                 return;
-            TryUncuff(ent, ent, cuffable: ent.Comp);
+            TryUncuff((ent, ent.Comp), ent);
             args.Handled = true;
         }
 
@@ -278,7 +279,7 @@ namespace Content.Shared.Cuffs
 
             Verb verb = new()
             {
-                Act = () => TryUncuff(uid, args.User, cuffable: component),
+                Act = () => TryUncuff((uid, component), args.User),
                 DoContactInteraction = true,
                 Text = Loc.GetString("uncuff-verb-get-data-text")
             };
@@ -585,41 +586,31 @@ namespace Content.Shared.Cuffs
             return true;
         }
 
+        /// <inheritdoc cref="TryUncuff(Entity{CuffableComponent?},EntityUid,Entity{HandcuffComponent?})"/>
+        public void TryUncuff(Entity<CuffableComponent?> target, EntityUid user)
+        {
+            if (!TryGetLastCuff(target, out var cuff))
+                return;
+
+            TryUncuff(target, user, cuff.Value);
+        }
+
         /// <summary>
         /// Attempt to uncuff a cuffed entity. Can be called by the cuffed entity, or another entity trying to help uncuff them.
         /// If the uncuffing succeeds, the cuffs will drop on the floor.
         /// </summary>
-        /// <param name="target"></param>
+        /// <param name="target">The entity we're trying to remove cuffs from.</param>
-        /// <param name="user">The cuffed entity</param>
+        /// <param name="user">The entity doing the cuffing.</param>
-        /// <param name="cuffsToRemove">Optional param for the handcuff entity to remove from the cuffed entity. If null, uses the most recently added handcuff entity.</param>
+        /// <param name="cuff">The handcuff entity we're attempting to remove.</param>
-        /// <param name="cuffable"></param>
+        public void TryUncuff(Entity<CuffableComponent?> target, EntityUid user, Entity<HandcuffComponent?> cuff)
-        /// <param name="cuff"></param>
-        public void TryUncuff(EntityUid target, EntityUid user, EntityUid? cuffsToRemove = null, CuffableComponent? cuffable = null, HandcuffComponent? cuff = null)
         {
-            if (!Resolve(target, ref cuffable))
+            if (!Resolve(target, ref target.Comp) || !Resolve(cuff, ref cuff.Comp))
                 return;
 
-            var isOwner = user == target;
+            var isOwner = user == target.Owner;
 
-            if (cuffsToRemove == null)
+            if (!target.Comp.Container.ContainedEntities.Contains(cuff))
-            {
-                if (cuffable.Container.ContainedEntities.Count == 0)
-                {
-                    return;
-                }
-
-                cuffsToRemove = cuffable.LastAddedCuffs;
-            }
-            else
-            {
-                if (!cuffable.Container.ContainedEntities.Contains(cuffsToRemove.Value))
-                {
                 Log.Warning("A user is trying to remove handcuffs that aren't in the owner's container. This should never happen!");
-                }
-            }
-
-            if (!Resolve(cuffsToRemove.Value, ref cuff))
-                return;
 
             var attempt = new UncuffAttemptEvent(user, target);
             RaiseLocalEvent(user, ref attempt, true);
@@ -629,29 +620,28 @@ namespace Content.Shared.Cuffs
                 return;
             }
 
-            if (!isOwner && !_interaction.InRangeUnobstructed(user, target))
+            if (!isOwner && !_interaction.InRangeUnobstructed(user, target.Owner))
             {
                 _popup.PopupClient(Loc.GetString("cuffable-component-cannot-remove-cuffs-too-far-message"), user, user);
                 return;
             }
 
-
+            var ev = new ModifyUncuffDurationEvent(user, target, isOwner ? cuff.Comp.BreakoutTime : cuff.Comp.UncuffTime);
-            var ev = new ModifyUncuffDurationEvent(user, target, isOwner ? cuff.BreakoutTime : cuff.UncuffTime);
             RaiseLocalEvent(user, ref ev);
             var uncuffTime = ev.Duration;
 
             if (isOwner)
             {
-                if (!TryComp(cuffsToRemove.Value, out UseDelayComponent? useDelay))
+                if (!TryComp(cuff, out UseDelayComponent? useDelay))
                     return;
 
-                if (!_delay.TryResetDelay((cuffsToRemove.Value, useDelay), true))
+                if (!_delay.TryResetDelay((cuff, useDelay), true))
                 {
                     return;
                 }
             }
 
-            var doAfterEventArgs = new DoAfterArgs(EntityManager, user, uncuffTime, new UnCuffDoAfterEvent(), target, target, cuffsToRemove)
+            var doAfterEventArgs = new DoAfterArgs(EntityManager, user, uncuffTime, new UnCuffDoAfterEvent(), target, target, cuff)
             {
                 BreakOnMove = true,
                 BreakOnWeightlessMove = false,
@@ -666,7 +656,7 @@ namespace Content.Shared.Cuffs
 
             _adminLog.Add(LogType.Action, LogImpact.High, $"{ToPrettyString(user):player} is trying to uncuff {ToPrettyString(target):subject}");
 
-            var popupText = user == target
+            var popupText = user == target.Owner
                 ? "cuffable-component-start-uncuffing-self-observer"
                 : "cuffable-component-start-uncuffing-observer";
             _popup.PopupEntity(
@@ -678,7 +668,7 @@ namespace Content.Shared.Cuffs
                     .RemoveWhere(e => e.AttachedEntity == target || e.AttachedEntity == user),
                 true);
 
-            if (target == user)
+            if (isOwner)
             {
                 _popup.PopupClient(Loc.GetString("cuffable-component-start-uncuffing-self"), user, user);
             }
@@ -694,7 +684,7 @@ namespace Content.Shared.Cuffs
                     target);
             }
 
-            _audio.PlayPredicted(isOwner ? cuff.StartBreakoutSound : cuff.StartUncuffSound, target, user);
+            _audio.PlayPredicted(isOwner ? cuff.Comp.StartBreakoutSound : cuff.Comp.StartUncuffSound, target, user);
         }
 
         public void Uncuff(EntityUid target, EntityUid? user, EntityUid cuffsToRemove, CuffableComponent? cuffable = null, HandcuffComponent? cuff = null)
@@ -818,9 +808,56 @@ namespace Content.Shared.Cuffs
 
         #endregion
 
-        public IReadOnlyList<EntityUid> GetAllCuffs(CuffableComponent component)
+        /// <summary>
+        /// Tries to get a list of all the handcuffs stored in an entity's <see cref="CuffableComponent"/>.
+        /// </summary>
+        /// <param name="entity">The cuffable entity in question.</param>
+        /// <param name="cuffs">A list of cuffs if it exists.</param>
+        /// <returns>True if a list of cuffs with cuffs exists. False if no list exists or if it is empty.</returns>
+        public bool TryGetAllCuffs(Entity<CuffableComponent?> entity, out IReadOnlyList<EntityUid> cuffs)
         {
-            return component.Container.ContainedEntities;
+            cuffs = GetAllCuffs(entity);
+
+            return cuffs.Count > 0;
+        }
+
+        /// <summary>
+        /// Tries to get a list of all the handcuffs stored in a entity's <see cref="CuffableComponent"/>.
+        /// </summary>
+        /// <param name="entity">The cuffable entity in question.</param>
+        /// <returns>A list of cuffs if it exists, or null if there are no cuffs.</returns>
+        public IReadOnlyList<EntityUid> GetAllCuffs(Entity<CuffableComponent?> entity)
+        {
+            if (!Resolve(entity, ref entity.Comp))
+                return [];
+
+            return entity.Comp.Container.ContainedEntities;
+        }
+
+        /// <summary>
+        /// Tries to get the most recently added pair of handcuffs added to an entity with <see cref="CuffableComponent"/>.
+        /// </summary>
+        /// <param name="entity">The cuffable entity in question.</param>
+        /// <param name="cuff">The most recently added cuff.</param>
+        /// <returns>Returns true if a cuff exists and false if one doesn't.</returns>
+        public bool TryGetLastCuff(Entity<CuffableComponent?> entity, [NotNullWhen(true)] out EntityUid? cuff)
+        {
+            cuff = GetLastCuffOrNull(entity);
+
+            return cuff != null;
+        }
+
+        /// <summary>
+        /// Tries to get the most recently added pair of handcuffs added to an entity with <see cref="CuffableComponent"/>.
+        /// </summary>
+        /// <param name="entity">The cuffable entity in question.</param>
+        /// <returns>The most recently added cuff or null if none exists.</returns>
+        public EntityUid? GetLastCuffOrNull(Entity<CuffableComponent?> entity)
+        {
+            if (!Resolve(entity, ref entity.Comp))
+                return null;
+
+            return entity.Comp.Container.ContainedEntities.Count == 0 ? null : entity.Comp.Container.ContainedEntities.Last();
         }
     }
 

Content.Shared/Strip/SharedStrippableSystem.cs

@@ -128,10 +128,10 @@ public abstract class SharedStrippableSystem : EntitySystem
 
         // Is the target a handcuff?
         if (TryComp<VirtualItemComponent>(heldEntity, out var virtualItem) &&
-            TryComp<CuffableComponent>(target.Owner, out var cuffable) &&
+            _cuffableSystem.TryGetAllCuffs(target.Owner, out var cuffs) &&
-            _cuffableSystem.GetAllCuffs(cuffable).Contains(virtualItem.BlockingEntity))
+            cuffs.Contains(virtualItem.BlockingEntity))
         {
-            _cuffableSystem.TryUncuff(target.Owner, user, virtualItem.BlockingEntity, cuffable);
+            _cuffableSystem.TryUncuff(target.Owner, user, virtualItem.BlockingEntity);
             return;
         }
 

Content.Shared/Trigger/Systems/UncuffOnTriggerSystem.cs

@@ -10,10 +10,10 @@ public sealed class UncuffOnTriggerSystem : XOnTriggerSystem<UncuffOnTriggerComp
 
     protected override void OnTrigger(Entity<UncuffOnTriggerComponent> ent, EntityUid target, ref TriggerEvent args)
     {
-        if (!TryComp<CuffableComponent>(target, out var cuffs) || cuffs.Container.ContainedEntities.Count < 1)
+        if (!TryComp<CuffableComponent>(target, out var cuffs) || !_cuffable.TryGetLastCuff(target, out var cuff))
             return;
 
-        _cuffable.Uncuff(target, args.User, cuffs.LastAddedCuffs);
+        _cuffable.Uncuff(target, args.User, cuff.Value);
         args.Handled = true;
     }
 }