Mixedrealitytoolkit-unity: [Suggestion] Use the base open standards repository for gLTF rather than embedded classes

[SimonDarksideJ]

Overview

gLFT is a standard and the Kronos group already provide an official GitHub repo source for this functionality.

I would recommend we SubModule this functionality rather than use separate copied code in the repo

https://github.com/KhronosGroup/UnityGLTF

Unless there is some reason for providing alternate code for this feature.

[StephenHodgson]

We've talked about this a few times before, but ultimately we decided submodules raise the barrier to entry for users with little to no experience using git.

[SimonDarksideJ]

True, however, it is standard use in GitHub and those users are more likely to only use the packaged MRTK asset which should include the version of the gLTF code (only the bits used by MRTK) in the package.

The primary mechanism for uses picking up the toolkit should be the compiled asset and not source. Source is for contributors and we can provide docs for how to get started, downloading the project and submitting for contributors

[StephenHodgson]

Agreed in all counts, but using submodules directly inside a Unity project is quite complex and has some substantial setup steps and advanced concepts.

[SimonDarksideJ]

Except you wouldn't have a submodule in Unity :D
you would either have:

• Source from the asset package (which would contain the required source), each new asset uses the latest source directly.
• Source from the Repo, would be just another folder. Only difference is that it's someone elses code. Not directly managed by MRTK. PR's could be needed to update the commit reference for the submodule to use a newer version.

Currently we are likely lagging behind improvements from the standard. (Which was updated only days ago)

[StephenHodgson]

It would be nice if they actually published releases in the form of asset packages as well.

[SimonDarksideJ]

Ha ha, Now that's a thought. I'll raise an issue on their Site. Would make life much easier :D

[StephenHodgson]

I already did, but realized I submitted a duplicate.

[SimonDarksideJ]

Yup, it's been there a while though
https://github.com/KhronosGroup/UnityGLTF/issues/7

[StephenHodgson]

There's this one too:
https://github.com/KhronosGroup/UnityGLTF/issues/80

[SimonDarksideJ]

Hmm, in Git lands, I think 7 trumps 80 :D

[keveleigh]

Currently we are likely lagging behind improvements from the standard. (Which was updated only days ago)

Good timing on bringing this up, as I've been migrating over the latest changes recently and updating some things, preparing a PR :)

For the controller model itself, everything we need is supported in the existing drop. There's been some refactoring work done in the UnityGLTF repo though (and some non-standard-related features I had to add here integrated in), so I'm bringing all that over now.

[SimonDarksideJ]

SO @keveleigh , are you looking to update the code in MRTK, or (better iMHO) create a submodule back to the base gLFT repo and test / update any integration code in MRTK?

[keveleigh]

I'm updating the code drop in the MRTK. We've had discussions in the past about submodules and tended to agree that it added a barrier to entry for the Toolkit that we didn't want. Specifically in this case, I'm not sure it's possible, as the UnityGLTF repo contains several projects, including non-Unity ones that we wouldn't want included in our project.

Is there something you feel a submodule adds to the project not accomplished by a commit reference in the readme, as we do now?

[SimonDarksideJ]

Mainly it's to avoid duplicate code and having to double up effort for a foreign repository.

As to causing devs issues, I'd have to disagree as the majority of devs will use the pre-packaged asset. Only experienced devs are likely to use source and most Git tools have a "Submodule" download.

Also having parts like this as submodules, ensure no one tries to tinker with the code :D (my 5p worth)

I've worked with a lot of projects that use submodules with external code / dependencies. Should avoid duplicating where possible.

[keveleigh]

Here's an additional discussion about the UnityGLTF repo as a submodule: https://github.com/KhronosGroup/UnityGLTF/issues/48

[wiwei]

Closing issues older than 180 days. If this is still an issue, please reactivate with recent information.