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

Fix Input.is_action_just_pressed flicker on joypad axes #82056

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

ErezShahaf
Copy link
Contributor

Fixes #81975
Pressed tick assignments were in the wrong scope, ended up updating pressed_frame even when it shouldn't and therefore the just_press would return true every time that the strength changes and not only when there's a new press.

@ErezShahaf ErezShahaf requested a review from a team as a code owner September 21, 2023 15:10
@AThousandShips AThousandShips changed the title Input just pressed jitter fix #81975 Fix Input.is_action_just_pressed jitter Sep 21, 2023
@AThousandShips AThousandShips added this to the 4.2 milestone Sep 21, 2023
core/input/input.cpp Show resolved Hide resolved
core/input/input.cpp Outdated Show resolved Hide resolved
core/input/input.cpp Outdated Show resolved Hide resolved
@Sauermann
Copy link
Contributor

With the exception of the comments, the changes are looking good. Only reviewed code, but didn't test it.

@Calinou
Copy link
Member

Calinou commented Oct 12, 2023

Tested locally, this is how the MRP linked in #81170 (comment) behaves:

master 42425ba

simplescreenrecorder-2023-10-12_20.08.24.mp4

This PR

simplescreenrecorder-2023-10-12_20.06.29.mp4

I still notice two issues:

  • On both master and this PR, the strength of the "Both" action (an action that is activated by both keyboard and gamepad events) remains on 100% when the keyboard is released if the gamepad is held still above 0%.
  • Controller strength sometimes flickers wildly, but I don't know exactly why. This may be related to mouse movement (I'm using a wired DualSense which has a touchpad that moves the mouse when you touch it).

Copy link
Contributor

@Eoin-ONeill-Yokai Eoin-ONeill-Yokai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once @Sauermann 's issues are resolved, I think this is good to go.

I've tested it with a few devices on my end and the axis "just_pressed" flag triggers appropriately only when entering the deadzone. Additionally, "just_released" works as expected as well.

I didn't observe any flickering inputs on my end compared to @Calinou 's observations. Having said that, I'm using mostly DS4, 8BitDo and Xbox Series X Controllers. The Series X controller on linux seems to register inputs twice still (looking into a solution to this still, if possible) but otherwise it seemed to behave akin to how master behaves.

@akien-mga
Copy link
Member

Please remember to squash the commits while addressing the code review comments. See PR workflow for instructions.

Pressed tick assignments were in the wrong scope, resulting in updating
`pressed_frame` even when it shouldn't and therefore the `just_pressed`
would return true every time that the strength changes and not only when
there's a new valid press.

Fixes godotengine#81975.
@akien-mga
Copy link
Member

I squashed the commits and addressed the review comments myself.

@akien-mga akien-mga changed the title Fix Input.is_action_just_pressed jitter Fix Input.is_action_just_pressed flicker on joypad axes Oct 16, 2023
@akien-mga akien-mga merged commit 2f5bb6c into godotengine:master Oct 16, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@ErezShahaf ErezShahaf deleted the Axisjustpressedjitterfix branch October 16, 2023 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Input.is_action_just_pressed flickers when action is tied to an input axis (joystick direction)
7 participants