mirror of
https://github.com/bevyengine/bevy
synced 2024-11-26 22:50:19 +00:00
12 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Gino Valente
|
2b4180ca8f
|
bevy_reflect: Function reflection terminology refactor (#14813)
# Objective One of the changes in #14704 made `DynamicFunction` effectively the same as `DynamicClosure<'static>`. This change meant that the de facto function type would likely be `DynamicClosure<'static>` instead of the intended `DynamicFunction`, since the former is much more flexible. We _could_ explore ways of making `DynamicFunction` implement `Copy` using some unsafe code, but it likely wouldn't be worth it. And users would likely still reach for the convenience of `DynamicClosure<'static>` over the copy-ability of `DynamicFunction`. The goal of this PR is to fix this confusion between the two types. ## Solution Firstly, the `DynamicFunction` type was removed. Again, it was no different than `DynamicClosure<'static>` so it wasn't a huge deal to remove. Secondly, `DynamicClosure<'env>` and `DynamicClosureMut<'env>` were renamed to `DynamicFunction<'env>` and `DynamicFunctionMut<'env>`, respectively. Yes, we still ultimately kept the naming of `DynamicFunction`, but changed its behavior to that of `DynamicClosure<'env>`. We need a term to refer to both functions and closures, and "function" was the best option. [Originally](https://discord.com/channels/691052431525675048/1002362493634629796/1274091992162242710), I was going to go with "callable" as the replacement term to encompass both functions and closures (e.g. `DynamciCallable<'env>`). However, it was [suggested](https://discord.com/channels/691052431525675048/1002362493634629796/1274653581777047625) by @SkiFire13 that the simpler "function" term could be used instead. While "callable" is perhaps the better umbrella term—being truly ambiguous over functions and closures— "function" is more familiar, used more often, easier to discover, and is subjectively just "better-sounding". ## Testing Most changes are purely swapping type names or updating documentation, but you can verify everything still works by running the following command: ``` cargo test --package bevy_reflect ``` |
||
|
Gino Valente
|
6183b56b5d
|
bevy_reflect: Reflect remote types (#6042)
# Objective The goal with this PR is to allow the use of types that don't implement `Reflect` within the reflection API. Rust's [orphan rule](https://doc.rust-lang.org/book/ch10-02-traits.html#implementing-a-trait-on-a-type) prevents implementing a trait on an external type when neither type nor trait are owned by the implementor. This means that if a crate, `cool_rust_lib`, defines a type, `Foo`, then a user cannot use it with reflection. What this means is that we have to ignore it most of the time: ```rust #[derive(Reflect)] struct SomeStruct { #[reflect(ignore)] data: cool_rust_lib::Foo } ``` Obviously, it's impossible to implement `Reflect` on `Foo`. But does it *have* to be? Most of reflection doesn't deal with concrete types— it's almost all using `dyn Reflect`. And being very metadata-driven, it should theoretically be possible. I mean, [`serde`](https://serde.rs/remote-derive.html) does it. ## Solution > Special thanks to @danielhenrymantilla for their help reviewing this PR and offering wisdom wrt safety. Taking a page out of `serde`'s book, this PR adds the ability to easily use "remote types" with reflection. In this context, a "remote type" is the external type for which we have no ability to implement `Reflect`. This adds the `#[reflect_remote(...)]` attribute macro, which is used to generate "remote type wrappers". All you have to do is define the wrapper exactly the same as the remote type's definition: ```rust // Pretend this is our external crate mod cool_rust_lib { #[derive(Default)] struct Foo { pub value: String } } #[reflect_remote(cool_rust_lib::Foo)] struct FooWrapper { pub value: String } ``` > **Note:** All fields in the external type *must* be public. This could be addressed with a separate getter/setter attribute either in this PR or in another one. The macro takes this user-defined item and transforms it into a newtype wrapper around the external type, marking it as `#[repr(transparent)]`. The fields/variants defined by the user are simply used to build out the reflection impls. Additionally, it generates an implementation of the new trait, `ReflectRemote`, which helps prevent accidental misuses of this API. Therefore, the output generated by the macro would look something like: ```rust #[repr(transparent)] struct FooWrapper(pub cool_rust_lib::Foo); impl ReflectRemote for FooWrapper { type Remote = cool_rust_lib::Foo; // transmutation methods... } // reflection impls... // these will acknowledge and make use of the `value` field ``` Internally, the reflection API will pass around the `FooWrapper` and [transmute](https://doc.rust-lang.org/std/mem/fn.transmute.html) it where necessary. All we have to do is then tell `Reflect` to do that. So rather than ignoring the field, we tell `Reflect` to use our wrapper using the `#[reflect(remote = ...)]` field attribute: ```rust #[derive(Reflect)] struct SomeStruct { #[reflect(remote = FooWrapper)] data: cool_rust_lib::Foo } ``` #### Other Macros & Type Data Because this macro consumes the defined item and generates a new one, we can't just put our macros anywhere. All macros that should be passed to the generated struct need to come *below* this macro. For example, to derive `Default` and register its associated type data: ```rust // ✅ GOOD #[reflect_remote(cool_rust_lib::Foo)] #[derive(Default)] #[reflect(Default)] struct FooWrapper { pub value: String } // ❌ BAD #[derive(Default)] #[reflect_remote(cool_rust_lib::Foo)] #[reflect(Default)] struct FooWrapper { pub value: String } ``` #### Generics Generics are forwarded to the generated struct as well. They should also be defined in the same order: ```rust #[reflect_remote(RemoteGeneric<'a, T1, T2>)] struct GenericWrapper<'a, T1, T2> { pub foo: &'a T1, pub bar: &'a T2, } ``` > Naming does *not* need to match the original definition's. Only order matters here. > Also note that the code above is just a demonstration and doesn't actually compile since we'd need to enforce certain bounds (e.g. `T1: Reflect`, `'a: 'static`, etc.) #### Nesting And, yes, you can nest remote types: ```rust #[reflect_remote(RemoteOuter)] struct OuterWrapper { #[reflect(remote = InnerWrapper)] pub inner: RemoteInner } #[reflect_remote(RemoteInner)] struct InnerWrapper(usize); ``` #### Assertions This macro will also generate some compile-time assertions to ensure that the correct types are used. It's important we catch this early so users don't have to wait for something to panic. And it also helps keep our `unsafe` a little safer. For example, a wrapper definition that does not match its corresponding remote type will result in an error: ```rust mod external_crate { pub struct TheirStruct(pub u32); } #[reflect_remote(external_crate::TheirStruct)] struct MyStruct(pub String); // ERROR: expected type `u32` but found `String` ``` <details> <summary>Generated Assertion</summary> ```rust const _: () = { #[allow(non_snake_case)] #[allow(unused_variables)] #[allow(unused_assignments)] #[allow(unreachable_patterns)] #[allow(clippy::multiple_bound_locations)] fn assert_wrapper_definition_matches_remote_type( mut __remote__: external_crate::TheirStruct, ) { __remote__.0 = (|| -> ::core::option::Option<String> { None })().unwrap(); } }; ``` </details> Additionally, using the incorrect type in a `#[reflect(remote = ...)]` attribute should result in an error: ```rust mod external_crate { pub struct TheirFoo(pub u32); pub struct TheirBar(pub i32); } #[reflect_remote(external_crate::TheirFoo)] struct MyFoo(pub u32); #[reflect_remote(external_crate::TheirBar)] struct MyBar(pub i32); #[derive(Reflect)] struct MyStruct { #[reflect(remote = MyBar)] // ERROR: expected type `TheirFoo` but found struct `TheirBar` foo: external_crate::TheirFoo } ``` <details> <summary>Generated Assertion</summary> ```rust const _: () = { struct RemoteFieldAssertions; impl RemoteFieldAssertions { #[allow(non_snake_case)] #[allow(clippy::multiple_bound_locations)] fn assert__foo__is_valid_remote() { let _: <MyBar as bevy_reflect::ReflectRemote>::Remote = (|| -> ::core::option::Option<external_crate::TheirFoo> { None })().unwrap(); } } }; ``` </details> ### Discussion There are a couple points that I think still need discussion or validation. - [x] 1. `Any` shenanigans ~~If we wanted to downcast our remote type from a `dyn Reflect`, we'd have to first downcast to the wrapper then extract the inner type. This PR has a [commit](b840db9f74cb6d357f951cb11b150d46bac89ee2) that addresses this by making all the `Reflect::*any` methods return the inner type rather than the wrapper type. This allows us to downcast directly to our remote type.~~ ~~However, I'm not sure if this is something we want to do. For unknowing users, it could be confusing and seemingly inconsistent. Is it worth keeping? Or should this behavior be removed?~~ I think this should be fine. The remote wrapper is an implementation detail and users should not need to downcast to the wrapper type. Feel free to let me know if there are other opinions on this though! - [x] 2. Implementing `Deref/DerefMut` and `From` ~~We don't currently do this, but should we implement other traits on the generated transparent struct? We could implement `Deref`/`DerefMut` to easily access the inner type. And we could implement `From` for easier conversion between the two types (e.g. `T: Into<Foo>`).~~ As mentioned in the comments, we probably don't need to do this. Again, the remote wrapper is an implementation detail, and should generally not be used directly. - [x] 3. ~~Should we define a getter/setter field attribute in this PR as well or leave it for a future one?~~ I think this should be saved for a future PR - [ ] 4. Any foreseeable issues with this implementation? #### Alternatives One alternative to defining our own `ReflectRemote` would be to use [bytemuck's `TransparentWrapper`](https://docs.rs/bytemuck/1.13.1/bytemuck/trait.TransparentWrapper.html) (as suggested by @danielhenrymantilla). This is definitely a viable option, as `ReflectRemote` is pretty much the same thing as `TransparentWrapper`. However, the cost would be bringing in a new crate— though, it is already in use in a few other sub-crates like bevy_render. I think we're okay just defining `ReflectRemote` ourselves, but we can go the bytemuck route if we'd prefer offloading that work to another crate. --- ## Changelog * Added the `#[reflect_remote(...)]` attribute macro to allow `Reflect` to be used on remote types * Added `ReflectRemote` trait for ensuring proper remote wrapper usage |
||
radiish
|
6ab8767d3b
|
reflect: implement the unique reflect rfc (#7207)
# Objective
- Implements the [Unique Reflect
RFC](https://github.com/nicopap/rfcs/blob/bevy-reflect-api/rfcs/56-better-reflect.md).
## Solution
- Implements the RFC.
- This implementation differs in some ways from the RFC:
- In the RFC, it was suggested `Reflect: Any` but `PartialReflect:
?Any`. During initial implementation I tried this, but we assume the
`PartialReflect: 'static` in a lot of places and the changes required
crept out of the scope of this PR.
- `PartialReflect::try_into_reflect` originally returned `Option<Box<dyn
Reflect>>` but i changed this to `Result<Box<dyn Reflect>, Box<dyn
PartialReflect>>` since the method takes by value and otherwise there
would be no way to recover the type. `as_full` and `as_full_mut` both
still return `Option<&(mut) dyn Reflect>`.
---
## Changelog
- Added `PartialReflect`.
- `Reflect` is now a subtrait of `PartialReflect`.
- Moved most methods on `Reflect` to the new `PartialReflect`.
- Added `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect}`.
- Added `PartialReflect::{try_as_reflect, try_as_reflect_mut,
try_into_reflect}`.
- Added `<dyn PartialReflect>::{try_downcast_ref, try_downcast_mut,
try_downcast, try_take}` supplementing the methods on `dyn Reflect`.
## Migration Guide
- Most instances of `dyn Reflect` should be changed to `dyn
PartialReflect` which is less restrictive, however trait bounds should
generally stay as `T: Reflect`.
- The new `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect, try_as_reflect, try_as_reflect_mut,
try_into_reflect}` methods as well as `Reflect::{as_reflect,
as_reflect_mut, into_reflect}` will need to be implemented for manual
implementors of `Reflect`.
## Future Work
- This PR is designed to be followed up by another "Unique Reflect Phase
2" that addresses the following points:
- Investigate making serialization revolve around `Reflect` instead of
`PartialReflect`.
- [Remove the `try_*` methods on `dyn PartialReflect` since they are
stop
gaps](https://github.com/bevyengine/bevy/pull/7207#discussion_r1083476050).
- Investigate usages like `ReflectComponent`. In the places they
currently use `PartialReflect`, should they be changed to use `Reflect`?
- Merging this opens the door to lots of reflection features we haven't
been able to implement.
- We could re-add [the `Reflectable`
trait](
|
||
|
Gino Valente
|
1042f09c2e
|
bevy_reflect: Add DynamicClosure and DynamicClosureMut (#14141)
# Objective As mentioned in [this](https://github.com/bevyengine/bevy/pull/13152#issuecomment-2198387297) comment, creating a function registry (see #14098) is a bit difficult due to the requirements of `DynamicFunction`. Internally, a `DynamicFunction` contains a `Box<dyn FnMut>` (the function that reifies reflected arguments and calls the actual function), which requires `&mut self` in order to be called. This means that users would require a mutable reference to the function registry for it to be useful— which isn't great. And they can't clone the `DynamicFunction` either because cloning an `FnMut` isn't really feasible (wrapping it in an `Arc` would allow it to be cloned but we wouldn't be able to call the clone since we need a mutable reference to the `FnMut`, which we can't get with multiple `Arc`s still alive, requiring us to also slap in a `Mutex`, which adds additional overhead). And we don't want to just replace the `dyn FnMut` with `dyn Fn` as that would prevent reflecting closures that mutate their environment. Instead, we need to introduce a new type to split the requirements of `DynamicFunction`. ## Solution Introduce new types for representing closures. Specifically, this PR introduces `DynamicClosure` and `DynamicClosureMut`. Similar to how `IntoFunction` exists for `DynamicFunction`, two new traits were introduced: `IntoClosure` and `IntoClosureMut`. Now `DynamicFunction` stores a `dyn Fn` with a `'static` lifetime. `DynamicClosure` also uses a `dyn Fn` but has a lifetime, `'env`, tied to its environment. `DynamicClosureMut` is most like the old `DynamicFunction`, keeping the `dyn FnMut` and also typing its lifetime, `'env`, to the environment Here are some comparison tables: | | `DynamicFunction` | `DynamicClosure` | `DynamicClosureMut` | | - | ----------------- | ---------------- | ------------------- | | Callable with `&self` | ✅ | ✅ | ❌ | | Callable with `&mut self` | ✅ | ✅ | ✅ | | Allows for non-`'static` lifetimes | ❌ | ✅ | ✅ | | | `IntoFunction` | `IntoClosure` | `IntoClosureMut` | | - | -------------- | ------------- | ---------------- | | Convert `fn` functions | ✅ | ✅ | ✅ | | Convert `fn` methods | ✅ | ✅ | ✅ | | Convert anonymous functions | ✅ | ✅ | ✅ | | Convert closures that capture immutable references | ❌ | ✅ | ✅ | | Convert closures that capture mutable references | ❌ | ❌ | ✅ | | Convert closures that capture owned values | ❌[^1] | ✅ | ✅ | [^1]: Due to limitations in Rust, `IntoFunction` can't be implemented for just functions (unless we forced users to manually coerce them to function pointers first). So closures that meet the trait requirements _can technically_ be converted into a `DynamicFunction` as well. To both future-proof and reduce confusion, though, we'll just pretend like this isn't a thing. ```rust let mut list: Vec<i32> = vec![1, 2, 3]; // `replace` is a closure that captures a mutable reference to `list` let mut replace = |index: usize, value: i32| -> i32 { let old_value = list[index]; list[index] = value; old_value }; // Convert the closure into a dynamic closure using `IntoClosureMut::into_closure_mut` let mut func: DynamicClosureMut = replace.into_closure_mut(); // Dynamically call the closure: let args = ArgList::default().push_owned(1_usize).push_owned(-2_i32); let value = func.call_once(args).unwrap().unwrap_owned(); // Check the result: assert_eq!(value.take::<i32>().unwrap(), 2); assert_eq!(list, vec![1, -2, 3]); ``` ### `ReflectFn`/`ReflectFnMut` To make extending the function reflection system easier (the blanket impls for `IntoFunction`, `IntoClosure`, and `IntoClosureMut` are all incredibly short), this PR generalizes callables with two new traits: `ReflectFn` and `ReflectFnMut`. These traits mimic `Fn` and `FnMut` but allow for being called via reflection. In fact, their blanket implementations are identical save for `ReflectFn` being implemented over `Fn` types and `ReflectFnMut` being implemented over `FnMut` types. And just as `Fn` is a subtrait of `FnMut`, `ReflectFn` is a subtrait of `ReflectFnMut`. So anywhere that expects a `ReflectFnMut` can also be given a `ReflectFn`. To reiterate, these traits aren't 100% necessary. They were added in purely for extensibility. If we decide to split things up differently or add new traits/types in the future, then those changes should be much simpler to implement. ### `TypedFunction` Because of the split into `ReflectFn` and `ReflectFnMut`, we needed a new way to access the function type information. This PR moves that concept over into `TypedFunction`. Much like `Typed`, this provides a way to access a function's `FunctionInfo`. By splitting this trait out, it helps to ensure the other traits are focused on a single responsibility. ### Internal Macros The original function PR (#13152) implemented `IntoFunction` using a macro which was passed into an `all_tuples!` macro invocation. Because we needed the same functionality for these new traits, this PR has copy+pasted that code for `ReflectFn`, `ReflectFnMut`, and `TypedFunction`— albeit with some differences between them. Originally, I was going to try and macro-ify the impls and where clauses such that we wouldn't have to straight up duplicate a lot of this logic. However, aside from being more complex in general, autocomplete just does not play nice with such heavily nested macros (tried in both RustRover and VSCode). And both of those problems told me that it just wasn't worth it: we need to ensure the crate is easily maintainable, even at the cost of duplicating code. So instead, I made sure to simplify the macro code by removing all fully-qualified syntax and cutting the where clauses down to the bare essentials, which helps to clean up a lot of the visual noise. I also tried my best to document the macro logic in certain areas (I may even add a bit more) to help with maintainability for future devs. ### Documentation Documentation for this module was a bit difficult for me. So many of these traits and types are very interconnected. And each trait/type has subtle differences that make documenting it in a single place, like at the module level, difficult to do cleanly. Describing the valid signatures is also challenging to do well. Hopefully what I have here is okay. I think I did an okay job, but let me know if there any thoughts on ways to improve it. We can also move such a task to a followup PR for more focused discussion. ## Testing You can test locally by running: ``` cargo test --package bevy_reflect ``` --- ## Changelog - Added `DynamicClosure` struct - Added `DynamicClosureMut` struct - Added `IntoClosure` trait - Added `IntoClosureMut` trait - Added `ReflectFn` trait - Added `ReflectFnMut` trait - Added `TypedFunction` trait - `IntoFunction` now only works for standard Rust functions - `IntoFunction` no longer takes a lifetime parameter - `DynamicFunction::call` now only requires `&self` - Removed `DynamicFunction::call_once` - Changed the `IntoReturn::into_return` signature to include a where clause ## Internal Migration Guide > [!important] > Function reflection was introduced as part of the 0.15 dev cycle. This migration guide was written for developers relying on `main` during this cycle, and is not a breaking change coming from 0.14. ### `IntoClosure` `IntoFunction` now only works for standard Rust functions. Calling `IntoFunction::into_function` on a closure that captures references to its environment (either mutable or immutable), will no longer compile. Instead, you will need to use either `IntoClosure::into_closure` to create a `DynamicClosure` or `IntoClosureMut::into_closure_mut` to create a `DynamicClosureMut`, depending on your needs: ```rust let punct = String::from("!"); let print = |value: String| { println!("{value}{punct}"); }; // BEFORE let func: DynamicFunction = print.into_function(); // AFTER let func: DynamicClosure = print.into_closure(); ``` ### `IntoFunction` lifetime Additionally, `IntoFunction` no longer takes a lifetime parameter as it always expects a `'static` lifetime. Usages will need to remove any lifetime parameters: ```rust // BEFORE fn execute<'env, F: IntoFunction<'env, Marker>, Marker>(f: F) {/* ... */} // AFTER fn execute<F: IntoFunction<Marker>, Marker>(f: F) {/* ... */} ``` ### `IntoReturn` `IntoReturn::into_return` now has a where clause. Any manual implementors will need to add this where clause to their implementation. |
||
Brezak
|
6522795889
|
Specify test group names in github summary for compile fail tests (#14330)
# Objective The github action summary titles every compile test group as `compile_fail_utils`.  ## Solution Manually specify group names for compile fail tests. ## Testing - Wait for compile fail tests to run. - Observe the generated summary. |
||
|
Gino Valente
|
aa241672e1
|
bevy_reflect: Nested TypeInfo getters (#13321)
# Objective
Right now, `TypeInfo` can be accessed directly from a type using either
`Typed::type_info` or `Reflect::get_represented_type_info`.
However, once that `TypeInfo` is accessed, any nested types must be
accessed via the `TypeRegistry`.
```rust
#[derive(Reflect)]
struct Foo {
bar: usize
}
let registry = TypeRegistry::default();
let TypeInfo::Struct(type_info) = Foo::type_info() else {
panic!("expected struct info");
};
let field = type_info.field("bar").unwrap();
let field_info = registry.get_type_info(field.type_id()).unwrap();
assert!(field_info.is::<usize>());;
```
## Solution
Enable nested types within a `TypeInfo` to be retrieved directly.
```rust
#[derive(Reflect)]
struct Foo {
bar: usize
}
let TypeInfo::Struct(type_info) = Foo::type_info() else {
panic!("expected struct info");
};
let field = type_info.field("bar").unwrap();
let field_info = field.type_info().unwrap();
assert!(field_info.is::<usize>());;
```
The particular implementation was chosen for two reasons.
Firstly, we can't just store `TypeInfo` inside another `TypeInfo`
directly. This is because some types are recursive and would result in a
deadlock when trying to create the `TypeInfo` (i.e. it has to create the
`TypeInfo` before it can use it, but it also needs the `TypeInfo` before
it can create it). Therefore, we must instead store the function so it
can be retrieved lazily.
I had considered also using a `OnceLock` or something to lazily cache
the info, but I figured we can look into optimizations later. The API
should remain the same with or without the `OnceLock`.
Secondly, a new wrapper trait had to be introduced: `MaybeTyped`. Like
`RegisterForReflection`, this trait is `#[doc(hidden)]` and only exists
so that we can properly handle dynamic type fields without requiring
them to implement `Typed`. We don't want dynamic types to implement
`Typed` due to the fact that it would make the return type
`Option<&'static TypeInfo>` for all types even though only the dynamic
types ever need to return `None` (see #6971 for details).
Users should never have to interact with this trait as it has a blanket
impl for all `Typed` types. And `Typed` is automatically implemented
when deriving `Reflect` (as it is required).
The one downside is we do need to return `Option<&'static TypeInfo>`
from all these new methods so that we can handle the dynamic cases. If
we didn't have to, we'd be able to get rid of the `Option` entirely. But
I think that's an okay tradeoff for this one part of the API, and keeps
the other APIs intact.
## Testing
This PR contains tests to verify everything works as expected. You can
test locally by running:
```
cargo test --package bevy_reflect
```
---
## Changelog
### Public Changes
- Added `ArrayInfo::item_info` method
- Added `NamedField::type_info` method
- Added `UnnamedField::type_info` method
- Added `ListInfo::item_info` method
- Added `MapInfo::key_info` method
- Added `MapInfo::value_info` method
- All active fields now have a `Typed` bound (remember that this is
automatically satisfied for all types that derive `Reflect`)
### Internal Changes
- Added `MaybeTyped` trait
## Migration Guide
All active fields for reflected types (including lists, maps, tuples,
etc.), must implement `Typed`. For the majority of users this won't have
any visible impact.
However, users implementing `Reflect` manually may need to update their
types to implement `Typed` if they weren't already.
Additionally, custom dynamic types will need to implement the new hidden
`MaybeTyped` trait.
|
||
|
Gino Valente
|
09d86bfb96
|
bevy_reflect: Re-enable reflection compile fail tests (#14165)
# Objective Looks like I accidentally disabled the reflection compile fail tests in #13152. These should be re-enabled. ## Solution Re-enable reflection compile fail tests. ## Testing CI should pass. You can also test locally by navigating to `crates/bevy_reflect/compile_fail/` and running: ``` cargo test --target-dir ../../../target ``` |
||
|
Gino Valente
|
276dd04001
|
bevy_reflect: Function reflection (#13152)
# Objective
We're able to reflect types sooooooo... why not functions?
The goal of this PR is to make functions callable within a dynamic
context, where type information is not readily available at compile
time.
For example, if we have a function:
```rust
fn add(left: i32, right: i32) -> i32 {
left + right
}
```
And two `Reflect` values we've already validated are `i32` types:
```rust
let left: Box<dyn Reflect> = Box::new(2_i32);
let right: Box<dyn Reflect> = Box::new(2_i32);
```
We should be able to call `add` with these values:
```rust
// ?????
let result: Box<dyn Reflect> = add.call_dynamic(left, right);
```
And ideally this wouldn't just work for functions, but methods and
closures too!
Right now, users have two options:
1. Manually parse the reflected data and call the function themselves
2. Rely on registered type data to handle the conversions for them
For a small function like `add`, this isn't too bad. But what about for
more complex functions? What about for many functions?
At worst, this process is error-prone. At best, it's simply tedious.
And this is assuming we know the function at compile time. What if we
want to accept a function dynamically and call it with our own
arguments?
It would be much nicer if `bevy_reflect` could alleviate some of the
problems here.
## Solution
Added function reflection!
This adds a `DynamicFunction` type to wrap a function dynamically. This
can be called with an `ArgList`, which is a dynamic list of
`Reflect`-containing `Arg` arguments. It returns a `FunctionResult`
which indicates whether or not the function call succeeded, returning a
`Reflect`-containing `Return` type if it did succeed.
Many functions can be converted into this `DynamicFunction` type thanks
to the `IntoFunction` trait.
Taking our previous `add` example, this might look something like
(explicit types added for readability):
```rust
fn add(left: i32, right: i32) -> i32 {
left + right
}
let mut function: DynamicFunction = add.into_function();
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
let result: Return = function.call(args).unwrap();
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```
And it also works on closures:
```rust
let add = |left: i32, right: i32| left + right;
let mut function: DynamicFunction = add.into_function();
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
let result: Return = function.call(args).unwrap();
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```
As well as methods:
```rust
#[derive(Reflect)]
struct Foo(i32);
impl Foo {
fn add(&mut self, value: i32) {
self.0 += value;
}
}
let mut foo = Foo(2);
let mut function: DynamicFunction = Foo::add.into_function();
let args: ArgList = ArgList::new().push_mut(&mut foo).push_owned(2_i32);
function.call(args).unwrap();
assert_eq!(foo.0, 4);
```
### Limitations
While this does cover many functions, it is far from a perfect system
and has quite a few limitations. Here are a few of the limitations when
using `IntoFunction`:
1. The lifetime of the return value is only tied to the lifetime of the
first argument (useful for methods). This means you can't have a
function like `(a: i32, b: &i32) -> &i32` without creating the
`DynamicFunction` manually.
2. Only 15 arguments are currently supported. If the first argument is a
(mutable) reference, this number increases to 16.
3. Manual implementations of `Reflect` will need to implement the new
`FromArg`, `GetOwnership`, and `IntoReturn` traits in order to be used
as arguments/return types.
And some limitations of `DynamicFunction` itself:
1. All arguments share the same lifetime, or rather, they will shrink to
the shortest lifetime.
2. Closures that capture their environment may need to have their
`DynamicFunction` dropped before accessing those variables again (there
is a `DynamicFunction::call_once` to make this a bit easier)
3. All arguments and return types must implement `Reflect`. While not a
big surprise coming from `bevy_reflect`, this implementation could
actually still work by swapping `Reflect` out with `Any`. Of course,
that makes working with the arguments and return values a bit harder.
4. Generic functions are not supported (unless they have been manually
monomorphized)
And general, reflection gotchas:
1. `&str` does not implement `Reflect`. Rather, `&'static str`
implements `Reflect` (the same is true for `&Path` and similar types).
This means that `&'static str` is considered an "owned" value for the
sake of generating arguments. Additionally, arguments and return types
containing `&str` will assume it's `&'static str`, which is almost never
the desired behavior. In these cases, the only solution (I believe) is
to use `&String` instead.
### Followup Work
This PR is the first of two PRs I intend to work on. The second PR will
aim to integrate this new function reflection system into the existing
reflection traits and `TypeInfo`. The goal would be to register and call
a reflected type's methods dynamically.
I chose not to do that in this PR since the diff is already quite large.
I also want the discussion for both PRs to be focused on their own
implementation.
Another followup I'd like to do is investigate allowing common container
types as a return type, such as `Option<&[mut] T>` and `Result<&[mut] T,
E>`. This would allow even more functions to opt into this system. I
chose to not include it in this one, though, for the same reasoning as
previously mentioned.
### Alternatives
One alternative I had considered was adding a macro to convert any
function into a reflection-based counterpart. The idea would be that a
struct that wraps the function would be created and users could specify
which arguments and return values should be `Reflect`. It could then be
called via a new `Function` trait.
I think that could still work, but it will be a fair bit more involved,
requiring some slightly more complex parsing. And it of course is a bit
more work for the user, since they need to create the type via macro
invocation.
It also makes registering these functions onto a type a bit more
complicated (depending on how it's implemented).
For now, I think this is a fairly simple, yet powerful solution that
provides the least amount of friction for users.
---
## Showcase
Bevy now adds support for storing and calling functions dynamically
using reflection!
```rust
// 1. Take a standard Rust function
fn add(left: i32, right: i32) -> i32 {
left + right
}
// 2. Convert it into a type-erased `DynamicFunction` using the `IntoFunction` trait
let mut function: DynamicFunction = add.into_function();
// 3. Define your arguments from reflected values
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
// 4. Call the function with your arguments
let result: Return = function.call(args).unwrap();
// 5. Extract the return value
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```
## Changelog
#### TL;DR
- Added support for function reflection
- Added a new `Function Reflection` example:
|
||
Alice Cecile
|
ec7b3490f6
|
Add on_unimplemented Diagnostics to Most Public Traits (#13347) (#13662)
# Objective - #13414 did not have the intended effect. - #13404 is still blocked ## Solution - Re-adds #13347. Co-authored-by: Zachary Harrold <zac@harrold.com.au> Co-authored-by: Jamie Ridding <Themayu@users.noreply.github.com> Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com> |
||
|
Alice Cecile
|
ee6dfd35c9
|
Revert "Add on_unimplemented Diagnostics to Most Public Traits" (#13413)
# Objective - Rust 1.78 breaks all Android support, see https://github.com/bevyengine/bevy/issues/13331 - We should not bump the MSRV to 1.78 until that's resolved in #13366. ## Solution - Temporarily revert https://github.com/bevyengine/bevy/pull/13347 Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com> |
||
Zachary Harrold
|
11f0a2dcde
|
Add on_unimplemented Diagnostics to Most Public Traits (#13347)
# Objective - Fixes #12377 ## Solution Added simple `#[diagnostic::on_unimplemented(...)]` attributes to some critical public traits providing a more approachable initial error message. Where appropriate, a `note` is added indicating that a `derive` macro is available. ## Examples <details> <summary>Examples hidden for brevity</summary> Below is a collection of examples showing the new error messages produced by this change. In general, messages will start with a more Bevy-centric error message (e.g., _`MyComponent` is not a `Component`_), and a note directing the user to an available derive macro where appropriate. ### Missing `#[derive(Resource)]` <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; struct MyResource; fn main() { App::new() .insert_resource(MyResource) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `MyResource` is not a `Resource` --> examples/app/empty.rs:7:26 | 7 | .insert_resource(MyResource) | --------------- ^^^^^^^^^^ invalid `Resource` | | | required by a bound introduced by this call | = help: the trait `Resource` is not implemented for `MyResource` = note: consider annotating `MyResource` with `#[derive(Resource)]` = help: the following other types implement trait `Resource`: AccessibilityRequested ManageAccessibilityUpdates bevy::bevy_a11y::Focus DiagnosticsStore FrameCount bevy::prelude::State<S> SystemInfo bevy::prelude::Axis<T> and 141 others note: required by a bound in `bevy::prelude::App::insert_resource` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:419:31 | 419 | pub fn insert_resource<R: Resource>(&mut self, resource: R) -> &mut Self { | ^^^^^^^^ required by this bound in `App::insert_resource` ``` </details> ### Putting A `QueryData` in a `QueryFilter` Slot <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; #[derive(Component)] struct A; #[derive(Component)] struct B; fn my_system(_query: Query<&A, &B>) {} fn main() { App::new() .add_systems(Update, my_system) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `&B` is not a valid `Query` filter --> examples/app/empty.rs:9:22 | 9 | fn my_system(_query: Query<&A, &B>) {} | ^^^^^^^^^^^^^ invalid `Query` filter | = help: the trait `QueryFilter` is not implemented for `&B` = help: the following other types implement trait `QueryFilter`: With<T> Without<T> bevy::prelude::Or<()> bevy::prelude::Or<(F0,)> bevy::prelude::Or<(F0, F1)> bevy::prelude::Or<(F0, F1, F2)> bevy::prelude::Or<(F0, F1, F2, F3)> bevy::prelude::Or<(F0, F1, F2, F3, F4)> and 28 others note: required by a bound in `bevy::prelude::Query` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_ecs\src\system\query.rs:349:51 | 349 | pub struct Query<'world, 'state, D: QueryData, F: QueryFilter = ()> { | ^^^^^^^^^^^ required by this bound in `Query` ``` </details> ### Missing `#[derive(Component)]` <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; struct A; fn my_system(mut commands: Commands) { commands.spawn(A); } fn main() { App::new() .add_systems(Startup, my_system) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `A` is not a `Bundle` --> examples/app/empty.rs:6:20 | 6 | commands.spawn(A); | ----- ^ invalid `Bundle` | | | required by a bound introduced by this call | = help: the trait `bevy::prelude::Component` is not implemented for `A`, which is required by `A: Bundle` = note: consider annotating `A` with `#[derive(Component)]` or `#[derive(Bundle)]` = help: the following other types implement trait `Bundle`: TransformBundle SceneBundle DynamicSceneBundle AudioSourceBundle<Source> SpriteBundle SpriteSheetBundle Text2dBundle MaterialMesh2dBundle<M> and 34 others = note: required for `A` to implement `Bundle` note: required by a bound in `bevy::prelude::Commands::<'w, 's>::spawn` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_ecs\src\system\commands\mod.rs:243:21 | 243 | pub fn spawn<T: Bundle>(&mut self, bundle: T) -> EntityCommands { | ^^^^^^ required by this bound in `Commands::<'w, 's>::spawn` ``` </details> ### Missing `#[derive(Asset)]` <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; struct A; fn main() { App::new() .init_asset::<A>() .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `A` is not an `Asset` --> examples/app/empty.rs:7:23 | 7 | .init_asset::<A>() | ---------- ^ invalid `Asset` | | | required by a bound introduced by this call | = help: the trait `Asset` is not implemented for `A` = note: consider annotating `A` with `#[derive(Asset)]` = help: the following other types implement trait `Asset`: Font AnimationGraph DynamicScene Scene AudioSource Pitch bevy::bevy_gltf::Gltf GltfNode and 17 others note: required by a bound in `init_asset` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_asset\src\lib.rs:307:22 | 307 | fn init_asset<A: Asset>(&mut self) -> &mut Self; | ^^^^^ required by this bound in `AssetApp::init_asset` ``` </details> ### Mismatched Input and Output on System Piping <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; fn producer() -> u32 { 123 } fn consumer(_: In<u16>) {} fn main() { App::new() .add_systems(Update, producer.pipe(consumer)) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `fn(bevy::prelude::In<u16>) {consumer}` is not a valid system with input `u32` and output `_` --> examples/app/empty.rs:11:44 | 11 | .add_systems(Update, producer.pipe(consumer)) | ---- ^^^^^^^^ invalid system | | | required by a bound introduced by this call | = help: the trait `bevy::prelude::IntoSystem<u32, _, _>` is not implemented for fn item `fn(bevy::prelude::In<u16>) {consumer}` = note: expecting a system which consumes `u32` and produces `_` note: required by a bound in `pipe` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_ecs\src\system\mod.rs:168:12 | 166 | fn pipe<B, Final, MarkerB>(self, system: B) -> PipeSystem<Self::System, B::System> | ---- required by a bound in this associated function 167 | where 168 | B: IntoSystem<Out, Final, MarkerB>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `IntoSystem::pipe` ``` </details> ### Missing Reflection <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; #[derive(Component)] struct MyComponent; fn main() { App::new() .register_type::<MyComponent>() .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `MyComponent` does not provide type registration information --> examples/app/empty.rs:8:26 | 8 | .register_type::<MyComponent>() | ------------- ^^^^^^^^^^^ the trait `GetTypeRegistration` is not implemented for `MyComponent` | | | required by a bound introduced by this call | = note: consider annotating `MyComponent` with `#[derive(Reflect)]` = help: the following other types implement trait `GetTypeRegistration`: bool char isize i8 i16 i32 i64 i128 and 443 others note: required by a bound in `bevy::prelude::App::register_type` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:619:29 | 619 | pub fn register_type<T: bevy_reflect::GetTypeRegistration>(&mut self) -> &mut Self { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `App::register_type` ``` </details> ### Missing `#[derive(States)]` Implementation <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; #[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Hash)] enum AppState { #[default] Menu, InGame { paused: bool, turbo: bool, }, } fn main() { App::new() .init_state::<AppState>() .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: the trait bound `AppState: FreelyMutableState` is not satisfied --> examples/app/empty.rs:15:23 | 15 | .init_state::<AppState>() | ---------- ^^^^^^^^ the trait `FreelyMutableState` is not implemented for `AppState` | | | required by a bound introduced by this call | = note: consider annotating `AppState` with `#[derive(States)]` note: required by a bound in `bevy::prelude::App::init_state` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:282:26 | 282 | pub fn init_state<S: FreelyMutableState + FromWorld>(&mut self) -> &mut Self { | ^^^^^^^^^^^^^^^^^^ required by this bound in `App::init_state` ``` </details> ### Adding a `System` with Unhandled Output <details> <summary>Example Code</summary> ```rust use bevy::prelude::*; fn producer() -> u32 { 123 } fn main() { App::new() .add_systems(Update, consumer) .run(); } ``` </details> <details> <summary>Error Generated</summary> ```error error[E0277]: `fn() -> u32 {producer}` does not describe a valid system configuration --> examples/app/empty.rs:9:30 | 9 | .add_systems(Update, producer) | ----------- ^^^^^^^^ invalid system configuration | | | required by a bound introduced by this call | = help: the trait `IntoSystem<(), (), _>` is not implemented for fn item `fn() -> u32 {producer}`, which is required by `fn() -> u32 {producer}: IntoSystemConfigs<_>` = help: the following other types implement trait `IntoSystemConfigs<Marker>`: <Box<(dyn bevy::prelude::System<In = (), Out = ()> + 'static)> as IntoSystemConfigs<()>> <NodeConfigs<Box<(dyn bevy::prelude::System<In = (), Out = ()> + 'static)>> as IntoSystemConfigs<()>> <(S0,) as IntoSystemConfigs<(SystemConfigTupleMarker, P0)>> <(S0, S1) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1)>> <(S0, S1, S2) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2)>> <(S0, S1, S2, S3) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2, P3)>> <(S0, S1, S2, S3, S4) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2, P3, P4)>> <(S0, S1, S2, S3, S4, S5) as IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2, P3, P4, P5)>> and 14 others = note: required for `fn() -> u32 {producer}` to implement `IntoSystemConfigs<_>` note: required by a bound in `bevy::prelude::App::add_systems` --> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:342:23 | 339 | pub fn add_systems<M>( | ----------- required by a bound in this associated function ... 342 | systems: impl IntoSystemConfigs<M>, | ^^^^^^^^^^^^^^^^^^^^ required by this bound in `App::add_systems` ``` </details> </details> ## Testing CI passed locally. ## Migration Guide Upgrade to version 1.78 (or higher) of Rust. ## Future Work - Currently, hints are not supported in this diagnostic. Ideally, suggestions like _"consider using ..."_ would be in a hint rather than a note, but that is the best option for now. - System chaining and other `all_tuples!(...)`-based traits have bad error messages due to the slightly different error message format. --------- Co-authored-by: Jamie Ridding <Themayu@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com> |
||
BD103
|
bdb4899978
|
Move compile fail tests (#13196)
# Objective
- Follow-up of #13184 :)
- We use `ui_test` to test compiler errors for our custom macros.
- There are four crates related to compile fail tests
- `bevy_ecs_compile_fail_tests`, `bevy_macros_compile_fail_tests`, and
`bevy_reflect_compile_fail_tests`, which actually test the macros.
-
[`bevy_compile_test_utils`](
|