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

Improve Support for Custom Extensions in Edit Mode #13

Open
jbienzss opened this issue Jan 21, 2024 · 2 comments
Open

Improve Support for Custom Extensions in Edit Mode #13

jbienzss opened this issue Jan 21, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@jbienzss
Copy link

jbienzss commented Jan 21, 2024

Is your feature request related to a problem? Please describe.

Currently it's quite difficult to add support for custom glTF extensions in Edit Mode because GltfImporter cannot be customized.

glTFast uses JSON deserialization for extensions. The C# "schema class" must inherit from GLTFast.Schema.RootBase.

GltfImport (which defines the Root schema type) is small enough and easy to replace. But GltfImporter (the Edit Mode ScriptedImporter), is hard-coded to use GltfImport and this cannot be changed. GltfImporter is extensive and not easy to replace. Therefore, it's quite difficult to add support for custom extensions in Edit Mode.

Describe the solution you'd like

I would like any option for allowing the type of GltfImportBase<TRoot> created by GltfImporter to be customized. Some options might include:

  1. Allowing a custom factory method to be specified in EditorImportSettings or ImportSettings.
  2. Adding a method to the ImportAddon base class that would allow a custom GltfImportBase<TRoot> object to be created.
  3. Adding a settable factory Func<GltfImportBase<TRoot>> property directly on GltfImporter itself.

Any other options would great.

Describe alternatives you've considered

Rewriting my own GltfImporter and making it the default ScriptedImporter. This would be a lot of code and would likely get out of sync with the package implementation.

Additional context

Although the package actually includes a version of GltfImport that uses Newtonsoft, the Newtonsoft version cannot be used for Edit Mode import because of the same reasons listed above. This makes the Newtsonsoft GltfImport class only useful for runtime loading and not editor importing.

@jbienzss jbienzss added the enhancement New feature or request label Jan 21, 2024
@atteneder
Copy link
Collaborator

@jbienzss Thanks for bringing this up.

The concept of fully customizable glTF imports is indeed very, very powerful and we've only scratched the surface with what ImportAddon can do at the moment.

I don't yet understand point 2. ImportAddon(instances) are supposed to be attached to a GltfImportBase, not create one. Can you elaborate what that means?

I have to double check that Newtonsoft cannot be used. It would be a great to just use it as default and allow for slower, but much more flexible JSON parsing.

Not sure when I'll have time to investigate myself, but if you can beat me to it, feel free to provide a working PoC of your idea.

Thanks!

@jbienzss
Copy link
Author

jbienzss commented Jan 31, 2024

Hello @atteneder. Nice to chat with you and thank you for taking the time. BTW, my LinkedIn post about Unity officially supporting your library is one of my highest ranking posts ever (over 2,000 impressions and counting). Really happy to see this.

ImportAddon(instances) are supposed to be attached to a GltfImportBase, not create one. Can you elaborate what that means?

Yes, I acknowledge that ImportAddonInstance is probably not the best place for this. I was thinking that somewhere in the pipeline we need to be able to specify a new GltfImportBase that includes the schema for the supported extensions. Currently, the primary extension method for glTFast is implementing an ImportAddon + ImportAddonInstance. I was thinking that one of those two might be provided with a mechanism to replace the schema object. You're right that it would make more sense to do this in the Addon rather than the Instance. The instance works with the schema, not defines it.

The more I think about this problem though the more I don't like the idea of having any one particular addon in control of the schema. glTFast is of course responsible for the core glTF v2.0 spec and any extensions it would like to map out of the box. Ideally, addons would be able to map their own schemas without having to replace the ImportBase. As you mention, this is where Netwonsoft would shine.

In the Newtonsoft world, it would be nice if GltfImportBase held every extension it didn't know as a JObject. Then, the Addon could grab the JObject from the base schema and use JObject.ToObject<CustomSchema> to deserialize it. CustomSchema would be known and provided by the addon.

Right now it's easy to use the Newtsonsoft version of GltfImport rather of the default one in code - you just change the namespace you're using. But I do not see any method for getting GltfImporter (the ScriptedImporter) to switch versions. I'd be happy to be wrong about that so let me know if I'm missing something.

I plan to sit down in the next few nights, create a fork, and see if I can provide a meaningful PR that addresses these issues above. I appreciate you taking a look at it. I'll touch base soon.

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants