Add Root::push() and Index::push() to assist with glTF construction.
#404
Conversation
|
In spirit, I like it. I must admit I have written similar functions. I'm hesitant on the naming/repurposing of Do you think we even need the |
Indeed, that's the part of this change I'm most unsure about.
That seems like a reasonable approach, and I like it a little better than the two methods Would you like me to rewrite this PR in terms of |
|
Yeah I think we should try |
These methods reduce the amount of boilerplate and possible mistakes involved in constructing a glTF asset from scratch. Most use cases are served by `Root::push()`; `Index::push()` is useful for cases like animation samplers where the vector of objects is not in the `Root`, or in algorithmically complex situations such as pushing both within and outside closures, which would produce borrow conflicts if `&mut Root` was required for everything. This introduces a new paradigm for child object access: implementing `AsMut<Vec<T>>`. This could also replace the `Get` trait's function via `AsRef<[T]>` implementations, but that would be a breaking change, so if that is to be done, let's do it separately. This commit includes those `AsRef` implementations, though, since they're harmless and parallel to the `AsMut` implementations.
This way, all `Index`es are guaranteed valid and the dependencies between the glTF objects are reflected in the program's variables. I have manually tested that the output of the example is valid glTF.
Done. It turned out that this could be a totally non-breaking change, leaving |
|
Thanks! 😄 |
These methods reduce the amount of boilerplate and possible mistakes involved in constructing a glTF asset from scratch.
Most use cases are served by
Root::push();Index::push()is useful for cases like animation samplers where the vector of objects is not in theRoot, or in algorithmically complex situations such as pushing both within and outside closures, which would produce borrow conflicts if&mut Rootwas required for everything.This requires some expansion of the(This PR no longer modifiesGettrait to support writing. By keeping the oldGet::get(), this is not a breaking change unless some dependent implementsGet, but if we choose to make general breaking changes, it would probably make sense to rename the trait, and perhaps even declare it an implementation detail, seal it, and hide its methods.Getand instead addsAsMut<Vec<T>>implementations.)This is based on a helper function I wrote and used extensively in my own code, and I've also verified that these methods work well in the same situations.