Skip to content
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

Model - nodesByName - use id if name is undefined #4870

Closed
wants to merge 1 commit into from
Closed

Conversation

lilleyse
Copy link
Contributor

model.nodeTransformations references nodes by their "name" property, which won't work if the node does not have a name. Since its a bit ambiguous what is the id vs. the name, this PR allows the model._runtime.nodesByName to be the id if the name is undefined.

A different approach for the same problem is modifying getNode to look inside nodesByName first and then nodes. Same with getMesh and getMateral.

This came up on the forum: https://groups.google.com/forum/#!topic/cesium-dev/Eqttp9vcR90

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 16, 2017

In glTF, name and id are separate concepts. name is a user-defined application-facing identifier, id is a glTF internal identifier to link together parts of the asset.

I don't think we should mash them together here or anywhere.

In #927, the plan was

[ ] Add ById versions of functions like getNode?

We close close this for now or, if you think this is timely, @shehzan10 could take the ById approach.

@lilleyse
Copy link
Contributor Author

Fine with me to close now, this was just a proof of concept.

@lilleyse lilleyse closed this Jan 16, 2017
@lilleyse lilleyse deleted the model-name branch January 16, 2017 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants