Add .with_system_set method to SystemSet

[alice-i-cecile]

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

Reusing system sets is currently challenging: users may for example want to run a collection of systems in several states at once.

What solution would you like?

Add with_system_set to match with_system on the SystemSet struct.

What alternative(s) have you considered?

More completely overhaul / reorganize system sets and scheduling.

Additional context

This should be relatively straightforward for newcomers who want to get to know the ECS a bit better. Follow the impl here:

bevy/crates/bevy_ecs/src/schedule/system_set.rs

Line 94 in c893b99

pub fn with_system<Params>(mut self, system: impl IntoSystemDescriptor<Params>) -> Self {

and push the systems from the new system set into self's corresponding field instead.

[Zengor]

Hi, looking into this. Two questions:

• What should be done when both system sets have run criteria? Prioritize self's?
• Should the other system set be consumed? Or taken as a reference + cloning its members? If the idea is to make reuse easier I guess the later is preferable but I'm not sure if this implicit cloning would be a problem. (actually it doesn't seem like SystemDescriptor even implements Clone)

[alice-i-cecile]

What should be done when both system sets have run criteria? Prioritize self's?

Hmm. Prioritize self IMO? That will need to be clearly documented though.

Should the other system set be consumed? Or taken as a reference + cloning its members? If the idea is to make reuse easier I guess the later is preferable but I'm not sure if this implicit cloning would be a problem. (actually it doesn't seem like SystemDescriptor even implements Clone)

I don't mind implicit cloning here: this should be a one-time cost. IMO we should try to impl Clone for SystemDescriptor in this PR and then use that. I could see it being useful elsewhere as well for similar reuse patterns.

[Zengor]

wrt implementing Clone, I ran into the issue that BoxedSystem and Box<dyn ExclusiveSystem> are not clonable and I'm not quite sure how to work around that (the systems themselves are functions using the function traits so I don't think they are clonable at all). Maybe there is some workaround that could be done with an Arc but I'm not sure where to go from here.

[alice-i-cecile]

Hmm. I think we should table this for the moment then; we can review it as the schedule and state story improves.

[Ratysz]

You can't clone systems - there is, ordinarily, no point to doing that, and adding that restriction just for the sake of this API isn't a good idea. This is not going to "improve" in the future.

What might be possible instead: introduce a Clone-like extension trait implemented on the descriptor obtained from clonable systems. I can't visualize how that's even going to work (descriptors are meant to be nongeneric), you might have to plumb in a whole pile of these traits and create a duplicate of most of descriptor logic but with cloning built in.

[cart]

I think other scenarios tie in to this:

• Running systems on a trigger (just passing an owned system each time it's needed isn't efficient, as has been proposed here run a system from a command #2234)
• Running specific systems as "reactions"

We already have ways to address systems:

• SystemId
• Labels

Maybe we need a way to say things like:

• "add all systems with LabelA to ScheduleThingY"
• "add pre-existing SystemIdB to ScheduleThingZ"
• "run SystemIdC once"
• "run all systems with LabelD once"

[Ratysz]

I think other scenarios tie in to this: [...]

They do, but this issue is kind of structured around the idea of cloning the systems.

To me, a good solution for those would be a dedicated out-of-schedule storage for systems, indexed by labels and/or a kind of a unique key (system name?).

Maybe we need a way to say things like: [...]

We've been referring to this as "label properties": i.e., do things to systems by doing things to their labels. I don't think this is feasible to implement until we untangle the stages/states/lifecycle doom triangle, we need a completely flat systems storage if we want to do this sanely.

[james7132]

SystemSets, as used prior to stageless, no longer exist. Closing this issue for now.