store ID and selectability in GLTF without relying on name

[andrewheumann]

BACKGROUND:

• In early days of Elements, the name of the object in the GLTF wasn't utilized for anything. Over time, the Hypar front end has evolved to rely on parsing out critical information from the name — specifically, the element Id, and whether or not it is meant to be selectable.

DESCRIPTION:

• Establishes an ad-hoc gltf extension called HYPAR_info which contains information necessary for the hypar front end to connect element data to GLTF mesh objects.

TESTING:

• I added a test which verifies that the data is written correctly to the GLTF.
• I have an in-progress explore branch which can read this info if it's present.

REQUIRED:

• All changes are up to date in CHANGELOG.md.

---

This change is 

[wynged]

Reviewed 5 of 7 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @andrewheumann)

---

Elements/src/Serialization/glTF/NodeUtilities.cs line 171 at r2 (raw file):

        }

        internal static int CreateNodeForMesh(int meshId, List<glTFLoader.Schema.Node> nodes, Guid elementId, Transform transform = null)

seems like elementId this should also be optional with default=null since it's optional in all other methods that take an elementId? Is there some reason we are sure to have a value here but not in the other methods?

---

Elements/src/Serialization/glTF/NodeUtilities.cs line 195 at r2 (raw file):

            if (selectable.HasValue)
            {
                node.Extensions["HYPAR_info"] = new Dictionary<string, object>

nit. I think my code preference would be to make the dictionary with the "id", optionally add the "selectable" property and then always assign the dictionary to the dictionary to Extensions["HYPAR_info"] once the correct dictionary is built. feels like a more extensible pattern.

[wynged]

mostly just nits.

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @andrewheumann)

[andrewheumann]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @wynged)

---

Elements/src/Serialization/glTF/NodeUtilities.cs line 171 at r2 (raw file):

Previously, wynged (Eric Wassail) wrote…

seems like elementId this should also be optional with default=null since it's optional in all other methods that take an elementId? Is there some reason we are sure to have a value here but not in the other methods?

Done.

---

Elements/src/Serialization/glTF/NodeUtilities.cs line 195 at r2 (raw file):

Previously, wynged (Eric Wassail) wrote…

nit. I think my code preference would be to make the dictionary with the "id", optionally add the "selectable" property and then always assign the dictionary to the dictionary to Extensions["HYPAR_info"] once the correct dictionary is built. feels like a more extensible pattern.

Done.

[wynged]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: complete! 1 of 1 approvals obtained