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

Change the default deadzone value for new actions from 0.5 to 0.2 #80299

Closed
wants to merge 0 commits into from

Conversation

rptfrg
Copy link
Contributor

@rptfrg rptfrg commented Aug 5, 2023

This changes the default deadzone value for new actions from 0.5 to 0.2 as discussed in godotengine/godot-proposals#7069.

This does not change the built-in actions.

This is my first pull request, so there is chance I have done something wrong - please check.

@rptfrg rptfrg requested review from a team as code owners August 5, 2023 15:36
@AThousandShips AThousandShips added this to the 4.x milestone Aug 5, 2023
@AThousandShips
Copy link
Member

You will probably need to add a compatibility note, but the CI will instruct for that

@AThousandShips
Copy link
Member

AThousandShips commented Aug 5, 2023

Needs a compatibility addition, follow the instructions here

Like so:

GH-80299
--------
Validate extension JSON: Error: Field 'classes/InputMap/methods/add_action/arguments/1': default_value changed value in new API, from "0.5" to "0.2".

Default deadzone value was changed.

Unsure what the comment should be here, if any further instructions are needed

@rptfrg rptfrg requested a review from a team as a code owner August 6, 2023 08:37
@rptfrg rptfrg force-pushed the deadzone-new-default branch 3 times, most recently from 9ebf120 to b92ab80 Compare August 6, 2023 08:50
@AThousandShips
Copy link
Member

Have you made sure this doesn't change input bindings from before this applied? I.e. make a binding with the default deadzone, and then open it with these changes

@rptfrg
Copy link
Contributor Author

rptfrg commented Aug 6, 2023

Have you made sure this doesn't change input bindings from before this applied? I.e. make a binding with the default deadzone, and then open it with these changes

Yes, I have tested for that, old actions are not changed.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected:

image

This implies a small run-time behavior change, as actions created from a script in InputMap will now have a lower deadzone by default. I think this is acceptable (especially since creating new actions from code is uncommon), but it needs to be clearly documented in the release notes.

@Calinou
Copy link
Member

Calinou commented Mar 26, 2024

Note that godotengine/godot-demo-projects#1024 changes the deadzone for all non-built-in inputs in demo projects to 0.2. Merging this PR would ensure we don't accidentally add new actions with a higher deadzone in demo projects 🙂

@Sauermann
Copy link
Contributor

I consider these changes beneficial and think, that they should be included.
Since this PR adjusts the API, perhaps it should be merged early in the 4.4 dev-cycle.

The following two places in the code base should also be investigated, if changes are necessary:

action["deadzone"] = Variant(0.5f);

// Since deadzone is 0.5, action_strength grows linearly from 0.5 to 1.0.

@Calinou
Copy link
Member

Calinou commented Sep 20, 2024

@reptofrog Could you look into rebasing to resolve merge conflicts? See PR workflow for instructions 🙂

The following two places in the code base should also be investigated, if changes are necessary:

The first instance is 3.x conversion code, so I'd keep it as-is. The second instance is a unit test where the deadzone is already set to 0.5 above, so it probably doesn't need to be changed.

Copy link
Member

Choose a reason for hiding this comment

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

Needs to be moved to the 4.3 file when rebasing

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.

Decrease the default deadzone value for actions to better match real world use cases
4 participants