Improve node encapsulation in ScheduleGraph

[ItsDoot]

Objective

• Part of Extensible Schedule Graphs #20115

We want to encapsulate each part of ScheduleGraph into its own specific struct to make parts of it easier to reuse and maintain.

Solution

• Pulled ScheduleGraph::systems and ScheduleGraph::system_conditions into a Systems struct and added a field for this new struct to ScheduleGraph
• Broke up ScheduleGraph::uninit into Systems::uninit and SystemSets::uninit to eliminate ScheduleGraph's direct field access of these types
• Removed node and condition accessors from ScheduleGraph; the same operations are now available on Systems and SystemSets instead (accessible via their pub fields on ScheduleGraph)
• Moved Systems, SystemSets, SystemNode, SystemWithAccess, and ConditionWithAccess into a separate file.

Testing

Added two new tests covering the API surface of Systems and SystemSets, respectively.

[chescock]

Yeah, this is getting much easier to understand!

[chescock]

I thought using Deref like this for "inheritence" was discouraged, and it was only supposed to be used for smart pointers.

[ItsDoot]

I added it to get rid of some system.system stutter, because for practically all intents and purposes a SystemWithAccess is a System. I'll leave this up to @alice-i-cecile

[alice-i-cecile]

Yeah Bevy's style guide disagrees with that advice. There's a reason we offer our own derive macros for Deref/DerefMut.

[ItsDoot]

The alternative I suppose is impl System for SystemWithAccess. I just want to avoid the stutter, really.

[hymm]

I feel like this would be more clear just storing Option<SystemWithNode> instead of using the SystemNode wrapper.

[hymm]

Just noticed this was done in a previous pr. Feel free to ignore this then.

[ItsDoot]

SystemNode will disappear entirely in my next PR (System Arcinization), so it shouldn't be staying around too much longer anyways.

[alice-i-cecile]

I'm happy enough with this :) Yay for smaller files too.

[alice-i-cecile]

The alternative I suppose is impl System for SystemWithAccess. I just want to avoid the stutter, really.

@ItsDoot I actually really like this idea. It's type-erased so it's not going to explode codegen. Are you willing to do that now?

[chescock]

The alternative I suppose is impl System for SystemWithAccess. I just want to avoid the stutter, really.

Another option here is to move the access to a separate storage, like another SecondaryMap. We need to store them for the multi-threaded executor, or if the ambiguity checker is on. But if we're running single-threaded and the detector is off them we could drop them to save space, just like we do with observers and registered systems. That might be easier if they're not part of the same structure.

Either way, thank you for cleaning up the mess I made when I stuck the access there :).

[ItsDoot]

@ItsDoot I actually really like this idea. It's type-erased so it's not going to explode codegen. Are you willing to do that now?

Done!

Another option here is to move the access to a separate storage, like another SecondaryMap. We need to store them for the multi-threaded executor, or if the ambiguity checker is on. But if we're running single-threaded and the detector is off them we could drop them to save space, just like we do with observers and registered systems. That might be easier if they're not part of the same structure.

Either way, thank you for cleaning up the mess I made when I stuck the access there :).

I quite like this idea, but I'll hold off on it at least until my next PR since those are likely to have similar blast-radii.