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

input: allow multiple gamepad inputs to be registered for one button in one frame #9446

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

soqb
Copy link
Contributor

@soqb soqb commented Aug 14, 2023

Objective

  • Currently, (AFAIC, accidentally) after registering an event for a Gilrs button event, we ignore all subsequent events for the same button in the same frame, because we don't update our filter. This is rare, but I noticed it while adding gamepad support to a terminal app rendering at 15fps.
  • Related to Lag seems to cause key release events to be missed #4664, but does not quite fix it.

Solution

  • Move the edit to the Axis<GamepadButton> resource to when we read the events from Gilrs.

@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Input Player input via keyboard, mouse, gamepad, and more labels Aug 15, 2023
@nicopap
Copy link
Contributor

nicopap commented Aug 15, 2023

This doesn't fix #4664. The issue mentions "key" and Alien cake addict, but it has no gamepad support only keyboard, this only fixes it for gamepad. So supposedly the issue exists for keyboard too.

@soqb
Copy link
Contributor Author

soqb commented Aug 15, 2023

ah that's unfortunate. i was not using winit in my testing so i would assume the issue lies in how we/them process window events.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 15, 2023
@alice-i-cecile
Copy link
Member

@soqb I've tweaked your original comment to reflect that this doesn't quite fix the linked issue so we don't have to re-open it / confuse future readers.

Merging now.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 15, 2023
@soqb
Copy link
Contributor Author

soqb commented Aug 15, 2023

yep, it slipped my mind to change that in the description.

really cool to see how fast small changes like this can be merged first hand! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Input Player input via keyboard, mouse, gamepad, and more C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants