General storage fixes (#34845)

* Fix storage stars

* Fix some more storage bugs

- Fix positions not saving.
- Fix the 1-tick delay between parent and child storage UIs opening / closing.
- Fix BackButton being visible sometimes when it shouldn't be.

* milon is a furry

Content.Client/PDA/PdaBoundUserInterface.cs

@@ -30,8 +30,7 @@ namespace Content.Client.PDA
 
         private void CreateMenu()
         {
-            _menu = this.CreateWindow<PdaMenu>();
+            _menu = this.CreateWindowCenteredLeft<PdaMenu>();
-            _menu.OpenCenteredLeft();
 
             _menu.FlashLightToggleButton.OnToggled += _ =>
             {

Content.Client/Storage/StorageBoundUserInterface.cs

@@ -1,8 +1,10 @@
+using System.Numerics;
 using Content.Client.UserInterface.Systems.Storage;
 using Content.Client.UserInterface.Systems.Storage.Controls;
 using Content.Shared.Storage;
 using JetBrains.Annotations;
 using Robust.Client.UserInterface;
+using Robust.Client.UserInterface.Controls;
 
 namespace Content.Client.Storage;
 
@@ -11,6 +13,8 @@ public sealed class StorageBoundUserInterface : BoundUserInterface
 {
     private StorageWindow? _window;
 
+    public Vector2? Position => _window?.Position;
+
     public StorageBoundUserInterface(EntityUid owner, Enum uiKey) : base(owner, uiKey)
     {
     }
@@ -21,7 +25,7 @@ public sealed class StorageBoundUserInterface : BoundUserInterface
 
         _window = IoCManager.Resolve<IUserInterfaceManager>()
             .GetUIController<StorageUIController>()
-            .CreateStorageWindow(Owner);
+            .CreateStorageWindow(this);
 
         if (EntMan.TryGetComponent(Owner, out StorageComponent? storage))
         {
@@ -50,10 +54,20 @@ public sealed class StorageBoundUserInterface : BoundUserInterface
     protected override void Dispose(bool disposing)
     {
         base.Dispose(disposing);
-
         Reclaim();
     }
 
+    public void CloseWindow(Vector2 position)
+    {
+        if (_window == null)
+            return;
+
+        // Update its position before potentially saving.
+        // Listen it makes sense okay.
+        LayoutContainer.SetPosition(_window, position);
+        _window?.Close();
+    }
+
     public void Hide()
     {
         if (_window == null)
@@ -70,6 +84,15 @@ public sealed class StorageBoundUserInterface : BoundUserInterface
         _window.Visible = true;
     }
 
+    public void Show(Vector2 position)
+    {
+        if (_window == null)
+            return;
+
+        Show();
+        LayoutContainer.SetPosition(_window, position);
+    }
+
     public void ReOpen()
     {
         _window?.Orphan();

Content.Client/Storage/Systems/StorageSystem.cs

@@ -19,6 +19,8 @@ public sealed class StorageSystem : SharedStorageSystem
 
     private Dictionary<EntityUid, ItemStorageLocation> _oldStoredItems = new();
 
+    private List<(StorageBoundUserInterface Bui, bool Value)> _queuedBuis = new();
+
     public override void Initialize()
     {
         base.Initialize();
@@ -72,7 +74,7 @@ public sealed class StorageSystem : SharedStorageSystem
             if (NestedStorage && player != null && ContainerSystem.TryGetContainingContainer((uid, null, null), out var container) &&
                 UI.TryGetOpenUi<StorageBoundUserInterface>(container.Owner, StorageComponent.StorageUiKey.Key, out var containerBui))
             {
-                containerBui.Hide();
+                _queuedBuis.Add((containerBui, false));
             }
         }
     }
@@ -89,7 +91,7 @@ public sealed class StorageSystem : SharedStorageSystem
     {
         if (UI.TryGetOpenUi<StorageBoundUserInterface>(uid, StorageComponent.StorageUiKey.Key, out var storageBui))
         {
-            storageBui.Hide();
+            _queuedBuis.Add((storageBui, false));
         }
     }
 
@@ -97,7 +99,7 @@ public sealed class StorageSystem : SharedStorageSystem
     {
         if (UI.TryGetOpenUi<StorageBoundUserInterface>(uid, StorageComponent.StorageUiKey.Key, out var storageBui))
         {
-            storageBui.Show();
+            _queuedBuis.Add((storageBui, true));
         }
     }
 
@@ -152,4 +154,30 @@ public sealed class StorageSystem : SharedStorageSystem
             }
         }
     }
+
+    public override void Update(float frameTime)
+    {
+        base.Update(frameTime);
+
+        if (!_timing.IsFirstTimePredicted)
+        {
+            return;
+        }
+
+        // This update loop exists just to synchronize with UISystem and avoid 1-tick delays.
+        // If deferred opens / closes ever get removed you can dump this.
+        foreach (var (bui, open) in _queuedBuis)
+        {
+            if (open)
+            {
+                bui.Show();
+            }
+            else
+            {
+                bui.Hide();
+            }
+        }
+
+        _queuedBuis.Clear();
+    }
 }

Content.Client/UserInterface/Systems/Storage/Controls/StorageWindow.cs

@@ -9,6 +9,7 @@ using Content.Shared.IdentityManagement;
 using Content.Shared.Input;
 using Content.Shared.Item;
 using Content.Shared.Storage;
+using Robust.Client.GameObjects;
 using Robust.Client.Graphics;
 using Robust.Client.UserInterface;
 using Robust.Client.UserInterface.Controls;
@@ -190,6 +191,26 @@ public sealed class StorageWindow : BaseWindow
         BuildGridRepresentation();
     }
 
+    private void CloseParent()
+    {
+        if (StorageEntity == null)
+            return;
+
+        var containerSystem = _entity.System<SharedContainerSystem>();
+        var uiSystem = _entity.System<UserInterfaceSystem>();
+
+        if (containerSystem.TryGetContainingContainer(StorageEntity.Value, out var container) &&
+            _entity.TryGetComponent(container.Owner, out StorageComponent? storage) &&
+            storage.Container.Contains(StorageEntity.Value) &&
+            uiSystem
+                .TryGetOpenUi<StorageBoundUserInterface>(container.Owner,
+                    StorageComponent.StorageUiKey.Key,
+                    out var parentBui))
+        {
+            parentBui.CloseWindow(Position);
+        }
+    }
+
     private void BuildGridRepresentation()
     {
         if (!_entity.TryGetComponent<StorageComponent>(StorageEntity, out var comp) || comp.Grid.Count == 0)
@@ -212,7 +233,9 @@ public sealed class StorageWindow : BaseWindow
         };
         exitButton.OnPressed += _ =>
         {
+            // Close ourselves and all parent BUIs.
             Close();
+            CloseParent();
         };
         exitButton.OnKeyBindDown += args =>
         {
@@ -220,6 +243,7 @@ public sealed class StorageWindow : BaseWindow
             if (!args.Handled && args.Function == ContentKeyFunctions.ActivateItemInWorld)
             {
                 Close();
+                CloseParent();
                 args.Handle();
             }
         };
@@ -258,7 +282,8 @@ public sealed class StorageWindow : BaseWindow
                 var containerSystem = _entity.System<SharedContainerSystem>();
 
                 if (containerSystem.TryGetContainingContainer(StorageEntity.Value, out var container) &&
-                    _entity.TryGetComponent(container.Owner, out StorageComponent? storage))
+                    _entity.TryGetComponent(container.Owner, out StorageComponent? storage) &&
+                    storage.Container.Contains(StorageEntity.Value))
                 {
                     Close();
 
@@ -267,7 +292,7 @@ public sealed class StorageWindow : BaseWindow
                             StorageComponent.StorageUiKey.Key,
                             out var parentBui))
                     {
-                        parentBui.Show();
+                        parentBui.Show(Position);
                     }
                 }
             };
@@ -412,6 +437,8 @@ public sealed class StorageWindow : BaseWindow
         {
             if (storageComp.StoredItems.TryGetValue(ent, out var updated))
             {
+                data.Control.Marked = IsMarked(ent);
+
                 if (data.Loc.Equals(updated))
                 {
                     DebugTools.Assert(data.Control.Location == updated);
@@ -450,12 +477,7 @@ public sealed class StorageWindow : BaseWindow
                 var gridPiece = new ItemGridPiece((ent, itemEntComponent), loc, _entity)
                 {
                     MinSize = size,
-                    Marked = _contained.IndexOf(ent) switch
+                    Marked = IsMarked(ent),
-                    {
-                        0 => ItemGridPieceMarks.First,
-                        1 => ItemGridPieceMarks.Second,
-                        _ => null,
-                    }
                 };
                 gridPiece.OnPiecePressed += OnPiecePressed;
                 gridPiece.OnPieceUnpressed += OnPieceUnpressed;
@@ -467,6 +489,16 @@ public sealed class StorageWindow : BaseWindow
         }
     }
 
+    private ItemGridPieceMarks? IsMarked(EntityUid uid)
+    {
+        return _contained.IndexOf(uid) switch
+        {
+            0 => ItemGridPieceMarks.First,
+            1 => ItemGridPieceMarks.Second,
+            _ => null,
+        };
+    }
+
     protected override void FrameUpdate(FrameEventArgs args)
     {
         base.FrameUpdate(args);
@@ -486,8 +518,9 @@ public sealed class StorageWindow : BaseWindow
         {
             if (StorageEntity != null && _entity.System<StorageSystem>().NestedStorage)
             {
+                // If parent container nests us then show back button
                 if (containerSystem.TryGetContainingContainer(StorageEntity.Value, out var container) &&
-                    _entity.HasComponent<StorageComponent>(container.Owner))
+                    _entity.TryGetComponent(container.Owner, out StorageComponent? storageComp) && storageComp.Container.Contains(StorageEntity.Value))
                 {
                     _backButton.Visible = true;
                 }

Content.Client/UserInterface/Systems/Storage/StorageUIController.cs

@@ -11,6 +11,7 @@ using Content.Shared.CCVar;
 using Content.Shared.Input;
 using Content.Shared.Interaction;
 using Content.Shared.Storage;
+using Robust.Client.GameObjects;
 using Robust.Client.Input;
 using Robust.Client.Player;
 using Robust.Client.UserInterface;
@@ -37,6 +38,7 @@ public sealed class StorageUIController : UIController, IOnSystemChanged<Storage
     [Dependency] private readonly IInputManager _input = default!;
     [Dependency] private readonly IPlayerManager _player = default!;
     [UISystemDependency] private readonly StorageSystem _storage = default!;
+    [UISystemDependency] private readonly UserInterfaceSystem _ui = default!;
 
     private readonly DragDropHelper<ItemGridPiece> _menuDragHelper;
 
@@ -107,7 +109,7 @@ public sealed class StorageUIController : UIController, IOnSystemChanged<Storage
         StaticStorageUIEnabled = obj;
     }
 
-    public StorageWindow CreateStorageWindow(EntityUid uid)
+    public StorageWindow CreateStorageWindow(StorageBoundUserInterface sBui)
     {
         var window = new StorageWindow();
         window.MouseFilter = Control.MouseFilterMode.Pass;
@@ -127,9 +129,25 @@ public sealed class StorageUIController : UIController, IOnSystemChanged<Storage
         }
         else
         {
-            window.OpenCenteredLeft();
+            // Open at parent position if it's open.
+            if (_ui.TryGetOpenUi<StorageBoundUserInterface>(EntityManager.GetComponent<TransformComponent>(sBui.Owner).ParentUid,
+                    StorageComponent.StorageUiKey.Key, out var bui) && bui.Position != null)
+            {
+                window.Open(bui.Position.Value);
+            }
+            // Open at the saved position if it exists.
+            else if (_ui.TryGetPosition(sBui.Owner, StorageComponent.StorageUiKey.Key, out var pos))
+            {
+                window.Open(pos);
+            }
+            // Open at the default position.
+            else
+            {
+                window.OpenCenteredLeft();
+            }
         }
 
+        _ui.RegisterControl(sBui, window);
         return window;
     }
 

Content.Shared/Storage/EntitySystems/SharedStorageSystem.cs

@@ -318,11 +318,33 @@ public abstract class SharedStorageSystem : EntitySystem
         args.Verbs.Add(verb);
     }
 
+    public void OpenStorageUI(EntityUid uid, EntityUid actor, StorageComponent? storageComp = null, bool silent = true)
+    {
+        // Handle recursively opening nested storages.
+        if (ContainerSystem.TryGetContainingContainer(uid, out var container) &&
+            UI.IsUiOpen(container.Owner, StorageComponent.StorageUiKey.Key, actor))
+        {
+            _nestedCheck = true;
+            HideStorageWindow(container.Owner, actor);
+            OpenStorageUIInternal(uid, actor, storageComp, silent: true);
+            _nestedCheck = false;
+        }
+        else
+        {
+            // If you need something more sophisticated for multi-UI you'll need to code some smarter
+            // interactions.
+            if (_openStorageLimit == 1)
+                UI.CloseUserUis<StorageComponent.StorageUiKey>(actor);
+
+            OpenStorageUIInternal(uid, actor, storageComp, silent: silent);
+        }
+    }
+
     /// <summary>
     ///     Opens the storage UI for an entity
     /// </summary>
     /// <param name="entity">The entity to open the UI for</param>
-    public void OpenStorageUI(EntityUid uid, EntityUid entity, StorageComponent? storageComp = null, bool silent = true)
+    private void OpenStorageUIInternal(EntityUid uid, EntityUid entity, StorageComponent? storageComp = null, bool silent = true)
     {
         if (!Resolve(uid, ref storageComp, false))
             return;
@@ -407,24 +429,7 @@ public abstract class SharedStorageSystem : EntitySystem
         }
         else
         {
-            // Handle recursively opening nested storages.
+            OpenStorageUI(uid, args.User, storageComp);
-            if (ContainerSystem.TryGetContainingContainer((args.Target, null, null), out var container) &&
-                UI.IsUiOpen(container.Owner, StorageComponent.StorageUiKey.Key, args.User))
-            {
-                _nestedCheck = true;
-                HideStorageWindow(container.Owner, args.User);
-                OpenStorageUI(uid, args.User, storageComp, silent: true);
-                _nestedCheck = false;
-            }
-            else
-            {
-                // If you need something more sophisticated for multi-UI you'll need to code some smarter
-                // interactions.
-                if (_openStorageLimit == 1)
-                    UI.CloseUserUis<StorageComponent.StorageUiKey>(args.User);
-
-                OpenStorageUI(uid, args.User, storageComp, silent: false);
-            }
         }
 
         args.Handled = true;