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

[Windows] Fix left/right Shift key regression #101763

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SupSuper
Copy link

@SupSuper SupSuper commented Jan 18, 2025

This fixes a regression introduced in #92415 and reported in #101384.

When multiple Shift keys are pressed, Windows only sends a KEYUP for the last key released, leaving the other stuck pressed. A workaround was introduced in #80231 to fake this missing KEYUP event.
However this workaround was broken in #92415. _get_mods() returns the modifier keys before the KEYUP, so there will always be a Shift key pressed before it's released.

This PR restores the original check and respective functionality. The MRP in #101384 can be used to reproduce and confirm the fix.

Thanks to @ghostsoft @andyprice @romlok for helping debug the issue.

Fixes godotengine#101384 by correctly checking the state of the Shift keys before sending the KEYUP event.
@Zireael07
Copy link
Contributor

Can you add a comment explaining WHY this function and not the other?

@bruvzg bruvzg self-requested a review January 18, 2025 16:14
@bruvzg
Copy link
Member

bruvzg commented Jan 18, 2025

The difference between mods/GetKeyboardState and GetAsyncKeyState is how they interact with message queue. For a fake key injection in the raw input handler, it is likely correct to use GetAsyncKeyState since normal input messages are not processed at this moment. Will test it tomorrow.

@SupSuper
Copy link
Author

@Zireael07

Can you add a comment explaining WHY this function and not the other?

The goal is to send a fake KEYUP event when a Shift key is released after a Shift key is pressed. (i.e. check if Shift keys are still pressed after the KEYUP).
mods/GetKeyboardState doesn't work for this because it returns the state before the KEYUP is resolved, so there will always be a Shift key pressed. You either end up sending no fake events or duplicate events.

The only solutions were:

  • Extend _get_mods to also handle Left/Right modifier keys so we can check for both pressed.
  • Rewrite everything to use RawInput.
  • Use GetAsyncKeyState to check the real key state (the old solution).

Since the old solution still worked, I opted for the simplest fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release platform:windows regression topic:input
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keys do not release properly if left shift and right shift are held at the same time
4 participants