forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve node encapsulation in ScheduleGraph (#20119)

Browse Source 
# Objective

- Part of #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.

---------

Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
This commit is contained in:
Christian Hughes 2025-07-15 01:29:52 -05:00 committed by GitHub
parent 0747b66602
commit 2be3bc5310
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 709 additions and 352 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs
View File

@ -4,13 +4,13 @@ use bevy_platform::collections::HashMap;
use crate::{
schedule::{SystemKey, SystemSetKey},
system::IntoSystem,
system::{IntoSystem, System},
world::World,
};
use super::{
is_apply_deferred, ApplyDeferred, DiGraph, Direction, NodeId, ReportCycles, ScheduleBuildError,
ScheduleBuildPass, ScheduleGraph, SystemNode,
ScheduleBuildPass, ScheduleGraph,
};
/// A [`ScheduleBuildPass`] that inserts [`ApplyDeferred`] systems into the schedule graph
@ -49,10 +49,7 @@ impl AutoInsertApplyDeferredPass {
fn add_auto_sync(&mut self, graph: &mut ScheduleGraph) -> SystemKey {
let key = graph
.systems
.insert(SystemNode::new(Box::new(IntoSystem::into_system(
ApplyDeferred,
))));
graph.system_conditions.insert(key, Vec::new());
.insert(Box::new(IntoSystem::into_system(ApplyDeferred)), Vec::new());
// ignore ambiguities with auto sync points
// They aren't under user control, so no one should know or care.
@ -81,7 +78,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
let topo = graph.topsort_graph(dependency_flattened, ReportCycles::Dependency)?;
fn set_has_conditions(graph: &ScheduleGraph, set: SystemSetKey) -> bool {
!graph.set_conditions_at(set).is_empty()
graph.system_sets.has_conditions(set)
|| graph
.hierarchy()
.graph()
@ -94,7 +91,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
}
fn system_has_conditions(graph: &ScheduleGraph, key: SystemKey) -> bool {
!graph.system_conditions[key].is_empty()
graph.systems.has_conditions(key)
|| graph
.hierarchy()
.graph()
@ -108,7 +105,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
let mut system_has_conditions_cache = HashMap::<SystemKey, bool>::default();
let mut is_valid_explicit_sync_point = |key: SystemKey| {
is_apply_deferred(&graph.systems[key].get().unwrap().system)
is_apply_deferred(&graph.systems[key])
&& !*system_has_conditions_cache
.entry(key)
.or_insert_with(|| system_has_conditions(graph, key))
@ -148,7 +145,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
} else if !node_needs_sync {
// No previous node has postponed sync points to add so check if the system itself
// has deferred params that require a sync point to apply them.
node_needs_sync = graph.systems[key].get().unwrap().system.has_deferred();
node_needs_sync = graph.systems[key].has_deferred();
}
for target in dependency_flattened.neighbors_directed(*node, Direction::Outgoing) {
@ -160,7 +157,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
let mut edge_needs_sync = node_needs_sync;
if node_needs_sync
&& !graph.systems[target].get().unwrap().system.is_exclusive()
&& !graph.systems[target].is_exclusive()
&& self
.no_sync_edges
.contains(&(*node, NodeId::System(target)))
@ -210,7 +207,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
continue;
}
if is_apply_deferred(&graph.systems[target].get().unwrap().system) {
if is_apply_deferred(&graph.systems[target]) {
// We don't need to insert a sync point since ApplyDeferred is a sync point
// already!
continue;

7
crates/bevy_ecs/src/schedule/executor/mod.rs
View File

@ -24,10 +24,7 @@ use crate::{
ConditionWithAccess, InternedSystemSet, SystemKey, SystemSetKey, SystemTypeSet,
SystemWithAccess,
},
system::{
RunSystemError, ScheduleSystem, System, SystemIn, SystemParamValidationError,
SystemStateFlags,
},
system::{RunSystemError, System, SystemIn, SystemParamValidationError, SystemStateFlags},
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
@ -157,7 +154,7 @@ impl SystemSchedule {
pub struct ApplyDeferred;
/// Returns `true` if the [`System`] is an instance of [`ApplyDeferred`].
pub(super) fn is_apply_deferred(system: &ScheduleSystem) -> bool {
pub(super) fn is_apply_deferred(system: &dyn System<In = (), Out = ()>) -> bool {
system.type_id() == TypeId::of::<ApplyDeferred>()
}

2
crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
View File

@ -712,7 +712,7 @@ impl ExecutorState {
// Move the full context object into the new future.
let context = *context;
if is_apply_deferred(system) {
if is_apply_deferred(&**system) {
// TODO: avoid allocation
let unapplied_systems = self.unapplied_systems.clone();
self.unapplied_systems.clear();

2
crates/bevy_ecs/src/schedule/executor/simple.rs
View File

@ -124,7 +124,7 @@ impl SystemExecutor for SimpleExecutor {
continue;
}
if is_apply_deferred(system) {
if is_apply_deferred(&**system) {
continue;
}

2
crates/bevy_ecs/src/schedule/executor/single_threaded.rs
View File

@ -125,7 +125,7 @@ impl SystemExecutor for SingleThreadedExecutor {
continue;
}
if is_apply_deferred(system) {
if is_apply_deferred(&**system) {
self.apply_deferred(schedule, world);
continue;
}

3
crates/bevy_ecs/src/schedule/mod.rs
View File

@ -4,13 +4,14 @@ mod auto_insert_apply_deferred;
mod condition;
mod config;
mod executor;
mod node;
mod pass;
mod schedule;
mod set;
mod stepping;
use self::graph::*;
pub use self::{condition::*, config::*, executor::*, schedule::*, set::*};
pub use self::{condition::*, config::*, executor::*, node::*, schedule::*, set::*};
pub use pass::ScheduleBuildPass;
pub use self::graph::NodeId;

615
crates/bevy_ecs/src/schedule/node.rs Normal file
View File

@ -0,0 +1,615 @@
use alloc::{boxed::Box, vec::Vec};
use bevy_utils::prelude::DebugName;
use core::{
any::TypeId,
ops::{Index, IndexMut, Range},
};
use bevy_platform::collections::HashMap;
use slotmap::{new_key_type, SecondaryMap, SlotMap};
use crate::{
component::{CheckChangeTicks, ComponentId, Tick},
prelude::{SystemIn, SystemSet},
query::FilteredAccessSet,
schedule::{BoxedCondition, InternedSystemSet},
system::{
ReadOnlySystem, RunSystemError, ScheduleSystem, System, SystemParamValidationError,
SystemStateFlags,
},
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
/// A [`SystemWithAccess`] stored in a [`ScheduleGraph`](crate::schedule::ScheduleGraph).
pub(crate) struct SystemNode {
pub(crate) inner: Option<SystemWithAccess>,
}
/// A [`ScheduleSystem`] stored alongside the access returned from [`System::initialize`].
pub struct SystemWithAccess {
/// The system itself.
pub system: ScheduleSystem,
/// The access returned by [`System::initialize`].
/// This will be empty if the system has not been initialized yet.
pub access: FilteredAccessSet<ComponentId>,
}
impl SystemWithAccess {
/// Constructs a new [`SystemWithAccess`] from a [`ScheduleSystem`].
/// The `access` will initially be empty.
pub fn new(system: ScheduleSystem) -> Self {
Self {
system,
access: FilteredAccessSet::new(),
}
}
}
impl System for SystemWithAccess {
type In = ();
type Out = ();
#[inline]
fn name(&self) -> DebugName {
self.system.name()
}
#[inline]
fn type_id(&self) -> TypeId {
self.system.type_id()
}
#[inline]
fn flags(&self) -> SystemStateFlags {
self.system.flags()
}
#[inline]
unsafe fn run_unsafe(
&mut self,
input: SystemIn<'_, Self>,
world: UnsafeWorldCell,
) -> Result<Self::Out, RunSystemError> {
// SAFETY: Caller ensures the same safety requirements.
unsafe { self.system.run_unsafe(input, world) }
}
#[cfg(feature = "hotpatching")]
#[inline]
fn refresh_hotpatch(&mut self) {
self.system.refresh_hotpatch();
}
#[inline]
fn apply_deferred(&mut self, world: &mut World) {
self.system.apply_deferred(world);
}
#[inline]
fn queue_deferred(&mut self, world: DeferredWorld) {
self.system.queue_deferred(world);
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Caller ensures the same safety requirements.
unsafe { self.system.validate_param_unsafe(world) }
}
#[inline]
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet<ComponentId> {
self.system.initialize(world)
}
#[inline]
fn check_change_tick(&mut self, check: CheckChangeTicks) {
self.system.check_change_tick(check);
}
#[inline]
fn default_system_sets(&self) -> Vec<InternedSystemSet> {
self.system.default_system_sets()
}
#[inline]
fn get_last_run(&self) -> Tick {
self.system.get_last_run()
}
#[inline]
fn set_last_run(&mut self, last_run: Tick) {
self.system.set_last_run(last_run);
}
}
/// A [`BoxedCondition`] stored alongside the access returned from [`System::initialize`].
pub struct ConditionWithAccess {
/// The condition itself.
pub condition: BoxedCondition,
/// The access returned by [`System::initialize`].
/// This will be empty if the system has not been initialized yet.
pub access: FilteredAccessSet<ComponentId>,
}
impl ConditionWithAccess {
/// Constructs a new [`ConditionWithAccess`] from a [`BoxedCondition`].
/// The `access` will initially be empty.
pub const fn new(condition: BoxedCondition) -> Self {
Self {
condition,
access: FilteredAccessSet::new(),
}
}
}
impl System for ConditionWithAccess {
type In = ();
type Out = bool;
#[inline]
fn name(&self) -> DebugName {
self.condition.name()
}
#[inline]
fn type_id(&self) -> TypeId {
self.condition.type_id()
}
#[inline]
fn flags(&self) -> SystemStateFlags {
self.condition.flags()
}
#[inline]
unsafe fn run_unsafe(
&mut self,
input: SystemIn<'_, Self>,
world: UnsafeWorldCell,
) -> Result<Self::Out, RunSystemError> {
// SAFETY: Caller ensures the same safety requirements.
unsafe { self.condition.run_unsafe(input, world) }
}
#[cfg(feature = "hotpatching")]
#[inline]
fn refresh_hotpatch(&mut self) {
self.condition.refresh_hotpatch();
}
#[inline]
fn apply_deferred(&mut self, world: &mut World) {
self.condition.apply_deferred(world);
}
#[inline]
fn queue_deferred(&mut self, world: DeferredWorld) {
self.condition.queue_deferred(world);
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Caller ensures the same safety requirements.
unsafe { self.condition.validate_param_unsafe(world) }
}
#[inline]
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet<ComponentId> {
self.condition.initialize(world)
}
#[inline]
fn check_change_tick(&mut self, check: CheckChangeTicks) {
self.condition.check_change_tick(check);
}
#[inline]
fn default_system_sets(&self) -> Vec<InternedSystemSet> {
self.condition.default_system_sets()
}
#[inline]
fn get_last_run(&self) -> Tick {
self.condition.get_last_run()
}
#[inline]
fn set_last_run(&mut self, last_run: Tick) {
self.condition.set_last_run(last_run);
}
}
impl SystemNode {
/// Create a new [`SystemNode`]
pub fn new(system: ScheduleSystem) -> Self {
Self {
inner: Some(SystemWithAccess::new(system)),
}
}
/// Obtain a reference to the [`SystemWithAccess`] represented by this node.
pub fn get(&self) -> Option<&SystemWithAccess> {
self.inner.as_ref()
}
/// Obtain a mutable reference to the [`SystemWithAccess`] represented by this node.
pub fn get_mut(&mut self) -> Option<&mut SystemWithAccess> {
self.inner.as_mut()
}
}
new_key_type! {
/// A unique identifier for a system in a [`ScheduleGraph`].
pub struct SystemKey;
/// A unique identifier for a system set in a [`ScheduleGraph`].
pub struct SystemSetKey;
}
/// Container for systems in a schedule.
#[derive(Default)]
pub struct Systems {
/// List of systems in the schedule.
nodes: SlotMap<SystemKey, SystemNode>,
/// List of conditions for each system, in the same order as `nodes`.
conditions: SecondaryMap<SystemKey, Vec<ConditionWithAccess>>,
/// Systems and their conditions that have not been initialized yet.
uninit: Vec<SystemKey>,
}
impl Systems {
/// Returns the number of systems in this container.
pub fn len(&self) -> usize {
self.nodes.len()
}
/// Returns `true` if this container is empty.
pub fn is_empty(&self) -> bool {
self.nodes.is_empty()
}
/// Returns a reference to the system with the given key, if it exists.
pub fn get(&self, key: SystemKey) -> Option<&SystemWithAccess> {
self.nodes.get(key).and_then(|node| node.get())
}
/// Returns a mutable reference to the system with the given key, if it exists.
pub fn get_mut(&mut self, key: SystemKey) -> Option<&mut SystemWithAccess> {
self.nodes.get_mut(key).and_then(|node| node.get_mut())
}
/// Returns a mutable reference to the system with the given key, panicking
/// if it does not exist.
///
/// # Panics
///
/// If the system with the given key does not exist in this container.
pub(crate) fn node_mut(&mut self, key: SystemKey) -> &mut SystemNode {
&mut self.nodes[key]
}
/// Returns `true` if the system with the given key has conditions.
pub fn has_conditions(&self, key: SystemKey) -> bool {
self.conditions
.get(key)
.is_some_and(|conditions| !conditions.is_empty())
}
/// Returns a reference to the conditions for the system with the given key, if it exists.
pub fn get_conditions(&self, key: SystemKey) -> Option<&[ConditionWithAccess]> {
self.conditions.get(key).map(Vec::as_slice)
}
/// Returns a mutable reference to the conditions for the system with the given key, if it exists.
pub fn get_conditions_mut(&mut self, key: SystemKey) -> Option<&mut Vec<ConditionWithAccess>> {
self.conditions.get_mut(key)
}
/// Returns an iterator over all systems and their conditions in this
/// container.
pub fn iter(
&self,
) -> impl Iterator<Item = (SystemKey, &ScheduleSystem, &[ConditionWithAccess])> + '_ {
self.nodes.iter().filter_map(|(key, node)| {
let system = &node.get()?.system;
let conditions = self
.conditions
.get(key)
.map(Vec::as_slice)
.unwrap_or_default();
Some((key, system, conditions))
})
}
/// Inserts a new system into the container, along with its conditions,
/// and queues it to be initialized later in [`Systems::initialize`].
///
/// We have to defer initialization of systems in the container until we have
/// `&mut World` access, so we store these in a list until
/// [`Systems::initialize`] is called. This is usually done upon the first
/// run of the schedule.
pub fn insert(
&mut self,
system: ScheduleSystem,
conditions: Vec<Box<dyn ReadOnlySystem<In = (), Out = bool>>>,
) -> SystemKey {
let key = self.nodes.insert(SystemNode::new(system));
self.conditions.insert(
key,
conditions
.into_iter()
.map(ConditionWithAccess::new)
.collect(),
);
self.uninit.push(key);
key
}
/// Returns `true` if all systems in this container have been initialized.
pub fn is_initialized(&self) -> bool {
self.uninit.is_empty()
}
/// Initializes all systems and their conditions that have not been
/// initialized yet.
pub fn initialize(&mut self, world: &mut World) {
for key in self.uninit.drain(..) {
let Some(system) = self.nodes.get_mut(key).and_then(|node| node.get_mut()) else {
continue;
};
system.access = system.system.initialize(world);
let Some(conditions) = self.conditions.get_mut(key) else {
continue;
};
for condition in conditions {
condition.access = condition.condition.initialize(world);
}
}
}
}
impl Index<SystemKey> for Systems {
type Output = SystemWithAccess;
#[track_caller]
fn index(&self, key: SystemKey) -> &Self::Output {
self.get(key)
.unwrap_or_else(|| panic!("System with key {:?} does not exist in the schedule", key))
}
}
impl IndexMut<SystemKey> for Systems {
#[track_caller]
fn index_mut(&mut self, key: SystemKey) -> &mut Self::Output {
self.get_mut(key)
.unwrap_or_else(|| panic!("System with key {:?} does not exist in the schedule", key))
}
}
/// Container for system sets in a schedule.
#[derive(Default)]
pub struct SystemSets {
/// List of system sets in the schedule.
sets: SlotMap<SystemSetKey, InternedSystemSet>,
/// List of conditions for each system set, in the same order as `sets`.
conditions: SecondaryMap<SystemSetKey, Vec<ConditionWithAccess>>,
/// Map from system sets to their keys.
ids: HashMap<InternedSystemSet, SystemSetKey>,
/// System sets that have not been initialized yet.
uninit: Vec<UninitializedSet>,
}
/// A system set's conditions that have not been initialized yet.
struct UninitializedSet {
key: SystemSetKey,
/// The range of indices in [`SystemSets::conditions`] that correspond
/// to conditions that have not been initialized yet.
///
/// [`SystemSets::conditions`] for a given set may be appended to
/// multiple times (e.g. when `configure_sets` is called multiple with
/// the same set), so we need to track which conditions in that list
/// are newly added and not yet initialized.
///
/// Systems don't need this tracking because each `add_systems` call
/// creates separate nodes in the graph with their own conditions,
/// so all conditions are initialized together.
uninitialized_conditions: Range<usize>,
}
impl SystemSets {
/// Returns the number of system sets in this container.
pub fn len(&self) -> usize {
self.sets.len()
}
/// Returns `true` if this container is empty.
pub fn is_empty(&self) -> bool {
self.sets.is_empty()
}
/// Returns `true` if the given set is present in this container.
pub fn contains(&self, set: impl SystemSet) -> bool {
self.ids.contains_key(&set.intern())
}
/// Returns a reference to the system set with the given key, if it exists.
pub fn get(&self, key: SystemSetKey) -> Option<&dyn SystemSet> {
self.sets.get(key).map(|set| &**set)
}
/// Returns the key for the given system set, inserting it into this
/// container if it does not already exist.
pub fn get_key_or_insert(&mut self, set: InternedSystemSet) -> SystemSetKey {
*self.ids.entry(set).or_insert_with(|| {
let key = self.sets.insert(set);
self.conditions.insert(key, Vec::new());
key
})
}
/// Returns `true` if the system set with the given key has conditions.
pub fn has_conditions(&self, key: SystemSetKey) -> bool {
self.conditions
.get(key)
.is_some_and(|conditions| !conditions.is_empty())
}
/// Returns a reference to the conditions for the system set with the given
/// key, if it exists.
pub fn get_conditions(&self, key: SystemSetKey) -> Option<&[ConditionWithAccess]> {
self.conditions.get(key).map(Vec::as_slice)
}
/// Returns a mutable reference to the conditions for the system set with
/// the given key, if it exists.
pub fn get_conditions_mut(
&mut self,
key: SystemSetKey,
) -> Option<&mut Vec<ConditionWithAccess>> {
self.conditions.get_mut(key)
}
/// Returns an iterator over all system sets in this container, along with
/// their conditions.
pub fn iter(
&self,
) -> impl Iterator<Item = (SystemSetKey, &dyn SystemSet, &[ConditionWithAccess])> {
self.sets.iter().filter_map(|(key, set)| {
let conditions = self.conditions.get(key)?.as_slice();
Some((key, &**set, conditions))
})
}
/// Inserts conditions for a system set into the container, and queues the
/// newly added conditions to be initialized later in [`SystemSets::initialize`].
///
/// If the set was not already present in the container, it is added automatically.
///
/// We have to defer initialization of system set conditions in the container
/// until we have `&mut World` access, so we store these in a list until
/// [`SystemSets::initialize`] is called. This is usually done upon the
/// first run of the schedule.
pub fn insert(
&mut self,
set: InternedSystemSet,
new_conditions: Vec<Box<dyn ReadOnlySystem<In = (), Out = bool>>>,
) -> SystemSetKey {
let key = self.get_key_or_insert(set);
if !new_conditions.is_empty() {
let current_conditions = &mut self.conditions[key];
let start = current_conditions.len();
self.uninit.push(UninitializedSet {
key,
uninitialized_conditions: start..(start + new_conditions.len()),
});
current_conditions.extend(new_conditions.into_iter().map(ConditionWithAccess::new));
}
key
}
/// Returns `true` if all system sets' conditions in this container have
/// been initialized.
pub fn is_initialized(&self) -> bool {
self.uninit.is_empty()
}
/// Initializes all system sets' conditions that have not been
/// initialized yet. Because a system set's conditions may be appended to
/// multiple times, we track which conditions were added since the last
/// initialization and only initialize those.
pub fn initialize(&mut self, world: &mut World) {
for uninit in self.uninit.drain(..) {
let Some(conditions) = self.conditions.get_mut(uninit.key) else {
continue;
};
for condition in &mut conditions[uninit.uninitialized_conditions] {
condition.access = condition.initialize(world);
}
}
}
}
impl Index<SystemSetKey> for SystemSets {
type Output = dyn SystemSet;
#[track_caller]
fn index(&self, key: SystemSetKey) -> &Self::Output {
self.get(key).unwrap_or_else(|| {
panic!(
"System set with key {:?} does not exist in the schedule",
key
)
})
}
}
#[cfg(test)]
mod tests {
use alloc::{boxed::Box, vec};
use crate::{
prelude::SystemSet,
schedule::{SystemSets, Systems},
system::IntoSystem,
world::World,
};
#[derive(SystemSet, Clone, Copy, PartialEq, Eq, Debug, Hash)]
pub struct TestSet;
#[test]
fn systems() {
fn empty_system() {}
let mut systems = Systems::default();
assert!(systems.is_empty());
assert_eq!(systems.len(), 0);
let system = Box::new(IntoSystem::into_system(empty_system));
let key = systems.insert(system, vec![]);
assert!(!systems.is_empty());
assert_eq!(systems.len(), 1);
assert!(systems.get(key).is_some());
assert!(systems.get_conditions(key).is_some());
assert!(systems.get_conditions(key).unwrap().is_empty());
assert!(systems.get_mut(key).is_some());
assert!(!systems.is_initialized());
assert!(systems.iter().next().is_some());
let mut world = World::new();
systems.initialize(&mut world);
assert!(systems.is_initialized());
}
#[test]
fn system_sets() {
fn always_true() -> bool {
true
}
let mut sets = SystemSets::default();
assert!(sets.is_empty());
assert_eq!(sets.len(), 0);
let condition = Box::new(IntoSystem::into_system(always_true));
let key = sets.insert(TestSet.intern(), vec![condition]);
assert!(!sets.is_empty());
assert_eq!(sets.len(), 1);
assert!(sets.get(key).is_some());
assert!(sets.get_conditions(key).is_some());
assert!(!sets.get_conditions(key).unwrap().is_empty());
assert!(!sets.is_initialized());
assert!(sets.iter().next().is_some());
let mut world = World::new();
sets.initialize(&mut world);
assert!(sets.is_initialized());
}
}

338
crates/bevy_ecs/src/schedule/schedule.rs
View File

@ -15,21 +15,18 @@ use bevy_utils::{default, prelude::DebugName, TypeIdMap};
use core::{
any::{Any, TypeId},
fmt::{Debug, Write},
ops::Range,
};
use fixedbitset::FixedBitSet;
use log::{error, info, warn};
use pass::ScheduleBuildPassObj;
use slotmap::{new_key_type, SecondaryMap, SlotMap};
use thiserror::Error;
#[cfg(feature = "trace")]
use tracing::info_span;
use crate::component::CheckChangeTicks;
use crate::{component::CheckChangeTicks, system::System};
use crate::{
component::{ComponentId, Components},
prelude::Component,
query::FilteredAccessSet,
resource::Resource,
schedule::*,
system::ScheduleSystem,
@ -391,20 +388,8 @@ impl Schedule {
let a = a.into_system_set();
let b = b.into_system_set();
let Some(&a_id) = self.graph.system_sets.ids.get(&a.intern()) else {
panic!(
"Could not mark system as ambiguous, `{:?}` was not found in the schedule.
Did you try to call `ambiguous_with` before adding the system to the world?",
a
);
};
let Some(&b_id) = self.graph.system_sets.ids.get(&b.intern()) else {
panic!(
"Could not mark system as ambiguous, `{:?}` was not found in the schedule.
Did you try to call `ambiguous_with` before adding the system to the world?",
b
);
};
let a_id = self.graph.system_sets.get_key_or_insert(a.intern());
let b_id = self.graph.system_sets.get_key_or_insert(b.intern());
self.graph
.ambiguous_with
@ -564,21 +549,21 @@ impl Schedule {
/// [`MAX_CHANGE_AGE`](crate::change_detection::MAX_CHANGE_AGE).
/// This prevents overflow and thus prevents false positives.
pub fn check_change_ticks(&mut self, check: CheckChangeTicks) {
for SystemWithAccess { system, .. } in &mut self.executable.systems {
for system in &mut self.executable.systems {
if !is_apply_deferred(system) {
system.check_change_tick(check);
}
}
for conditions in &mut self.executable.system_conditions {
for system in conditions {
system.condition.check_change_tick(check);
for condition in conditions {
condition.check_change_tick(check);
}
}
for conditions in &mut self.executable.set_conditions {
for system in conditions {
system.condition.check_change_tick(check);
for condition in conditions {
condition.check_change_tick(check);
}
}
}
@ -659,141 +644,16 @@ impl Dag {
}
}
/// A [`SystemWithAccess`] stored in a [`ScheduleGraph`].
pub struct SystemNode {
inner: Option<SystemWithAccess>,
}
/// A [`ScheduleSystem`] stored alongside the access returned from [`System::initialize`](crate::system::System::initialize).
pub struct SystemWithAccess {
/// The system itself.
pub system: ScheduleSystem,
/// The access returned by [`System::initialize`](crate::system::System::initialize).
/// This will be empty if the system has not been initialized yet.
pub access: FilteredAccessSet<ComponentId>,
}
impl SystemWithAccess {
/// Constructs a new [`SystemWithAccess`] from a [`ScheduleSystem`].
/// The `access` will initially be empty.
pub fn new(system: ScheduleSystem) -> Self {
Self {
system,
access: FilteredAccessSet::new(),
}
}
}
/// A [`BoxedCondition`] stored alongside the access returned from [`System::initialize`](crate::system::System::initialize).
pub struct ConditionWithAccess {
/// The condition itself.
pub condition: BoxedCondition,
/// The access returned by [`System::initialize`](crate::system::System::initialize).
/// This will be empty if the system has not been initialized yet.
pub access: FilteredAccessSet<ComponentId>,
}
impl ConditionWithAccess {
/// Constructs a new [`ConditionWithAccess`] from a [`BoxedCondition`].
/// The `access` will initially be empty.
pub const fn new(condition: BoxedCondition) -> Self {
Self {
condition,
access: FilteredAccessSet::new(),
}
}
}
impl SystemNode {
/// Create a new [`SystemNode`]
pub fn new(system: ScheduleSystem) -> Self {
Self {
inner: Some(SystemWithAccess::new(system)),
}
}
/// Obtain a reference to the [`SystemWithAccess`] represented by this node.
pub fn get(&self) -> Option<&SystemWithAccess> {
self.inner.as_ref()
}
/// Obtain a mutable reference to the [`SystemWithAccess`] represented by this node.
pub fn get_mut(&mut self) -> Option<&mut SystemWithAccess> {
self.inner.as_mut()
}
}
new_key_type! {
/// A unique identifier for a system in a [`ScheduleGraph`].
pub struct SystemKey;
/// A unique identifier for a system set in a [`ScheduleGraph`].
pub struct SystemSetKey;
}
/// A node in a [`ScheduleGraph`] with a system or conditions that have not been
/// initialized yet.
///
/// We have to defer initialization of nodes in the graph until we have
/// `&mut World` access, so we store these in a list ([`ScheduleGraph::uninit`])
/// until then. In most cases, initialization occurs upon the first run of the
/// schedule.
enum UninitializedId {
/// A system and its conditions that have not been initialized yet.
System(SystemKey),
/// A system set's conditions that have not been initialized yet.
Set {
key: SystemSetKey,
/// The range of indices in [`SystemSets::conditions`] that correspond
/// to conditions that have not been initialized yet.
///
/// [`SystemSets::conditions`] for a given set may be appended to
/// multiple times (e.g. when `configure_sets` is called multiple with
/// the same set), so we need to track which conditions in that list
/// are newly added and not yet initialized.
///
/// Systems don't need this tracking because each `add_systems` call
/// creates separate nodes in the graph with their own conditions,
/// so all conditions are initialized together.
uninitialized_conditions: Range<usize>,
},
}
/// Metadata for system sets in a schedule.
#[derive(Default)]
struct SystemSets {
/// List of system sets in the schedule
sets: SlotMap<SystemSetKey, InternedSystemSet>,
/// List of conditions for each system set, in the same order as `system_sets`
conditions: SecondaryMap<SystemSetKey, Vec<ConditionWithAccess>>,
/// Map from system set to node id
ids: HashMap<InternedSystemSet, SystemSetKey>,
}
impl SystemSets {
fn get_or_add_set(&mut self, set: InternedSystemSet) -> SystemSetKey {
*self.ids.entry(set).or_insert_with(|| {
let key = self.sets.insert(set);
self.conditions.insert(key, Vec::new());
key
})
}
}
/// Metadata for a [`Schedule`].
///
/// The order isn't optimized; calling `ScheduleGraph::build_schedule` will return a
/// `SystemSchedule` where the order is optimized for execution.
#[derive(Default)]
pub struct ScheduleGraph {
/// List of systems in the schedule
pub systems: SlotMap<SystemKey, SystemNode>,
/// List of conditions for each system, in the same order as `systems`
pub system_conditions: SecondaryMap<SystemKey, Vec<ConditionWithAccess>>,
/// Data about system sets in the schedule
system_sets: SystemSets,
/// Systems, their conditions, and system set conditions that need to be
/// initialized before the schedule can be run.
uninit: Vec<UninitializedId>,
/// Container of systems in the schedule.
pub systems: Systems,
/// Container of system sets in the schedule.
pub system_sets: SystemSets,
/// Directed acyclic graph of the hierarchy (which systems/sets are children of which sets)
hierarchy: Dag,
/// Directed acyclic graph of the dependency (which systems/sets have to run before which other systems/sets)
@ -812,10 +672,8 @@ impl ScheduleGraph {
/// Creates an empty [`ScheduleGraph`] with default settings.
pub fn new() -> Self {
Self {
systems: SlotMap::with_key(),
system_conditions: SecondaryMap::new(),
systems: Systems::default(),
system_sets: SystemSets::default(),
uninit: Vec::new(),
hierarchy: Dag::new(),
dependency: Dag::new(),
ambiguous_with: UnGraph::default(),
@ -828,78 +686,6 @@ impl ScheduleGraph {
}
}
/// Returns the system at the given [`SystemKey`], if it exists.
pub fn get_system_at(&self, key: SystemKey) -> Option<&ScheduleSystem> {
self.systems
.get(key)
.and_then(|system| system.get())
.map(|system| &system.system)
}
/// Returns `true` if the given system set is part of the graph. Otherwise, returns `false`.
pub fn contains_set(&self, set: impl SystemSet) -> bool {
self.system_sets.ids.contains_key(&set.intern())
}
/// Returns the system at the given [`NodeId`].
///
/// Panics if it doesn't exist.
#[track_caller]
pub fn system_at(&self, key: SystemKey) -> &ScheduleSystem {
self.get_system_at(key)
.unwrap_or_else(|| panic!("system with key {key:?} does not exist in this Schedule"))
}
/// Returns the set at the given [`NodeId`], if it exists.
pub fn get_set_at(&self, key: SystemSetKey) -> Option<&dyn SystemSet> {
self.system_sets.sets.get(key).map(|set| &**set)
}
/// Returns the set at the given [`NodeId`].
///
/// Panics if it doesn't exist.
#[track_caller]
pub fn set_at(&self, id: SystemSetKey) -> &dyn SystemSet {
self.get_set_at(id)
.unwrap_or_else(|| panic!("set with id {id:?} does not exist in this Schedule"))
}
/// Returns the conditions for the set at the given [`SystemSetKey`], if it exists.
pub fn get_set_conditions_at(&self, key: SystemSetKey) -> Option<&[ConditionWithAccess]> {
self.system_sets.conditions.get(key).map(Vec::as_slice)
}
/// Returns the conditions for the set at the given [`SystemSetKey`].
///
/// Panics if it doesn't exist.
#[track_caller]
pub fn set_conditions_at(&self, key: SystemSetKey) -> &[ConditionWithAccess] {
self.get_set_conditions_at(key)
.unwrap_or_else(|| panic!("set with key {key:?} does not exist in this Schedule"))
}
/// Returns an iterator over all systems in this schedule, along with the conditions for each system.
pub fn systems(
&self,
) -> impl Iterator<Item = (SystemKey, &ScheduleSystem, &[ConditionWithAccess])> {
self.systems.iter().filter_map(|(key, system_node)| {
let system = &system_node.inner.as_ref()?.system;
let conditions = self.system_conditions.get(key)?;
Some((key, system, conditions.as_slice()))
})
}
/// Returns an iterator over all system sets in this schedule, along with the conditions for each
/// system set.
pub fn system_sets(
&self,
) -> impl Iterator<Item = (SystemSetKey, &dyn SystemSet, &[ConditionWithAccess])> {
self.system_sets.sets.iter().filter_map(|(key, set)| {
let conditions = self.system_sets.conditions.get(key)?.as_slice();
Some((key, &**set, conditions))
})
}
/// Returns the [`Dag`] of the hierarchy.
///
/// The hierarchy is a directed acyclic graph of the systems and sets,
@ -1059,17 +845,7 @@ impl ScheduleGraph {
/// Add a [`ScheduleConfig`] to the graph, including its dependencies and conditions.
fn add_system_inner(&mut self, config: ScheduleConfig<ScheduleSystem>) -> SystemKey {
let key = self.systems.insert(SystemNode::new(config.node));
self.system_conditions.insert(
key,
config
.conditions
.into_iter()
.map(ConditionWithAccess::new)
.collect(),
);
// system init has to be deferred (need `&mut World`)
self.uninit.push(UninitializedId::System(key));
let key = self.systems.insert(config.node, config.conditions);
// graph updates are immediate
self.update_graphs(NodeId::System(key), config.metadata);
@ -1083,26 +859,11 @@ impl ScheduleGraph {
}
/// Add a single `ScheduleConfig` to the graph, including its dependencies and conditions.
fn configure_set_inner(&mut self, set: ScheduleConfig<InternedSystemSet>) -> SystemSetKey {
let ScheduleConfig {
node: set,
metadata,
conditions,
} = set;
let key = self.system_sets.get_or_add_set(set);
fn configure_set_inner(&mut self, config: ScheduleConfig<InternedSystemSet>) -> SystemSetKey {
let key = self.system_sets.insert(config.node, config.conditions);
// graph updates are immediate
self.update_graphs(NodeId::Set(key), metadata);
// system init has to be deferred (need `&mut World`)
let system_set_conditions = self.system_sets.conditions.entry(key).unwrap().or_default();
let start = system_set_conditions.len();
self.uninit.push(UninitializedId::Set {
key,
uninitialized_conditions: start..(start + conditions.len()),
});
system_set_conditions.extend(conditions.into_iter().map(ConditionWithAccess::new));
self.update_graphs(NodeId::Set(key), config.metadata);
key
}
@ -1129,7 +890,7 @@ impl ScheduleGraph {
for key in sets
.into_iter()
.map(|set| self.system_sets.get_or_add_set(set))
.map(|set| self.system_sets.get_key_or_insert(set))
{
self.hierarchy.graph.add_edge(NodeId::Set(key), id);
@ -1141,7 +902,7 @@ impl ScheduleGraph {
dependencies
.into_iter()
.map(|Dependency { kind, set, options }| {
(kind, self.system_sets.get_or_add_set(set), options)
(kind, self.system_sets.get_key_or_insert(set), options)
})
{
let (lhs, rhs) = match kind {
@ -1162,7 +923,7 @@ impl ScheduleGraph {
Ambiguity::IgnoreWithSet(ambiguous_with) => {
for key in ambiguous_with
.into_iter()
.map(|set| self.system_sets.get_or_add_set(set))
.map(|set| self.system_sets.get_key_or_insert(set))
{
self.ambiguous_with.add_edge(id, NodeId::Set(key));
}
@ -1173,28 +934,11 @@ impl ScheduleGraph {
}
}
/// Initializes any newly-added systems and conditions by calling [`System::initialize`](crate::system::System)
/// Initializes any newly-added systems and conditions by calling
/// [`System::initialize`](crate::system::System).
pub fn initialize(&mut self, world: &mut World) {
for id in self.uninit.drain(..) {
match id {
UninitializedId::System(key) => {
let system = self.systems[key].get_mut().unwrap();
system.access = system.system.initialize(world);
for condition in &mut self.system_conditions[key] {
condition.access = condition.condition.initialize(world);
}
}
UninitializedId::Set {
key,
uninitialized_conditions,
} => {
for condition in &mut self.system_sets.conditions[key][uninitialized_conditions]
{
condition.access = condition.condition.initialize(world);
}
}
}
}
self.systems.initialize(world);
self.system_sets.initialize(world);
}
/// Build a [`SystemSchedule`] optimized for scheduler access from the [`ScheduleGraph`].
@ -1286,9 +1030,9 @@ impl ScheduleGraph {
HashMap<SystemSetKey, HashSet<SystemKey>>,
) {
let mut set_systems: HashMap<SystemSetKey, Vec<SystemKey>> =
HashMap::with_capacity_and_hasher(self.system_sets.sets.len(), Default::default());
HashMap::with_capacity_and_hasher(self.system_sets.len(), Default::default());
let mut set_system_sets: HashMap<SystemSetKey, HashSet<SystemKey>> =
HashMap::with_capacity_and_hasher(self.system_sets.sets.len(), Default::default());
HashMap::with_capacity_and_hasher(self.system_sets.len(), Default::default());
for &id in hierarchy_topsort.iter().rev() {
let NodeId::Set(set_key) = id else {
continue;
@ -1419,9 +1163,9 @@ impl ScheduleGraph {
"Encountered a non-system node in the flattened disconnected results: {b:?}"
);
};
let system_a = self.systems[a].get().unwrap();
let system_b = self.systems[b].get().unwrap();
if system_a.system.is_exclusive() || system_b.system.is_exclusive() {
let system_a = &self.systems[a];
let system_b = &self.systems[b];
if system_a.is_exclusive() || system_b.is_exclusive() {
conflicting_systems.push((a, b, Vec::new()));
} else {
let access_a = &system_a.access;
@ -1487,7 +1231,7 @@ impl ScheduleGraph {
// ignore system sets that have no conditions
// ignore system type sets (already covered, they don't have conditions)
let key = id.as_set()?;
(!self.system_sets.conditions[key].is_empty()).then_some((i, key))
self.system_sets.has_conditions(key).then_some((i, key))
})
.unzip();
@ -1567,7 +1311,7 @@ impl ScheduleGraph {
ignored_ambiguities: &BTreeSet<ComponentId>,
schedule_label: InternedScheduleLabel,
) -> Result<(), ScheduleBuildError> {
if !self.uninit.is_empty() {
if !self.systems.is_initialized() || !self.system_sets.is_initialized() {
return Err(ScheduleBuildError::Uninitialized);
}
@ -1578,8 +1322,8 @@ impl ScheduleGraph {
.zip(schedule.systems.drain(..))
.zip(schedule.system_conditions.drain(..))
{
self.systems[key].inner = Some(system);
self.system_conditions[key] = conditions;
self.systems.node_mut(key).inner = Some(system);
*self.systems.get_conditions_mut(key).unwrap() = conditions;
}
for (key, conditions) in schedule
@ -1587,21 +1331,21 @@ impl ScheduleGraph {
.drain(..)
.zip(schedule.set_conditions.drain(..))
{
self.system_sets.conditions[key] = conditions;
*self.system_sets.get_conditions_mut(key).unwrap() = conditions;
}
*schedule = self.build_schedule(world, schedule_label, ignored_ambiguities)?;
// move systems into new schedule
for &key in &schedule.system_ids {
let system = self.systems[key].inner.take().unwrap();
let conditions = core::mem::take(&mut self.system_conditions[key]);
let system = self.systems.node_mut(key).inner.take().unwrap();
let conditions = core::mem::take(self.systems.get_conditions_mut(key).unwrap());
schedule.systems.push(system);
schedule.system_conditions.push(conditions);
}
for &key in &schedule.set_ids {
let conditions = core::mem::take(&mut self.system_sets.conditions[key]);
let conditions = core::mem::take(self.system_sets.get_conditions_mut(key).unwrap());
schedule.set_conditions.push(conditions);
}
@ -1656,7 +1400,7 @@ impl ScheduleGraph {
fn get_node_name_inner(&self, id: &NodeId, report_sets: bool) -> String {
match *id {
NodeId::System(key) => {
let name = self.systems[key].get().unwrap().system.name();
let name = self.systems[key].name();
let name = if self.settings.use_shortnames {
name.shortname().to_string()
} else {
@ -1676,7 +1420,7 @@ impl ScheduleGraph {
}
}
NodeId::Set(key) => {
let set = &self.system_sets.sets[key];
let set = &self.system_sets[key];
if set.is_anonymous() {
self.anonymous_set_name(id)
} else {
@ -1902,7 +1646,7 @@ impl ScheduleGraph {
set_systems: &HashMap<SystemSetKey, Vec<SystemKey>>,
) -> Result<(), ScheduleBuildError> {
for (&key, systems) in set_systems {
let set = &self.system_sets.sets[key];
let set = &self.system_sets[key];
if set.system_type().is_some() {
let instances = systems.len();
let ambiguous_with = self.ambiguous_with.edges(NodeId::Set(key));
@ -2009,7 +1753,7 @@ impl ScheduleGraph {
fn names_of_sets_containing_node(&self, id: &NodeId) -> Vec<String> {
let mut sets = <HashSet<_>>::default();
self.traverse_sets_containing_node(*id, &mut |key| {
self.system_sets.sets[key].system_type().is_none() && sets.insert(key)
self.system_sets[key].system_type().is_none() && sets.insert(key)
});
let mut sets: Vec<_> = sets
.into_iter()

37
release-content/migration-guides/schedule_cleanup.md Normal file
View File

@ -0,0 +1,37 @@
---
title: Schedule API Cleanup
pull_requests: [19352, 20119]
---
In order to support removing systems from schedules, `Vec`s storing `System`s and
`SystemSet`s have been replaced with `SlotMap`s which allow safely removing nodes and
reusing indices. The maps are respectively keyed by `SystemKey`s and `SystemSetKey`s.
The following signatures were changed:
- `NodeId::System`: Now stores a `SystemKey` instead of a plain `usize`
- `NodeId::Set`: Now stores a `SystemSetKey` instead of a plain `usize`
- `ScheduleBuildPass::collapse_set`: Now takes the type-specific keys. Wrap them back into a `NodeId` if necessary.
- The following functions now return the type-specific keys. Wrap them back into a `NodeId` if necessary.
- `Schedule::systems`
- `ScheduleGraph::conflicting_systems`
The following functions were replaced. Those that took or returned `NodeId` now
take or return `SystemKey` or `SystemSetKey`. Wrap/unwrap them as necessary.
- `ScheduleGraph::contains_set`: Use `ScheduleGraph::system_sets` and `SystemSets::contains`.
- `ScheduleGraph::get_set_at`: Use `ScheduleGraph::system_sets` and `SystemSets::get`.
- `ScheduleGraph::set_at`: Use `ScheduleGraph::system_sets` and `SystemSets::index` (`system_sets[key]`).
- `ScheduleGraph::get_set_conditions_at`: Use `ScheduleGraph::system_sets` and `SystemSets::get_conditions`.
- `ScheduleGraph::system_sets`: Use `ScheduleGraph::system_sets` and `SystemSets::iter`.
- `ScheduleGraph::get_system_at`: Use `ScheduleGraph::systems` and `Systems::get`.
- `ScheduleGraph::system_at`: Use `ScheduleGraph::systems` and `Systems::index` (`systems[key]`).
- `ScheduleGraph::systems`: Use `ScheduleGraph::systems` and `Systems::iter`.
The following functions were removed:
- `NodeId::index`: You should match on and use the `SystemKey` and `SystemSetKey` instead.
- `NodeId::cmp`: Use the `PartialOrd` and `Ord` traits instead.
- `ScheduleGraph::set_conditions_at`: If needing to check presence of conditions,
use `ScheduleGraph::system_sets` and `SystemSets::has_conditions`.
Otherwise, use `SystemSets::get_conditions`.

34
release-content/migration-guides/schedule_slotmaps.md
View File

@ -1,34 +0,0 @@
---
title: Schedule SlotMaps
pull_requests: [19352]
---
In order to support removing systems from schedules, `Vec`s storing `System`s and
`SystemSet`s have been replaced with `SlotMap`s which allow safely removing and
reusing indices. The maps are respectively keyed by `SystemKey`s and `SystemSetKey`s.
The following signatures were changed:
- `NodeId::System`: Now stores a `SystemKey` instead of a plain `usize`
- `NodeId::Set`: Now stores a `SystemSetKey` instead of a plain `usize`
- `ScheduleBuildPass::collapse_set`: Now takes the type-specific keys. Wrap them back into a `NodeId` if necessary.
- The following functions now return the type-specific keys. Wrap them back into a `NodeId` if necessary.
- `Schedule::systems`
- `ScheduleGraph::systems`
- `ScheduleGraph::system_sets`
- `ScheduleGraph::conflicting_systems`
- Use the appropriate key types to index these structures rather than bare `usize`s:
- `ScheduleGraph::systems` field
- `ScheduleGraph::system_conditions`
- The following functions now take the type-specific keys. Use pattern matching to extract them from `NodeId`s, if necessary:
- `ScheduleGraph::get_system_at`
- `ScheduleGraph::system_at`
- `ScheduleGraph::get_set_at`
- `ScheduleGraph::set_at`
- `ScheduleGraph::get_set_conditions_at`
- `ScheduleGraph::set_conditions_at`
The following functions were removed:
- `NodeId::index`: You should match on and use the `SystemKey` and `SystemSetKey` instead.
- `NodeId::cmp`: Use the `PartialOrd` and `Ord` traits instead.