Remove confusion between node id and jointId

[pjcozzi]

As discussed in #193, tracing the indirection for skinning is pretty confusing. At the least, can we change the converter to not use the node name for a node's jointId? For example, do not do this:

        "locator047Node": {
            "children": [
                "locator051Node",
                "locator103Node",
                "locator106Node"
            ],
            "jointId": "locator047Node",

Then it will be clear when something (an animation, for example) is targeting the node or (a skin, for example) is referencing the jointId.

[pjcozzi]

Let's also consider renaming skins.joints to skins.jointIds. What do you think?

[pjcozzi]

Let's also consider renaming skins.joints to skins.jointIds. What do you think?

I'm working on the schema now. Thoughts on renaming skins.joints to skins.jointIds?

[fabrobinet]

in other places we always refer to ids but do not specify id, for instance in mesh we have material and not materialId why should this case be different ?

[fabrobinet]

also, it is a given as we can only refer object by ids (because of JSON), but maybe I missed something on this specific case

[fabrobinet]

I guess it is jointId who is inconsistent here

[pjcozzi]

Our convention is that the name of the JSON object, e.g., node, material, etc., is the id, and then we reference it without explicitly appending id as you said. Given this, perhaps it is OK to not rename skins.joints to skins.jointIds since jointId is really like a second id for a node. Do you have any other suggestions? I'm not sure that renaming jointId to joint or jointName is better.

[pjcozzi]

@fabrobinet thoughts here?

[fabrobinet]

to be consistent I am for renaming jointId to joint.

[pjcozzi]

Thinking about this more, I agree. I'll update the schema now. Can you update the converter in dev-6?

[fabrobinet]

yes that will be fixed for dev-6

[fabrobinet]

That is done, I'll push this along with other changes, or sooner if anyone wants to try it before. (it will be on format change branch in that case)

[fabrobinet]

@pjcozzi I am implementing 0.8 in my viewer and I believe the decision we took is wrong here. jointId was actually declaring "what is the joint id for a given node" e.g it was not a reference to another node. But still "id" as used in glTF was wrong as this jointId was not necessarily unique.
Instead I would propose to rename it jointName to reflect more what it is...
But now we have possibly name and jointName side by side, depending on situation they might be the same but they could also be different or name could be absent too.
Also we would need to rename skin.joints as skin.jointNames (or jointsNames ?) so thing would be clear.

[pjcozzi]

I don't know that it is needed, but I am open to:

• joint - > jointName
• skin.joints -> skin.jointNames

Let's discuss on the call today.

[fabrobinet]

Agreed today on the call, moving forward with this.

[pjcozzi]

Updated schema. 7e509fc

How does this impact instanceSkin.skeletons, which is currently defined as "Joint id (joint property) of skeleton nodes?" Should it just be "Joint name (jointName property)...?"

Can you please update the converter and binaries today? Otherwise, I will not be able to put them in Cesium 1.0.

[fabrobinet]

Yes "Joint name" is fine.

@pjcozzi converter for osx has been updated, I'll upload the windows version that @tfili will give me, but you can probably get it sooner from him directly.

[pjcozzi]

Thanks @fabrobinet. We were able to update Cesium quickly, cesium/#1995.

[fabrobinet]

NP. I am glad we are all set for 0.8.