Add on_unimplemented Diagnostics to Most Public Traits

[bushrat011899]

Objective

• Fixes Use the #[diagnostic::on_unimplemented] attribute to improve ECS (system and query) error messages #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

Examples hidden for brevity

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)]

Example Code

use bevy::prelude::*;

struct MyResource;

fn main() {
    App::new()
        .insert_resource(MyResource)
        .run();
}

Error Generated

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`

Putting A QueryData in a QueryFilter Slot

Example Code

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();
}

Error Generated

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`

Missing #[derive(Component)]

Example Code

use bevy::prelude::*;

struct A;

fn my_system(mut commands: Commands) {
    commands.spawn(A);
}

fn main() {
    App::new()
        .add_systems(Startup, my_system)
        .run();
}

Error Generated

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`

Missing #[derive(Asset)]

Example Code

use bevy::prelude::*;

struct A;

fn main() {
    App::new()
        .init_asset::<A>()
        .run();
}

Error Generated

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`

Mismatched Input and Output on System Piping

Example Code

use bevy::prelude::*;

fn producer() -> u32 {
    123
}

fn consumer(_: In<u16>) {}

fn main() {
    App::new()
        .add_systems(Update, producer.pipe(consumer))
        .run();
}

Error Generated

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`

Missing Reflection

Example Code

use bevy::prelude::*;

#[derive(Component)]
struct MyComponent;

fn main() {
    App::new()
        .register_type::<MyComponent>()
        .run();
}

Error Generated

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`

Missing #[derive(States)] Implementation

Example Code

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();
}

Error Generated

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`

Adding a System with Unhandled Output

Example Code

use bevy::prelude::*;

fn producer() -> u32 {
    123
}

fn main() {
    App::new()
        .add_systems(Update, consumer)
        .run();
}

Error Generated

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`

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.

[alice-i-cecile]

I'm quite pleased with this and would like to ship this in 0.14. What remains to be done?

I'm frustrated that we can't get truly better error messages for our all_tuples traits yet, but we'll get there :)

[cart]

Adding the diagnostic for "simple" cases like this is essentially an assertion that Rust is not capable of producing solid error messages for the straightforward impl MyTrait for MyStruct cases. If we implement this for cases like Animatable, we are essentially saying "every trait in Bevy should have a custom diagnostic" and in a wider scope "every Rust library should implement custom diagnostics for every trait". That is a very strong statement. If we agree with that, we should start knocking on the Rust team's door immediately to work out how to resolve that problem.

Lets compare the error messages:

With Diagnostic

Without Diagnostic

Comparison

"With Diagnostic" does read as plain English:

Thing can not be animated
invalid animateable

Lets compare to "Without Diagnostic"

the trait bound Thing: bevy::prelude::Animatable is not satisfied
the trait bevy::prelude::Animatable is not implemented for Thing

On the surface, "plain english" might feel more user friendly. However I will assert that in practice, this is not more user friendly. "cannot be animated' introduces the new verb "animated" which has no direct technical ties to the code at hand. First: "animation" might happen in other traits. Maybe Thing can be animated in other contexts. Additionally, Animatable has other functionality outside of "animation", namely Animatable::post_process. By using "cannot be animated", we are applying a lossy mental model to a context that might only be doing post processing. "Likewise "invalid animatable", while being the same word as Animatable, is not the actual symbol. The user has to "couple" these concepts in their head, instead of directly thinking about Animatable.

In both cases we have decoupled ourselves from the actual reality of the code. The "friendly english" obfuscates what the actual scope of the problem is.

"Without diagnostic", on the other hand, tells us directly exactly what is wrong with the code and fully encapsulates the problem space.

I think we should remove all instances of things in this class, and use on_unimplemented only in specific cases:

1. Cases where a trait is derivable and that derive is used in a high percentage of cases.
2. Cases where we have complicated implementations of traits that result in nasty / hard to understand error messages.

[alice-i-cecile]

Yep, I'm on board with that decision.

[bushrat011899]

Agreed, this is the main reason I haven't taken this off draft just yet. I started by adding basic implementations to all public traits as a maximal goal to then pair back based on actual utility.

I'm going to remove a large number of these custom messages roughly in line with how you've described (derive traits, particularly ugly error messages, etc.)

The other thing I want to do is contrast this with bevycheck and the default error messages to ensure this is a strict improvement over the generic case.

[bushrat011899]

Sorting through custom error messages to keep highest value options.

[bushrat011899]

Marking as ready for review as I'm now unaware of any changes that could improve this PR. Please experiment and suggest any changes you can think of.

[Themayu]

Was looking through this PR, and I found a few places where the message given by diagnostics might be improved somewhat.

Feel free to ignore any of these you don't want to change.

[alice-i-cecile]

There's a couple of suggestions that I think help a bit, but overall I'm happy with this selection of traits. Hopefully in the future we can get better information about why exactly things aren't e.g. SystemParam, but this is a great start.

[Themayu]

Left a couple comments of my own on Alice's suggestions. Again, feel free to ignore my comments if you disagree with them.

[BD103]

Looks good! Just two quick, non-blocking things. :)

[BD103]

Just as a note, this was reverted in #13413 because Rust 1.78 broke Android support. This will be unblocked by #13366, though!