-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[Merged by Bors] - add #[reflect(Default)]
to create default value for reflected types
#3733
Conversation
Maybe |
Personally, I think we should add both because there will probably be cases where we need to create the "default" from world state using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
use crate::{FromType, Reflect}; | ||
|
||
#[derive(Clone)] | ||
pub struct ReflectDefault { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I know it's a relatively straightforward struct, but I think a doc comment could still be nice.
@@ -24,6 +24,8 @@ mod impls { | |||
} | |||
|
|||
pub mod serde; | |||
pub mod std_traits; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Can you go from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a really solid, and probably quite important addition. Would be incredibly helpful for any use case of reflection.
@@ -24,6 +24,8 @@ mod impls { | |||
} | |||
|
|||
pub mod serde; | |||
pub mod std_traits; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very simple and solid. @jakobhellermann can you rebase and add the module the std_traits
module to the prelude?
Rebased and added |
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?
#[reflect(Default)]
to create default value for reflected types#[reflect(Default)]
to create default value for reflected types
…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 <[email protected]>
…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 <[email protected]>
…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 <[email protected]>
…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 <[email protected]>
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
ofReflectComponent
to add a new component to an entity because you can't know what the initial value should be.Solution
ReflectDefault
type#[reflect(Default)]
to all component types that implementDefault
and are user facing (so notComputedSize
,CubemapVisibleEntities
etc.)This makes it possible to add the default value of a component to an entity without any compile-time information:
Open questions
ReflectDefault
orReflectFromWorld
or both?