[Merged by Bors] - add `#[reflect(Default)]` to create default value for reflected types #3733

jakobhellermann · 2022-01-20T22:33:05Z

Problem

It currently isn't possible to construct the default value of a reflected type. Because of that, it isn't possible to use add_component of ReflectComponent to add a new component to an entity because you can't know what the initial value should be.

Solution

add ReflectDefault type

#[derive(Clone)]
pub struct ReflectDefault {
    default: fn() -> Box<dyn Reflect>,
}

impl ReflectDefault {
    pub fn default(&self) -> Box<dyn Reflect> {
        (self.default)()
    }
}

impl<T: Reflect + Default> FromType<T> for ReflectDefault {
    fn from_type() -> Self {
        ReflectDefault {
            default: || Box::new(T::default()),
        }
    }
}

add #[reflect(Default)] to all component types that implement Default and are user facing (so not ComputedSize, CubemapVisibleEntities etc.)

This makes it possible to add the default value of a component to an entity without any compile-time information:

fn main() {
    let mut app = App::new();
    app.register_type::<Camera>();

    let type_registry = app.world.get_resource::<TypeRegistry>().unwrap();
    let type_registry = type_registry.read();

    let camera_registration = type_registry.get(std::any::TypeId::of::<Camera>()).unwrap();
    let reflect_default = camera_registration.data::<ReflectDefault>().unwrap();
    let reflect_component = camera_registration
        .data::<ReflectComponent>()
        .unwrap()
        .clone();

    let default = reflect_default.default();

    drop(type_registry);

    let entity = app.world.spawn().id();
    reflect_component.add_component(&mut app.world, entity, &*default);

    let camera = app.world.entity(entity).get::<Camera>().unwrap();
    dbg!(&camera);
}

Open questions

should we have ReflectDefault or ReflectFromWorld or both?

…user-facing

jakobhellermann · 2022-01-25T20:44:30Z

Maybe ReflectDefault should return a Box<dyn Any> and we could have a way to go from &Any -> &Reflect? That way you could also use default values for types where reflect isn't implemented.

MrGVSV · 2022-03-06T08:39:34Z

should we have ReflectDefault or ReflectFromWorld or both?

Personally, I think we should add both because there will probably be cases where we need to create the "default" from world state using ReflectFromWorld. But we also need ReflectDefault to be able to create an instance when we don't have or need the world.

MrGVSV

LGTM!

I think if you do want to add ReflectFromWorld, maybe that should go in another PR? I think this one is pretty good as is.

MrGVSV · 2022-04-23T04:17:20Z

crates/bevy_reflect/src/std_traits.rs

+use crate::{FromType, Reflect};
+
+#[derive(Clone)]
+pub struct ReflectDefault {


Nit: I know it's a relatively straightforward struct, but I think a doc comment could still be nice.

MrGVSV · 2022-04-23T04:20:48Z

crates/bevy_reflect/src/lib.rs

@@ -24,6 +24,8 @@ mod impls {
 }

 pub mod serde;
+pub mod std_traits;


I like this module. It makes it obvious where to find std-based Reflect*** structs.

Though, imo this module should also be put in the prelude since I can see them being used a lot (as far as reflection usage goes at least haha).

I agree that this makes more sense to go in prelude. Would be annoying to want to slap standard derives on a component and need to add one additional import to do that.

MrGVSV · 2022-04-23T04:26:29Z

Maybe ReflectDefault should return a Box<dyn Any> and we could have a way to go from &Any -> &Reflect? That way you could also use default values for types where reflect isn't implemented.

Can you go from &dyn Any to &dyn Reflect? I'm not sure that's possible (at least not easily).

PROMETHIA-27

I think this is a really solid, and probably quite important addition. Would be incredibly helpful for any use case of reflection.

PROMETHIA-27 · 2022-04-29T19:47:06Z

crates/bevy_reflect/src/lib.rs

@@ -24,6 +24,8 @@ mod impls {
 }

 pub mod serde;
+pub mod std_traits;


I agree that this makes more sense to go in prelude. Would be annoying to want to slap standard derives on a component and need to add one additional import to do that.

alice-i-cecile

Very simple and solid. @jakobhellermann can you rebase and add the module the std_traits module to the prelude?

jakobhellermann · 2022-05-03T08:53:21Z

Rebased and added std_traits to the bevy_reflect prelude.

alice-i-cecile · 2022-05-03T19:19:57Z

bors r+

…#3733) ### Problem It currently isn't possible to construct the default value of a reflected type. Because of that, it isn't possible to use `add_component` of `ReflectComponent` to add a new component to an entity because you can't know what the initial value should be. ### Solution 1. add `ReflectDefault` type ```rust #[derive(Clone)] pub struct ReflectDefault { default: fn() -> Box<dyn Reflect>, } impl ReflectDefault { pub fn default(&self) -> Box<dyn Reflect> { (self.default)() } } impl<T: Reflect + Default> FromType<T> for ReflectDefault { fn from_type() -> Self { ReflectDefault { default: || Box::new(T::default()), } } } ``` 2. add `#[reflect(Default)]` to all component types that implement `Default` and are user facing (so not `ComputedSize`, `CubemapVisibleEntities` etc.) This makes it possible to add the default value of a component to an entity without any compile-time information: ```rust fn main() { let mut app = App::new(); app.register_type::<Camera>(); let type_registry = app.world.get_resource::<TypeRegistry>().unwrap(); let type_registry = type_registry.read(); let camera_registration = type_registry.get(std::any::TypeId::of::<Camera>()).unwrap(); let reflect_default = camera_registration.data::<ReflectDefault>().unwrap(); let reflect_component = camera_registration .data::<ReflectComponent>() .unwrap() .clone(); let default = reflect_default.default(); drop(type_registry); let entity = app.world.spawn().id(); reflect_component.add_component(&mut app.world, entity, &*default); let camera = app.world.entity(entity).get::<Camera>().unwrap(); dbg!(&camera); } ``` ### Open questions - should we have `ReflectDefault` or `ReflectFromWorld` or both?

bors · 2022-05-03T19:33:35Z

Pull request successfully merged into main.

Build succeeded:

…bevyengine#3733) ### Problem It currently isn't possible to construct the default value of a reflected type. Because of that, it isn't possible to use `add_component` of `ReflectComponent` to add a new component to an entity because you can't know what the initial value should be. ### Solution 1. add `ReflectDefault` type ```rust #[derive(Clone)] pub struct ReflectDefault { default: fn() -> Box<dyn Reflect>, } impl ReflectDefault { pub fn default(&self) -> Box<dyn Reflect> { (self.default)() } } impl<T: Reflect + Default> FromType<T> for ReflectDefault { fn from_type() -> Self { ReflectDefault { default: || Box::new(T::default()), } } } ``` 2. add `#[reflect(Default)]` to all component types that implement `Default` and are user facing (so not `ComputedSize`, `CubemapVisibleEntities` etc.) This makes it possible to add the default value of a component to an entity without any compile-time information: ```rust fn main() { let mut app = App::new(); app.register_type::<Camera>(); let type_registry = app.world.get_resource::<TypeRegistry>().unwrap(); let type_registry = type_registry.read(); let camera_registration = type_registry.get(std::any::TypeId::of::<Camera>()).unwrap(); let reflect_default = camera_registration.data::<ReflectDefault>().unwrap(); let reflect_component = camera_registration .data::<ReflectComponent>() .unwrap() .clone(); let default = reflect_default.default(); drop(type_registry); let entity = app.world.spawn().id(); reflect_component.add_component(&mut app.world, entity, &*default); let camera = app.world.entity(entity).get::<Camera>().unwrap(); dbg!(&camera); } ``` ### Open questions - should we have `ReflectDefault` or `ReflectFromWorld` or both?

…4140) # Objective Currently, `FromReflect` makes a couple assumptions: * Ignored fields must implement `Default` * Active fields must implement `FromReflect` * The reflected must be fully populated for active fields (can't use an empty `DynamicStruct`) However, one or both of these requirements might be unachievable, such as for external types. In these cases, it might be nice to tell `FromReflect` to use a custom default. ## Solution Added the `#[reflect(default)]` derive helper attribute. This attribute can be applied to any field (ignored or not) and will allow a default value to be specified in place of the regular `from_reflect()` call. It takes two forms: `#[reflect(default)]` and `#[reflect(default = "some_func")]`. The former specifies that `Default::default()` should be used while the latter specifies that `some_func()` should be used. This is pretty much [how serde does it](https://serde.rs/field-attrs.html#default). ### Example ```rust #[derive(Reflect, FromReflect)] struct MyStruct { // Use `Default::default()` #[reflect(default)] foo: String, // Use `get_bar_default()` #[reflect(default = "get_bar_default")] #[reflect(ignore)] bar: usize, } fn get_bar_default() -> usize { 123 } ``` ### Active Fields As an added benefit, this also allows active fields to be completely missing from their dynamic object. This is because the attribute tells `FromReflect` how to handle missing active fields (it still tries to use `from_reflect` first so the `FromReflect` trait is still required). ```rust let dyn_struct = DynamicStruct::default(); // We can do this without actually including the active fields since they have `#[reflect(default)]` let my_struct = <MyStruct as FromReflect>::from_reflect(&dyn_struct); ``` ### Container Defaults Also, with the addition of #3733, people will likely start adding `#[reflect(Default)]` to their types now. Just like with the fields, we can use this to mark the entire container as "defaultable". This grants us the ability to completely remove the field markers altogether if our type implements `Default` (and we're okay with fields using that instead of their own `Default` impls): ```rust #[derive(Reflect, FromReflect)] #[reflect(Default)] struct MyStruct { foo: String, #[reflect(ignore)] bar: usize, } impl Default for MyStruct { fn default() -> Self { Self { foo: String::from("Hello"), bar: 123, } } } // Again, we can now construct this from nothing pretty much let dyn_struct = DynamicStruct::default(); let my_struct = <MyStruct as FromReflect>::from_reflect(&dyn_struct); ``` Now if _any_ field is missing when using `FromReflect`, we simply fallback onto the container's `Default` implementation. This behavior can be completely overridden on a per-field basis, of course, by simply defining those same field attributes like before. ### Related * #3733 * #1395 * #2377 --- ## Changelog * Added `#[reflect(default)]` field attribute for `FromReflect` * Allows missing fields to be given a default value when using `FromReflect` * `#[reflect(default)]` - Use the field's `Default` implementation * `#[reflect(default = "some_fn")]` - Use a custom function to get the default value * Allow `#[reflect(Default)]` to have a secondary usage as a container attribute * Allows missing fields to be given a default value based on the container's `Default` impl when using `FromReflect` Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>

…evyengine#4140) # Objective Currently, `FromReflect` makes a couple assumptions: * Ignored fields must implement `Default` * Active fields must implement `FromReflect` * The reflected must be fully populated for active fields (can't use an empty `DynamicStruct`) However, one or both of these requirements might be unachievable, such as for external types. In these cases, it might be nice to tell `FromReflect` to use a custom default. ## Solution Added the `#[reflect(default)]` derive helper attribute. This attribute can be applied to any field (ignored or not) and will allow a default value to be specified in place of the regular `from_reflect()` call. It takes two forms: `#[reflect(default)]` and `#[reflect(default = "some_func")]`. The former specifies that `Default::default()` should be used while the latter specifies that `some_func()` should be used. This is pretty much [how serde does it](https://serde.rs/field-attrs.html#default). ### Example ```rust #[derive(Reflect, FromReflect)] struct MyStruct { // Use `Default::default()` #[reflect(default)] foo: String, // Use `get_bar_default()` #[reflect(default = "get_bar_default")] #[reflect(ignore)] bar: usize, } fn get_bar_default() -> usize { 123 } ``` ### Active Fields As an added benefit, this also allows active fields to be completely missing from their dynamic object. This is because the attribute tells `FromReflect` how to handle missing active fields (it still tries to use `from_reflect` first so the `FromReflect` trait is still required). ```rust let dyn_struct = DynamicStruct::default(); // We can do this without actually including the active fields since they have `#[reflect(default)]` let my_struct = <MyStruct as FromReflect>::from_reflect(&dyn_struct); ``` ### Container Defaults Also, with the addition of bevyengine#3733, people will likely start adding `#[reflect(Default)]` to their types now. Just like with the fields, we can use this to mark the entire container as "defaultable". This grants us the ability to completely remove the field markers altogether if our type implements `Default` (and we're okay with fields using that instead of their own `Default` impls): ```rust #[derive(Reflect, FromReflect)] #[reflect(Default)] struct MyStruct { foo: String, #[reflect(ignore)] bar: usize, } impl Default for MyStruct { fn default() -> Self { Self { foo: String::from("Hello"), bar: 123, } } } // Again, we can now construct this from nothing pretty much let dyn_struct = DynamicStruct::default(); let my_struct = <MyStruct as FromReflect>::from_reflect(&dyn_struct); ``` Now if _any_ field is missing when using `FromReflect`, we simply fallback onto the container's `Default` implementation. This behavior can be completely overridden on a per-field basis, of course, by simply defining those same field attributes like before. ### Related * bevyengine#3733 * bevyengine#1395 * bevyengine#2377 --- ## Changelog * Added `#[reflect(default)]` field attribute for `FromReflect` * Allows missing fields to be given a default value when using `FromReflect` * `#[reflect(default)]` - Use the field's `Default` implementation * `#[reflect(default = "some_fn")]` - Use a custom function to get the default value * Allow `#[reflect(Default)]` to have a secondary usage as a container attribute * Allows missing fields to be given a default value based on the container's `Default` impl when using `FromReflect` Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>

…bevyengine#3733) ### Problem It currently isn't possible to construct the default value of a reflected type. Because of that, it isn't possible to use `add_component` of `ReflectComponent` to add a new component to an entity because you can't know what the initial value should be. ### Solution 1. add `ReflectDefault` type ```rust #[derive(Clone)] pub struct ReflectDefault { default: fn() -> Box<dyn Reflect>, } impl ReflectDefault { pub fn default(&self) -> Box<dyn Reflect> { (self.default)() } } impl<T: Reflect + Default> FromType<T> for ReflectDefault { fn from_type() -> Self { ReflectDefault { default: || Box::new(T::default()), } } } ``` 2. add `#[reflect(Default)]` to all component types that implement `Default` and are user facing (so not `ComputedSize`, `CubemapVisibleEntities` etc.) This makes it possible to add the default value of a component to an entity without any compile-time information: ```rust fn main() { let mut app = App::new(); app.register_type::<Camera>(); let type_registry = app.world.get_resource::<TypeRegistry>().unwrap(); let type_registry = type_registry.read(); let camera_registration = type_registry.get(std::any::TypeId::of::<Camera>()).unwrap(); let reflect_default = camera_registration.data::<ReflectDefault>().unwrap(); let reflect_component = camera_registration .data::<ReflectComponent>() .unwrap() .clone(); let default = reflect_default.default(); drop(type_registry); let entity = app.world.spawn().id(); reflect_component.add_component(&mut app.world, entity, &*default); let camera = app.world.entity(entity).get::<Camera>().unwrap(); dbg!(&camera); } ``` ### Open questions - should we have `ReflectDefault` or `ReflectFromWorld` or both?

…evyengine#4140) # Objective Currently, `FromReflect` makes a couple assumptions: * Ignored fields must implement `Default` * Active fields must implement `FromReflect` * The reflected must be fully populated for active fields (can't use an empty `DynamicStruct`) However, one or both of these requirements might be unachievable, such as for external types. In these cases, it might be nice to tell `FromReflect` to use a custom default. ## Solution Added the `#[reflect(default)]` derive helper attribute. This attribute can be applied to any field (ignored or not) and will allow a default value to be specified in place of the regular `from_reflect()` call. It takes two forms: `#[reflect(default)]` and `#[reflect(default = "some_func")]`. The former specifies that `Default::default()` should be used while the latter specifies that `some_func()` should be used. This is pretty much [how serde does it](https://serde.rs/field-attrs.html#default). ### Example ```rust #[derive(Reflect, FromReflect)] struct MyStruct { // Use `Default::default()` #[reflect(default)] foo: String, // Use `get_bar_default()` #[reflect(default = "get_bar_default")] #[reflect(ignore)] bar: usize, } fn get_bar_default() -> usize { 123 } ``` ### Active Fields As an added benefit, this also allows active fields to be completely missing from their dynamic object. This is because the attribute tells `FromReflect` how to handle missing active fields (it still tries to use `from_reflect` first so the `FromReflect` trait is still required). ```rust let dyn_struct = DynamicStruct::default(); // We can do this without actually including the active fields since they have `#[reflect(default)]` let my_struct = <MyStruct as FromReflect>::from_reflect(&dyn_struct); ``` ### Container Defaults Also, with the addition of bevyengine#3733, people will likely start adding `#[reflect(Default)]` to their types now. Just like with the fields, we can use this to mark the entire container as "defaultable". This grants us the ability to completely remove the field markers altogether if our type implements `Default` (and we're okay with fields using that instead of their own `Default` impls): ```rust #[derive(Reflect, FromReflect)] #[reflect(Default)] struct MyStruct { foo: String, #[reflect(ignore)] bar: usize, } impl Default for MyStruct { fn default() -> Self { Self { foo: String::from("Hello"), bar: 123, } } } // Again, we can now construct this from nothing pretty much let dyn_struct = DynamicStruct::default(); let my_struct = <MyStruct as FromReflect>::from_reflect(&dyn_struct); ``` Now if _any_ field is missing when using `FromReflect`, we simply fallback onto the container's `Default` implementation. This behavior can be completely overridden on a per-field basis, of course, by simply defining those same field attributes like before. ### Related * bevyengine#3733 * bevyengine#1395 * bevyengine#2377 --- ## Changelog * Added `#[reflect(default)]` field attribute for `FromReflect` * Allows missing fields to be given a default value when using `FromReflect` * `#[reflect(default)]` - Use the field's `Default` implementation * `#[reflect(default = "some_fn")]` - Use a custom function to get the default value * Allow `#[reflect(Default)]` to have a secondary usage as a container attribute * Allows missing fields to be given a default value based on the container's `Default` impl when using `FromReflect` Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>

jakobhellermann added 2 commits January 20, 2022 23:04

add #[reflect(Default)]

6c4e2cc

add #[reflect(Default)] to components that implement Default and are …

8f9ef02

…user-facing

jakobhellermann added A-Reflection Runtime information about types C-Enhancement A new feature labels Jan 20, 2022

github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 20, 2022

jakobhellermann removed the S-Needs-Triage This issue needs to be labelled label Jan 20, 2022

jakobhellermann mentioned this pull request Jan 20, 2022

Feature request: Add/remove component buttons jakobhellermann/bevy_editor_pls#9

Open

Reflect for NotShadow{Caster,Receiver}

11b293d

MrGVSV mentioned this pull request Mar 7, 2022

[Merged by Bors] - bevy_reflect: Add #[reflect(default)] attribute for FromReflect #4140

Closed

MrGVSV approved these changes Apr 23, 2022

View reviewed changes

alice-i-cecile added this to the Bevy 0.8 milestone Apr 25, 2022

MrGVSV mentioned this pull request Apr 27, 2022

[Merged by Bors] - Add macro to implement reflect for struct types and migrate glam types #4540

Closed

PROMETHIA-27 reviewed Apr 29, 2022

View reviewed changes

alice-i-cecile requested changes May 2, 2022

View reviewed changes

jakobhellermann added 2 commits May 3, 2022 10:09

add std_traits to prelude

091e41f

Merge branch 'main' into reflect-default

516b1ee

add doc comment

7ca8801

bors bot changed the title ~~add #[reflect(Default)] to create default value for reflected types~~ [Merged by Bors] - add #[reflect(Default)] to create default value for reflected types May 3, 2022

bors bot closed this May 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - add `#[reflect(Default)]` to create default value for reflected types #3733

[Merged by Bors] - add `#[reflect(Default)]` to create default value for reflected types #3733

jakobhellermann commented Jan 20, 2022 •

edited

Loading

jakobhellermann commented Jan 25, 2022

MrGVSV commented Mar 6, 2022

MrGVSV left a comment

MrGVSV Apr 23, 2022

MrGVSV Apr 23, 2022

PROMETHIA-27 Apr 29, 2022

MrGVSV commented Apr 23, 2022

PROMETHIA-27 left a comment •

edited

Loading

PROMETHIA-27 Apr 29, 2022

alice-i-cecile left a comment

jakobhellermann commented May 3, 2022

alice-i-cecile commented May 3, 2022

bors bot commented May 3, 2022

[Merged by Bors] - add #[reflect(Default)] to create default value for reflected types #3733

[Merged by Bors] - add #[reflect(Default)] to create default value for reflected types #3733

Conversation

jakobhellermann commented Jan 20, 2022 • edited Loading

Problem

Solution

Open questions

jakobhellermann commented Jan 25, 2022

MrGVSV commented Mar 6, 2022

MrGVSV left a comment

Choose a reason for hiding this comment

MrGVSV Apr 23, 2022

Choose a reason for hiding this comment

MrGVSV Apr 23, 2022

Choose a reason for hiding this comment

PROMETHIA-27 Apr 29, 2022

Choose a reason for hiding this comment

MrGVSV commented Apr 23, 2022

PROMETHIA-27 left a comment • edited Loading

Choose a reason for hiding this comment

PROMETHIA-27 Apr 29, 2022

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

jakobhellermann commented May 3, 2022

alice-i-cecile commented May 3, 2022

bors bot commented May 3, 2022

[Merged by Bors] - add `#[reflect(Default)]` to create default value for reflected types #3733

[Merged by Bors] - add `#[reflect(Default)]` to create default value for reflected types #3733

jakobhellermann commented Jan 20, 2022 •

edited

Loading

PROMETHIA-27 left a comment •

edited

Loading