]> git.smokeofanarchy.ru Git - space-station-14.git/commitdiff
Fix `ItemSlotSystem` popup Logic (#28856)
authorBrandon Li <48413902+aspiringLich@users.noreply.github.com>
Sat, 2 Nov 2024 01:33:26 +0000 (21:33 -0400)
committerGitHub <noreply@github.com>
Sat, 2 Nov 2024 01:33:26 +0000 (20:33 -0500)
* move popup call out of `CanInsert` into `OnInteractUsing`

* im stupid and `reason` is completely unnecessary

Signed-off-by: Brandon Li <sirbrandonthenerd@gmail.com>
* return early when `itemSlots.Slots.Count == 0`

* tweak logic for triggering popups

* change popup logic again

* Consolidate whitelist check

* Get any popup message not just last failed slot

* Apply suggestions from code review

Co-authored-by: chromiumboy <50505512+chromiumboy@users.noreply.github.com>
* yoink

Co-authored-by: ShadowCommander <10494922+ShadowCommander@users.noreply.github.com>
---------

Signed-off-by: Brandon Li <sirbrandonthenerd@gmail.com>
Co-authored-by: ShadowCommander <10494922+ShadowCommander@users.noreply.github.com>
Co-authored-by: chromiumboy <50505512+chromiumboy@users.noreply.github.com>
Content.Shared/Containers/ItemSlot/ItemSlotsSystem.cs

index f25273f40394c1470c44f386b32f3111d49b7430..479690847c3645b58063ff23eed19f399fcc61f7 100644 (file)
@@ -60,6 +60,7 @@ namespace Content.Shared.Containers.ItemSlots
         }
 
         #region ComponentManagement
+
         /// <summary>
         ///     Spawn in starting items for any item slots that should have one.
         /// </summary>
@@ -70,7 +71,8 @@ namespace Content.Shared.Containers.ItemSlots
                 if (slot.HasItem || string.IsNullOrEmpty(slot.StartingItem))
                     continue;
 
-                var item = EntityManager.SpawnEntity(slot.StartingItem, EntityManager.GetComponent<TransformComponent>(uid).Coordinates);
+                var item = Spawn(slot.StartingItem, Transform(uid).Coordinates);
+                    
                 if (slot.ContainerSlot != null)
                     _containers.Insert(item, slot.ContainerSlot);
             }
@@ -99,7 +101,8 @@ namespace Content.Shared.Containers.ItemSlots
             if (itemSlots.Slots.TryGetValue(id, out var existing))
             {
                 if (existing.Local)
-                    Log.Error($"Duplicate item slot key. Entity: {EntityManager.GetComponent<MetaDataComponent>(uid).EntityName} ({uid}), key: {id}");
+                    Log.Error(
+                        $"Duplicate item slot key. Entity: {EntityManager.GetComponent<MetaDataComponent>(uid).EntityName} ({uid}), key: {id}");
                 else
                     // server state takes priority
                     slot.CopyFrom(existing);
@@ -134,7 +137,10 @@ namespace Content.Shared.Containers.ItemSlots
                 Dirty(uid, itemSlots);
         }
 
-        public bool TryGetSlot(EntityUid uid, string slotId, [NotNullWhen(true)] out ItemSlot? itemSlot, ItemSlotsComponent? component = null)
+        public bool TryGetSlot(EntityUid uid,
+            string slotId,
+            [NotNullWhen(true)] out ItemSlot? itemSlot,
+            ItemSlotsComponent? component = null)
         {
             itemSlot = null;
 
@@ -143,9 +149,11 @@ namespace Content.Shared.Containers.ItemSlots
 
             return component.Slots.TryGetValue(slotId, out itemSlot);
         }
+
         #endregion
 
         #region Interactions
+
         /// <summary>
         ///     Attempt to take an item from a slot, if any are set to EjectOnInteract.
         /// </summary>
@@ -201,20 +209,50 @@ namespace Content.Shared.Containers.ItemSlots
             if (!EntityManager.TryGetComponent(args.User, out HandsComponent? hands))
                 return;
 
+            if (itemSlots.Slots.Count == 0)
+                return;
+
+            // If any slot can be inserted into don't show popup.
+            // If any whitelist passes, but slot is locked, then show locked.
+            // If whitelist fails all, show whitelist fail.
+
+            // valid, insertable slots (if any)
             var slots = new List<ItemSlot>();
+
+            string? whitelistFailPopup = null;
+            string? lockedFailPopup = null;
             foreach (var slot in itemSlots.Slots.Values)
             {
                 if (!slot.InsertOnInteract)
                     continue;
 
-                if (!CanInsert(uid, args.Used, args.User, slot, swap: slot.Swap, popup: args.User))
-                    continue;
+                if (CanInsert(uid, args.Used, args.User, slot, slot.Swap))
+                {
+                    slots.Add(slot);
+                }
+                else
+                {
+                    var allowed = CanInsertWhitelist(args.Used, slot);
+                    if (lockedFailPopup == null && slot.LockedFailPopup != null && allowed && slot.Locked)
+                        lockedFailPopup = slot.LockedFailPopup;
 
-                slots.Add(slot);
+                    if (whitelistFailPopup == null && slot.WhitelistFailPopup != null)
+                        whitelistFailPopup = slot.WhitelistFailPopup;
+                }
             }
 
             if (slots.Count == 0)
+            {
+                // it's a bit weird that the popupMessage is stored with the item slots themselves, but in practice
+                // the popup messages will just all be the same, so it's probably fine.
+                //
+                // doing a check to make sure that they're all the same or something is probably frivolous
+                if (lockedFailPopup != null)
+                    _popupSystem.PopupClient(Loc.GetString(lockedFailPopup), uid, args.User);
+                else if (whitelistFailPopup != null)
+                    _popupSystem.PopupClient(Loc.GetString(whitelistFailPopup), uid, args.User);
                 return;
+            }
 
             // Drop the held item onto the floor. Return if the user cannot drop.
             if (!_handsSystem.TryDrop(args.User, args.Used, handsComp: hands))
@@ -236,23 +274,31 @@ namespace Content.Shared.Containers.ItemSlots
                 return;
             }
         }
+
         #endregion
 
         #region Insert
+
         /// <summary>
         ///     Insert an item into a slot. This does not perform checks, so make sure to also use <see
         ///     cref="CanInsert"/> or just use <see cref="TryInsert"/> instead.
         /// </summary>
         /// <param name="excludeUserAudio">If true, will exclude the user when playing sound. Does nothing client-side.
         /// Useful for predicted interactions</param>
-        private void Insert(EntityUid uid, ItemSlot slot, EntityUid item, EntityUid? user, bool excludeUserAudio = false)
+        private void Insert(EntityUid uid,
+            ItemSlot slot,
+            EntityUid item,
+            EntityUid? user,
+            bool excludeUserAudio = false)
         {
             bool? inserted = slot.ContainerSlot != null ? _containers.Insert(item, slot.ContainerSlot) : null;
             // ContainerSlot automatically raises a directed EntInsertedIntoContainerMessage
 
             // Logging
             if (inserted != null && inserted.Value && user != null)
-                _adminLogger.Add(LogType.Action, LogImpact.Low, $"{ToPrettyString(user.Value)} inserted {ToPrettyString(item)} into {slot.ContainerSlot?.ID + " slot of "}{ToPrettyString(uid)}");
+                _adminLogger.Add(LogType.Action,
+                    LogImpact.Low,
+                    $"{ToPrettyString(user.Value)} inserted {ToPrettyString(item)} into {slot.ContainerSlot?.ID + " slot of "}{ToPrettyString(uid)}");
 
             _audioSystem.PlayPredicted(slot.InsertSound, uid, excludeUserAudio ? user : null);
         }
@@ -261,46 +307,53 @@ namespace Content.Shared.Containers.ItemSlots
         ///     Check whether a given item can be inserted into a slot. Unless otherwise specified, this will return
         ///     false if the slot is already filled.
         /// </summary>
-        /// <remarks>
-        ///     If a popup entity is given, and if the item slot is set to generate a popup message when it fails to
-        ///     pass the whitelist or due to slot being locked, then this will generate an appropriate popup.
-        /// </remarks>
-        public bool CanInsert(EntityUid uid, EntityUid usedUid, EntityUid? user, ItemSlot slot, bool swap = false, EntityUid? popup = null)
+        public bool CanInsert(EntityUid uid,
+            EntityUid usedUid,
+            EntityUid? user,
+            ItemSlot slot,
+            bool swap = false)
         {
             if (slot.ContainerSlot == null)
                 return false;
 
-            if (_whitelistSystem.IsWhitelistFail(slot.Whitelist, usedUid) || _whitelistSystem.IsBlacklistPass(slot.Blacklist, usedUid))
-            {
-                if (popup.HasValue && slot.WhitelistFailPopup.HasValue)
-                    _popupSystem.PopupClient(Loc.GetString(slot.WhitelistFailPopup), uid, popup.Value);
+            if (slot.HasItem && (!swap || swap && !CanEject(uid, user, slot)))
                 return false;
-            }
 
-            if (slot.Locked)
-            {
-                if (popup.HasValue && slot.LockedFailPopup.HasValue)
-                    _popupSystem.PopupClient(Loc.GetString(slot.LockedFailPopup), uid, popup.Value);
+            if (!CanInsertWhitelist(usedUid, slot))
                 return false;
-            }
 
-            if (slot.HasItem && (!swap || (swap && !CanEject(uid, user, slot))))
+            if (slot.Locked)
                 return false;
 
             var ev = new ItemSlotInsertAttemptEvent(uid, usedUid, user, slot);
             RaiseLocalEvent(uid, ref ev);
             RaiseLocalEvent(usedUid, ref ev);
             if (ev.Cancelled)
+            {
                 return false;
+            }
 
             return _containers.CanInsert(usedUid, slot.ContainerSlot, assumeEmpty: swap);
         }
 
+        private bool CanInsertWhitelist(EntityUid usedUid, ItemSlot slot)
+        {
+            if (_whitelistSystem.IsWhitelistFail(slot.Whitelist, usedUid)
+                || _whitelistSystem.IsBlacklistPass(slot.Blacklist, usedUid))
+                return false;
+            return true;
+        }
+
         /// <summary>
         ///     Tries to insert item into a specific slot.
         /// </summary>
         /// <returns>False if failed to insert item</returns>
-        public bool TryInsert(EntityUid uid, string id, EntityUid item, EntityUid? user, ItemSlotsComponent? itemSlots = null, bool excludeUserAudio = false)
+        public bool TryInsert(EntityUid uid,
+            string id,
+            EntityUid item,
+            EntityUid? user,
+            ItemSlotsComponent? itemSlots = null,
+            bool excludeUserAudio = false)
         {
             if (!Resolve(uid, ref itemSlots))
                 return false;
@@ -315,7 +368,11 @@ namespace Content.Shared.Containers.ItemSlots
         ///     Tries to insert item into a specific slot.
         /// </summary>
         /// <returns>False if failed to insert item</returns>
-        public bool TryInsert(EntityUid uid, ItemSlot slot, EntityUid item, EntityUid? user, bool excludeUserAudio = false)
+        public bool TryInsert(EntityUid uid,
+            ItemSlot slot,
+            EntityUid item,
+            EntityUid? user,
+            bool excludeUserAudio = false)
         {
             if (!CanInsert(uid, item, user, slot))
                 return false;
@@ -329,7 +386,11 @@ namespace Content.Shared.Containers.ItemSlots
         ///     Does not check action blockers.
         /// </summary>
         /// <returns>False if failed to insert item</returns>
-        public bool TryInsertFromHand(EntityUid uid, ItemSlot slot, EntityUid user, HandsComponent? hands = null, bool excludeUserAudio = false)
+        public bool TryInsertFromHand(EntityUid uid,
+            ItemSlot slot,
+            EntityUid user,
+            HandsComponent? hands = null,
+            bool excludeUserAudio = false)
         {
             if (!Resolve(user, ref hands, false))
                 return false;
@@ -443,6 +504,7 @@ namespace Content.Shared.Containers.ItemSlots
 
             return 1;
         }
+
         #endregion
 
         #region Eject
@@ -462,7 +524,7 @@ namespace Content.Shared.Containers.ItemSlots
                 return false;
             }
 
-            if (slot.ContainerSlot?.ContainedEntity is not {} item)
+            if (slot.ContainerSlot?.ContainedEntity is not { } item)
                 return false;
 
             var ev = new ItemSlotEjectAttemptEvent(uid, item, user, slot);
@@ -487,7 +549,9 @@ namespace Content.Shared.Containers.ItemSlots
 
             // Logging
             if (ejected != null && ejected.Value && user != null)
-                _adminLogger.Add(LogType.Action, LogImpact.Low, $"{ToPrettyString(user.Value)} ejected {ToPrettyString(item)} from {slot.ContainerSlot?.ID + " slot of "}{ToPrettyString(uid)}");
+                _adminLogger.Add(LogType.Action,
+                    LogImpact.Low,
+                    $"{ToPrettyString(user.Value)} ejected {ToPrettyString(item)} from {slot.ContainerSlot?.ID + " slot of "}{ToPrettyString(uid)}");
 
             _audioSystem.PlayPredicted(slot.EjectSound, uid, excludeUserAudio ? user : null);
         }
@@ -496,7 +560,11 @@ namespace Content.Shared.Containers.ItemSlots
         ///     Try to eject an item from a slot.
         /// </summary>
         /// <returns>False if item slot is locked or has no item inserted</returns>
-        public bool TryEject(EntityUid uid, ItemSlot slot, EntityUid? user, [NotNullWhen(true)] out EntityUid? item, bool excludeUserAudio = false)
+        public bool TryEject(EntityUid uid,
+            ItemSlot slot,
+            EntityUid? user,
+            [NotNullWhen(true)] out EntityUid? item,
+            bool excludeUserAudio = false)
         {
             item = null;
 
@@ -518,8 +586,12 @@ namespace Content.Shared.Containers.ItemSlots
         ///     Try to eject item from a slot.
         /// </summary>
         /// <returns>False if the id is not valid, the item slot is locked, or it has no item inserted</returns>
-        public bool TryEject(EntityUid uid, string id, EntityUid? user,
-            [NotNullWhen(true)] out EntityUid? item, ItemSlotsComponent? itemSlots = null, bool excludeUserAudio = false)
+        public bool TryEject(EntityUid uid,
+            string id,
+            EntityUid? user,
+            [NotNullWhen(true)] out EntityUid? item,
+            ItemSlotsComponent? itemSlots = null,
+            bool excludeUserAudio = false)
         {
             item = null;
 
@@ -550,12 +622,16 @@ namespace Content.Shared.Containers.ItemSlots
 
             return true;
         }
+
         #endregion
 
         #region Verbs
-        private void AddAlternativeVerbs(EntityUid uid, ItemSlotsComponent itemSlots, GetVerbsEvent<AlternativeVerb> args)
+
+        private void AddAlternativeVerbs(EntityUid uid,
+            ItemSlotsComponent itemSlots,
+            GetVerbsEvent<AlternativeVerb> args)
         {
-            if (args.Hands == null || !args.CanAccess ||!args.CanInteract)
+            if (args.Hands == null || !args.CanAccess || !args.CanInteract)
             {
                 return;
             }
@@ -649,7 +725,9 @@ namespace Content.Shared.Containers.ItemSlots
             }
         }
 
-        private void AddInteractionVerbsVerbs(EntityUid uid, ItemSlotsComponent itemSlots, GetVerbsEvent<InteractionVerb> args)
+        private void AddInteractionVerbsVerbs(EntityUid uid,
+            ItemSlotsComponent itemSlots,
+            GetVerbsEvent<InteractionVerb> args)
         {
             if (args.Hands == null || !args.CanAccess || !args.CanInteract)
                 return;
@@ -708,7 +786,7 @@ namespace Content.Shared.Containers.ItemSlots
                         new SpriteSpecifier.Texture(
                             new ResPath("/Textures/Interface/VerbIcons/insert.svg.192dpi.png"));
                 }
-                else if(slot.EjectOnInteract)
+                else if (slot.EjectOnInteract)
                 {
                     // Inserting/ejecting is a primary interaction for this entity. Instead of using the insert
                     // category, we will use a single "Place <item>" verb.
@@ -727,9 +805,11 @@ namespace Content.Shared.Containers.ItemSlots
                 args.Verbs.Add(insertVerb);
             }
         }
+
         #endregion
 
         #region BUIs
+
         private void HandleButtonPressed(EntityUid uid, ItemSlotsComponent component, ItemSlotButtonPressedEvent args)
         {
             if (!component.Slots.TryGetValue(args.SlotId, out var slot))
@@ -740,6 +820,7 @@ namespace Content.Shared.Containers.ItemSlots
             else if (args.TryInsert && !slot.HasItem)
                 TryInsertFromHand(uid, slot, args.Actor);
         }
+
         #endregion
 
         /// <summary>