Partially fix UI assert (#5100)

* Partially fix UI assert

* Avoid breaking change in BoundKeyEventArgs

This is a public constructor, as much as it maybe shouldn't be. Adding this parameter is a breaking change.

* Replace .Disposed checks with ! .VisibleInTree

Control disposal should not be used anymore.

* Release notes

---------

Co-authored-by: Pieter-Jan Briers <pieterjan.briers+git@gmail.com>

RELEASE-NOTES.md

@@ -39,11 +39,11 @@ END TEMPLATE-->
 
 ### New features
 
-*None yet*
+* Added `BoundKeyEventArgs.IsRepeat`.
 
 ### Bugfixes
 
-*None yet*
+* Fix assert trip when holding repeatable keybinds.
 
 ### Other
 

Robust.Client/Input/InputManager.cs

@@ -346,7 +346,7 @@ namespace Robust.Client.Input
                 {
                     if (binding.CanRepeat)
                     {
-                        return SetBindState(binding, BoundKeyState.Down, uiOnly);
+                        return SetBindState(binding, BoundKeyState.Down, uiOnly, isRepeat);
                     }
 
                     return true;
@@ -375,7 +375,7 @@ namespace Robust.Client.Input
             SetBindState(binding, BoundKeyState.Up);
         }
 
-        private bool SetBindState(KeyBinding binding, BoundKeyState state, bool uiOnly = false)
+        private bool SetBindState(KeyBinding binding, BoundKeyState state, bool uiOnly = false, bool isRepeat = false)
         {
             if (binding.BindingType == KeyBindingType.Command && state == BoundKeyState.Down)
             {
@@ -387,6 +387,7 @@ namespace Robust.Client.Input
             // I honestly have no idea what the best solution here is.
             // note from the future: context switches won't cause re-entrancy anymore because InputContextContainer defers context switches
             DebugTools.Assert(!_currentlyFindingViewport, "Re-entrant key events??");
+            DebugTools.Assert(!isRepeat || binding.CanRepeat);
 
             try
             {
@@ -399,7 +400,7 @@ namespace Robust.Client.Input
                 binding.State = state;
 
                 var eventArgs = new BoundKeyEventArgs(binding.Function, binding.State,
-                    MouseScreenPosition, binding.CanFocus);
+                    MouseScreenPosition, binding.CanFocus, isRepeat);
 
                 // UI returns true here into blockPass if it wants to prevent us from giving input events
                 // to the viewport, but doesn't want it hard-handled so we keep processing possible key actions.

Robust.Client/UserInterface/UserInterfaceManager.Input.cs

@@ -114,8 +114,10 @@ internal partial class UserInterfaceManager
             args.Handle();
         }
 
-        // Attempt to ensure that keybind-up events only get raised after a single keybind-down.
-        DebugTools.Assert(!_focusedControls.ContainsKey(args.Function));
+        // Attempt to ensure that keybind-up events get raised after a keybind-down.
+        DebugTools.Assert(!_focusedControls.TryGetValue(args.Function, out var existing)
+                          || !existing.VisibleInTree
+                          || args.IsRepeat && existing == control);
         _focusedControls[args.Function] = control;
 
         OnKeyBindDown?.Invoke(control);
@@ -124,7 +126,7 @@ internal partial class UserInterfaceManager
     public void KeyBindUp(BoundKeyEventArgs args)
     {
         // Only raise keybind-up for the control on which we previously raised keybind-down
-        if (!_focusedControls.Remove(args.Function, out var control) || control.Disposed)
+        if (!_focusedControls.Remove(args.Function, out var control) || !control.VisibleInTree)
             return;
 
         var guiArgs = new GUIBoundKeyEventArgs(args.Function, args.State, args.PointerLocation, args.CanFocus,

Robust.Shared/Input/BoundKeyEventArgs.cs

@@ -33,6 +33,11 @@ namespace Robust.Shared.Input
 
         public bool Handled { get; private set; }
 
+        /// <summary>
+        /// Is this a repeated keypress (i.e., are they holding down the key)?
+        /// </summary>
+        public readonly bool IsRepeat;
+
         /// <summary>
         ///     Constructs a new instance of <see cref="BoundKeyEventArgs"/>.
         /// </summary>
@@ -47,6 +52,23 @@ namespace Robust.Shared.Input
             CanFocus = canFocus;
         }
 
+        /// <summary>
+        ///     Constructs a new instance of <see cref="BoundKeyEventArgs"/>.
+        /// </summary>
+        /// <param name="function">Bound key that that is changing.</param>
+        /// <param name="state">New state of the function.</param>
+        /// <param name="pointerLocation">Current Pointer location in screen coordinates.</param>
+        /// <param name="isRepeat"></param>
+        public BoundKeyEventArgs(
+            BoundKeyFunction function,
+            BoundKeyState state,
+            ScreenCoordinates pointerLocation,
+            bool canFocus,
+            bool isRepeat = false) : this(function, state, pointerLocation, canFocus)
+        {
+            IsRepeat = isRepeat;
+        }
+
         /// <summary>
         ///     Mark this event as handled.
         /// </summary>