Skip to content

Prevent held non-modifier keys from re-triggering bindings after modifier release#6736

Merged
peppy merged 8 commits intoppy:masterfrom
Piotrr0:fix-keybinding-ghost-keys
Apr 28, 2026
Merged

Prevent held non-modifier keys from re-triggering bindings after modifier release#6736
peppy merged 8 commits intoppy:masterfrom
Piotrr0:fix-keybinding-ghost-keys

Conversation

@Piotrr0
Copy link
Copy Markdown
Contributor

@Piotrr0 Piotrr0 commented Apr 23, 2026

Fixes a bug where releasing a modifier key while a non-modifier key is still held causes the non-modifier to "ghost", remaining in pressedInputKeys without belonging to any active binding. Any subsequent key press could then accidentally satisfy a binding that includes the held non-modifier, even though the user never made a new intentional press of it.

Closes ppy/osu#36779

@Piotrr0 Piotrr0 marked this pull request as draft April 23, 2026 21:24
@Piotrr0 Piotrr0 marked this pull request as ready for review April 23, 2026 21:52
@Susko3
Copy link
Copy Markdown
Member

Susko3 commented Apr 23, 2026

This needs some tests that fail on master, but pass on this PR. A test for the failure case you described in the PR would be a great start.

Comment thread osu.Framework.Tests/Visual/Input/TestSceneKeyBindingContainer.cs Fixed
@pull-request-size pull-request-size Bot added size/M and removed size/S labels Apr 24, 2026
Comment thread osu.Framework.Tests/Visual/Input/TestSceneKeyBindingContainer.cs Fixed
AddStep("add container", () =>
{
pressedActions.Clear();
Child = new TestKeyBindingContainer(matchingMode: KeyCombinationMatchingMode.Modifiers)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peppy peppy self-requested a review April 28, 2026 09:10
Copy link
Copy Markdown
Member

@peppy peppy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense. Fixes the observed issues and cannot find new breakage so let's give it a try.

@peppy peppy merged commit 8c8becf into ppy:master Apr 28, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pressing speed change shortcut + enter moves selection up/down

4 participants