-
Notifications
You must be signed in to change notification settings - Fork 628
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
fix(useAnimations): passing root to mixer init if it is Object3D #1307
fix(useAnimations): passing root to mixer init if it is Object3D #1307
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 75226c1:
|
sorry for the long delay. i think useState is the wrong place, it should probably be a useLayoutEffect that sets the property? otherwise useState would be stale and couldn't react to changing the root from one to another? |
Okay, I think I see. You're saying if the root of the mixer changes via a direct call then the actualRef will be out of sync with the mixer and the useEffect cleanup won't work? I admit I didn't think to do any tests with multiple objects interacting with the mixer. Sorry, I will revisit this. |
I did some testing and I'm more confused. I don't understand when or why the root object would change as other objects can be passed as optional root for a new action or create a separate mixer. The class doesn't seem to have a function that changes it, even unchacheRoot didn't change it, so, I don't know. Please let me know what I should do. Should I delete the pull request? |
🎉 This PR is included in version 9.66.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #429
Why
Trying to create a new AnimationClips in useAnimations mixer fails because mixer is instantiated with no reference to the root scene as described in issue #429
What
passed the root scene to the constructor function after and instanceof check
Checklist
This is my first ever pull request, the fix worked for me but I'm not sure if this is breaking some intentional design or I'm missing something else. Be GeNtLe :)