From 5454d1caa1927428fe339e3d5b47ddfe45f9dfb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Sun, 6 Aug 2023 17:40:55 +0200
Subject: [PATCH] Remove global action container input queue workaround

As described in #24248, the workaround employed by
`GlobalActionContainer`, wherein it tried to handle actions with
priority before its children by being placed in front of the children
and not _actually containing_ said children, is blocking the resolution
of some rather major input handling issues that allow key releases to be
received by deparented drawables.

To resolve, migrate `GlobalActionContainer` to use `Prioritised`, which
can be done without regressing certain mouse button flows after
ppy/osu-framework#5966.
---
 .../Input/Bindings/GlobalActionContainer.cs   | 34 +++++--------------
 osu.Game/OsuGameBase.cs                       | 19 ++++++-----
 .../Visual/OsuManualInputManagerTestScene.cs  |  8 ++++-
 3 files changed, 25 insertions(+), 36 deletions(-)

diff --git a/osu.Game/Input/Bindings/GlobalActionContainer.cs b/osu.Game/Input/Bindings/GlobalActionContainer.cs
index 9a0a2d5c15..296232d9ea 100644
--- a/osu.Game/Input/Bindings/GlobalActionContainer.cs
+++ b/osu.Game/Input/Bindings/GlobalActionContainer.cs
@@ -3,33 +3,26 @@
 
 using System.Collections.Generic;
 using System.Linq;
-using osu.Framework.Graphics;
 using osu.Framework.Input;
 using osu.Framework.Input.Bindings;
+using osu.Framework.Input.Events;
 using osu.Framework.Localisation;
 using osu.Game.Localisation;
 
 namespace osu.Game.Input.Bindings
 {
-    public partial class GlobalActionContainer : DatabasedKeyBindingContainer<GlobalAction>, IHandleGlobalKeyboardInput
+    public partial class GlobalActionContainer : DatabasedKeyBindingContainer<GlobalAction>, IHandleGlobalKeyboardInput, IKeyBindingHandler<GlobalAction>
     {
-        private readonly Drawable? handler;
-
-        private InputManager? parentInputManager;
+        private readonly IKeyBindingHandler<GlobalAction>? handler;
 
         public GlobalActionContainer(OsuGameBase? game)
             : base(matchingMode: KeyCombinationMatchingMode.Modifiers)
         {
-            if (game is IKeyBindingHandler<GlobalAction>)
-                handler = game;
+            if (game is IKeyBindingHandler<GlobalAction> h)
+                handler = h;
         }
 
-        protected override void LoadComplete()
-        {
-            base.LoadComplete();
-
-            parentInputManager = GetContainingInputManager();
-        }
+        protected override bool Prioritised => true;
 
         // IMPORTANT: Take care when changing order of the items in the enumerable.
         // It is used to decide the order of precedence, with the earlier items having higher precedence.
@@ -161,20 +154,9 @@ namespace osu.Game.Input.Bindings
             new KeyBinding(InputKey.F3, GlobalAction.MusicPlay)
         };
 
-        protected override IEnumerable<Drawable> KeyBindingInputQueue
-        {
-            get
-            {
-                // To ensure the global actions are handled with priority, this GlobalActionContainer is actually placed after game content.
-                // It does not contain children as expected, so we need to forward the NonPositionalInputQueue from the parent input manager to correctly
-                // allow the whole game to handle these actions.
+        public bool OnPressed(KeyBindingPressEvent<GlobalAction> e) => handler?.OnPressed(e) == true;
 
-                // An eventual solution to this hack is to create localised action containers for individual components like SongSelect, but this will take some rearranging.
-                var inputQueue = parentInputManager?.NonPositionalInputQueue ?? base.KeyBindingInputQueue;
-
-                return handler != null ? inputQueue.Prepend(handler) : inputQueue;
-            }
-        }
+        public void OnReleased(KeyBindingReleaseEvent<GlobalAction> e) => handler?.OnReleased(e);
     }
 
     public enum GlobalAction
diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs
index 6737caa5f9..75b46a0a4d 100644
--- a/osu.Game/OsuGameBase.cs
+++ b/osu.Game/OsuGameBase.cs
@@ -392,17 +392,18 @@ namespace osu.Game
             {
                 SafeAreaOverrideEdges = SafeAreaOverrideEdges,
                 RelativeSizeAxes = Axes.Both,
-                Child = CreateScalingContainer().WithChildren(new Drawable[]
+                Child = CreateScalingContainer().WithChild(globalBindings = new GlobalActionContainer(this)
                 {
-                    (GlobalCursorDisplay = new GlobalCursorDisplay
+                    Children = new Drawable[]
                     {
-                        RelativeSizeAxes = Axes.Both
-                    }).WithChild(content = new OsuTooltipContainer(GlobalCursorDisplay.MenuCursor)
-                    {
-                        RelativeSizeAxes = Axes.Both
-                    }),
-                    // to avoid positional input being blocked by children, ensure the GlobalActionContainer is above everything.
-                    globalBindings = new GlobalActionContainer(this)
+                        (GlobalCursorDisplay = new GlobalCursorDisplay
+                        {
+                            RelativeSizeAxes = Axes.Both
+                        }).WithChild(content = new OsuTooltipContainer(GlobalCursorDisplay.MenuCursor)
+                        {
+                            RelativeSizeAxes = Axes.Both
+                        }),
+                    }
                 })
             });
 
diff --git a/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs b/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs
index 37260b3b13..ffe40243ab 100644
--- a/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs
+++ b/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs
@@ -57,7 +57,13 @@ namespace osu.Game.Tests.Visual
             }
 
             if (CreateNestedActionContainer)
-                mainContent.Add(new GlobalActionContainer(null));
+            {
+                var globalActionContainer = new GlobalActionContainer(null)
+                {
+                    Child = mainContent
+                };
+                mainContent = globalActionContainer;
+            }
 
             base.Content.AddRange(new Drawable[]
             {