Make ItemSlots more verbose + AME proof of concept (#25779)

* Make itemslots more verbose with AME proof of concept.

* Remove unnecessary [DataField] strings and use null coalescing in whitelist checks.

* Change optional popup fields into LocId.

Content.Server/Ame/EntitySystems/AmeControllerSystem.cs

@@ -234,6 +234,8 @@ public sealed class AmeControllerSystem : EntitySystem
 
         UpdateUi(uid, controller);
 
+        _itemSlots.SetLock(uid, controller.FuelSlot, value);
+
         // Logging
         if (!HasComp<MindContainerComponent>(user))
             return;

Content.Shared/Containers/ItemSlot/ItemSlotsComponent.cs

@@ -21,7 +21,7 @@ namespace Content.Shared.Containers.ItemSlots
         ///     The dictionary that stores all of the item slots whose interactions will be managed by the <see
         ///     cref="ItemSlotsSystem"/>.
         /// </summary>
-        [DataField("slots", readOnly:true)]
+        [DataField(readOnly:true)]
         public Dictionary<string, ItemSlot> Slots = new();
 
         // There are two ways to use item slots:
@@ -67,17 +67,17 @@ namespace Content.Shared.Containers.ItemSlots
             CopyFrom(other);
         }
 
-        [DataField("whitelist")]
+        [DataField]
         [Access(typeof(ItemSlotsSystem), Other = AccessPermissions.ReadWriteExecute)]
         public EntityWhitelist? Whitelist;
 
-        [DataField("blacklist")]
+        [DataField]
         public EntityWhitelist? Blacklist;
 
-        [DataField("insertSound")]
+        [DataField]
         public SoundSpecifier InsertSound = new SoundPathSpecifier("/Audio/Weapons/Guns/MagIn/revolver_magin.ogg");
 
-        [DataField("ejectSound")]
+        [DataField]
         public SoundSpecifier EjectSound = new SoundPathSpecifier("/Audio/Weapons/Guns/MagOut/revolver_magout.ogg");
 
         /// <summary>
@@ -87,7 +87,7 @@ namespace Content.Shared.Containers.ItemSlots
         ///     This will be passed through Loc.GetString. If the name is an empty string, then verbs will use the name
         ///     of the currently held or currently inserted entity instead.
         /// </remarks>
-        [DataField("name", readOnly: true)]
+        [DataField(readOnly: true)]
         [Access(typeof(ItemSlotsSystem), Other = AccessPermissions.ReadWriteExecute)] // FIXME Friends
         public string Name = string.Empty;
 
@@ -99,7 +99,7 @@ namespace Content.Shared.Containers.ItemSlots
         ///     property of that component (e.g., cell slot size category), and this can lead to unnecessary changes
         ///     when mapping.
         /// </remarks>
-        [DataField("startingItem", readOnly: true, customTypeSerializer: typeof(PrototypeIdSerializer<EntityPrototype>))]
+        [DataField(readOnly: true, customTypeSerializer: typeof(PrototypeIdSerializer<EntityPrototype>))]
         [Access(typeof(ItemSlotsSystem), Other = AccessPermissions.ReadWriteExecute)] // FIXME Friends
         [NonSerialized]
         public string? StartingItem;
@@ -111,7 +111,7 @@ namespace Content.Shared.Containers.ItemSlots
         ///     This doesn't have to mean the slot is somehow physically locked. In the case of the item cabinet, the
         ///     cabinet may simply be closed at the moment and needs to be opened first.
         /// </remarks>
-        [DataField("locked", readOnly: true)]
+        [DataField(readOnly: true)]
         [ViewVariables(VVAccess.ReadWrite)]
         public bool Locked = false;
 
@@ -121,14 +121,14 @@ namespace Content.Shared.Containers.ItemSlots
         /// <remarks>
         ///     This does not affect EjectOnInteract, since if you do that you probably want ejecting to work.
         /// </remarks>
-        [DataField("disableEject"), ViewVariables(VVAccess.ReadWrite)]
+        [DataField, ViewVariables(VVAccess.ReadWrite)]
         public bool DisableEject = false;
 
         /// <summary>
         ///     Whether the item slots system will attempt to insert item from the user's hands into this slot when interacted with.
         ///     It doesn't block other insertion methods, like verbs.
         /// </summary>
-        [DataField("insertOnInteract")]
+        [DataField]
         public bool InsertOnInteract = true;
 
         /// <summary>
@@ -139,7 +139,7 @@ namespace Content.Shared.Containers.ItemSlots
         ///     there are some exceptions. For example item cabinets and charging stations should probably eject their
         ///     contents when clicked on normally.
         /// </remarks>
-        [DataField("ejectOnInteract")]
+        [DataField]
         public bool EjectOnInteract = false;
 
         /// <summary>
@@ -151,21 +151,21 @@ namespace Content.Shared.Containers.ItemSlots
         ///     'Z' to open UI). Unlike <see cref="EjectOnInteract"/>, this will not make any changes to the context
         ///     menu, nor will it disable alt-click interactions.
         /// </remarks>
-        [DataField("ejectOnUse")]
+        [DataField]
         public bool EjectOnUse = false;
 
         /// <summary>
         ///     Override the insert verb text. Defaults to using the slot's name (if specified) or the name of the
         ///     targeted item. If specified, the verb will not be added to the default insert verb category.
         /// </summary>
-        [DataField("insertVerbText")]
+        [DataField]
         public string? InsertVerbText;
 
         /// <summary>
         ///     Override the eject verb text. Defaults to using the slot's name (if specified) or the name of the
         ///     targeted item. If specified, the verb will not be added to the default eject verb category
         /// </summary>
-        [DataField("ejectVerbText")]
+        [DataField]
         public string? EjectVerbText;
 
         [ViewVariables, NonSerialized]
@@ -178,7 +178,7 @@ namespace Content.Shared.Containers.ItemSlots
         /// <remarks>
         ///     The actual deconstruction logic is handled by the server-side EmptyOnMachineDeconstructSystem.
         /// </remarks>
-        [DataField("ejectOnDeconstruct")]
+        [DataField]
         [Access(typeof(ItemSlotsSystem), Other = AccessPermissions.ReadWriteExecute)]
         [NonSerialized]
         public bool EjectOnDeconstruct = true;
@@ -187,17 +187,30 @@ namespace Content.Shared.Containers.ItemSlots
         ///     If this slot belongs to some breakable or destructible entity, should the item inside the slot be
         ///     ejected when it is broken or destroyed?
         /// </summary>
-        [DataField("ejectOnBreak")]
+        [DataField]
         [Access(typeof(ItemSlotsSystem), Other = AccessPermissions.ReadWriteExecute)]
         [NonSerialized]
         public bool EjectOnBreak = false;
 
         /// <summary>
-        ///     If this is not an empty string, this will generate a popup when someone attempts to insert a bad item
+        ///     When specified, a popup will be generated whenever someone attempts to insert a bad item into this slot.
-        ///     into this slot. This string will be passed through localization.
         /// </summary>
-        [DataField("whitelistFailPopup")]
+        [DataField]
-        public string WhitelistFailPopup = string.Empty;
+        public LocId? WhitelistFailPopup;
+
+        /// <summary>
+        ///     When specified, a popup will be generated whenever someone attempts to insert a valid item, or eject an item
+        ///     from the slot while that slot is locked.
+        /// </summary>
+        [DataField]
+        public LocId? LockedFailPopup;
+
+        /// <summary>
+        ///     When specified, a popup will be generated whenever someone successfully inserts a valid item into this slot.
+        ///     This is also used for insertions resulting from swapping.
+        /// </summary>
+        [DataField]
+        public LocId? InsertSuccessPopup;
 
         /// <summary>
         ///     If the user interacts with an entity with an already-filled item slot, should they attempt to swap out the item?
@@ -206,7 +219,7 @@ namespace Content.Shared.Containers.ItemSlots
         ///     Useful for things like chem dispensers, but undesirable for things like the ID card console, where you
         ///     want to insert more than one item that matches the same whitelist.
         /// </remarks>
-        [DataField("swap")]
+        [DataField]
         [Access(typeof(ItemSlotsSystem), Other = AccessPermissions.ReadWriteExecute)]
         public bool Swap = true;
 
@@ -219,7 +232,7 @@ namespace Content.Shared.Containers.ItemSlots
         /// <summary>
         ///     Priority for use with the eject & insert verbs for this slot.
         /// </summary>
-        [DataField("priority")]
+        [DataField]
         public int Priority = 0;
 
         /// <summary>
@@ -244,6 +257,8 @@ namespace Content.Shared.Containers.ItemSlots
             InsertVerbText = other.InsertVerbText;
             EjectVerbText = other.EjectVerbText;
             WhitelistFailPopup = other.WhitelistFailPopup;
+            LockedFailPopup = other.LockedFailPopup;
+            InsertSuccessPopup = other.InsertSuccessPopup;
             Swap = other.Swap;
             Priority = other.Priority;
         }

Content.Shared/Containers/ItemSlot/ItemSlotsSystem.cs

@@ -9,12 +9,9 @@ using Content.Shared.Interaction;
 using Content.Shared.Interaction.Events;
 using Content.Shared.Popups;
 using Content.Shared.Verbs;
-using Robust.Shared.Audio;
 using Robust.Shared.Audio.Systems;
 using Robust.Shared.Containers;
 using Robust.Shared.GameStates;
-using Robust.Shared.Network;
-using Robust.Shared.Timing;
 using Robust.Shared.Utility;
 
 namespace Content.Shared.Containers.ItemSlots
@@ -22,10 +19,12 @@ namespace Content.Shared.Containers.ItemSlots
     /// <summary>
     ///     A class that handles interactions related to inserting/ejecting items into/from an item slot.
     /// </summary>
+    /// <remarks>
+    ///     Note when using popups on entities with many slots with InsertOnInteract, EjectOnInteract or EjectOnUse:
+    ///     A single use will try to insert to/eject from every slot and generate a popup for each that fails.
+    /// </remarks>
     public sealed class ItemSlotsSystem : EntitySystem
     {
-        [Dependency] private readonly IGameTiming _timing = default!;
-        [Dependency] private readonly INetManager _netManager = default!;
         [Dependency] private readonly ISharedAdminLogManager _adminLogger = default!;
         [Dependency] private readonly ActionBlockerSystem _actionBlockerSystem = default!;
         [Dependency] private readonly SharedContainerSystem _containers = default!;
@@ -153,7 +152,7 @@ namespace Content.Shared.Containers.ItemSlots
 
             foreach (var slot in itemSlots.Slots.Values)
             {
-                if (slot.Locked || !slot.EjectOnInteract || slot.Item == null)
+                if (!slot.EjectOnInteract || slot.Item == null || !CanEject(uid, args.User, slot, popup: args.User))
                     continue;
 
                 args.Handled = true;
@@ -172,7 +171,7 @@ namespace Content.Shared.Containers.ItemSlots
 
             foreach (var slot in itemSlots.Slots.Values)
             {
-                if (slot.Locked || !slot.EjectOnUse || slot.Item == null)
+                if (!slot.EjectOnUse || slot.Item == null || !CanEject(uid, args.User, slot, popup: args.User))
                     continue;
 
                 args.Handled = true;
@@ -214,6 +213,10 @@ namespace Content.Shared.Containers.ItemSlots
                     _handsSystem.TryPickupAnyHand(args.User, slot.Item.Value, handsComp: hands);
 
                 Insert(uid, slot, args.Used, args.User, excludeUserAudio: true);
+
+                if (slot.InsertSuccessPopup.HasValue)
+                    _popupSystem.PopupClient(Loc.GetString(slot.InsertSuccessPopup), uid, args.User);
+
                 args.Handled = true;
                 return;
             }
@@ -245,27 +248,29 @@ namespace Content.Shared.Containers.ItemSlots
         /// </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, then this will generate a popup.
+        ///     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)
         {
             if (slot.ContainerSlot == null)
                 return false;
 
-            if (slot.Locked)
+            if ((!slot.Whitelist?.IsValid(usedUid) ?? false) ||
-                return false;
+                (slot.Blacklist?.IsValid(usedUid) ?? false))
-
-            if (!swap && slot.HasItem)
-                return false;
-
-            if ((slot.Whitelist != null && !slot.Whitelist.IsValid(usedUid)) || (slot.Blacklist != null && slot.Blacklist.IsValid(usedUid)))
             {
-                if (_netManager.IsClient && _timing.IsFirstTimePredicted && popup.HasValue && !string.IsNullOrWhiteSpace(slot.WhitelistFailPopup))
+                if (popup.HasValue && slot.WhitelistFailPopup.HasValue)
-                    _popupSystem.PopupEntity(Loc.GetString(slot.WhitelistFailPopup), uid, popup.Value);
+                    _popupSystem.PopupClient(Loc.GetString(slot.WhitelistFailPopup), uid, popup.Value);
                 return false;
             }
 
-            if (swap && slot.HasItem && !CanEject(uid, user, slot))
+            if (slot.Locked)
+            {
+                if (popup.HasValue && slot.LockedFailPopup.HasValue)
+                    _popupSystem.PopupClient(Loc.GetString(slot.LockedFailPopup), uid, popup.Value);
+                return false;
+            }
+
+            if (slot.HasItem && (!swap || (swap && !CanEject(uid, user, slot))))
                 return false;
 
             var ev = new ItemSlotInsertAttemptEvent(uid, usedUid, user, slot);
@@ -332,9 +337,22 @@ namespace Content.Shared.Containers.ItemSlots
 
         #region Eject
 
-        public bool CanEject(EntityUid uid, EntityUid? user, ItemSlot slot)
+        /// <summary>
+        ///     Check whether an ejection from a given slot may happen.
+        /// </summary>
+        /// <remarks>
+        ///     If a popup entity is given, this will generate a popup message if any are configured on the the item slot.
+        /// </remarks>
+        public bool CanEject(EntityUid uid, EntityUid? user, ItemSlot slot, EntityUid? popup = null)
         {
-            if (slot.Locked || slot.ContainerSlot?.ContainedEntity is not {} item)
+            if (slot.Locked)
+            {
+                if (popup.HasValue && slot.LockedFailPopup.HasValue)
+                    _popupSystem.PopupClient(Loc.GetString(slot.LockedFailPopup), uid, popup.Value);
+                return false;
+            }
+
+            if (slot.ContainerSlot?.ContainedEntity is not {} item)
                 return false;
 
             var ev = new ItemSlotEjectAttemptEvent(uid, item, user, slot);
@@ -347,7 +365,7 @@ namespace Content.Shared.Containers.ItemSlots
         }
 
         /// <summary>
-        ///     Eject an item into a slot. This does not perform checks (e.g., is the slot locked?), so you should
+        ///     Eject an item from a slot. This does not perform checks (e.g., is the slot locked?), so you should
         ///     probably just use <see cref="TryEject"/> instead.
         /// </summary>
         /// <param name="excludeUserAudio">If true, will exclude the user when playing sound. Does nothing client-side.

Resources/Locale/en-US/ame/components/ame-controller-component.ftl

@@ -1,4 +1,6 @@
-ame-controller-component-interact-using-whitelist-fail = You can't put that in the controller...
+ame-controller-component-fuel-slot-fail-whitelist = You can't put that in the controller...
+ame-controller-component-fuel-slot-fail-locked = Power down the AME before manipulating its fuel.
+ame-controller-component-fuel-slot-success-insert = You insert the jar into the fuel slot.
 
 ## UI
 

Resources/Prototypes/Entities/Structures/Power/Generation/ame.yml

@@ -54,8 +54,9 @@
       whitelist:
         components:
         - AmeFuelContainer
-      whitelistFailPopup: ame-controller-component-interact-using-whitelist-fail
+      whitelistFailPopup: ame-controller-component-fuel-slot-fail-whitelist
-      swap: false
+      lockedFailPopup: ame-controller-component-fuel-slot-fail-locked
+      insertSuccessPopup: ame-controller-component-fuel-slot-success-insert
   - type: Explosive
     explosionType: Default
     intensitySlope: 5