nicopap / bevy_mod_sysfail

Decorate your bevy system with the sysfail macro attribute to make them handle cleanly failure mods.
Apache License 2.0
25 stars 5 forks source link

Exclusive systems not supported by `#[sysfail(log)]` #3

Closed dmlary closed 10 months ago

dmlary commented 1 year ago

This plugin is great, but I get a compiler warning when using #[sysfail(log)] with an exclusive system. I've reproduced the error in the bevy_mod_sysfail example with the following changes.

diff --git a/examples/all_attributes.rs b/examples/all_attributes.rs
index ddb1285..f0df584 100644
--- a/examples/all_attributes.rs
+++ b/examples/all_attributes.rs
@@ -15,7 +15,8 @@ enum GizmoError {
 fn main() {
     let mut app = App::new();
     app.add_plugins((MinimalPlugins, bevy::log::LogPlugin::default()))
-        .add_systems(Update, (drag_gizmo, (delete_gizmo, place_gizmo)).chain());
+        .add_systems(Update, (drag_gizmo, (delete_gizmo, place_gizmo)).chain())
+        .add_systems(Update, exclusive_system);
     app.update();
 }

@@ -46,3 +47,9 @@ fn delete_gizmo(time: Res<Time>, mut query: Query<&mut Transform>, foos: Query<E
     let _ = None?;
     println!("This will never print");
 }
+
+#[sysfail(log)]
+fn exclusive_system(_world: &mut World) -> anyhow::Result<()> {
+    let _ = Err(GizmoError::Error)?;
+    Ok(())
+}

It looks like the generated code assumes its safe to add additional arguments to the function, but that's not the case with fn system(world: &mut World). The generated function appears to be fn(&'a mut bevy::prelude::World, bevy::prelude::Res<'b, bevy::prelude::Time>, Local<'c, LoggedErrors<Result<(), anyhow::Error>>>).

error[E0277]: the trait bound `for<'a, 'b, 'c> fn(&'a mut bevy::prelude::World, bevy::prelude::Res<'b, bevy::prelude::Time>, Local<'c, LoggedErrors<Result<(), anyhow::Error>>>) {exclusive_sy
stem}: IntoSystem<(), (), _>` is not satisfied
   --> examples/all_attributes.rs:19:30
    |
19  |         .add_systems(Update, exclusive_system);
    |          -----------         ^^^^^^^^^^^^^^^^ the trait `IntoSystem<(), (), _>` is not implemented for fn item `for<'a, 'b, 'c> fn(&'a mut bevy::prelude::World, bevy::prelude::Res<'b, 
bevy::prelude::Time>, Local<'c, LoggedErrors<Result<(), anyhow::Error>>>) {exclusive_system}`
    |          |
    |          required by a bound introduced by this call
    |
    = help: the following other types implement trait `bevy::prelude::IntoSystemConfigs<Marker>`:
              <Box<(dyn bevy::prelude::System<In = (), Out = ()> + 'static)> as bevy::prelude::IntoSystemConfigs<()>>
              <NodeConfigs<Box<(dyn bevy::prelude::System<In = (), Out = ()> + 'static)>> as bevy::prelude::IntoSystemConfigs<()>>
              <(S0,) as bevy::prelude::IntoSystemConfigs<(SystemConfigTupleMarker, P0)>>
              <(S0, S1) as bevy::prelude::IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1)>>
              <(S0, S1, S2) as bevy::prelude::IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2)>>
              <(S0, S1, S2, S3) as bevy::prelude::IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2, P3)>>
              <(S0, S1, S2, S3, S4) as bevy::prelude::IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2, P3, P4)>>
              <(S0, S1, S2, S3, S4, S5) as bevy::prelude::IntoSystemConfigs<(SystemConfigTupleMarker, P0, P1, P2, P3, P4, P5)>>
            and 14 others
    = note: required for `for<'a, 'b, 'c> fn(&'a mut bevy::prelude::World, bevy::prelude::Res<'b, bevy::prelude::Time>, Local<'c, LoggedErrors<Result<(), anyhow::Error>>>) {exclusive_system}
` to implement `bevy::prelude::IntoSystemConfigs<_>`
note: required by a bound in `bevy::prelude::App::add_systems`
   --> /Users/dmlary/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_app-0.12.0/src/app.rs:416:23
    |
413 |     pub fn add_systems<M>(
    |            ----------- required by a bound in this associated function
...
416 |         systems: impl IntoSystemConfigs<M>,
    |                       ^^^^^^^^^^^^^^^^^^^^ required by this bound in `App::add_systems`

For more information about this error, try `rustc --explain E0277`.
nicopap commented 1 year ago

The macro requires access to the bevy Time resource in order to implement the cooldown feature (Locals are safe as exclusive system parameters). I guess if you still want to log things, we could avoid adding the Res<Time> when cooldown is explicitly set to 0.

Workaround

In the current version, you can avoid the additional params by using the silent log-level.