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

[rcore] IsKeyPressed() missing quick inputs #4591

Open
exformation opened this issue Dec 10, 2024 · 10 comments
Open

[rcore] IsKeyPressed() missing quick inputs #4591

exformation opened this issue Dec 10, 2024 · 10 comments

Comments

@exformation
Copy link

exformation commented Dec 10, 2024

For me, IsKeyPressed fails to see a press if the release happens immediately afterwards.

My keyboard firmware sends some keys as:

register_key(key);
unregister_key(key);

with no delay between them by default. Raylib doesn't see these presses (unless I double tap the key or increase the firmware's tap delay). I know this is because of my keyboard firmware, but the key has been pressed, so Raylib should still see it. Other applications/games don't have issues with this, and qmk is a very popular firmware, so more people will run into this issue in the future.

@exformation
Copy link
Author

exformation commented Dec 10, 2024

Sorry I didn't see that when searching IsKeyPressed issues. I mentioned the context for some clarity, but I still consider this a bug. Raylib received a press event and a release event, yet it doesn't register that the key was pressed. At least most QMK users are tech savvy enough to hopefully figure out what's wrong and update their firmware if they play a Raylib game. For any future QMK users finding this:

// config.h
#define TAP_CODE_DELAY 20

However, I confirmed that setting that delay to lower values could still cause missed inputs.

Framerate shouldn't determine whether Raylib can see input. This won't just be a problem for QMK/ZMK. Razer, Corsair, Wooting, and more are increasingly creating features which allow users to input some incredibly quick key presses. If they happen to release their key immediately after it triggers, Raylib will miss their input (although this can happen with a normal keyboard too, it's just easier with features like this). FPS drops also cause input drops, regardless of keyboard.

For now, I'll use GetKeyPressed to avoid inconsistencies.

@exformation
Copy link
Author

exformation commented Dec 10, 2024

Regardless, an input shouldn't be dropped because of lower FPS, and an input shouldn't be dropped because someone tapped a key quickly. While typing on a normal keyboard I registered keypresses that only lasted 10-20ms. This can be dropped even at 60 FPS.

@exformation
Copy link
Author

exformation commented Dec 10, 2024

Are there examples of polling on a thread? I asked a different question about timestamping user input with a thread in the discord and JeffM told me it wouldn't be possible with custom frame control and that I should just use SDL:
image

@exformation
Copy link
Author

exformation commented Dec 10, 2024

For that amount of precision ...

I've been messing with GLFW and SDL2 and neither have issue seeing the press/release events.

The function here will trigger on both PRESS and RELEASE:
https://github.com/raysan5/raylib/blob/aeb33e6301fd614e1f38aed66ab6b219a824df3c/src/platforms/rcore_desktop_glfw.c#L1788C13-L1788C24
The entire issue is the whole previousKeyState/currentKeyState pattern. currentKeyState is set to 1 and back to 0 before IsKeyPressed can be called, but it doesn't seem like IsKeyPressed does anything much different than checking if key is in keyPressedQueue but without emptying the queue. Most of the time that queue wouldn't have more than a couple items in it so it'd be quick to check.

bool IsKeyPressed(int key)
{
    if ((key > 0) && (key < MAX_KEYBOARD_KEYS))
    {
        for (int i = 0; i < CORE.Input.Keyboard.keyPressedQueueCount; i++)
        {
            if (CORE.Input.Keyboard.keyPressedQueue[i] == key)
            {
                return true;
            }
        }
    }
    return false;
}

@raysan5 raysan5 changed the title [rcore] IsKeyPressed missing quick inputs [rcore] IsKeyPressed() missing quick inputs Dec 12, 2024
@veins1
Copy link
Contributor

veins1 commented Dec 12, 2024

If IsKeyPressed and IsKeyReleased are supposed to return whether a key was pressed or released at least once from the last frame then that seems very easy to implement.
The only nuance is that would be a "breaking" change, since we would be able to have a state that is unrepresantable now, like IsKeyDown and IsKeyReleased both returning true in the same frame. But I imagine breaking would be minimal.

@exformation
Copy link
Author

exformation commented Dec 13, 2024

like IsKeyDown and IsKeyReleased both returning true in the same frame

Do you mean IsKeyDown and IsKeyUp, or IsKeyPressed and IsKeyReleased? Assuming the latter.

@veins1
Copy link
Contributor

veins1 commented Dec 13, 2024

I meant IsKeyDown and IsKeyReleased , but IsKeyPressed and IsKeyReleased is another example.

@exformation
Copy link
Author

exformation commented Dec 13, 2024

I suppose SDL users have dealt with this just fine? If they get a press and release in the same frame, then the event queue will have both of them in there, so the game would handle the down event and then the up event. For raylib, you'd just have to make sure your logic for handling down comes before up, and make sure they aren't exclusive.

while PollEvent(&evt) 
  switch(evt.kind)
    case KeyDown: if(evt.key==...) doThingA()
    case KeyUp:   if(evt.key==...) doThingB()

// functionally equivalent to this?
if(IsKeyPressed(...))  doThingA()
if(IsKeyReleased(...)) doThingB()

@veins1
Copy link
Contributor

veins1 commented Dec 13, 2024

Yeah, that's all fine, I'm just saying some people might not expect that.
Reviewing this further, although the fix seems pretty simple code-wise, it requires changes in rcore, AutomationEvents and PollInputEvents for all platforms.

@CodingMadness
Copy link

Yeah, that's all fine, I'm just saying some people might not expect that. Reviewing this further, although the fix seems pretty simple code-wise, it requires changes in rcore, AutomationEvents and PollInputEvents for all platforms.

would it be that bad if this got applied cause I noticed indeed similar issues as OP, or rather not issue but a feature I would like to work as he described aswell tbh.

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

No branches or pull requests

3 participants