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

Skeleton._sortBones() crash for bones with parents from other skeletons #12415

Merged
merged 1 commit into from
Apr 20, 2022

Conversation

EvgenyRodygin
Copy link
Contributor

It is possible for a bone to have a parent bone from a skeleton different from the original one.
Currently, the method crashes when this is the case.
This simple check fixes it. Notice that "index" might be -1 when the parent is from another skeleton in the previous iteration.

@azure-pipelines
Copy link

Please make sure to tag your PR with "bug", "new feature" or "breaking change" tags.

@azure-pipelines
Copy link

Snapshot stored with reference name:
refs/pull/12415/merge

Test environment:
https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12415/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12415/merge/index.html#WGZLGJ#4600

To test the snapshot in the playground itself use (for example):

https://playground.babylonjs.com/?snapshot=refs/pull/12415/merge#BCU1XR#0

Copy link
Contributor

@deltakosh deltakosh left a comment

Choose a reason for hiding this comment

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

I'm deeply surprise you could get a bone with a different skeleton than the rest of the bones. This is clearly not supported by design

How do you end up with that?

Anyway this fix is great

@deltakosh deltakosh merged commit 152de45 into BabylonJS:master Apr 20, 2022
@EvgenyRodygin
Copy link
Contributor Author

I'm deeply surprise you could get a bone with a different skeleton than the rest of the bones. This is clearly not supported by design

How do you end up with that?

Anyway this fix is great

Yeah. Shouldn't be the case I know. You won't get something like this importing regular files I think (might be wrong though).
In our app we create a separate IK skeleton and the root bone of this IK skeleton is attached to the bone from the FK-skeleton. Might be not the best design though that what I encountered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants