bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Add `Configure` trait as a receiver-free alternative to `Plugin`

Open benfrankel opened this issue 1 year ago • 0 comments

What problem does this solve or what need does it fill?

It solves the code organization problems encountered when defining types that require some configuration in order to work, illustrated in the "alternatives considered" section.

What solution would you like?

Proposed API:

fn plugin(app: &mut App) {
    app.configure::<(MySystemSet, MyEvent, MyComponent)>();
}

#[derive(SystemSet, ...)]
pub enum MySystemSet { A, B, C, D, E }

impl Configure for MySystemSet {
    fn configure(app: &mut App) {
        app.configure_sets(Update, (Self::A, Self::B, Self::C, Self::D, Self::E).chain());
    }
}

// ...

Implementation:

pub trait Configure {
    fn configure(app: &mut App);
}

macro_rules! impl_configure {
    ($($T:ident),*)  => {
        impl<$($T: Configure),*> Configure for ($($T,)*) {
            fn configure(app: &mut App) {
                $($T::configure(app);)*
                let _ = app;
            }
        }
    }
}

all_tuples!(impl_configure, 0, 15, T);

pub trait AppExtConfigure {
    fn configure<T: Configure>(&mut self) -> &mut Self;
}

impl AppExtConfigure for App {
    fn configure<T: Configure>(&mut self) -> &mut Self {
        T::configure(self);
        self
    }
}

What alternative(s) have you considered?

Inline the configuration

fn plugin(app: &mut App) {
    // Configure MySystemSet.
    app.configure_sets(Update, (
        MySystemSet::A,
        MySystemSet::B,
        MySystemSet::C,
        MySystemSet::D,
        MySystemSet::E,
    ));

    // Configure MyEvent
    // ...

    // Configure MyComponent
    // ...
}

This is usually done for internal types, but there's a number of drawbacks:

  1. This is not re-usable, which is especially relevant for generic types like MySystemSet<T>.
  2. The configure code may be far from where the type itself is defined, so e.g. if a new variant MySystemSet::F is added, you have to remember to track down where the type was configured and update that as well.
  3. The plugin has to "know" how to configure MySystemSet, which is an inversion of responsibility.

Define an extension trait on App for each type

This is often done for 3rd-party crates, but it's a lot of boilerplate to implement, requires users to import the extension traits, and requires users to learn new methods like app.configure_my_system_set().

Define a separate Plugin for each type

fn plugin(app: &mut App) {
    app.add_plugins((configure_my_system_set, configure_my_event, configure_my_component));
}

fn configure_my_system_set(app: &mut App) {
    app.configure_sets(Update, (Self::A, Self::B, Self::C, Self::D, Self::E).chain());
}

// ...

This isn't that bad, but there is a drawback: there's no connection between MySystemSet and configure_my_system_set. So if MySystemSet is a 3rd-party type for example, now you have to know that a plugin for that type exists, know what it's named, and import it separately.

Implement Plugin for each type

fn plugin(app: &mut App) {
    app.add_plugins((MySystemSet::A, MyEvent::default(), MyComponent(0)));
}

impl Plugin for MySystemSet {
    fn build(&self, app: &mut App) {
        app.configure_sets(Update, (Self::A, Self::B, Self::C, Self::D, Self::E).chain());
    }
}

// ...

Now configuring MySystemSet requires passing a dummy value by reference that will be totally ignored.

benfrankel avatar Jul 10 '24 08:07 benfrankel