Update GLTFExporter / GLTFLoader signatures #196
Conversation
| /** | ||
| * @deprecated parse() expects options as the fourth argument now. | ||
| */ | ||
| parse( | ||
| input: Object3D | Object3D[], | ||
| onDone: (gltf: { [key: string]: any }) => void, | ||
| options?: GLTFExporterOptions, | ||
| ): void; |
There was a problem hiding this comment.
how do you think about this @deprecated signature?
There was a problem hiding this comment.
I would get rid of this, because if someone used the method like this it would not work.
There was a problem hiding this comment.
It does work since the compat is still implemented: https://github.com/mrdoob/three.js/blob/c7d06c02e302ab9c20fe8b33eade4b61c6712654/examples/jsm/exporters/GLTFExporter.js#L93-L122
I assume that your potential opinion alternatives can be:
- "Using deprecated functions must be punished from TypeScript type checking".
- "We just don't want to track deprecated functions in the repository".
I will agree in either way.
Just want to make sure your / the repository's opinion for my further contributions 😄
There was a problem hiding this comment.
"We just don't want to track deprecated functions in the repository".
I think this is the most straight forward for us.
| /** | ||
| * @deprecated parse() expects options as the fourth argument now. | ||
| */ | ||
| parse( | ||
| input: Object3D | Object3D[], | ||
| onDone: (gltf: { [key: string]: any }) => void, | ||
| options?: GLTFExporterOptions, | ||
| ): void; |
There was a problem hiding this comment.
I would get rid of this, because if someone used the method like this it would not work.
|
I'm sorry for leaving this one for a long time, gonna fix this |
See: https://github.com/mrdoob/three.js/blob/c7d06c02e302ab9c20fe8b33eade4b61c6712654/examples/jsm/exporters/GLTFExporter.js#L93-L122 See: https://github.com/mrdoob/three.js/blob/f9c68e393a4176e18be6fe0b0a00192f3652c63a/docs/examples/en/exporters/GLTFExporter.html Context: three-types#170
add GLTFWriter and GLTFExporterPlugin The interfaces of GLTFWriter is not exhaustive yet See: https://github.com/mrdoob/three.js/blob/c7d06c02e302ab9c20fe8b33eade4b61c6712654/examples/jsm/exporters/GLTFExporter.js#L69-L91
|
rebased |
|
@joshuaellis re-requested the review 🙇 |
Will resolve #170
Why
To catch up with the latest GLTFExporter / GLTFLoader APIs
What
Links for clues are inside of each commit logs
Points need review
ErrorEventfor the error type ofonError. I've just followed how it was in GLTFLoader definitions but have no confidence about itErrororanyChecklist
master, next goesdev)masterchanges