Skip to content

Commit

Permalink
Remove global action container input queue workaround
Browse files Browse the repository at this point in the history
As described in ppy#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.
  • Loading branch information
bdach committed Aug 21, 2023
1 parent aa5680a commit 5454d1c
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 36 deletions.
34 changes: 8 additions & 26 deletions osu.Game/Input/Bindings/GlobalActionContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -161,20 +154,9 @@ protected override void LoadComplete()
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.

// 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;
public bool OnPressed(KeyBindingPressEvent<GlobalAction> e) => handler?.OnPressed(e) == true;

return handler != null ? inputQueue.Prepend(handler) : inputQueue;
}
}
public void OnReleased(KeyBindingReleaseEvent<GlobalAction> e) => handler?.OnReleased(e);
}

public enum GlobalAction
Expand Down
19 changes: 10 additions & 9 deletions osu.Game/OsuGameBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -392,17 +392,18 @@ private void load(ReadableKeyCombinationProvider keyCombinationProvider, Framewo
{
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
}),
}
})
});

Expand Down
8 changes: 7 additions & 1 deletion osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,13 @@ protected OsuManualInputManagerTestScene()
}

if (CreateNestedActionContainer)
mainContent.Add(new GlobalActionContainer(null));
{
var globalActionContainer = new GlobalActionContainer(null)
{
Child = mainContent
};
mainContent = globalActionContainer;
}

base.Content.AddRange(new Drawable[]
{
Expand Down

0 comments on commit 5454d1c

Please sign in to comment.