-
Notifications
You must be signed in to change notification settings - Fork 250
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
Output correct glTF type based on extension #117
Conversation
@JoshuaStorm Can you add a test making sure the exception gets thrown? |
@lasalvavida I am actually unsure how we would go about testing |
@JoshuaStorm You should move this check into |
@@ -96,11 +97,12 @@ function processFile(inputPath, options, callback) { | |||
} | |||
|
|||
function writeFile(gltf, outputPath, options, callback) { | |||
var fileExtension = path.extname(outputPath); |
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.
I don't know how all of this is wired together, but can't we just make sure that binary
is set to true
before this is called; otherwise, this is forcing some of the command-line logic deeper in the engine.
I think this is fine for now. |
@pjcozzi I think this check and exception should be thrown in |
@lasalvavida That makes sense, I imagine we should do the same for the other two exceptions in |
@JoshuaStorm That sounds good to me. Maybe add a |
@lasalvavida On second thought, wouldn't it make most sense to check for these exceptions within the |
That could work too; I'm fine with either. |
@lasalvavida please merge when you guys are happy. |
Tested; this resolves #116 correctly. Good job @JoshuaStorm! |
See #116
Also added a
DeveloperError
to be thrown if output path given has neither.glb
or.gltf
file extensions.