vpenades / SharpGLTF

glTF reader and writer for .NET Standard
MIT License
457 stars 72 forks source link

Allow client code to supply its own extensions #94

Closed emackey closed 3 years ago

emackey commented 3 years ago

I'm not sure if you want this change or not, but this allows client apps to register and use their own extensions.

Fixes #93.

vpenades commented 3 years ago

The reason why this constructor is needed is because I use a trick to connect parents and children, which allows bidirectional navigation through the schema tree, and also it's being used by the deserializer. So, to some degree it's an implementation detail and not a constructor that should be exposed to the end user.

Since the current code is internal, it's not possible for users right now to call the API, and since theoretically I know what I'm doing, I am careful to call the constructor with the right parent object.

By making it public, it could be possible for a user to create a Node Extension, using Node A, and then adding the extension to NodeB, so additional checks would be needed to ensure the owner object is correctly set on the extension when you add it to the Node. But that's not possible because it's not stored anywhere because it's only used in some constructors and not stored anyway.

So, by making it public, and callable by users, it would allow users to do mistakes, and there would be no way to check what went wrong.

What's really needed is a UseExtension<> method that calls to ExtensionsFactory.Create if the extension doesn't exist.

Something like this:

public T UseExtension<T>() where T : JsonSerializable
        {
            var value = GetExtension<T>();
            if (value != null) return value;

            var name = ExtensionsFactory.Identify(this.GetType(), typeof(T));
            Guard.NotNull(name, nameof(T));

            value = ExtensionsFactory.Create(this, name) as T;
            Guard.NotNull(value, nameof(T));

            _extensions.Add(value);

            return value;
        }

By enforcing the creation of extensions through this method, you ensure two things:

emackey commented 3 years ago

Ah OK, I'll close the PR, no worries.