d3012df755
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
tmstorey
|
c55c69e3fc
|
Add NonNilUuid support to bevy_reflect (#18604)
# Objective - If using a `NonNilUuid` in Bevy, it's difficult to reflect it. ## Solution - Adds `NonNilUuid` using `impl_reflect_opaque!`. ## Testing - Built with no issues found locally. - Essentially the same as the `Uuid` support except without `Default`. Co-authored-by: TM Storey <mail@tmstorey.id.au> |
||
|
Gino Valente
|
f5210c54d2
|
bevy_reflect: Reflection-based cloning (#13432)
# Objective Using `Reflect::clone_value` can be somewhat confusing to those unfamiliar with how Bevy's reflection crate works. For example take the following code: ```rust let value: usize = 123; let clone: Box<dyn Reflect> = value.clone_value(); ``` What can we expect to be the underlying type of `clone`? If you guessed `usize`, then you're correct! Let's try another: ```rust #[derive(Reflect, Clone)] struct Foo(usize); let value: Foo = Foo(123); let clone: Box<dyn Reflect> = value.clone_value(); ``` What about this code? What is the underlying type of `clone`? If you guessed `Foo`, unfortunately you'd be wrong. It's actually `DynamicStruct`. It's not obvious that the generated `Reflect` impl actually calls `Struct::clone_dynamic` under the hood, which always returns `DynamicStruct`. There are already some efforts to make this a bit more apparent to the end-user: #7207 changes the signature of `Reflect::clone_value` to instead return `Box<dyn PartialReflect>`, signaling that we're potentially returning a dynamic type. But why _can't_ we return `Foo`? `Foo` can obviously be cloned— in fact, we already derived `Clone` on it. But even without the derive, this seems like something `Reflect` should be able to handle. Almost all types that implement `Reflect` either contain no data (trivially clonable), they contain a `#[reflect_value]` type (which, by definition, must implement `Clone`), or they contain another `Reflect` type (which recursively fall into one of these three categories). This PR aims to enable true reflection-based cloning where you get back exactly the type that you think you do. ## Solution Add a `Reflect::reflect_clone` method which returns `Result<Box<dyn Reflect>, ReflectCloneError>`, where the `Box<dyn Reflect>` is guaranteed to be the same type as `Self`. ```rust #[derive(Reflect)] struct Foo(usize); let value: Foo = Foo(123); let clone: Box<dyn Reflect> = value.reflect_clone().unwrap(); assert!(clone.is::<Foo>()); ``` Notice that we didn't even need to derive `Clone` for this to work: it's entirely powered via reflection! Under the hood, the macro generates something like this: ```rust fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> { Ok(Box::new(Self { // The `reflect_clone` impl for `usize` just makes use of its `Clone` impl 0: Reflect::reflect_clone(&self.0)?.take().map_err(/* ... */)?, })) } ``` If we did derive `Clone`, we can tell `Reflect` to rely on that instead: ```rust #[derive(Reflect, Clone)] #[reflect(Clone)] struct Foo(usize); ``` <details> <summary>Generated Code</summary> ```rust fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> { Ok(Box::new(Clone::clone(self))) } ``` </details> Or, we can specify our own cloning function: ```rust #[derive(Reflect)] #[reflect(Clone(incremental_clone))] struct Foo(usize); fn incremental_clone(value: &usize) -> usize { *value + 1 } ``` <details> <summary>Generated Code</summary> ```rust fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> { Ok(Box::new(incremental_clone(self))) } ``` </details> Similarly, we can specify how fields should be cloned. This is important for fields that are `#[reflect(ignore)]`'d as we otherwise have no way to know how they should be cloned. ```rust #[derive(Reflect)] struct Foo { #[reflect(ignore, clone)] bar: usize, #[reflect(ignore, clone = "incremental_clone")] baz: usize, } fn incremental_clone(value: &usize) -> usize { *value + 1 } ``` <details> <summary>Generated Code</summary> ```rust fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> { Ok(Box::new(Self { bar: Clone::clone(&self.bar), baz: incremental_clone(&self.baz), })) } ``` </details> If we don't supply a `clone` attribute for an ignored field, then the method will automatically return `Err(ReflectCloneError::FieldNotClonable {/* ... */})`. `Err` values "bubble up" to the caller. So if `Foo` contains `Bar` and the `reflect_clone` method for `Bar` returns `Err`, then the `reflect_clone` method for `Foo` also returns `Err`. ### Attribute Syntax You might have noticed the differing syntax between the container attribute and the field attribute. This was purely done for consistency with the current attributes. There are PRs aimed at improving this. #7317 aims at making the "special-cased" attributes more in line with the field attributes syntactically. And #9323 aims at moving away from the stringified paths in favor of just raw function paths. ### Compatibility with Unique Reflect This PR was designed with Unique Reflect (#7207) in mind. This method actually wouldn't change that much (if at all) under Unique Reflect. It would still exist on `Reflect` and it would still `Option<Box<dyn Reflect>>`. In fact, Unique Reflect would only _improve_ the user's understanding of what this method returns. We may consider moving what's currently `Reflect::clone_value` to `PartialReflect` and possibly renaming it to `partial_reflect_clone` or `clone_dynamic` to better indicate how it differs from `reflect_clone`. ## Testing You can test locally by running the following command: ``` cargo test --package bevy_reflect ``` --- ## Changelog - Added `Reflect::reflect_clone` method - Added `ReflectCloneError` error enum - Added `#[reflect(Clone)]` container attribute - Added `#[reflect(clone)]` field attribute |
||
|
raldone01
|
1b7db895b7
|
Harden proc macro path resolution and add integration tests. (#17330)
This pr uses the `extern crate self as` trick to make proc macros behave the same way inside and outside bevy. # Objective - Removes noise introduced by `crate as` in the whole bevy repo. - Fixes #17004. - Hardens proc macro path resolution. ## TODO - [x] `BevyManifest` needs cleanup. - [x] Cleanup remaining `crate as`. - [x] Add proper integration tests to the ci. ## Notes - `cargo-manifest-proc-macros` is written by me and based/inspired by the old `BevyManifest` implementation and [`bkchr/proc-macro-crate`](https://github.com/bkchr/proc-macro-crate). - What do you think about the new integration test machinery I added to the `ci`? More and better integration tests can be added at a later stage. The goal of these integration tests is to simulate an actual separate crate that uses bevy. Ideally they would lightly touch all bevy crates. ## Testing - Needs RA test - Needs testing from other users - Others need to run at least `cargo run -p ci integration-test` and verify that they work. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Gino Valente
|
83356b12c9
|
bevy_reflect: Replace "value" terminology with "opaque" (#15240)
# Objective Currently, the term "value" in the context of reflection is a bit overloaded. For one, it can be used synonymously with "data" or "variable". An example sentence would be "this function takes a reflected value". However, it is also used to refer to reflected types which are `ReflectKind::Value`. These types are usually either primitives, opaque types, or types that don't fall into any other `ReflectKind` (or perhaps could, but don't due to some limitation/difficulty). An example sentence would be "this function takes a reflected value type". This makes it difficult to write good documentation or other learning material without causing some amount of confusion to readers. Ideally, we'd be able to move away from the `ReflectKind::Value` usage and come up with a better term. ## Solution This PR replaces the terminology of "value" with "opaque" across `bevy_reflect`. This includes in documentation, type names, variant names, and macros. The term "opaque" was chosen because that's essentially how the type is treated within the reflection API. In other words, its internal structure is hidden. All we can do is work with the type itself. ### Primitives While primitives are not technically opaque types, I think it's still clearer to refer to them as "opaque" rather than keep the confusing "value" terminology. We could consider adding another concept for primitives (e.g. `ReflectKind::Primitive`), but I'm not sure that provides a lot of benefit right now. In most circumstances, they'll be treated just like an opaque type. They would also likely use the same macro (or two copies of the same macro but with different names). ## Testing You can test locally by running: ``` cargo test --package bevy_reflect --all-features ``` --- ## Migration Guide The reflection concept of "value type" has been replaced with a clearer "opaque type". The following renames have been made to account for this: - `ReflectKind::Value` → `ReflectKind::Opaque` - `ReflectRef::Value` → `ReflectRef::Opaque` - `ReflectMut::Value` → `ReflectMut::Opaque` - `ReflectOwned::Value` → `ReflectOwned::Opaque` - `TypeInfo::Value` → `TypeInfo::Opaque` - `ValueInfo` → `OpaqueInfo` - `impl_reflect_value!` → `impl_reflect_opaque!` - `impl_from_reflect_value!` → `impl_from_reflect_opaque!` Additionally, declaring your own opaque types no longer uses `#[reflect_value]`. This attribute has been replaced by `#[reflect(opaque)]`: ```rust // BEFORE #[derive(Reflect)] #[reflect_value(Default)] struct MyOpaqueType(u32); // AFTER #[derive(Reflect)] #[reflect(opaque)] #[reflect(Default)] struct MyOpaqueType(u32); ``` Note that the order in which `#[reflect(opaque)]` appears does not matter. |
||
|
James Liu
|
512b7463a3
|
Disentangle bevy_utils/bevy_core's reexported dependencies (#12313)
# Objective Make bevy_utils less of a compilation bottleneck. Tackle #11478. ## Solution * Move all of the directly reexported dependencies and move them to where they're actually used. * Remove the UUID utilities that have gone unused since `TypePath` took over for `TypeUuid`. * There was also a extraneous bytemuck dependency on `bevy_core` that has not been used for a long time (since `encase` became the primary way to prepare GPU buffers). * Remove the `all_tuples` macro reexport from bevy_ecs since it's accessible from `bevy_utils`. --- ## Changelog Removed: Many of the reexports from bevy_utils (petgraph, uuid, nonmax, smallvec, and thiserror). Removed: bevy_core's reexports of bytemuck. ## Migration Guide bevy_utils' reexports of petgraph, uuid, nonmax, smallvec, and thiserror have been removed. bevy_core' reexports of bytemuck's types has been removed. Add them as dependencies in your own crate instead. |
||
|
Gino Valente
|
aeeb20ec4c
|
bevy_reflect: FromReflect Ergonomics Implementation (#6056)
# Objective **This implementation is based on https://github.com/bevyengine/rfcs/pull/59.** --- Resolves #4597 Full details and motivation can be found in the RFC, but here's a brief summary. `FromReflect` is a very powerful and important trait within the reflection API. It allows Dynamic types (e.g., `DynamicList`, etc.) to be formed into Real ones (e.g., `Vec<i32>`, etc.). This mainly comes into play concerning deserialization, where the reflection deserializers both return a `Box<dyn Reflect>` that almost always contain one of these Dynamic representations of a Real type. To convert this to our Real type, we need to use `FromReflect`. It also sneaks up in other ways. For example, it's a required bound for `T` in `Vec<T>` so that `Vec<T>` as a whole can be made `FromReflect`. It's also required by all fields of an enum as it's used as part of the `Reflect::apply` implementation. So in other words, much like `GetTypeRegistration` and `Typed`, it is very much a core reflection trait. The problem is that it is not currently treated like a core trait and is not automatically derived alongside `Reflect`. This makes using it a bit cumbersome and easy to forget. ## Solution Automatically derive `FromReflect` when deriving `Reflect`. Users can then choose to opt-out if needed using the `#[reflect(from_reflect = false)]` attribute. ```rust #[derive(Reflect)] struct Foo; #[derive(Reflect)] #[reflect(from_reflect = false)] struct Bar; fn test<T: FromReflect>(value: T) {} test(Foo); // <-- OK test(Bar); // <-- Panic! Bar does not implement trait `FromReflect` ``` #### `ReflectFromReflect` This PR also automatically adds the `ReflectFromReflect` (introduced in #6245) registration to the derived `GetTypeRegistration` impl— if the type hasn't opted out of `FromReflect` of course. <details> <summary><h4>Improved Deserialization</h4></summary> > **Warning** > This section includes changes that have since been descoped from this PR. They will likely be implemented again in a followup PR. I am mainly leaving these details in for archival purposes, as well as for reference when implementing this logic again. And since we can do all the above, we might as well improve deserialization. We can now choose to deserialize into a Dynamic type or automatically convert it using `FromReflect` under the hood. `[Un]TypedReflectDeserializer::new` will now perform the conversion and return the `Box`'d Real type. `[Un]TypedReflectDeserializer::new_dynamic` will work like what we have now and simply return the `Box`'d Dynamic type. ```rust // Returns the Real type let reflect_deserializer = UntypedReflectDeserializer::new(®istry); let mut deserializer = ron:🇩🇪:Deserializer::from_str(input)?; let output: SomeStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?; // Returns the Dynamic type let reflect_deserializer = UntypedReflectDeserializer::new_dynamic(®istry); let mut deserializer = ron:🇩🇪:Deserializer::from_str(input)?; let output: DynamicStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?; ``` </details> --- ## Changelog * `FromReflect` is now automatically derived within the `Reflect` derive macro * This includes auto-registering `ReflectFromReflect` in the derived `GetTypeRegistration` impl * ~~Renamed `TypedReflectDeserializer::new` and `UntypedReflectDeserializer::new` to `TypedReflectDeserializer::new_dynamic` and `UntypedReflectDeserializer::new_dynamic`, respectively~~ **Descoped** * ~~Changed `TypedReflectDeserializer::new` and `UntypedReflectDeserializer::new` to automatically convert the deserialized output using `FromReflect`~~ **Descoped** ## Migration Guide * `FromReflect` is now automatically derived within the `Reflect` derive macro. Items with both derives will need to remove the `FromReflect` one. ```rust // OLD #[derive(Reflect, FromReflect)] struct Foo; // NEW #[derive(Reflect)] struct Foo; ``` If using a manual implementation of `FromReflect` and the `Reflect` derive, users will need to opt-out of the automatic implementation. ```rust // OLD #[derive(Reflect)] struct Foo; impl FromReflect for Foo {/* ... */} // NEW #[derive(Reflect)] #[reflect(from_reflect = false)] struct Foo; impl FromReflect for Foo {/* ... */} ``` <details> <summary><h4>Removed Migrations</h4></summary> > **Warning** > This section includes changes that have since been descoped from this PR. They will likely be implemented again in a followup PR. I am mainly leaving these details in for archival purposes, as well as for reference when implementing this logic again. * The reflect deserializers now perform a `FromReflect` conversion internally. The expected output of `TypedReflectDeserializer::new` and `UntypedReflectDeserializer::new` is no longer a Dynamic (e.g., `DynamicList`), but its Real counterpart (e.g., `Vec<i32>`). ```rust let reflect_deserializer = UntypedReflectDeserializer::new_dynamic(®istry); let mut deserializer = ron:🇩🇪:Deserializer::from_str(input)?; // OLD let output: DynamicStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?; // NEW let output: SomeStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?; ``` Alternatively, if this behavior isn't desired, use the `TypedReflectDeserializer::new_dynamic` and `UntypedReflectDeserializer::new_dynamic` methods instead: ```rust // OLD let reflect_deserializer = UntypedReflectDeserializer::new(®istry); // NEW let reflect_deserializer = UntypedReflectDeserializer::new_dynamic(®istry); ``` </details> --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com> |
||
|
Joaquín León
|
af4336c501
|
Reflect UUID (#8905)
For those who wish to be able to `#[reflect]` stuff using the `Uuid` type I'm very unfamiliar with the codebase, so please tell me if I'm missing something |