Storage CanInsert() tweaks (#21623)

Content.Client/Storage/Systems/StorageSystem.cs

@@ -23,10 +23,10 @@ public sealed class StorageSystem : SharedStorageSystem
         SubscribeNetworkEvent<AnimateInsertingEntitiesEvent>(HandleAnimatingInsertingEntities);
     }
 
-    public override void UpdateUI(EntityUid uid, StorageComponent component)
+    public override void UpdateUI(Entity<StorageComponent?> entity)
     {
-        // Should we wrap this in some prediction call maybe?
+        if (Resolve(entity.Owner, ref entity.Comp))
-        StorageUpdated?.Invoke(uid, component);
+            StorageUpdated?.Invoke(entity.Owner, entity.Comp);
     }
 
     /// <inheritdoc />

Content.Server/Item/ItemSystem.cs

@@ -1,24 +1,7 @@
-using Content.Server.Storage.Components;
+using Content.Shared.Item;
-using Content.Server.Storage.EntitySystems;
-using Content.Shared.Item;
-using Content.Shared.Stacks;
-using Content.Shared.Storage;
 
 namespace Content.Server.Item;
 
 public sealed class ItemSystem : SharedItemSystem
 {
-    [Dependency] private readonly StorageSystem _storage = default!;
-
-    protected override void OnStackCountChanged(EntityUid uid, ItemComponent component, StackCountChangedEvent args)
-    {
-        base.OnStackCountChanged(uid, component, args);
-
-        if (!Container.TryGetContainingContainer(uid, out var container) ||
-            !TryComp<StorageComponent>(container.Owner, out var storage))
-            return;
-
-        _storage.RecalculateStorageUsed(container.Owner, storage);
-        _storage.UpdateUI(container.Owner, storage);
-    }
 }

Content.Server/Stack/StackSystem.cs

@@ -1,10 +1,7 @@
-using Content.Server.Storage.EntitySystems;
 using Content.Shared.Popups;
 using Content.Shared.Stacks;
-using Content.Shared.Storage;
 using Content.Shared.Verbs;
 using JetBrains.Annotations;
-using Robust.Server.Containers;
 using Robust.Shared.Map;
 using Robust.Shared.Prototypes;
 
@@ -17,8 +14,6 @@ namespace Content.Server.Stack
     [UsedImplicitly]
     public sealed class StackSystem : SharedStackSystem
     {
-        [Dependency] private readonly ContainerSystem _container = default!;
-        [Dependency] private readonly StorageSystem _storage = default!;
         [Dependency] private readonly IPrototypeManager _prototypeManager = default!;
 
         public static readonly int[] DefaultSplitAmounts = { 1, 5, 10, 20, 30, 50 };
@@ -165,12 +160,6 @@ namespace Content.Server.Stack
             if (Split(uid, amount, userTransform.Coordinates, stack) is not {} split)
                 return;
 
-            if (_container.TryGetContainingContainer(uid, out var container) &&
-                TryComp<StorageComponent>(container.Owner, out var storage))
-            {
-                _storage.UpdateUI(container.Owner, storage);
-            }
-
             Hands.PickupOrDrop(userUid, split);
 
             Popup.PopupCursor(Loc.GetString("comp-stack-split"), userUid);

Content.Server/Storage/EntitySystems/StorageSystem.cs

@@ -90,7 +90,7 @@ public sealed partial class StorageSystem : SharedStorageSystem
         if (!_uiSystem.IsUiOpen(uid, args.UiKey))
         {
             storageComp.IsUiOpen = false;
-            UpdateStorageVisualization(uid, storageComp);
+            UpdateAppearance((uid, storageComp, null));
 
             if (storageComp.StorageCloseSound is not null)
                 Audio.Play(storageComp.StorageCloseSound, Filter.Pvs(uid, entityManager: EntityManager), uid, true, storageComp.StorageCloseSound.Params);

Content.Shared/Hands/EntitySystems/SharedHandsSystem.Drop.cs

@@ -27,6 +27,20 @@ public abstract partial class SharedHandsSystem
         RaiseLocalEvent(uid, didUnequip);
     }
 
+    /// <summary>
+    ///     Checks whether an entity can drop a given entity. Will return false if they are not holding the entity.
+    /// </summary>
+    public bool CanDrop(EntityUid uid, EntityUid entity, HandsComponent? handsComp = null, bool checkActionBlocker = true)
+    {
+        if (!Resolve(uid, ref handsComp))
+            return false;
+
+        if (!IsHolding(uid, entity, out var hand, handsComp))
+            return false;
+
+        return CanDropHeld(uid, hand, checkActionBlocker);
+    }
+
     /// <summary>
     ///     Checks if the contents of a hand is able to be removed from its container.
     /// </summary>

Content.Shared/Item/SharedItemSystem.cs

@@ -1,6 +1,5 @@
 using Content.Shared.Hands.EntitySystems;
 using Content.Shared.Interaction;
-using Content.Shared.Stacks;
 using Content.Shared.Verbs;
 using Content.Shared.Examine;
 using JetBrains.Annotations;
@@ -22,7 +21,6 @@ public abstract class SharedItemSystem : EntitySystem
         base.Initialize();
         SubscribeLocalEvent<ItemComponent, GetVerbsEvent<InteractionVerb>>(AddPickupVerb);
         SubscribeLocalEvent<ItemComponent, InteractHandEvent>(OnHandInteract, before: new []{typeof(SharedItemSystem)});
-        SubscribeLocalEvent<ItemComponent, StackCountChangedEvent>(OnStackCountChanged);
 
         SubscribeLocalEvent<ItemComponent, ComponentGetState>(OnGetState);
         SubscribeLocalEvent<ItemComponent, ComponentHandleState>(OnHandleState);
@@ -80,11 +78,6 @@ public abstract class SharedItemSystem : EntitySystem
         args.Handled = _handsSystem.TryPickup(args.User, uid, animateUser: false);
     }
 
-    protected virtual void OnStackCountChanged(EntityUid uid, ItemComponent component, StackCountChangedEvent args)
-    {
-
-    }
-
     private void OnHandleState(EntityUid uid, ItemComponent component, ref ComponentHandleState args)
     {
         if (args.Current is not ItemComponentState state)

Content.Shared/Storage/EntitySystems/SharedStorageSystem.cs

@@ -69,9 +69,11 @@ public abstract class SharedStorageSystem : EntitySystem
         SubscribeLocalEvent<StorageComponent, DestructionEventArgs>(OnDestroy);
         SubscribeLocalEvent<StorageComponent, StorageComponent.StorageInsertItemMessage>(OnInsertItemMessage);
         SubscribeLocalEvent<StorageComponent, BoundUIOpenedEvent>(OnBoundUIOpen);
+        SubscribeLocalEvent<MetaDataComponent, StackCountChangedEvent>(OnStackCountChanged);
 
-        SubscribeLocalEvent<StorageComponent, EntInsertedIntoContainerMessage>(OnStorageItemInserted);
+        SubscribeLocalEvent<StorageComponent, EntInsertedIntoContainerMessage>(OnContainerModified);
-        SubscribeLocalEvent<StorageComponent, EntRemovedFromContainerMessage>(OnStorageItemRemoved);
+        SubscribeLocalEvent<StorageComponent, EntRemovedFromContainerMessage>(OnContainerModified);
+        SubscribeLocalEvent<StorageComponent, ContainerIsInsertingAttemptEvent>(OnInsertAttempt);
 
         SubscribeLocalEvent<StorageComponent, AreaPickupDoAfterEvent>(OnDoAfter);
 
@@ -80,31 +82,11 @@ public abstract class SharedStorageSystem : EntitySystem
 
     private void OnComponentInit(EntityUid uid, StorageComponent storageComp, ComponentInit args)
     {
-        // ReSharper disable once StringLiteralTypo
+        storageComp.Container = _containerSystem.EnsureContainer<Container>(uid, StorageComponent.ContainerId);
-        storageComp.Container = _containerSystem.EnsureContainer<Container>(uid, "storagebase");
+        UpdateAppearance((uid, storageComp, null));
-        UpdateStorage(uid, storageComp);
     }
 
-    /// <summary>
+    public virtual void UpdateUI(Entity<StorageComponent?> entity) {}
-    /// Updates the storage UI, visualizer, etc.
-    /// </summary>
-    /// <param name="uid"></param>
-    /// <param name="component"></param>
-    private void UpdateStorage(EntityUid uid, StorageComponent component)
-    {
-        // TODO: I had this.
-        // We can get states being applied before the container is ready.
-        // ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract
-        if (component.Container == default)
-            return;
-
-        RecalculateStorageUsed(uid, component);
-        UpdateStorageVisualization(uid, component);
-        UpdateUI(uid, component);
-        Dirty(uid, component);
-    }
-
-    public virtual void UpdateUI(EntityUid uid, StorageComponent component) {}
 
     public virtual void OpenStorageUI(EntityUid uid, EntityUid entity, StorageComponent? storageComp = null, bool silent = false) { }
 
@@ -120,7 +102,7 @@ public abstract class SharedStorageSystem : EntitySystem
 
         // if the target is storage, add a verb to transfer storage.
         if (TryComp(args.Target, out StorageComponent? targetStorage)
-            && (!TryComp(uid, out LockComponent? targetLock) || !targetLock.Locked))
+            && (!TryComp(args.Target, out LockComponent? targetLock) || !targetLock.Locked))
         {
             UtilityVerb verb = new()
             {
@@ -142,8 +124,6 @@ public abstract class SharedStorageSystem : EntitySystem
         if (args.Handled || !storageComp.ClickInsert || TryComp(uid, out LockComponent? lockComponent) && lockComponent.Locked)
             return;
 
-        Log.Debug($"Storage (UID {uid}) attacked by user (UID {args.User}) with entity (UID {args.Used}).");
-
         if (HasComp<PlaceableSurfaceComponent>(uid))
             return;
 
@@ -184,7 +164,7 @@ public abstract class SharedStorageSystem : EntitySystem
     /// <returns></returns>
     private void AfterInteract(EntityUid uid, StorageComponent storageComp, AfterInteractEvent args)
     {
-        if (!args.CanReach)
+        if (args.Handled || !args.CanReach)
             return;
 
         // Pick up all entities in a radius around the clicked location.
@@ -217,6 +197,7 @@ public abstract class SharedStorageSystem : EntitySystem
                 };
 
                 _doAfterSystem.TryStartDoAfter(doAfterArgs);
+                args.Handled = true;
             }
 
             return;
@@ -245,6 +226,7 @@ public abstract class SharedStorageSystem : EntitySystem
                     _transform
                 );
 
+                args.Handled = true;
                 if (PlayerInsertEntityInWorld((uid, storageComp), args.User, target))
                 {
                     RaiseNetworkEvent(new AnimateInsertingEntitiesEvent(GetNetEntity(uid),
@@ -261,6 +243,7 @@ public abstract class SharedStorageSystem : EntitySystem
         if (args.Handled || args.Cancelled)
             return;
 
+        args.Handled = true;
         var successfullyInserted = new List<EntityUid>();
         var successfullyInsertedPositions = new List<EntityCoordinates>();
         var successfullyInsertedAngles = new List<Angle>();
@@ -374,44 +357,57 @@ public abstract class SharedStorageSystem : EntitySystem
         if (!storageComp.IsUiOpen)
         {
             storageComp.IsUiOpen = true;
-            UpdateStorageVisualization(uid, storageComp);
+            UpdateAppearance((uid, storageComp, null));
         }
     }
 
-    private void OnStorageItemInserted(EntityUid uid, StorageComponent component, EntInsertedIntoContainerMessage args)
+    private void OnContainerModified(EntityUid uid, StorageComponent component, ContainerModifiedMessage args)
     {
-        UpdateStorage(uid, component);
+        // ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract
-    }
+        if (component.Container == null)
-
-    private void OnStorageItemRemoved(EntityUid uid, StorageComponent storageComp, EntRemovedFromContainerMessage args)
-    {
-        UpdateStorage(uid, storageComp);
-    }
-
-    protected void UpdateStorageVisualization(EntityUid uid, StorageComponent storageComp)
-    {
-        if (!TryComp<AppearanceComponent>(uid, out var appearance))
             return;
 
-        _appearance.SetData(uid, StorageVisuals.Open, storageComp.IsUiOpen, appearance);
+        if (args.Container.ID != StorageComponent.ContainerId)
-        _appearance.SetData(uid, SharedBagOpenVisuals.BagState, storageComp.IsUiOpen ? SharedBagState.Open : SharedBagState.Closed);
+            return;
 
-        if (HasComp<ItemCounterComponent>(uid))
+        UpdateAppearance((uid, component, null));
-            _appearance.SetData(uid, StackVisuals.Hide, !storageComp.IsUiOpen);
+        UpdateUI((uid, component));
     }
 
-    public void RecalculateStorageUsed(EntityUid uid, StorageComponent storageComp)
+    private void OnInsertAttempt(EntityUid uid, StorageComponent component, ContainerIsInsertingAttemptEvent args)
     {
-        if (storageComp.MaxSlots == null)
+        if (args.Cancelled || args.Container.ID != StorageComponent.ContainerId)
+            return;
+
+        if (!CanInsert(uid, args.EntityUid, out _, component, ignoreStacks: true))
+            args.Cancel();
+    }
+
+    public void UpdateAppearance(Entity<StorageComponent?, AppearanceComponent?> entity)
+    {
+        // TODO STORAGE remove appearance data and just use the data on the component.
+        var (uid, storage, appearance) = entity;
+        if (!Resolve(uid, ref storage, ref appearance, false))
+            return;
+
+        int capacity;
+        int used;
+        if (storage.MaxSlots == null)
         {
-            _appearance.SetData(uid, StorageVisuals.StorageUsed, GetCumulativeItemSizes(uid, storageComp));
+            used = GetCumulativeItemSizes(uid, storage);
-            _appearance.SetData(uid, StorageVisuals.Capacity, storageComp.MaxTotalWeight);
+            capacity = storage.MaxTotalWeight;
         }
         else
         {
-            _appearance.SetData(uid, StorageVisuals.StorageUsed, storageComp.Container.ContainedEntities.Count);
+            capacity = storage.MaxSlots.Value;
-            _appearance.SetData(uid, StorageVisuals.Capacity, storageComp.MaxSlots.Value);
+            used = storage.Container.ContainedEntities.Count;
         }
+
+        _appearance.SetData(uid, StorageVisuals.StorageUsed, used, appearance);
+        _appearance.SetData(uid, StorageVisuals.Capacity, capacity, appearance);
+        _appearance.SetData(uid, StorageVisuals.Open, storage.IsUiOpen, appearance);
+        _appearance.SetData(uid, SharedBagOpenVisuals.BagState, storage.IsUiOpen ? SharedBagState.Open : SharedBagState.Closed, appearance);
+        _appearance.SetData(uid, StackVisuals.Hide, !storage.IsUiOpen, appearance);
     }
 
     /// <summary>
@@ -449,9 +445,9 @@ public abstract class SharedStorageSystem : EntitySystem
     /// <param name="storageComp"></param>
     /// <param name="item"></param>
     /// <returns>true if it can be inserted, false otherwise</returns>
-    public bool CanInsert(EntityUid uid, EntityUid insertEnt, out string? reason, StorageComponent? storageComp = null, ItemComponent? item = null)
+    public bool CanInsert(EntityUid uid, EntityUid insertEnt, out string? reason, StorageComponent? storageComp = null, ItemComponent? item = null, bool ignoreStacks = false)
     {
-        if (!Resolve(uid, ref storageComp) || !Resolve(insertEnt, ref item))
+        if (!Resolve(uid, ref storageComp) || !Resolve(insertEnt, ref item, false))
         {
             reason = null;
             return false;
@@ -475,36 +471,41 @@ public abstract class SharedStorageSystem : EntitySystem
             return false;
         }
 
-        if (!_stackQuery.TryGetComponent(insertEnt, out var stack) || !HasSpaceInStacks(uid, stack.StackTypeId))
+        if (!ignoreStacks
+            && _stackQuery.TryGetComponent(insertEnt, out var stack)
+            && HasSpaceInStacks((uid, storageComp), stack.StackTypeId))
         {
-            var maxSize = _item.GetSizePrototype(GetMaxItemSize((uid, storageComp)));
+            reason = null;
-            if (_item.GetSizePrototype(item.Size) > maxSize)
+            return true;
-            {
+        }
-                reason = "comp-storage-too-big";
-                return false;
-            }
 
-            if (TryComp<StorageComponent>(insertEnt, out var insertStorage)
+        var maxSize = _item.GetSizePrototype(GetMaxItemSize((uid, storageComp)));
-                && _item.GetSizePrototype(GetMaxItemSize((insertEnt, insertStorage))) >= maxSize)
+        if (_item.GetSizePrototype(item.Size) > maxSize)
-            {
+        {
-                reason = "comp-storage-too-big";
+            reason = "comp-storage-too-big";
-                return false;
+            return false;
-            }
+        }
 
-            if (storageComp.MaxSlots != null)
+        if (TryComp<StorageComponent>(insertEnt, out var insertStorage)
-            {
+            && _item.GetSizePrototype(GetMaxItemSize((insertEnt, insertStorage))) >= maxSize)
-                if (storageComp.Container.ContainedEntities.Count >= storageComp.MaxSlots)
+        {
-                {
+            reason = "comp-storage-too-big";
-                    reason = "comp-storage-insufficient-capacity";
+            return false;
-                    return false;
+        }
-                }
+
-            }
+        if (storageComp.MaxSlots != null)
-            else if (_item.GetItemSizeWeight(item.Size) + GetCumulativeItemSizes(uid, storageComp) > storageComp.MaxTotalWeight)
+        {
+            if (storageComp.Container.ContainedEntities.Count >= storageComp.MaxSlots)
             {
                 reason = "comp-storage-insufficient-capacity";
                 return false;
             }
         }
+        else if (_item.GetItemSizeWeight(item.Size) + GetCumulativeItemSizes(uid, storageComp) > storageComp.MaxTotalWeight)
+        {
+            reason = "comp-storage-insufficient-capacity";
+            return false;
+        }
 
         reason = null;
         return true;
@@ -513,7 +514,8 @@ public abstract class SharedStorageSystem : EntitySystem
     /// <summary>
     ///     Inserts into the storage container
     /// </summary>
-    /// <returns>true if the entity was inserted, false otherwise</returns>
+    /// <returns>true if the entity was inserted, false otherwise. This will also return true if a stack was partially
+    /// inserted.</returns>
     public bool Insert(
         EntityUid uid,
         EntityUid insertEnt,
@@ -528,7 +530,8 @@ public abstract class SharedStorageSystem : EntitySystem
     /// <summary>
     ///     Inserts into the storage container
     /// </summary>
-    /// <returns>true if the entity was inserted, false otherwise</returns>
+    /// <returns>true if the entity was inserted, false otherwise. This will also return true if a stack was partially
+    /// inserted</returns>
     public bool Insert(
         EntityUid uid,
         EntityUid insertEnt,
@@ -541,7 +544,7 @@ public abstract class SharedStorageSystem : EntitySystem
         stackedEntity = null;
         reason = null;
 
-        if (!Resolve(uid, ref storageComp) || !CanInsert(uid, insertEnt, out reason, storageComp))
+        if (!Resolve(uid, ref storageComp))
             return false;
 
         /*
@@ -553,56 +556,42 @@ public abstract class SharedStorageSystem : EntitySystem
          * For now we just treat items as always being the same size regardless of stack count.
          */
 
-        // If it's stackable then prefer to stack it
+        if (!_stackQuery.TryGetComponent(insertEnt, out var insertStack))
-        if (_stackQuery.TryGetComponent(insertEnt, out var insertStack))
         {
-            var toInsertCount = insertStack.Count;
+            if (!_containerSystem.Insert(insertEnt, storageComp.Container))
+                return false;
 
-            foreach (var ent in storageComp.Container.ContainedEntities)
+            if (playSound)
-            {
+                Audio.PlayPredicted(storageComp.StorageInsertSound, uid, user);
-                if (!_stackQuery.TryGetComponent(ent, out var containedStack))
-                    continue;
 
-                if (!_stack.TryAdd(insertEnt, ent, insertStack, containedStack))
+            return true;
-                    continue;
-
-                stackedEntity = ent;
-                var remaining = insertStack.Count;
-                toInsertCount -= toInsertCount - remaining;
-
-                if (remaining > 0)
-                    continue;
-
-                break;
-            }
-
-            // Still stackable remaining
-            if (toInsertCount > 0)
-            {
-                // Try to insert it as a new stack.
-                if (!CanInsert(uid, insertEnt, out _, storageComp) ||
-                    !storageComp.Container.Insert(insertEnt))
-                {
-                    UpdateUI(uid, storageComp);
-
-                    // If we also didn't do any stack fills above then just end
-                    // otherwise play sound and update UI anyway.
-                    if (toInsertCount == insertStack.Count)
-                        return false;
-                }
-            }
-            else
-            {
-                UpdateUI(uid, storageComp);
-            }
         }
-        // Non-stackable but no insertion for reasons.
+
-        else if (!storageComp.Container.Insert(insertEnt))
+        var toInsertCount = insertStack.Count;
+
+        foreach (var ent in storageComp.Container.ContainedEntities)
         {
+            if (!_stackQuery.TryGetComponent(ent, out var containedStack))
+                continue;
+
+            if (!_stack.TryAdd(insertEnt, ent, insertStack, containedStack))
+                continue;
+
+            stackedEntity = ent;
+            if (insertStack.Count == 0)
+                break;
+        }
+
+        // Still stackable remaining
+        if (insertStack.Count > 0
+            && !_containerSystem.Insert(insertEnt, storageComp.Container)
+            && toInsertCount == insertStack.Count)
+        {
+            // Failed to insert anything.
             return false;
         }
 
-        if (playSound && storageComp.StorageInsertSound is not null)
+        if (playSound)
             Audio.PlayPredicted(storageComp.StorageInsertSound, uid, user);
 
         return true;
@@ -628,7 +617,7 @@ public abstract class SharedStorageSystem : EntitySystem
             return false;
         }
 
-        if (!_sharedHandsSystem.TryDrop(player, toInsert.Value, handsComp: hands))
+        if (!_sharedHandsSystem.CanDrop(player, toInsert.Value, hands))
         {
             _popupSystem.PopupClient(Loc.GetString("comp-storage-cant-drop"), uid, player);
             return false;
@@ -737,6 +726,16 @@ public abstract class SharedStorageSystem : EntitySystem
         return sizes[Math.Max(currentSizeIndex - 1, 0)].ID;
     }
 
+    private void OnStackCountChanged(EntityUid uid, MetaDataComponent component, StackCountChangedEvent args)
+    {
+        if (_containerSystem.TryGetContainingContainer(uid, out var container, component) &&
+            container.ID == StorageComponent.ContainerId)
+        {
+            UpdateAppearance(container.Owner);
+            UpdateUI(container.Owner);
+        }
+    }
+
     public FixedPoint2 GetStorageFillPercentage(Entity<StorageComponent?> uid)
     {
         if (!Resolve(uid, ref uid.Comp))

Content.Shared/Storage/StorageComponent.cs

@@ -16,6 +16,8 @@ namespace Content.Shared.Storage
     [RegisterComponent, NetworkedComponent, AutoGenerateComponentState]
     public sealed partial class StorageComponent : Component
     {
+        public static string ContainerId = "storagebase";
+
         // TODO: This fucking sucks
         [ViewVariables(VVAccess.ReadWrite), DataField("isOpen"), AutoNetworkedField]
         public bool IsUiOpen;