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

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

Open
ghostsoft opened this issue Jan 10, 2025 · 9 comments · May be fixed by #101763
Open

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

ghostsoft opened this issue Jan 10, 2025 · 9 comments · May be fixed by #101763

Comments

@ghostsoft
Copy link

Tested versions

  • Reproducible in 4.3.stable.official, 4.4.dev7.official

System information

Godot v4.3.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 (NVIDIA; 32.0.15.6603) - Intel(R) Core(TM) i5-8400 CPU @ 2.80GHz (6 Threads)

Issue description

If you create input actions for left shift (Physical left) and right shift (Physical right) they work fine as long as you press one at a time, but if you press both shift keys at the same time they will not release properly.

If you release one key, both will still count as being still pressed and when you let go of the other key that key will properly release while the first key will still count as pressed and is now stuck in a perpetual "key pressed" state. This is the same regardless of if using Input.is_key_pressed() or the _input(event) function with exact_match = true

I was planning to use this feature for a pinball game and was very excited Godot finally supports physical left and right shift keys but due to this bug they are currently unusable. The problem still persists in the 4.4 dev snapshot.

Steps to reproduce

Add new action for left shift (Physical left) and right shift (Physical right) in Input Map, press both shift keys at the same time and then release them.

Minimal reproduction project (MRP)

shiftstuck.zip

@TheWatcher
Copy link

Just tried to reproduce with the provided shiftstuck.zip and I experience the same behaviour.

System is: Godot v4.4.dev7 - Windows 10 (build 19045) - Multi-window, 3 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3080 (NVIDIA; 32.0.15.6109) - AMD Ryzen 5 2600 Six-Core Processor (12 threads)

@Sauermann
Copy link
Contributor

Can you please check, if the same bug also applies to left-Ctrl and right-Ctrl?

@ghostsoft
Copy link
Author

Can you please check, if the same bug also applies to left-Ctrl and right-Ctrl?

I hope you'll excuse me for not posting another MRP but this was interesting - This does work properly with Ctrl

So the bug, whatever it may be seems to be with the shift keys specifically.

@Sauermann
Copy link
Contributor

Thank you, that helps to narrow down the problem.

@tmilker
Copy link

tmilker commented Jan 17, 2025

@romlok I've reviewed the original PR for your commit that added this support and see that it was left at partial support. Is the rest of the support going to be added so this can be fixed for Windows? 4.4 is coming quick and like you said about 4.3, it would be nice to have this at 100% without waiting for another major version.

@romlok
Copy link
Contributor

romlok commented Jan 17, 2025

Thanks for the ping! If this is what I found while doing that PR, then this should be Windows-specific and, as discovered here, shift-specific. Windows does a thing where it doesn't send a key-up event until all shifts are released 🙃

That said, I thought I addressed that in the PR. So it could be that I missed something, or that this is triggered somewhere else. Either way, I don't actually have a Windows box to test/verify on, so it would be difficult to investigate further myself 😕

That said, I just tested the MRP on my Linux box, and I obviously don't see your issue, but I do see that event.is_action_pressed returns false if another key of the same type is already pressed (and there hasn't been a release of it since). Maybe related, maybe not. 😕

@ghostsoft Can you check if eg. Input.is_action_pressed("right_shift") works as expected for you? I was experimenting with your MRP, and that function seemed to consistently return the expected value, at least on my machine.

@ghostsoft
Copy link
Author

ghostsoft commented Jan 18, 2025

@ghostsoft Can you check if eg. Input.is_action_pressed("right_shift") works as expected for you? I was experimenting with your MRP, and that function seemed to consistently return the expected value, at least on my machine.

I alluded to this in the initial post but I should've probably included it in the MRP too - when the key is stuck (but not physically pressed) the Input.is_action_pressed() function still returns true.

As an example, if both left and right shift are pressed, then right shift is released and then left shift is released, Input.is_action_pressed("left_shift") now returns false but Input.is_action_pressed("right_shift") still returns true despite no keys being down. I've spoken with some friends who are better than me about understanding the source code and they agree that it's a Windows-specific bug and that it's "in the detection code and not the action matching code" but I barely understood what they were talking about.

@andyprice
Copy link
Contributor

they agree that it's a Windows-specific bug and that it's "in the detection code and not the action matching code"

To add a little detail, it seems there's a separate action matching issue that also occurs on Linux so I asked @ghostsoft to test this modification to the MRP to make sure their problem is not related to action matching:

func _input(event: InputEvent) -> void:
    if event is not InputEventKey || event.keycode != KEY_SHIFT:
        return
    if event.is_pressed():
        if event.location == KEY_LOCATION_LEFT:
            left.modulate = Color.RED
        if event.location == KEY_LOCATION_RIGHT:
            right.modulate = Color.RED
    else:
        if event.location == KEY_LOCATION_LEFT:
            left.modulate = Color.WHITE
        if event.location == KEY_LOCATION_RIGHT:
            right.modulate = Color.WHITE

Result: this works fine on Linux but apparently reproduces @ghostsoft's issue on Windows.

SupSuper added a commit to SupSuper/godot that referenced this issue Jan 18, 2025
Fixes godotengine#101384 by correctly checking the state of the Shift keys before sending the KEYUP event.
@SupSuper SupSuper linked a pull request Jan 18, 2025 that will close this issue
@SupSuper
Copy link

I've submitted a fix for this issue on Windows: #101763

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants