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 units and docs for Joint3D/2D stiffness and damping #96523

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Sep 3, 2024

Similar to what #94862 does for VehicleWheel3D, this PR adds units for the Joint2D/3D-derived nodes.

General info on stiffness and damping in metric:

  • Stiffness is measured in different units depending on the type of stiffness. For linear stiffness, it is measured in Newtons per meter, or kg/s² in SI base units. For angular stiffness, it is measured in Newton-meters per radian, or kg⋅m²/s²/rad in SI base units. This difference is because linear stiffness involves force divided by distance (N / m), while angular stiffness involves torque divided by angle (N⋅m / rad).

  • Damping is measured in different units depending on the type of damping. For linear damping, it is measured in Newton-seconds per meter, or kg/s in SI base units. For angular damping, it is measured in Newton-meter-seconds per radian, or kg⋅m²/s/rad in SI base units. This difference is because linear damping involves impulse divided by distance (N⋅s / m), while angular damping involves torque-impulse divided by angle (N⋅m⋅s / rad).

Godot-specific info:

  • The "softness" property in 3D is incorrectly named, it's actually stiffness. We should rename this in the future.

  • The "softness" property in PinJoint2D is actually softness, inverse stiffness, so it's s²/kg instead of kg/s². This may be worth replacing in the future for consistency with other joints using stiffness. The conversion is 1.0 / softness == stiffness and 1.0 / stiffness == softness.

  • The "relaxation" property on HingeJoint3D and ConeTwistJoint3D is inverse damping, so it's rad⋅s/kg/m² or rad/N/m/s. This may be worth replacing in the future for consistency with other joints using damping. The conversion is 1.0 / relaxation == damping and 1.0 / damping == relaxation.

Screenshot 2024-09-03 at 4 48 29 AM

@aaronfranke
Copy link
Member Author

I updated the PR to include an overhaul to the documentation of these values. I was planning on just adding the units to the documentation, but the descriptions were really bad and often plain wrong. I'm adding the bug label because I believe the old documentation was so bad that this qualifies as a bug fix.

On PR #94862, @akien-mga mentioned that we should only use the Newton-based units for vehicle wheel suspension. Should we do the same here? Note that for 2D, we can't use Newtons, because Godot's 2D doesn't use meters.

@aaronfranke aaronfranke changed the title Add units for Joint3D/2D stiffness and damping Add units and docs for Joint3D/2D stiffness and damping Sep 18, 2024
@aaronfranke aaronfranke requested a review from a team as a code owner October 4, 2024 23:29
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

I do think that the newly found verbosity in this PR is quite daunting. However, it clearly comes from actual knowledge of the subject, so it is very accurate and exhaustive.

</member>
<member name="angular_limit_y/restitution" type="float" setter="set_param_y" getter="get_param_y" default="0.0">
The amount of rotational restitution across the Y axis. The lower, the more restitution occurs.
The amount of rotational restitution across the Y axis. The lower, the more restitution occurs. Restitution is a dimensionless value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure if lack of units is worth specifying, given every other description is now very explicit about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is worth specifying, just to be extra clear.

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.

Code and docs look good to me. I'm not a physics expert so I don't know if the units used are correct though.

@aaronfranke aaronfranke modified the milestones: 4.4, 4.x Feb 13, 2025
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.

3 participants