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

PropertyBinding: Remove fallback to root node on incorrect path names. #25329

Merged
merged 1 commit into from
Jan 27, 2023

Conversation

hybridherbst
Copy link
Contributor

Related issue:

Description

Currently, incorrect path names in property bindings can lead to confusing results, as ALL properties that can't be matched are instead applied to the root of the animation. This PR removes the fallbacks for that; instead, an error will be logged (as before) for those specific tracks but remaining tracks will work fine.

This contribution is funded by Needle

@Mugen87 Mugen87 added this to the r150 milestone Jan 24, 2023
Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

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

To be on the safe side, I have added the PR to the r150 release. In this way, there is plenty of time to work with this change on dev.

@hybridherbst
Copy link
Contributor Author

Could you clarify what that means in practice – it's going to get merged after r149 becomes live?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 24, 2023

Yes, I suggest to merge the PR after r149 has been published. So at the beginning of the next dev cycle.

@Mugen87 Mugen87 changed the title [Animation] Property binding should not fall back to root node on incorrect path names PropertyBinding: Remove fallback to root node on incorrect path names. Jan 25, 2023
@Mugen87 Mugen87 merged commit 5da06be into mrdoob:dev Jan 27, 2023
@hybridherbst hybridherbst deleted the fix-propertybinding-root-fallback branch March 27, 2023 15:43
@yoshikiohshima
Copy link
Contributor

This commit broke our application.

The relevant part of our application is here:

https://github.com/croquet/microverse/blob/6302d8d8a1052e3e0ac077386794a9665fbab278/behaviors/croquet/fullBodyAvatar.js#L83

which loads animation keyframes from shared GLB, and applies it to a set of avatar models created on Ready Player Me.

Perhaps falling back to root may be wrong but setting targetObject to be null seems to be also wrong.

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.

PropertyBinding falls back to root node for incorrect paths/nodes
3 participants