Slightly improve action ui performance (#20799)

Content.Client/Actions/ActionsSystem.cs

@@ -29,7 +29,6 @@ namespace Content.Client.Actions
 
         public event Action<EntityUid>? OnActionAdded;
         public event Action<EntityUid>? OnActionRemoved;
-        public event OnActionReplaced? ActionReplaced;
         public event Action? ActionsUpdated;
         public event Action<ActionsComponent>? LinkActions;
         public event Action? UnlinkActions;

Content.Client/UserInterface/Systems/Actions/ActionUIController.cs

@@ -42,6 +42,7 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
     [Dependency] private readonly IOverlayManager _overlays = default!;
     [Dependency] private readonly IGameTiming _timing = default!;
     [Dependency] private readonly IPlayerManager _playerManager = default!;
+    [Dependency] private readonly IEntityManager _entMan = default!;
 
     [UISystemDependency] private readonly ActionsSystem? _actionsSystem = default;
     [UISystemDependency] private readonly InteractionOutlineSystem? _interactionOutline = default;
@@ -113,12 +114,11 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
         {
             _actionsSystem.OnActionAdded += OnActionAdded;
             _actionsSystem.OnActionRemoved += OnActionRemoved;
-            _actionsSystem.ActionReplaced += OnActionReplaced;
             _actionsSystem.ActionsUpdated += OnActionsUpdated;
         }
 
         UpdateFilterLabel();
-        SearchAndDisplay();
+        QueueWindowUpdate();
 
         _dragShadow.Orphan();
         UIManager.PopupRoot.AddChild(_dragShadow);
@@ -307,6 +307,8 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
     {
         if (ActionButton != null)
             ActionButton.Pressed = true;
+
+        SearchAndDisplay();
     }
 
     private void OnWindowClosed()
@@ -321,7 +323,6 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
         {
             _actionsSystem.OnActionAdded -= OnActionAdded;
             _actionsSystem.OnActionRemoved -= OnActionRemoved;
-            _actionsSystem.ActionReplaced -= OnActionReplaced;
             _actionsSystem.ActionsUpdated -= OnActionsUpdated;
         }
 
@@ -345,6 +346,9 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
 
     private void ChangePage(int index)
     {
+        if (_actionsSystem == null)
+            return;
+
         var lastPage = _pages.Count - 1;
         if (index < 0)
         {
@@ -357,7 +361,7 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
 
         _currentPageIndex = index;
         var page = _pages[_currentPageIndex];
-        _container?.SetActionData(page);
+        _container?.SetActionData(_actionsSystem, page);
 
         ActionsBar!.PageButtons.Label.Text = $"{_currentPageIndex + 1}";
     }
@@ -424,7 +428,6 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
         }
 
         AppendAction(actionId);
-        SearchAndDisplay();
     }
 
     private void OnActionRemoved(EntityUid actionId)
@@ -454,24 +457,11 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
                 }
             }
         }
-
-        SearchAndDisplay();
-    }
-
-    private void OnActionReplaced(EntityUid actionId)
-    {
-        if (_container == null)
-            return;
-
-        foreach (var button in _container.GetButtons())
-        {
-            if (button.ActionId == actionId)
-                button.UpdateData(actionId);
-        }
     }
 
     private void OnActionsUpdated()
     {
+        QueueWindowUpdate();
         if (_container == null)
             return;
 
@@ -538,27 +528,56 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
 
     private void PopulateActions(IEnumerable<(EntityUid Id, BaseActionComponent Comp)> actions)
     {
-        if (_window == null)
+        if (_window is not { Disposed: false, IsOpen: true })
             return;
 
-        ClearList();
+        if (_actionsSystem == null)
+            return;
 
+        _window.UpdateNeeded = false;
+
+        List<ActionButton> existing = new(_window.ResultsGrid.ChildCount);
+        foreach (var child in _window.ResultsGrid.Children)
+        {
+            if (child is ActionButton button)
+                existing.Add(button);
+        }
+
+        int i = 0;
         foreach (var action in actions)
         {
-            var button = new ActionButton {Locked = true};
+            if (i < existing.Count)
+            {
+                existing[i++].UpdateData(action.Id, _actionsSystem);
+                continue;
+            }
 
-            button.UpdateData(action.Id);
+            var button = new ActionButton(_entMan, _spriteSystem, this) {Locked = true};
             button.ActionPressed += OnWindowActionPressed;
             button.ActionUnpressed += OnWindowActionUnPressed;
             button.ActionFocusExited += OnWindowActionFocusExisted;
-
+            button.UpdateData(action.Id, _actionsSystem);
             _window.ResultsGrid.AddChild(button);
         }
+
+        for (; i < existing.Count; i++)
+        {
+            existing[i].Dispose();
+        }
+    }
+
+    public void QueueWindowUpdate()
+    {
+        if (_window != null)
+            _window.UpdateNeeded = true;
     }
 
     private void SearchAndDisplay()
     {
-        if (_window is not { Disposed: false } || _actionsSystem == null)
+        if (_window is not { Disposed: false, IsOpen: true })
+            return;
+
+        if (_actionsSystem == null)
             return;
 
         if (_playerManager.LocalPlayer?.ControlledEntity is not { } player)
@@ -598,6 +617,9 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
 
     private void SetAction(ActionButton button, EntityUid? actionId)
     {
+        if (_actionsSystem == null)
+            return;
+
         int position;
 
         if (actionId == null)
@@ -611,7 +633,7 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
             return;
         }
 
-        if (button.TryReplaceWith(actionId.Value) &&
+        if (button.TryReplaceWith(actionId.Value, _actionsSystem) &&
             _container != null &&
             _container.TryGetButtonIndex(button, out position))
         {
@@ -648,18 +670,18 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
         _window.SearchBar.Clear();
         _window.FilterButton.DeselectAll();
         UpdateFilterLabel();
-        SearchAndDisplay();
+        QueueWindowUpdate();
     }
 
     private void OnSearchChanged(LineEditEventArgs args)
     {
-        SearchAndDisplay();
+        QueueWindowUpdate();
     }
 
     private void OnFilterSelected(ItemPressedEventArgs args)
     {
         UpdateFilterLabel();
-        SearchAndDisplay();
+        QueueWindowUpdate();
     }
 
     private void OnWindowActionPressed(GUIBoundKeyEventArgs args, ActionButton action)
@@ -849,12 +871,15 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
 
     private void AssignSlots(List<SlotAssignment> assignments)
     {
+        if (_actionsSystem == null)
+            return;
+
         foreach (ref var assignment in CollectionsMarshal.AsSpan(assignments))
         {
             _pages[assignment.Hotbar][assignment.Slot] = assignment.ActionId;
         }
 
-        _container?.SetActionData(_pages[_currentPageIndex]);
+        _container?.SetActionData(_actionsSystem, _pages[_currentPageIndex]);
     }
 
     public void RemoveActionContainer()
@@ -881,19 +906,24 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
     public override void FrameUpdate(FrameEventArgs args)
     {
         _menuDragHelper.Update(args.DeltaSeconds);
+        if (_window is {UpdateNeeded: true})
+            SearchAndDisplay();
     }
 
     private void OnComponentLinked(ActionsComponent component)
     {
+        if (_actionsSystem == null)
+            return;
+
         LoadDefaultActions(component);
-        _container?.SetActionData(_pages[DefaultPageIndex]);
+        _container?.SetActionData(_actionsSystem, _pages[DefaultPageIndex]);
-        SearchAndDisplay();
+        QueueWindowUpdate();
     }
 
     private void OnComponentUnlinked()
     {
         _container?.ClearActionData();
-        SearchAndDisplay();
+        QueueWindowUpdate();
         StopTargeting();
     }
 

Content.Client/UserInterface/Systems/Actions/Controls/ActionButton.cs

@@ -8,8 +8,6 @@ using Robust.Client.GameObjects;
 using Robust.Client.Graphics;
 using Robust.Client.UserInterface;
 using Robust.Client.UserInterface.Controls;
-using Robust.Client.Utility;
-using Robust.Shared.Graphics;
 using Robust.Shared.Input;
 using Robust.Shared.Timing;
 using Robust.Shared.Utility;
@@ -21,11 +19,9 @@ namespace Content.Client.UserInterface.Systems.Actions.Controls;
 
 public sealed class ActionButton : Control, IEntityControl
 {
-    private IEntityManager? _entities;
+    private IEntityManager _entities;
-
+    private SpriteSystem? _spriteSys;
-    private ActionUIController Controller => UserInterfaceManager.GetUIController<ActionUIController>();
+    private ActionUIController? _controller;
-    private IEntityManager Entities => _entities ??= IoCManager.Resolve<IEntityManager>();
-    private ActionsSystem Actions => Entities.System<ActionsSystem>();
     private bool _beingHovered;
     private bool _depressed;
     private bool _toggled;
@@ -54,14 +50,21 @@ public sealed class ActionButton : Control, IEntityControl
     private readonly SpriteView _bigItemSpriteView;
 
     public EntityUid? ActionId { get; private set; }
+    private BaseActionComponent? _action;
     public bool Locked { get; set; }
 
     public event Action<GUIBoundKeyEventArgs, ActionButton>? ActionPressed;
     public event Action<GUIBoundKeyEventArgs, ActionButton>? ActionUnpressed;
     public event Action<ActionButton>? ActionFocusExited;
 
-    public ActionButton()
+    public ActionButton(IEntityManager entities, SpriteSystem? spriteSys = null, ActionUIController? controller = null)
     {
+        // TODO why is this constructor so slooooow. The rest of the code is fine
+
+        _entities = entities;
+        _spriteSys = spriteSys;
+        _controller = controller;
+
         MouseFilter = MouseFilterMode.Pass;
         Button = new TextureRect
         {
@@ -180,7 +183,7 @@ public sealed class ActionButton : Control, IEntityControl
 
     private Control? SupplyTooltip(Control sender)
     {
-        if (!Entities.TryGetComponent(ActionId, out MetaDataComponent? metadata))
+        if (!_entities.TryGetComponent(ActionId, out MetaDataComponent? metadata))
             return null;
 
         var name = FormattedMessage.FromMarkupPermissive(Loc.GetString(metadata.EntityName));
@@ -196,9 +199,8 @@ public sealed class ActionButton : Control, IEntityControl
 
     private void UpdateItemIcon()
     {
-        if (!Actions.TryGetActionData(ActionId, out var action) ||
+        if (_action is not {EntityIcon: { } entity} ||
-            action is not {EntityIcon: { } entity} ||
+            !_entities.HasComponent<SpriteComponent>(entity))
-            !Entities.HasComponent<SpriteComponent>(entity))
         {
             _bigItemSpriteView.Visible = false;
             _bigItemSpriteView.SetEntity(null);
@@ -207,7 +209,7 @@ public sealed class ActionButton : Control, IEntityControl
         }
         else
         {
-            switch (action.ItemIconStyle)
+            switch (_action.ItemIconStyle)
             {
                 case ItemActionIconStyle.BigItem:
                     _bigItemSpriteView.Visible = true;
@@ -233,17 +235,17 @@ public sealed class ActionButton : Control, IEntityControl
 
     private void SetActionIcon(Texture? texture)
     {
-        if (!Actions.TryGetActionData(ActionId, out var action) || texture == null)
+        if (_action == null || texture == null)
         {
             _bigActionIcon.Texture = null;
             _bigActionIcon.Visible = false;
             _smallActionIcon.Texture = null;
             _smallActionIcon.Visible = false;
         }
-        else if (action.EntityIcon != null && action.ItemIconStyle == ItemActionIconStyle.BigItem)
+        else if (_action.EntityIcon != null && _action.ItemIconStyle == ItemActionIconStyle.BigItem)
         {
             _smallActionIcon.Texture = texture;
-            _smallActionIcon.Modulate = action.IconColor;
+            _smallActionIcon.Modulate = _action.IconColor;
             _smallActionIcon.Visible = true;
             _bigActionIcon.Texture = null;
             _bigActionIcon.Visible = false;
@@ -251,7 +253,7 @@ public sealed class ActionButton : Control, IEntityControl
         else
         {
             _bigActionIcon.Texture = texture;
-            _bigActionIcon.Modulate = action.IconColor;
+            _bigActionIcon.Modulate = _action.IconColor;
             _bigActionIcon.Visible = true;
             _smallActionIcon.Texture = null;
             _smallActionIcon.Visible = false;
@@ -262,39 +264,43 @@ public sealed class ActionButton : Control, IEntityControl
     {
         UpdateItemIcon();
 
-        if (!Actions.TryGetActionData(ActionId, out var action))
+        if (_action == null)
         {
             SetActionIcon(null);
             return;
         }
 
-        if ((Controller.SelectingTargetFor == ActionId || action.Toggled) && action.IconOn != null)
+        _controller ??= UserInterfaceManager.GetUIController<ActionUIController>();
-            SetActionIcon(action.IconOn.Frame0());
+        _spriteSys ??= _entities.System<SpriteSystem>();
+        if ((_controller.SelectingTargetFor == ActionId || _action.Toggled) && _action.IconOn != null)
+            SetActionIcon(_spriteSys.Frame0(_action.IconOn));
         else
-            SetActionIcon(action.Icon?.Frame0());
+            SetActionIcon(_action.Icon != null ? _spriteSys.Frame0(_action.Icon) : null);
     }
 
-    public bool TryReplaceWith(EntityUid actionId)
+    public bool TryReplaceWith(EntityUid actionId, ActionsSystem system)
     {
         if (Locked)
         {
             return false;
         }
 
-        UpdateData(actionId);
+        UpdateData(actionId, system);
         return true;
     }
 
-    public void UpdateData(EntityUid actionId)
+    public void UpdateData(EntityUid? actionId, ActionsSystem system)
     {
         ActionId = actionId;
-        Label.Visible = true;
+        system.TryGetActionData(actionId, out _action);
+        Label.Visible = actionId != null;
         UpdateIcons();
     }
 
     public void ClearData()
     {
         ActionId = null;
+        _action = null;
         Cooldown.Visible = false;
         Cooldown.Progress = 1;
         Label.Visible = false;
@@ -305,19 +311,17 @@ public sealed class ActionButton : Control, IEntityControl
     {
         base.FrameUpdate(args);
 
-        if (!Actions.TryGetActionData(ActionId, out var action))
+        if (_action == null)
-        {
             return;
+
+        if (_action.Cooldown != null)
+        {
+            Cooldown.FromTime(_action.Cooldown.Value.Start, _action.Cooldown.Value.End);
         }
 
-        if (action.Cooldown != null)
+        if (ActionId != null && _toggled != _action.Toggled)
         {
-            Cooldown.FromTime(action.Cooldown.Value.Start, action.Cooldown.Value.End);
+            _toggled = _action.Toggled;
-        }
-
-        if (ActionId != null && _toggled != action.Toggled)
-        {
-            _toggled = action.Toggled;
         }
     }
 
@@ -344,7 +348,7 @@ public sealed class ActionButton : Control, IEntityControl
     public void Depress(GUIBoundKeyEventArgs args, bool depress)
     {
         // action can still be toggled if it's allowed to stay selected
-        if (!Actions.TryGetActionData(ActionId, out var action) || action is not {Enabled: true})
+        if (_action is not {Enabled: true})
             return;
 
         if (_depressed && !depress)
@@ -362,14 +366,15 @@ public sealed class ActionButton : Control, IEntityControl
         HighlightRect.Visible = _beingHovered;
 
         // always show the normal empty button style if no action in this slot
-        if (!Actions.TryGetActionData(ActionId, out var action))
+        if (_action == null)
         {
             SetOnlyStylePseudoClass(ContainerButton.StylePseudoClassNormal);
             return;
         }
 
         // show a hover only if the action is usable or another action is being dragged on top of this
-        if (_beingHovered && (Controller.IsDragging || action.Enabled))
+        _controller ??= UserInterfaceManager.GetUIController<ActionUIController>();
+        if (_beingHovered && (_controller.IsDragging || _action.Enabled))
         {
             SetOnlyStylePseudoClass(ContainerButton.StylePseudoClassHover);
         }
@@ -384,16 +389,16 @@ public sealed class ActionButton : Control, IEntityControl
         }
 
         // if it's toggled on, always show the toggled on style (currently same as depressed style)
-        if (action.Toggled || Controller.SelectingTargetFor == ActionId)
+        if (_action.Toggled || _controller.SelectingTargetFor == ActionId)
         {
             // when there's a toggle sprite, we're showing that sprite instead of highlighting this slot
-            SetOnlyStylePseudoClass(action.IconOn != null
+            SetOnlyStylePseudoClass(_action.IconOn != null
                 ? ContainerButton.StylePseudoClassNormal
                 : ContainerButton.StylePseudoClassPressed);
             return;
         }
 
-        if (!action.Enabled)
+        if (!_action.Enabled)
         {
             SetOnlyStylePseudoClass(ContainerButton.StylePseudoClassDisabled);
             return;

Content.Client/UserInterface/Systems/Actions/Controls/ActionButtonContainer.cs

@@ -1,3 +1,4 @@
+using Content.Client.Actions;
 using Robust.Client.UserInterface;
 using Robust.Client.UserInterface.Controls;
 
@@ -21,7 +22,7 @@ public class ActionButtonContainer : GridContainer
         get => (ActionButton) GetChild(index);
     }
 
-    public void SetActionData(params EntityUid?[] actionTypes)
+    public void SetActionData(ActionsSystem system, params EntityUid?[] actionTypes)
     {
         ClearActionData();
 
@@ -31,7 +32,7 @@ public class ActionButtonContainer : GridContainer
             if (action == null)
                 continue;
 
-            ((ActionButton) GetChild(i)).UpdateData(action.Value);
+            ((ActionButton) GetChild(i)).UpdateData(action.Value, system);
         }
     }
 

Content.Client/UserInterface/Systems/Actions/Widgets/ActionsBar.xaml.cs

@@ -3,16 +3,18 @@ using Content.Shared.Input;
 using Robust.Client.AutoGenerated;
 using Robust.Client.UserInterface.Controls;
 using Robust.Client.UserInterface.XAML;
-using Robust.Shared.Input;
 
 namespace Content.Client.UserInterface.Systems.Actions.Widgets;
 
 [GenerateTypedNameReferences]
 public sealed partial class ActionsBar : UIWidget
 {
+    [Dependency] private readonly IEntityManager _entity = default!;
+
     public ActionsBar()
     {
         RobustXamlLoader.Load(this);
+        IoCManager.InjectDependencies(this);
 
         var keys = ContentKeyFunctions.GetHotbarBoundKeys();
         for (var index = 1; index < keys.Length; index++)
@@ -24,7 +26,7 @@ public sealed partial class ActionsBar : UIWidget
         ActionButton MakeButton(int index)
         {
             var boundKey = keys[index];
-            var button = new ActionButton();
+            var button = new ActionButton(_entity);
             button.KeyBind = boundKey;
             button.Label.Text = index.ToString();
             return button;

Content.Client/UserInterface/Systems/Actions/Windows/ActionsWindow.xaml.cs

@@ -10,6 +10,11 @@ public sealed partial class ActionsWindow : DefaultWindow
 {
     public MultiselectOptionButton<Filters> FilterButton { get; private set; }
 
+    /// <summary>
+    /// Whether the displayed actions or search filter needs updating.
+    /// </summary>
+    public bool UpdateNeeded;
+
     public ActionsWindow()
     {
         RobustXamlLoader.Load(this);