Skip to content

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

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
soqb:laggy-gamepad-support
Aug 15, 2023
Merged

input: allow multiple gamepad inputs to be registered for one button in one frame#9446
alice-i-cecile merged 1 commit intobevyengine:mainfrom
soqb:laggy-gamepad-support

Conversation

@soqb
Copy link
Copy Markdown
Contributor

@soqb soqb commented Aug 14, 2023 •
edited by alice-i-cecile
Loading

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
Copy Markdown
Contributor

github-actions bot commented Aug 14, 2023

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

alice-i-cecile commented Aug 15, 2023

@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
Copy Markdown
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! 🚀

Merged via the queue into bevyengine:main with commit 632ef0c Aug 15, 2023
@100-TomatoJuice 100-TomatoJuice mentioned this pull request Oct 24, 2023
Closed
This was referenced May 3, 2024
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@maniwani maniwani Awaiting requested review from maniwani

+1 more reviewer

@nicopap nicopap nicopap approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@soqb @nicopap @alice-i-cecile