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

Add non-consuming as_json method to Document struct #406

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Add non-consuming as_json method to Document struct #406

merged 1 commit into from
Jan 25, 2024

Conversation

AlexTMjugador
Copy link
Contributor

@AlexTMjugador AlexTMjugador commented Dec 21, 2023

Currently, when serializing a glTF Document, the responsible code must own the Document to call the into_json method. However, this requirement deviates from the usual read-only borrow requirement for serializing data. To overcome this, client code can arrange for passing around references to the inner Root struct, but doing so sacrifices the benefits provided by the Gltf and Document structs for better binary data handling. (The Deref implementation for Gltf can also be used, but its syntax is somewhat unintuitive, and that does not apply to Documents themselves.)

To improve on this, let's introduce a non-consuming as_json unwrapping method. This method takes the Document by reference and returns a reference to the inner Root structure. By doing this, its usage across various use cases is simplified, making the code more straightforward and intuitive.

Currently, when serializing a glTF `Document`, the responsible code must own the
`Document` to call the `into_json` method. However, this requirement deviates
from the usual read-only borrow requirement for serializing data. To overcome
this, client code can arrange for passing around references to the inner `Root`
struct, but doing so sacrifices the benefits provided by the `Gltf` and
`Document` structs for better binary data handling.

To improve on this, let's introduce a non-consuming `as_json` unwrapping method.
This method takes the `Document` by reference and returns a reference to the
inner `Root` structure. By doing this, its usage across various use cases is
simplified, making the code more straightforward and intuitive.
@AlexTMjugador
Copy link
Contributor Author

Hi @alteous! I noticed that since I opened this PR the issue #409 was created. Do you think this PR aligns with the direction you want to move this crate forward to?

@alteous
Copy link
Member

alteous commented Jan 25, 2024

This is fine, thanks. 👍

@alteous alteous merged commit d7750db into gltf-rs:main Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants