How should we add systems to schedules?

[cart]

Bevy now has a number of ways to add systems to schedules:

1. app.add_system(foo.in_schedule(X))
2. Context-specific App builder methods: app.add_startup_system(foo).
   • This is equivalent to foo.in_schedule(CoreSchedule::Startup)
3. Context-specific SystemConfig builder methods: app.add_system(foo.on_startup())

There are proposals to deprecate add_startup_system for on_startup (#7694).

There are also proposals to add new on_X functions, such as foo.on_enter(X) (equivalent to foo.in_schedule(OnEnter(X)) and foo.on_exit(X).

Before we start embracing a pattern as the "one way to do things", we should discuss the problem being solved, motivation, and alternatives.

Here is a roll-up of the current options:

// Option 1
app.add_system(foo.on_startup())
app.add_system(bar.on_enter(X))
app.add_system(baz)

// Option 2
app.add_system_to(Startup, foo)
app.add_system_to(Enter(X), bar)
app.add_system(baz)

// Option 3
app.edit_schedule(Startup, | schedule| { schedule.add_system(foo))
app.edit_schedule(Enter(X), | schedule| { schedule.add_system(bar))
app.add_system(baz)

// Option 4
app.add_system(foo.in_schedule(Startup))
app.add_system(foo.in_schedule(Enter(X)))
app.add_system(baz)

// Option 5
app.add_system(Startup, foo)
app.add_system(Enter(X), bar)
app.add_system(Main, baz)

[cart]

Option 1: app.add_system(foo.on_x())?

Use "trait extension methods" to add shorthand for foo.in_schedule(X)

Pros

• It has the potential to improve ergonomics (but this is not always the case):
  • Improvement

    // before
    app.add_system(foo.in_schedule(OnEnter(X)))
    
    // after
    app.add_system(foo.on_enter(X))

  • Improvement

    // before
    app.add_system(foo.in_schedule(CoreSchedule::Startup))
    // before (top level Startup variant)
    app.add_system(foo.in_schedule(Startup))
    
    // after
    app.add_system(foo.on_startup())

  • Regression

    // before
    app.add_startup_system(foo)
    
    // after
    app.add_system(foo.on_startup())

• It simplifies the top level App api by embracing the schedule as "per system config"

Cons

• The on_X pattern requires "trait extension methods". Not a huge problem for built in schedules, but this is a significant complexity burden for users:

  // Note that this relies on consolidating IntoSystemAppConfig into IntoSystemAppConfigs
  // otherwise two traits and impls will be required
  pub trait OnXBuilder<Marker>: IntoSystemAppConfigs<Marker> {
      fn on_x(self) -> SystemAppConfigs {
          self.in_schedule(self)
      }
  }
  
  impl<T, Marker> OnXBuilder for T where T: IntoSystemAppConfigs<Marker> { }

• The on_X pattern completely abstracts out the concept of a "schedule". This will almost certainly hamper some users from building correct mental models.
• on_X can be called multiple times for the same system: foo.on_enter().on_exit().

[cart]

Option 2: app.add_system_to(X, foo)

We would also deprecate foo.in_schedule(X). Also note that in practice we are planning on deprecating add_system, so this would be add_systems_to. But for comparison parity with Option 1 I'm only considering add_system_to.

Pros

• It has the potential to improve ergonomics (but this is not always the case):
  • Improvement

    // before
    app.add_system(foo.in_schedule(OnEnter(X)))
    
    // after
    app.add_system_to(OnEnter(X), foo)

  • Improvement

    // before
    app.add_system(foo.in_schedule(CoreSchedule::Startup))
    // before (top level Startup variant)
    app.add_system(foo.in_schedule(Startup))
    
    // after
    app.add_system_to(Startup, foo)

  • Regression

    // before
    app.add_startup_system(foo)
    
    // after
    app.add_system_to(Startup, foo)

• It does not require extension methods / boilerplate like Option 1. Users can use this "for free" with their existing schedules.
• It is consistent: we don't have both foo.in_schedule(X) and foo.on_x() like Option 1.
• It does not completely erase the schedule concept like Option 1.
• app.add_system_to(X, foo) follows rust Key->Value (X->Foo) idioms more closely. Ex: hashmap.insert(X, foo)
• app.add_system_to(X, foo) cannot be called "twice" for the same foo system, which is a problem with "system config" variants: foo.in_schedule(X).in_schedule(Y) and foo.on_enter().on_exit().
• This moves the "schedule" property outside of "system config". Some might also consider this a con.
• This forces "formatting alignment", which puts the schedules in the same column with each other:

  app
      .add_system_to(Startup, foo.in_set(A))
      .add_system_to(OnEnter(X), bar)
      .add_system_to(OnExit(X), baz.in_set(B))
  
  // compare to this:
  app
      .add_system(foo.in_set(A).on_startup())
      .add_system(bar.on_enter(X))
      .add_system(baz.on_exit(X).in_set(B))

Cons

• Option 1 can be slightly more ergonomic sometimes (but not always!)

  // Option 1
  app.add_system(foo.on_enter(X))
  // Option 2
  app.add_system_to(OnEnter(X), foo)
  
  // Option 1
  app.add_system(foo.on_startup())
  // Option 2
  app.add_system_to(Startup, foo)

• This moves the "schedule" property outside of "system config". Some might also consider this a pro.

  // Notably, this prevents:
  app.add_systems((
      foo.on_enter(X),
      bar.on_exit(X),
  ))
  // Instead, you need
  app
      .add_system_to(OnEnter(X), foo)
      .add_system_to(OnExit(X), bar)
  // But remember this is also a pro! This invalid configuration is no longer expressible
  app.add_systems((
      foo.on_enter(X),
      bar.on_exit(X),
  ).chain())
  // Nor is this!
  app.add_system(foo.on_enter(X).on_exit(X))

[cart]

Note that we would probably also want to discuss aligning Schedule names:

• OnThing: OnStartup, OnEnter(X), OnExit(X)
• Thing: Startup, Enter(X), Exit(X)

I like the terseness of Thing.

[alice-i-cecile]

I like the terser names, but I think that's orthogonal here :) Note that we're also hoping to add Transition(X, Y) schedules too for states in #7936.

[alice-i-cecile]

I still prefer add_system_to_schedule over add_system_to, if we end up going this route. If one of the pros of this approach is that it's explict about what's a schedule, we should actually note that we're adding things to schedules, rather than using a sentence fragment.

[alice-i-cecile]

I also think that those ergonomics examples are all regressions. It's very much a matter of taste, but I don't think there's a clear improvement there.

[cart]

I like the terser names, but I think that's orthogonal here :) Note that we're also hoping to add Transition(X, Y) schedules too for states in #7936.

I disagree. I think a primary motivator for the on_X stuff is "ergonomics". Terser names are relevant to that conversation. Same argument for add_system_to_schedule vs add_system_to. I think people will try to come up with reasons not to use "long" apis in this context (ex: someone will re-invent on_x).

One other option is just adding this to add_systems:

app
  .add_systems(Main, foo)
  .add_systems(Startup, bar)
  .add_systems(Enter(X), baz)

It would mean adding Main everywhere (which is certainly a controversial change), but full parity + api alignment does feel compelling.

[alice-i-cecile]

Option 3

Just use edit_schedule.

Instead of:

app.add_system(foo.in_schedule(Bar))

You can use:

app.edit_schedule(Bar, | schedule| { schedule.add_system(foo))

(This is mostly a joke, but since it was the inital API shipped in the ScheduleV3 PR it's worth at least ruling out).

Pros

• It is consistent: we don't have both foo.in_schedule(X) and foo.on_x() like Option 1.
• It does not completely erase the schedule concept like Option 1.
• Nice small API surface.
• This moves the "schedule" property outside of "system config".
• You can do many operations on the same schedule at once.

Cons

• So much boilerplate
• Closures are really tricky for beginners to read
• When working with states, you have a few operations in many diferrent schedules, rather than many operations on one schedule. This is not true for FixedUpdate or Startup, and I'd probably endorse using this pattern for that!

[alice-i-cecile]

Option 4

Keep in_schedule, don't add any extenstion methods. Remove add_startup_system and shorten schedule names to a single word exported in the prelude.

app.add_system(spawn_player.in_schedule(Startup))

Pros

• very explicit about what's a schedule
• low on boilerplate
• easy to refactor in common ways (e.g. startup -> on-enter)
• no need to write extension traits
• system sets retain schedule information, allowing you to quickly swap them in and out of various state transitions
• simplifies the top level App api by embracing the schedule as "per system config"

Cons

• can add systems to multiple schedules by accident (like for base sets)
• run time panic when the above occurs
• a little tedious to always call .in_schedule(Exit(State::Running) everywhere compared to special-cased builder methods

[alice-i-cecile]

I think it's important to compare across the same level of explictness:

app.add_system(foo.in_schedule(Startup))
app.add_system_to_schedule(Startup, foo)

or

app.add_system(foo.to(Startup))
app.add_system_to(Startup, foo)

[cart]

I didn't want to propose to() because removing it from the top level makes it way less clear imo. The Key->Value style api and the proximity of to to add_system buys us context that add_system(foo.to(Startup)) does not. I think the question "what is to adding to?" is significantly less clear in that context, especially when combined with system sets: add_system(foo.in_set(X).to(Startup))

[cart]

Putting _to at the top level helps imply a Key->Value / parent child relationship.

[alice-i-cecile]

What about in? app.add_system(foo.in(Startup)) is much clearer to me than the to form.

[cart]

The parity is definitely better, but I still think it suffers from the same problems listed above.

[cart]

Option 5: app.add_systems(X, foo)

Require specifying a schedule in add_system:

app
  .add_system(Main, foo)
  .add_system(Startup, bar)
  .add_system(Enter(X), baz)

(note that in practice we will likely deprecate add_system in favor of add_systems everywhere)

Pros

• Same pros as Option 2 (app.add_systems_to)
• Terser than Option 2 when specifying non-default schedules
• One single API for everything
• Full columnar alignment of schedules. Reads well
• No more "implicit default schedule" and therefore less confusion.

Cons

• Same cons as Option 2
• Requires specifying Main whenever adding systems. This adds extra boilerplate to the common app.add_systems((foo, bar)) operation.

[cart]

I really like the simplicity / terseness / alignment here, but requiring Main when adding normal systems is a very big ask.