Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Handle the case where mesh2shape returns null #41

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

Elettrotecnica
Copy link

This can happen, for instance, when a gltf-model has not finished loading, hence a bounding box cannot be calculated. We try again after the object3D has been set in this case, the same as when "shape" is missing.

This can happen, for instance, when a gltf-model has not finished loading, hence a bounding box cannot be calculated. We try again after the object3D has been set in this case, the same as when "shape" is missing.
@diarmidmackenzie
Copy link
Member

Are you able to make this change to remove the parentheses, then I'll merge? Thank you.

@Elettrotecnica
Copy link
Author

Hi! I am not sure I understand what should be changed, if you refer to this line:

({ shape, offset, orientation } = shapeInfo);

the round parentheses are syntax. One can do without only when the variable is being declared. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#syntax

I was personally not familiar with this syntax and found out the hard way :-)

@diarmidmackenzie
Copy link
Member

Ok, my error. Thanks for clarifying, and for the link.

Parentheses are required, because in the absence of a const, let or var, the opening brace for the destructuring assignment will be parsed as an opening brace of a new code block.

Including the parentheses allows the destructing assignment to work correctly.

@diarmidmackenzie diarmidmackenzie merged commit ea1630d into c-frame:master Aug 30, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants