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 validation for rotation axis to SpringBoneSimulator3D #101571

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

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Jan 15, 2025

The rotation axis locking is worked by snapping to a plane. Also, SpringBone obtains the next motion from the movement of the Vector3 coordinates by integrating and predicting. Then it cannot account for twisting due to the movement of child joints.

For example, if the forward and rotational axes are the same, the child joints may twist, but SpringBone's calculations cannot take account this force.

So, if the forward and rotation axes are the same unintended rotation will occur (although the corruption of Matrix is prevented), so this PR adds a validation to warn the user.

Also, added to the documentation that scaling is not recommended.

@TokageItLab TokageItLab added this to the 4.4 milestone Jan 15, 2025
@TokageItLab TokageItLab requested a review from a team January 15, 2025 04:36
@TokageItLab TokageItLab requested review from a team as code owners January 15, 2025 04:36
@TokageItLab TokageItLab force-pushed the validate-rotation-axis-spring-bone branch from 3ea28b2 to 2a4e2a1 Compare January 15, 2025 04:39
@fire
Copy link
Member

fire commented Jan 15, 2025

This is a quality-of-life fix that avoids unintended rotation.

@TokageItLab TokageItLab force-pushed the validate-rotation-axis-spring-bone branch from 2a4e2a1 to dac6173 Compare January 15, 2025 06:11
doc/classes/SpringBoneCollision3D.xml Outdated Show resolved Hide resolved
doc/classes/SpringBoneSimulator3D.xml Outdated Show resolved Hide resolved
@TokageItLab TokageItLab force-pushed the validate-rotation-axis-spring-bone branch from dac6173 to 48e74af Compare January 15, 2025 09:29
@fire
Copy link
Member

fire commented Jan 15, 2025

This is a general thing, but how do we deal with things that may spam per frame, and the logging is worse than the bug?

@TokageItLab
Copy link
Member Author

TokageItLab commented Jan 16, 2025

Since validation occurs only at the timing of value set, this should only cause spam if you ignore the warning and continue to change the value of the joint settings. In other words, the warning will not occur frame by frame unless you keep changing the joint settings.

However, if you have a chain of bones, warnings for all those joints may look like spam.

So when checking multiple axes at the same time, it might be bale to store a pair of setting and joints in a dictionary, so that we notify it in only one warning. However, I don't remember any other place in core that does that, so I don't know if I should do that.

At any rate, the possibility of spamming each frame does not occur in normal use, so I don't think there is any problem with merging as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Approved, Waiting for Production
Development

Successfully merging this pull request may close these issues.

3 participants