Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix KeyBindingContainer propagating release events to removed drawables #5975

Merged
merged 3 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 51 additions & 14 deletions osu.Framework.Tests/Visual/Input/TestSceneKeyBindingContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,14 @@ public void TestPressKeyBeforeKeyBindingContainerAdded()
});

AddStep("press key A", () => InputManager.PressKey(Key.A));
AddAssert("only one action triggered", () => pressedActions.Count == 1);
AddAssert("ActionA triggered", () => pressedActions[0] == TestAction.ActionA);
AddAssert("no actions released", () => releasedActions.Count == 0);
AddAssert("only one action triggered", () => pressedActions, () => Has.Count.EqualTo(1));
AddAssert("ActionA triggered", () => pressedActions[0], () => Is.EqualTo(TestAction.ActionA));
AddAssert("no actions released", () => releasedActions, () => Is.Empty);

AddStep("release key A", () => InputManager.ReleaseKey(Key.A));
AddAssert("only one action triggered", () => pressedActions.Count == 1);
AddAssert("only one action released", () => releasedActions.Count == 1);
AddAssert("ActionA released", () => releasedActions[0] == TestAction.ActionA);
AddAssert("only one action triggered", () => pressedActions, () => Has.Count.EqualTo(1));
AddAssert("only one action released", () => releasedActions, () => Has.Count.EqualTo(1));
AddAssert("ActionA released", () => releasedActions[0], () => Is.EqualTo(TestAction.ActionA));
}

[Test]
Expand Down Expand Up @@ -126,8 +126,8 @@ public void TestKeyHandledByOtherDrawableDoesNotTrigger()
AddStep("release enter", () => InputManager.ReleaseKey(Key.Enter));
AddStep("release mouse button", () => InputManager.ReleaseButton(MouseButton.Left));

AddAssert("no pressed actions", () => pressedActions.Count == 0);
AddAssert("no released actions", () => releasedActions.Count == 0);
AddAssert("no pressed actions", () => pressedActions, () => Is.Empty);
AddAssert("no released actions", () => releasedActions, () => Is.Empty);
}

[Test]
Expand Down Expand Up @@ -196,18 +196,18 @@ public void TestSingleKeyRepeatEvents()
});

AddStep("press A", () => InputManager.PressKey(Key.A));
AddAssert("press received", () => pressedReceived == 1);
AddAssert("press received", () => pressedReceived, () => Is.EqualTo(1));

for (int i = 0; i < 10; i++)
{
int localI = i + 1;
AddUntilStep($"repeat #{1 + i} received", () => repeatedReceived >= localI);
AddUntilStep($"repeat #{1 + i} received", () => repeatedReceived, () => Is.GreaterThanOrEqualTo(localI));
}

AddStep("release A", () => InputManager.ReleaseKey(Key.A));
AddAssert("release received", () => releasedReceived);

AddAssert("only one press received", () => pressedReceived == 1);
AddAssert("only one press received", () => pressedReceived, () => Is.EqualTo(1));
}

[Test]
Expand Down Expand Up @@ -236,18 +236,18 @@ public void TestKeyRepeatDoesntFireWhenNotAlive()
});

AddStep("press A", () => InputManager.PressKey(Key.A));
AddUntilStep("wait for non-zero repeated", () => repeatedReceived > 0);
AddUntilStep("wait for non-zero repeated", () => repeatedReceived, () => Is.GreaterThan(0));

AddStep("hide receptor", () => receptor.Hide());

int stopReceivingCheck = 0;
AddStep("store count", () => stopReceivingCheck = repeatedReceived);
AddWaitStep("wait some", 5);
AddAssert("ensure not incrementing", () => stopReceivingCheck == repeatedReceived);
AddAssert("ensure not incrementing", () => stopReceivingCheck, () => Is.EqualTo(repeatedReceived));

AddStep("release A", () => InputManager.ReleaseKey(Key.A));
AddAssert("release received", () => releasedReceived);
AddAssert("only one press received", () => pressedReceived == 1);
AddAssert("only one press received", () => pressedReceived, () => Is.EqualTo(1));
}

[Test]
Expand Down Expand Up @@ -349,6 +349,43 @@ public void TestPrioritisedPositionalInput([Values] bool prioritised)
AddAssert("container did not receive input", () => !containerReceivedInput);
}

[Test]
public void TestReleaseKeyAfterReceptorRemovedFromHierarchy()
{
TestKeyBindingContainer container = null!;
TestKeyBindingReceptor receptor = null!;
List<TestAction> pressedActions = new List<TestAction>();
List<TestAction> releasedActions = new List<TestAction>();

AddStep("add container", () =>
{
pressedActions.Clear();
releasedActions.Clear();

Child = container = new TestKeyBindingContainer
{
Child = receptor = new TestKeyBindingReceptor
{
Pressed = a => pressedActions.Add(a),
Released = a => releasedActions.Add(a)
}
};
});

AddStep("press key A", () => InputManager.PressKey(Key.A));
AddAssert("only one action triggered", () => pressedActions, () => Has.Count.EqualTo(1));
AddAssert("ActionA triggered", () => pressedActions[0], () => Is.EqualTo(TestAction.ActionA));
AddAssert("no actions released", () => releasedActions, () => Is.Empty);

AddStep("remove receptor", () => container.Remove(receptor, disposeImmediately: false));

AddStep("release key A", () => InputManager.ReleaseKey(Key.A));
AddAssert("only one action triggered", () => pressedActions, () => Has.Count.EqualTo(1));
AddAssert("no actions released", () => releasedActions, () => Is.Empty);

AddStep("dispose of receptor", () => receptor.Dispose());
}

private partial class TestKeyBindingReceptor : Drawable, IKeyBindingHandler<TestAction>
{
public Action<TestAction>? Pressed;
Expand Down
2 changes: 1 addition & 1 deletion osu.Framework/Input/Bindings/KeyBindingContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ private void handleNewReleased(InputState state, InputKey releasedKey)
{
pressedBindings.Remove(binding);

PropagateReleased(getInputQueue(binding), state, binding.GetAction<T>());
PropagateReleased(getInputQueue(binding).Where(d => d.IsRootedAt(this)), state, binding.GetAction<T>());
keyBindingQueues[binding].Clear();
}
}
Expand Down
Loading