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

Add note about inertia being required for apply_torque on various Node types #68098

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

compmstr
Copy link
Contributor

@compmstr compmstr commented Oct 31, 2022

@compmstr compmstr requested a review from a team as a code owner October 31, 2022 23:37
@Calinou Calinou added this to the 4.0 milestone Nov 1, 2022
@Calinou
Copy link
Member

Calinou commented Nov 1, 2022

The same note is likely relevant for RigidBody3D and PhysicsDirectBodyState3D. Can you check there?

@compmstr
Copy link
Contributor Author

compmstr commented Nov 1, 2022

Checked in the code, and calculating angular velocity uses _inv_inertia_tensor, so it appears that inertia is required for 3D as well. Added a similar note to apply_torque functions for RigidBody3D

@mhilbrunner
Copy link
Member

Please squash the commits into one :)

@compmstr
Copy link
Contributor Author

compmstr commented Nov 2, 2022

I think that commit squashing is done upon merging? Or am I missing an option in Github?

@Zireael07
Copy link
Contributor

@compmstr: It can be done upon merging, but Godot engine repo doesn't use this option

@compmstr compmstr force-pushed the apply-torque-inertia-note branch from 47a3895 to 469c004 Compare November 3, 2022 09:19
@compmstr
Copy link
Contributor Author

compmstr commented Nov 3, 2022

I followed https://docs.godotengine.org/en/stable/community/contributing/pr_workflow.html#the-interactive-rebase, and I think I've got it set up correctly. TIL about manually squashing commits, thank you :)

I also added the note to PhysicsDirectBodyState3D.

@compmstr compmstr changed the title Add note about inertia being required for RigidBody2D.apply_torque Add note about inertia being required for apply_torque on various Node types Nov 3, 2022
@mhilbrunner mhilbrunner requested a review from a team November 6, 2022 05:43
@compmstr compmstr force-pushed the apply-torque-inertia-note branch from 469c004 to 1f3806a Compare November 6, 2022 13:34
@compmstr
Copy link
Contributor Author

compmstr commented Nov 6, 2022

I fixed the CI issues:

  • had to change 'param' to 'member' for the inertia members
  • I had the class reference tags wrong (it's just [ClassName] instead of [class ClassName])

@@ -76,6 +76,7 @@
<param index="0" name="torque" type="float" />
<description>
Applies a rotational force without affecting position. A force is time dependent and meant to be applied every physics update.
[b]Note:[/b] [member inverse_inertia] is required for this to work. To have [member inverse_inertia], an active [CollisionShape2D] must be a child of the node, or you can manually set [member inverse_inertia].
Copy link
Member

Choose a reason for hiding this comment

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

We only use one space after a period in the docs. (Same for all added sentences.)

@akien-mga
Copy link
Member

Note that the email associated with your Git identity for this commit is not valid (see https://github.com/godotengine/godot/pull/68098.patch). It's not a problem per se but it means the commit can't be attributed to your GitHub account. If you want to fix that, you should change your Git identity locally and amend the commit with git commit --amend --reset-author.
https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/setting-your-commit-email-address#setting-your-commit-email-address-in-git

@compmstr compmstr force-pushed the apply-torque-inertia-note branch from 1f3806a to 7673f97 Compare November 6, 2022 15:14
  - RigidBody2D
  - PhysicsDirectBodyState2D
  - RigidBody3D
  - PhysicsDirectBodyState3D
@compmstr compmstr force-pushed the apply-torque-inertia-note branch from 7673f97 to f19de2a Compare November 6, 2022 15:17
@compmstr
Copy link
Contributor Author

compmstr commented Nov 6, 2022

Thanks for the note about the email address. I fixed that, and the double spaces after periods.

@akien-mga akien-mga merged commit 56e1520 into godotengine:master Nov 8, 2022
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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.

apply_torque on RigidBody2D does not work if bodys CollisionShape2D is disabled
5 participants