-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Prevent axis-based actions from getting stuck #81170
Conversation
9a9bf6f
to
3204ec2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is difficult to review, because I cant test the PR.
I'm not sure if there is a better way to fix the bug, but I can follow the argumentation of the shortcomings in the OP and understand the changes.
So this is a "it will not worsen the situation" review approval.
This doesn't appear to work correctly on my end (using keyboard and wired DualSense on Linux at the same time). In fact, I can still notice #45628 happening on my end (look at times when I press keyboard keys while keeping the gamepad axis still): simplescreenrecorder-2023-08-30_23.27.10.mp4CheckBox indicates state of Testing project: test_analog_and_digital_input_2.zip |
Fixed. It was the same issue - the action was released multiple times in a row. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Origin for this PR is #80859, which originally intended to fix #45628. Since I can't replicate that issue, I can't verify that.
#80859 introduced #81164 which this PR intends to fix. I can confirm #81164 in v4.2.dev4.official [549fcce]
With this PR applied to v4.2.dev.custom_build [fb4edf5], I tried for over a minute to cause, #81164 but was unsuccessful to do that. So I am confident, that this PR fixes that issue.
Comment #81170 (comment) mentioned a problem, that a previous version of this PR introduced and I have tested and can confirm, that this has been fixed in the latest update.
The concerns of my comments regarding the implementation also have been solved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work fine, and I don't see any regression on the joypads demo.
Thanks! |
Fixes #81164
Not sure how much correct is that. This assumes that only InputEventJoypadMotion has this problem and that you have only single axis event assigned to an action (which is true in 99.9% cases). Also a special handler for one event type feels hacky.