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

docs(example): Example how to insert reflected Component onto entities #17836

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

favilo
Copy link

@favilo favilo commented Feb 13, 2025

Objective

We needed an example that showed how to insert a Component onto an entity that was deserialized into a type that implements Reflect.

ReflectCommandExt::insert_reflect was not easy to discover.

Solution

This new example shows how to use insert_reflect(), while also showing how to deserialize data from glTF extras, and add the deserialized Components automatically to the corresponding scene's children entities on instantiation.

Testing

  • Did you test these changes? If so, how?

Tested locally on a Linux machine. Runs fine.

  • Are there any parts that need more testing?

It's an example using standard features, so I assume it'll work where ever

  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
$ cargo run --example load_gltf_components
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?

Linux desktop machine. I could potentially test on other platforms like wasm, but I haven't yet.

…into entities.

 # Objective

We needed an example that showed how to insert a `Component` onto an
entity that was deserialized into a type that implements `Reflect`.

`ReflectCommandExt::insert_reflect` was not easy to discover.

 ## Solution

This new example shows how to use `insert_reflect()`, while also showing how to
deserialize data from glTF extras, and add the deserialized `Component`s automatically
to the corresponding scene's children entities on instantiation.
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@ChristopherBiscardi
Copy link
Contributor

IMO this example should be greatly reduced to essentially only the deserialize_extra that is written and it should be moved to the reflection folder with the more minimal result. gltf extras handling is already shown in the gltf extras example.

Something closer to this would make sense, without any gltf code.

let json_component = r#"{"skein::tests::SomeThings":{"OneThing":{"name":"testing"}}}"#
let type_registry = type_registry.read();

let reflect_deserializer = ReflectDeserializer::new(&type_registry);
let mut deserializer = serde_json::Deserializer::from_str(value);
let reflect_value = reflect_deserializer
    .deserialize(json_component)
    .unwrap();

commands
    .entity(entity)
    .insert_reflect(reflect_value);

Also ron is used in the other examples, I'm not sure if the Bevy repo has a preference for ron over serde_json for the examples

Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@favilo
Copy link
Author

favilo commented Feb 13, 2025

@ChristopherBiscardi, what do you think about this example. I've got both json and ron deserializing, and verify that the component is constructed properly.

Copy link
Contributor

@ChristopherBiscardi ChristopherBiscardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall lgtm.

A lot of the example is the same as examples/reflection/reflection.rs but I think showing two different serializations (ron/json) is enough to validate the additional example even though insert_reflect could be added to the other example.

I also dislike Foo/Bar naming but the other example this is extended from also uses Foo/Bar so matching that is probably better for ease of recognizing commonalities. CI is failing on the variable foo though so that will have to change to merge.

Notably insert_reflect is hard to find in the documentation because it is on an extension trait, so having an example for it is a good thing.

@ChristopherBiscardi ChristopherBiscardi added C-Docs An addition or correction to our documentation S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-Reflection Runtime information about types labels Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Docs An addition or correction to our documentation S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants