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

Inconsistent ordering of GLTF JSON name property #226

Open
ziriax opened this issue Dec 5, 2018 · 3 comments
Open

Inconsistent ordering of GLTF JSON name property #226

ziriax opened this issue Dec 5, 2018 · 3 comments

Comments

@ziriax
Copy link
Contributor

ziriax commented Dec 5, 2018

It seems that sometimes the name property is written first, and sometimes last.

That's because the base class GLTF::Object::writeJSON is not always called in the beginning of each override writeJSON method.

GLTF::Object::writeJSON also writes other properties like extras and extensions.

I think it is easier to debug and read the JSON when the name property always comes first?

@lasalvavida
Copy link
Contributor

Seems reasonable - I think we'll do this as part of #94.

@ziriax
Copy link
Contributor Author

ziriax commented Dec 5, 2018

Okay, nice. I already patched the code in a fork, and I also noticed that many objects derive from GLTF::Object are not children-of-the-root, and according to the official GLTF schema, these should not have a name. So we might need to add an extra base-class GLTF::ChildOfRoot or something, like in the schema.

@lasalvavida
Copy link
Contributor

lasalvavida commented Dec 5, 2018

Okay, nice. I already patched the code in a fork, and I also noticed that many objects derive from GLTF::Object are not children-of-the-root, and according to the official GLTF schema, these should not have a name.

Yes, I did see that conversation on the glTF repo - opened up #228 for tracking it here.

So we might need to add an extra base-class GLTF::ChildOfRoot or something, like in the schema.

I'd be open to that as a solution. Inheritance-wise it's probably easier to remove the concept of name from GLTF::Object and create a subclass called GLTF::RootObject that has a name and all root objects inherit from that. (edit: I misread, I think this is pretty much what you said above). Then, create a GLTF::NameExtra class that we can add to the extras on non-root objects with COLLADA names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants