Remove global action container input queue workaround #24610
Conversation
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.
| 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 | ||
| }), | ||
| } |
There was a problem hiding this comment.
Out of interest, aside from the usage with CreateScalingContainer(), is there a reason this uses the WithChild() style of hierarchy creation?
There was a problem hiding this comment.
Nope, not in this case.
There was a problem hiding this comment.
It's just stylistic. Was used to avoid too much nesting where it made things hard to read (due to nested object initialisers and such).
There was a problem hiding this comment.
right, in this case it's due to the abstracted out constructor meaning it's the only way we could structure this hierarchically.
There was a problem hiding this comment.
The one usage by CreateScalingContainer() is required yeah. The other could use plain .Child but it's like whatever.
There was a problem hiding this comment.
Turns out it's required due to the inner reference to GlobalCursorDisplay.MenuCursor and execution order woes 😅
peppy
force-pushed
the
remove-global-action-container-hack
branch
from
200247f to
5454d1c
Compare
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
GlobalActionContainerto usePrioritised, which can be done without regressing certain mouse button flows after ppy/osu-framework#5966.