Affinitize System Sets to Schedules

[cart]

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

Sometimes people do things like:

#[derive(SystemSet, Debug, Clone, Copy, PartialEq, Eq, Hash)]
struct X;

app
  .configure_sets(Update, X.run_if(in_state(GameState::Playing)))
  .add_systems(Update, foo.in_set(X))
  .add_systems(PostUpdate, foo.in_set(X))

Then they get confused when foo runs unconditionally in PostUpdate (because sets are "per schedule" configuration).

What solution would you like?

One way to ensure this never happens is to require a specified ScheduleLabel type when implementing SystemSet.

#[derive(SystemSet, Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[system_set(Update)]
struct X;

app
  .configure_sets(Update, X.run_if(in_state(GameState::Playing)))
  .add_systems(Update, foo.in_set(X))
  // This fails to compile
  .add_systems(PostUpdate, foo.in_set(X))

I haven't fully thought this one through yet / I'm not sure we actually want this solution. Just putting the idea out there.

[cart]

Oops posted this too early by pressing "enter". Bear with me for a sec as I fill it in.

[cart]

One problem with this is that some ScheduleLabels are "by value":

.add_systems(OnExit(GameState::Playing), teardown)
.add_systems(OnEnter(GameState::GameOver), display_score)

[hymm]

I was considering something like this. I think it'd be fine to have an opt out #[system_set_any_schedule]

[B-Reif]

I don't love the thought of repeating myself like this / remembering to type schedules out in both places.

I suspect this confusion is happening specifically as a hiccup of finding 0.11 equivalents to existing configurations. For example, in the renet PR:

// main, v.10: Schedule is optional and left unspecified.
app.configure_set(RenetSet::Server.run_if(resource_exists::<RenetServer>()));

// PR, v.11: Schedule required. No equivalent statement to the above exists (in isolation).
// Migration requires the author to understand every schedule this set runs in.
app.configure_set(Update, RenetSet::Server.run_if(resource_exists::<RenetServer>()));

Going forward, I think the existing API is reasonably obvious. The schedule-first argument to configure_set pretty strongly implies the importance of the given schedule value. I'd suggest waiting to see if this problem persists in new code, when the previous API is a more distant memory.

[Shatur]

I think it'd be fine to have an opt out #[system_set_any_schedule]

I think it makes sense...
For example, I working on a multiplayer game and I going to have a lot of systems that check if client / server.
This condition will run for each system. I wish we could have the ability to run same conditions once.

[vacuus]

https://docs.rs/bevy/latest/bevy/app/struct.App.html#method.configure_set
https://docs.rs/bevy/latest/bevy/app/struct.App.html#method.configure_sets (basically the same documentation)

Configures a collection of system sets in the default schedule, adding any sets that do not exist.

The existing documentation doesn't reflect the Schedule-First changes in 0.11.0

For the current behavior it should be something like

Configures a collection of system sets in the given schedule, adding any sets that do not exist. System sets are configured on a per-schedule basis: the configuration of a system set in one schedule won't carry over to other schedules.

[rlidwka]

Then they get confused when foo runs unconditionally in PostUpdate (because sets are "per schedule" configuration).

What is the reason for them being "per schedule"? Why can't set configuration apply to any schedule a set is put into?

One way to ensure this never happens is to require a specified ScheduleLabel type when implementing SystemSet.

Sounds very reasonable to limit each set to a specific schedule.

And if you were to do that, perhaps the add syntax can be simplified to .add_systems(X, foo) (with set as a first argument), and bevy would know that X is a part of Update.

[deifactor]

For most of the sets I define in my own game, I'm positive that I never want any system in that set unless it's in a given schedule. All my DamageStep systems need to happen in PostUpdate, because the physics step runs there and I can't do collision detection until after the physics has happened. So I'd really like a feature like this. But there's nothing stopping me from accidentally doing app.add_systems(Update, my_system.in_set(DamageStep)).

[vonforum]

I'd like to point out my idea for this in #8353

[alice-i-cecile]

#9792 addresses this problem too, but proposes a different solution.