System sets do not share their configuration across schedules

[alice-i-cecile]

Bevy version

0.11

What you did

1. Create a system set MySystemSet.
2. Give it some configuration using configure_set(Update, MySystemSet).
3. Add some systems in both the Update schedule and another schedule to this set.

What went wrong

The configuration is silently missing for systems in other schedules.

If a system set defines a set of behaviors (like running in a certain state), then I would expect that behavior to be true across schedules (where possible).

Additional information

The fact that this will happen is thankfully much clearer now that we require explicit schedules everywhere, but it's still a nuisance and a footgun.

[alice-i-cecile]

My first thought was that we should store the metadata about what a system set should do at the Schedules level, and attempt to apply it for wherever it is present.

However, doing so would break the simpler single-schedule use cases, since then they no longer have a conception of system sets.

We could do a lookup across all schedules, maybe?

In terms of band-aids:

1. We could warn if a system is in a set with no configuration. This is almost always a bug in user code, but not library code, so that would need to be silenceable (SystemSet trait const maybe?).
2. We could add a method to on App to configure a set across all schedules, and then add the definition to each schedule. This is bad duplication, and introduces even worse builder-order problems, as schedules could be added later.

Rubber-ducking, I think the best solution may be to add "universal" system sets, which are stored at the Schedules level, in addition to those stored on Schedule.

[vacuus]

What's a use case for a system set that spans multiple schedules?

[alice-i-cecile]

So, a common example in my games is "gameplay logic". These systems should not run if the game is paused, or if the assets are not yet loaded. System sets let us configure this behaviour in one convenient spot, but only if we're only using a single schedule.

[cBournhonesque]

So would the solution be to add all sets at the Schedules level?
i.e. a given Set is always shared between all Schedules?

[alice-i-cecile]

Yep, that's my intended fix. There's one wrinkle though: we need to figure out what to do with system sets for a single schedule, which some Bevy users will be working with.

I think the right design here is to have system sets defined at the schedule level, whose config then gets additively mirrored both up and down to Schedules. There may be a simpler solution though!

[cBournhonesque]

What do you mean by system sets for a single schedule?
I thought the idea was that set configuration would become independent of schedules?

i.e. if we have the set added to a single schedule

app.configure_sets(Update, set_a.run_if(in_state(Playing)))

the set is included in schedule Update, but the configuration part is schedule-independent

[alice-i-cecile]

Hmm. So, in theory Schedule can be used and configured on its own. This can be useful for plugin customizability, or exotic light-weight use cases.

On reflection, let's just lift this up to Schedules and see if anyone complains. If they do, we can consider the more complex architecture.

[cBournhonesque]

I see, do you have an example of Schedule being configured on its own? Or is there one in the code-base?
I can try to make a small PR that lifts up the set configs to the Schedules and see how that looks.

The only thing i'm worried about is if users were intentionally having different configs for the same SystemSet in different schedules, but I guess in those cases they can just shift to creating different sets

[cBournhonesque]

I'm also worried about ergonomics. It's pretty confusing that
app.configure_sets(Update, set_a.run_if(..))

• adds set_a to the Update schedule
• adds the NodeConfig in all schedules that contain Update

I understand the problem (and ran into it myself a couple times), but the mental model can become a tad confusing

[alice-i-cecile]

With this change, we can / should make configure_sets not take a ScheduleLabel arg. I think that should help clarify semantics substantially, and it's nice that it's a bit more terse.

I see, do you have an example of Schedule being configured on its own? Or is there one in the code-base?

I don't think I've seen this in the wild, and it's definitely not in Bevy's codebase. Don't worry about it for now IMO.

[cBournhonesque]

I see, so to summarize:

// sets are not tied to schedules at all, they are just a way to provide hierarchy/ordering/run_conditions to a group of systems, independently of which schedules the systems are part of
app.configure_set(SetA.run_if(in_state(Playing));

// systems are added to a specific schedule; if they are part of a set, then the set's config (hierarchy/ordering/run_conditions) apply to the system as well
// here, both systems would only run in state Playing, even though they are in different schedules
app.add_systems(Update, system_a.in_set(SetA));
app.add_systems(PostUpdate, system_b.in_set(SetA));

That works for me! The implementation seems pretty complicated though, that's actual a big change

[alice-i-cecile]

Yep, exactly!