From 6471f55a49c2548e437a1a101f92c3416c676893 Mon Sep 17 00:00:00 2001
From: Leon Friedrich <60421075+ElectroJr@users.noreply.github.com>
Date: Mon, 29 Apr 2024 20:36:18 +1200
Subject: [PATCH] Action UI fixes (#27468)

---
 .../Systems/Actions/ActionUIController.cs     | 108 +++++++++---------
 .../Systems/Actions/Controls/ActionButton.cs  |  30 +++--
 2 files changed, 70 insertions(+), 68 deletions(-)

diff --git a/Content.Client/UserInterface/Systems/Actions/ActionUIController.cs b/Content.Client/UserInterface/Systems/Actions/ActionUIController.cs
index c79f0f80f9..5d9706452c 100644
--- a/Content.Client/UserInterface/Systems/Actions/ActionUIController.cs
+++ b/Content.Client/UserInterface/Systems/Actions/ActionUIController.cs
@@ -15,6 +15,7 @@ using Content.Shared.Actions;
 using Content.Shared.Input;
 using Robust.Client.GameObjects;
 using Robust.Client.Graphics;
+using Robust.Client.Input;
 using Robust.Client.Player;
 using Robust.Client.UserInterface;
 using Robust.Client.UserInterface.Controllers;
@@ -42,6 +43,7 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
     [Dependency] private readonly IGameTiming _timing = default!;
     [Dependency] private readonly IPlayerManager _playerManager = default!;
     [Dependency] private readonly IEntityManager _entMan = default!;
+    [Dependency] private readonly IInputManager _input = default!;
 
     [UISystemDependency] private readonly ActionsSystem? _actionsSystem = default;
     [UISystemDependency] private readonly InteractionOutlineSystem? _interactionOutline = default;
@@ -356,6 +358,10 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
     {
         QueueWindowUpdate();
 
+        // TODO ACTIONS allow buttons to persist across state applications
+        // Then we don't have to interrupt drags any time the buttons get rebuilt.
+        _menuDragHelper.EndDrag();
+
         if (_actionsSystem != null)
             _container?.SetActionData(_actionsSystem, _actions.ToArray());
     }
@@ -516,7 +522,8 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
             button.ClearData();
             if (_container?.TryGetButtonIndex(button, out position) ?? false)
             {
-                _actions.RemoveAt(position);
+                if (_actions.Count > position && position >= 0)
+                    _actions.RemoveAt(position);
             }
         }
         else if (button.TryReplaceWith(actionId.Value, _actionsSystem) &&
@@ -539,23 +546,22 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
 
     private void DragAction()
     {
+        if (_menuDragHelper.Dragged is not {ActionId: {} action} dragged)
+        {
+            _menuDragHelper.EndDrag();
+            return;
+        }
+
         EntityUid? swapAction = null;
-        if (UIManager.CurrentlyHovered is ActionButton button)
+        var currentlyHovered = UIManager.MouseGetControl(_input.MouseScreenPosition);
+        if (currentlyHovered is ActionButton button)
         {
-            if (!_menuDragHelper.IsDragging || _menuDragHelper.Dragged?.ActionId is not { } type)
-            {
-                _menuDragHelper.EndDrag();
-                return;
-            }
-
             swapAction = button.ActionId;
-            SetAction(button, type, false);
+            SetAction(button, action, false);
         }
 
-        if (_menuDragHelper.Dragged is {Parent: ActionButtonContainer} old)
-        {
-            SetAction(old, swapAction, false);
-        }
+        if (dragged.Parent is ActionButtonContainer)
+            SetAction(dragged, swapAction, false);
 
         if (_actionsSystem != null)
             _container?.SetActionData(_actionsSystem, _actions.ToArray());
@@ -610,27 +616,27 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
 
     private void OnActionPressed(GUIBoundKeyEventArgs args, ActionButton button)
     {
-        if (args.Function == EngineKeyFunctions.UIClick)
-        {
-            if (button.ActionId == null)
-            {
-                var ev = new FillActionSlotEvent();
-                EntityManager.EventBus.RaiseEvent(EventSource.Local, ev);
-                if (ev.Action != null)
-                    SetAction(button, ev.Action);
-            }
-            else
-            {
-                _menuDragHelper.MouseDown(button);
-            }
-
-            args.Handle();
-        }
-        else if (args.Function == EngineKeyFunctions.UIRightClick)
+        if (args.Function == EngineKeyFunctions.UIRightClick)
         {
             SetAction(button, null);
             args.Handle();
+            return;
         }
+
+        if (args.Function != EngineKeyFunctions.UIClick)
+            return;
+
+        args.Handle();
+        if (button.ActionId != null)
+        {
+            _menuDragHelper.MouseDown(button);
+            return;
+        }
+
+        var ev = new FillActionSlotEvent();
+        EntityManager.EventBus.RaiseEvent(EventSource.Local, ev);
+        if (ev.Action != null)
+            SetAction(button, ev.Action);
     }
 
     private void OnActionUnpressed(GUIBoundKeyEventArgs args, ActionButton button)
@@ -638,33 +644,31 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
         if (args.Function != EngineKeyFunctions.UIClick || _actionsSystem == null)
             return;
 
-        //todo: make dragging onto the same spot NOT trigger again
-        if (UIManager.CurrentlyHovered == button)
-        {
-            _menuDragHelper.EndDrag();
+        args.Handle();
 
-            if (_actionsSystem.TryGetActionData(button.ActionId, out var baseAction))
-            {
-                if (baseAction is BaseTargetActionComponent action)
-                {
-                    // for target actions, we go into "select target" mode, we don't
-                    // message the server until we actually pick our target.
-
-                    // if we're clicking the same thing we're already targeting for, then we simply cancel
-                    // targeting
-                    ToggleTargeting(button.ActionId.Value, action);
-                    return;
-                }
-
-                _actionsSystem?.TriggerAction(button.ActionId.Value, baseAction);
-            }
-        }
-        else
+        if (_menuDragHelper.IsDragging)
         {
             DragAction();
+            return;
         }
 
-        args.Handle();
+        _menuDragHelper.EndDrag();
+
+        if (!_actionsSystem.TryGetActionData(button.ActionId, out var baseAction))
+            return;
+
+        if (baseAction is not BaseTargetActionComponent action)
+        {
+            _actionsSystem?.TriggerAction(button.ActionId.Value, baseAction);
+            return;
+        }
+
+        // for target actions, we go into "select target" mode, we don't
+        // message the server until we actually pick our target.
+
+        // if we're clicking the same thing we're already targeting for, then we simply cancel
+        // targeting
+        ToggleTargeting(button.ActionId.Value, action);
     }
 
     private bool OnMenuBeginDrag()
diff --git a/Content.Client/UserInterface/Systems/Actions/Controls/ActionButton.cs b/Content.Client/UserInterface/Systems/Actions/Controls/ActionButton.cs
index 2372d98f8d..2cdd4cb79b 100644
--- a/Content.Client/UserInterface/Systems/Actions/Controls/ActionButton.cs
+++ b/Content.Client/UserInterface/Systems/Actions/Controls/ActionButton.cs
@@ -152,16 +152,8 @@ public sealed class ActionButton : Control, IEntityControl
 
         OnThemeUpdated();
 
-        OnKeyBindDown += args =>
-        {
-            Depress(args, true);
-            OnPressed(args);
-        };
-        OnKeyBindUp += args =>
-        {
-            Depress(args, false);
-            OnUnpressed(args);
-        };
+        OnKeyBindDown += OnPressed;
+        OnKeyBindUp += OnUnpressed;
 
         TooltipSupplier = SupplyTooltip;
     }
@@ -175,11 +167,23 @@ public sealed class ActionButton : Control, IEntityControl
 
     private void OnPressed(GUIBoundKeyEventArgs args)
     {
+        if (args.Function != EngineKeyFunctions.UIClick && args.Function != EngineKeyFunctions.UIRightClick)
+            return;
+
+        if (args.Function == EngineKeyFunctions.UIRightClick)
+            Depress(args, true);
+
         ActionPressed?.Invoke(args, this);
     }
 
     private void OnUnpressed(GUIBoundKeyEventArgs args)
     {
+        if (args.Function != EngineKeyFunctions.UIClick && args.Function != EngineKeyFunctions.UIRightClick)
+            return;
+
+        if (args.Function == EngineKeyFunctions.UIRightClick)
+            Depress(args, false);
+
         ActionUnpressed?.Invoke(args, this);
     }
 
@@ -378,12 +382,6 @@ public sealed class ActionButton : Control, IEntityControl
         if (_action is not {Enabled: true})
             return;
 
-        if (_depressed && !depress)
-        {
-            // fire the action
-            OnUnpressed(args);
-        }
-
         _depressed = depress;
         DrawModeChanged();
     }