ButtonDownInputQueue includes sibling drawables to the one handling it

[frenzibyte]

As the title says, the ButtonDownInputQueue in ButtonEventManager includes sibling drawables to the one handling it, which is probably incorrect as the down queue should only include the drawable handling the event and its children that are built to the queue.

I presume the logic behind this:

osu-framework/osu.Framework/Input/ButtonEventManager.cs

Lines 75 to 76 in 6697d05

	var count = inputQueue.IndexOf(handledBy) + 1;
	inputQueue.RemoveRange(count, inputQueue.Count - count);

is meant to allow children to receive click events when their parent has handled a mouse button down event, so removing this and letting only the handled drawable to be in the queue will not work well. (osu!lazer becomes unplayable from testing)

Here's a simple reproducing (demonstrating) example:

Children = new[]
{
    new KeyEventHandler(Key.A),
    new KeyEventHandler(Key.B),
    new KeyEventHandler(Key.C),    
};

private class KeyEventHandler : Component
{
    override OnKeyDown(KeyDownEvent e) => e.Key == associatedKey;

    override OnKeyUp(KeyUpEvent e) => Logger.Log($"Receptor{ChildID}: Key {e.Key} has been released");
}

where it would log (with written comments):

Receptor3: Key A has been released // (didn't handle and yet received)
Receptor2: Key A has been released // (didn't handle and yet received)
Receptor1: Key A has been released // (correctly received)
Receptor3: Key B has been released // (didn't handle and yet received)
Receptor2: Key B has been released // (correctly received)
Receptor3: Key C has been released // (correctly received)

Here's also a test case attachable to KeyboardInputTest.cs that would log what the receptor received, same as above. (requires some minor changes to declaration of KeyDown and KeyUp to work).

---

I've tried fixing this by doing some simple algorithm to cut siblings / ancestors of the handler from above, but it turns out to be impossible to do as there's no simple way to tell if a drawable is an ancestor / sibling to another.

So as per above, the only fix I'm finding here is by introducing something like a ParentDepth (definitely needs another name) that provides how deep the drawable is in the hierarchy in comparison to another, open for suggestions if I completely missed something that can fix this.

[smoogipoo]

This is intentional. Receiving an OnKeyUp does not mean that you've returned true from OnKeyDown, it means that you've received an OnKeyDown event. This is what the xmldoc says:

osu-framework/osu.Framework/Graphics/Drawable.cs

Line 2196 in 6697d05

/// This is guaranteed to be invoked if <see cref="OnKeyDown"/> was invoked.

This could be explained better though (XMLDocs reworded?), and the wiki doesn't seem to mention this at all.

As for the XMLDocs, I've tried to make a distinction between when true also acts as a blocker for receiving other events vs when true only acts as a blocker for propagation:

Only as a blocker for propagation:

Will always be worded in the form of ... is guaranteed to be invoked if X was invoked.

osu-framework/osu.Framework/Graphics/Drawable.cs

Line 2104 in 6697d05

/// This is guaranteed to be invoked if <see cref="OnMouseDown"/> was invoked.

osu-framework/osu.Framework/Graphics/Drawable.cs

Line 2088 in 6697d05

/// This is guaranteed to be invoked if <see cref="OnHover"/> was invoked.

osu-framework/osu.Framework/Graphics/Drawable.cs

Line 2196 in 6697d05

/// This is guaranteed to be invoked if <see cref="OnKeyDown"/> was invoked.

Also as a blocker also for receiving other events:

Will always be worded in the form of ... will only be invoked on the Drawable that returned true from X.

osu-framework/osu.Framework/Graphics/Drawable.cs

Line 2142 in 6697d05

/// This will only be invoked on the <see cref="Drawable"/> that returned <code>true</code> from a previous <see cref="OnDragStart"/> invocation.

osu-framework/osu.Framework/Graphics/Drawable.cs

Line 2167 in 6697d05

/// This will only be invoked on the <see cref="Drawable"/> that returned <code>true</code> from both <see cref="AcceptsFocus"/> and a previous <see cref="OnClick"/> invocation.

osu-framework/osu.Framework/Graphics/Drawable.cs

Line 2125 in 6697d05

/// This will only be invoked on the <see cref="Drawable"/> that returned <code>true</code> from a previous <see cref="OnClick"/> invocation.

[smoogipoo]

Going to close on the original premise of the issue being intended behaviour. Would accept a separate issue mentioning similar to my above comment regarding documentation.

[peppy]

Completely forgot that was something we decided on. Definitely should be better documented.