bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Trait fn returning `impl Fn()` cannot be used as a system when called from generic type

Open benfrankel opened this issue 1 year ago • 1 comments

Bevy version

0.13.2

What you did

trait Foo {
    fn foo() -> impl Fn() {
        || {}
    }
}

enum ConcreteFoo {}

impl Foo for ConcreteFoo {}

fn frobnicate<GenericFoo: Foo>(app: &mut App) {
    app.add_systems(Update, <ConcreteFoo as Foo>::foo()); // Is a system
    app.add_systems(Update, <GenericFoo as Foo>::foo()); // Is not a system
}

What went wrong

The second foo() is not considered a system, so it doesn't compile.

Additional information

This looks like a Rust bug, but I have no idea what the source of the bug is (something to do with RPITIT?), and this affects my usage of Bevy, so I'm creating an issue here for now.

benfrankel avatar May 20 '24 09:05 benfrankel

Workaround:

trait Foo {
    fn foo() -> BoxedSystem {
        Box::new(IntoSystem::into_system(|| {}))
    }
}

benfrankel avatar May 20 '24 10:05 benfrankel

This bug applies to run conditions as well, but BoxedCondition is not a proper workaround because it doesn't impl Condition. You have to use .run_if_dyn instead of .run_if, but .run_if_dyn is a real pain to use because it doesn't return Self:

fn my_system() {}

fn always_box() -> BoxedCondition {
    Box::new(IntoSystem::into_system(|| true))
}

fn foo(app: &mut App) {
    app.add_systems(Update, my_system.run_if(always_box())); // Does not compile
    app.add_systems(Update, {
        let mut configs = my_system.into_configs();
        configs.run_if_dyn(always_box());
        configs
    }); // Does compile
}

benfrankel avatar May 20 '24 18:05 benfrankel

I think you need fn foo() -> impl Fn() + Sync + Send + 'static {.

System requires those trait bounds, and for ConcreteFoo the compiler can see the actual type and validate them. But for GenericFoo it has to protect against something like

struct EvilFoo;
impl Foo for EvilFoo {
    fn foo() -> impl Fn() {
        let x = Cell::new(0);
        // This is not `Sync` and so cannot be a `System`.
        move || {
            x.set(0);
        }
    }
}

chescock avatar May 20 '24 18:05 chescock

Interesting. That works. I guess this isn't a bug in Rust or Bevy, but it is surprising.

benfrankel avatar May 20 '24 19:05 benfrankel