Skip to content

Back to Base Sets #16432

@andriyDev

Description

@andriyDev

Background

Today, we have many schedules. Main, First, PreUpdate, Update, etc. Primarily, these are used for general ordering of systems. This has necessitated a bunch of sad parts like MainScheduleOrder. This is just code duplication. We already have a way to order a bunch of systems: system sets! In theory, instead of Update being a schedule, it could be a system set and get basically the same behaviour. In theory we could even have systems that run between PreUpdate and Update (without needing a schedule in between). Schedules reduce parallelism and system set constraints aren't (currently) shared across schedules.

Base Sets

In #8079, we removed base sets. I believe this was the correct move at the time. The main problems listed in that PR:

  • "It is a new concept that users need to content with"
    • Agreed! However I primarily believe this was in the nomenclature. To change a system's base set, you had to call in_base_set which was different from in_set.
  • "It was easy to accidentally create invalid orderings: add_system(foo.after(TransformPropagate)) would fail because foo is implicitly in CoreSet::Update"
    • This is a problem because Update is nowhere to be seen, and no way to really "reach" it with "Go to Definition".
  • "They were largely added to enable 'default base sets'"
    • I would argue this is the root of the problem.

I am not trying to bring back base sets as they were. However, they have a nice advantage: there's pretty much one schedule to deal with: Main.

Bringing them back in style

The current syntax for adding systems is very nice:

app.add_systems(Update, my_system);

Let's keep that! I propose adding a trait like:

pub trait SystemLocation {
    fn get_schedule_set_pair(&self) -> (Box<dyn ScheduleLabel>, Option<Box<dyn SystemSet>>);
}

impl<T: ScheduleLabel> SystemLocation for T {
    fn get_schedule_set_pair(&self) -> (Box<dyn ScheduleLabel>, Option<Box<dyn SystemSet>>) {
        (self.dyn_clone(), None)
    }
}

Then add_systems becomes:

pub fn add_systems<M>(
    &mut self,
    system_location: impl SystemLocation,
    systems: impl IntoSystemConfigs<M>
) -> &mut App {
    // ...
}

So far, nothing has changed: we can still add systems to a schedule like normal. We can also still add systems to system sets like normal:

render_app.add_systems(Render, queue_donkey.in_set(RenderSet::Queue));

The kicker comes from the next step. First, we need to bring back the CoreSet enum (part of it at least):

#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)]
pub enum CoreSet {
    First,
    PreUpdate,
    RunFixedMainLoop, // We can probably remove this one.
    Update,
    SpawnScene,
    PostUpdate,
    Last,
}

And finally, we change Update and friends, to the following:

pub struct Update;

impl SystemLocation for Update {
    fn get_schedule_set_pair(&self) -> (Box<dyn ScheduleLabel>, Option<Box<dyn SystemSet>>) {
        (Box::new(Main), Some(Box::new(CoreSet::Update)))
    }
}

Now look what this does:

app.add_systems(Update, move_donkey.in_set(MovementSystemSet));

Wait... nothing changed... Exactly! Under the hood, Update is no longer a schedule. It is a system set in the Main schedule. Provided that Bevy adds a .chain() for all the CoreSet, #9822 will automatically insert a sync point between the "base sets". Now we've just made some syntactic sugar for:

app.add_systems(Main, move_donkey.in_set(MovementSystemSet).in_set(CoreSet::Update));

User's don't need to learn anything about base sets, they just silently use them. There's also nothing special about base sets. Users can create their own "base sets" by just making a new struct that impls SystemLocation.

app.add_systems(avian2d::PhysicsSet::Prepare, explode);

Even rendering can take advantage of this. As far as I can tell, rendering uses one schedule with a bunch of system sets for performance reasons. Now we can have a nicer API:

render_app.add_systems(Queue, queue_donkey);

MainScheduleOrder can be replaced with just configure_sets calls:

#[derive(SystemSets)]
struct PrePreUpdate;
app.configure_sets(Main, (First, PrePreUpdate, PreUpdate).chain());

Alternatives

  • We could make CoreSet implement SystemLocation. Then our add_systems calls would look like app.add_systems(CoreSet::Update, move_donkey).
  • We could make system sets always be associated with one schedule. Then we can just allow system sets to be used in add_systems. (I'm not a fan of this since it reduces what we can do with system sets).
  • We don't strictly need this. It's mostly cleaning up and "simplifying" things (simplifying in the sense that there are fewer moving parts).

Risks

  • This can be confusing since we're now passing in schedules and things that look like system sets into add_systems. This can be confusing to learn about the difference between Update, CoreSet::Update, and Main. (it may be confusing that you can add systems to a system set in two ways). I think this is fine since this is mostly just an implementation detail. From a user's perspective they are doing the same thing that schedules do today.
  • The schedule is now implicit. I think this is fine since there's still something to "Go to Definition" on. It's fairly easy to go to the definition of Update and (assuming its SystemLocation impl is close), see that it belongs to the Main schedule.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-ECSEntities, components, systems, and eventsC-FeatureA new feature, making something new possibleS-Needs-Design-DocThis issue or PR is particularly complex, and needs an approved design doc before it can be mergedX-ControversialThere is active debate or serious implications around merging this PR

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions