-
-
Notifications
You must be signed in to change notification settings - Fork 35.4k
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
ObjectLoader: Add fallback for Geometry #15336
Conversation
src/Three.Legacy.js
Outdated
@@ -1888,14 +1888,6 @@ export function CanvasRenderer() { | |||
|
|||
// | |||
|
|||
export function JSONLoader() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's necessary to remove this entry otherwise 'JSONLoader' in THREE
always evaluates to true
. That's a bit unfortunate for users who just want to create an instance of JSONLoader
. They will get the runtime error THREE.JSONLoader is not a constructor
and not a log message if they don't manually include THREE.JSONLoader
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can keep that one and call the "new" one THREE.LegacyJSONLoader
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, renamed the loader from THREE.JSONLoader
to THREE.LegacyJSONLoader
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Phew, it's really not easy to get rid of the old stuff...
Yay! Thanks! |
Update the migration guide: https://github.com/mrdoob/three.js/wiki/Migration-Guide#r98--r99 |
It still doesn't work. |
It's animated character so it think you have to complain here: #15314 |
see #15310 (comment)