Back to Base Sets
- Dominant language
- Rust
- Stars
- 48.2k
- Forks
- 4.8k
- Avg merge
- 3d 16h
- Merged PRs (30d)
- 171
Description
# 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:
```rust
pub trait SystemLocation {
fn get_schedule_set_pair(&self) -> (Box, Option>);
}
impl SystemLocation for T {
fn get_schedule_set_pair(&self) -> (Box, Option>) {
(self.dyn_clone(), None)
}
}
```
Then `add_systems` becomes:
```rust
pub fn add_systems(
&mut self,
system_location: impl SystemLocation,
systems: impl IntoSystemConfigs
) -> &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:
```rust
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):
```rust
#[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:
```rust
pub struct Update;
impl SystemLocation for Update {
fn get_schedule_set_pair(&self) -> (Box, Option>) {
(Box::new(Main), Some(Box::new(CoreSet::Update)))
}
}
```
Now look what this does:
```rust
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:
```rust
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`.
```rust
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:
```rust
render_app.add_systems(Queue, queue_donkey);
```
`MainScheduleOrder` can be replaced with just `configure_sets` calls:
```rust
#[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.
Contributor guide
Research direction
Start by reading App::add_systems, SystemLocation, CoreSet, and MainScheduleOrder, then review the scheduling discussion and the relationship to #9822. No file or test is named, and the proposal lists alternatives and risks rather than a settled implementation scope; done would require an agreed design plus corresponding implementation and validation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- game-dev
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100