Commit graph

7 commits

Author SHA1 Message Date
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
```
2024-07-05 20:49:03 +00:00
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:
ba727898f2/examples/reflection/function_reflection.rs (L1-L157)

#### Details

Added the following items:

- `ArgError` enum
- `ArgId` enum
- `ArgInfo` struct
- `ArgList` struct
- `Arg` enum
- `DynamicFunction` struct
- `FromArg` trait (derived with `derive(Reflect)`)
- `FunctionError` enum
- `FunctionInfo` struct
- `FunctionResult` alias
- `GetOwnership` trait (derived with `derive(Reflect)`)
- `IntoFunction` trait (with blanket implementation)
- `IntoReturn` trait (derived with `derive(Reflect)`)
- `Ownership` enum
- `ReturnInfo` struct
- `Return` enum

---------

Co-authored-by: Periwink <charlesbour@gmail.com>
2024-07-01 13:49:08 +00:00
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>
2024-06-04 00:31:34 +00:00
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>
2024-05-17 17:00:43 +00:00
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>
2024-05-17 00:49:05 +00:00
Brezak
423a4732c3
Update compile test to use ui_test 0.23 (#13245)
# Objective

Closes #13241

## Solution

Update test utils to use `ui_test` 0.23.0.

## Testing

- Run compile tests for bevy_ecs.

cc @BD103
2024-05-05 22:17:56 +00:00
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`](64c1c65783/crates/bevy_compile_test_utils),
which provides helpers and common patterns for these tests.
- All of these crates reside within the `crates` directory.
- This can be confusing, especially for newcomers. All of the other
folders in `crates` are actual published libraries, except for these 4.

## Solution

- Move all compile fail tests to a `compile_fail` folder under their
corresponding crate.
- E.g. `crates/bevy_ecs_compile_fail_tests` would be moved to
`crates/bevy_ecs/compile_fail`.
- Move `bevy_compile_test_utils` to `tools/compile_fail_utils`.

There are a few benefits to this approach:

1. An internal testing detail is less intrusive (and confusing) for
those who just want to browse the public Bevy interface.
2. Follows a pre-existing approach of organizing related crates inside a
larger crate's folder.
   - See `bevy_gizmos/macros` for an example.
4. Makes consistent the terms `compile_test`, `compile_fail`, and
`compile_fail_test` in code. It's all just `compile_fail` now, because
we are specifically testing the error messages on compiler failures.
- To be clear it can still be referred to by these terms in comments and
speech, just the names of the crates and the CI command are now
consistent.

## Testing

Run the compile fail CI command:

```shell
cargo run -p ci -- compile-fail
```

If it still passes, then my refactor was successful.
2024-05-03 13:35:21 +00:00