diff --git a/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs b/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs index 524b198358..4e4cbd7eeb 100644 --- a/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs +++ b/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs @@ -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::::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; diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 12030c2580..655a71caa7 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -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) -> bool { system.type_id() == TypeId::of::() } diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index 006faa8fe4..b2ccfbddf7 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -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(); diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index 17f3f3b8a0..f33cfcf27e 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -124,7 +124,7 @@ impl SystemExecutor for SimpleExecutor { continue; } - if is_apply_deferred(system) { + if is_apply_deferred(&**system) { continue; } diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index 4d321bdaff..36f7b259f2 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -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; } diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 8b559b33c3..7f07600ab2 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -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; diff --git a/crates/bevy_ecs/src/schedule/node.rs b/crates/bevy_ecs/src/schedule/node.rs new file mode 100644 index 0000000000..bc96d4871e --- /dev/null +++ b/crates/bevy_ecs/src/schedule/node.rs @@ -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, +} + +/// 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, +} + +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 { + // 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 { + 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 { + 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, +} + +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 { + // 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 { + 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 { + 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, + /// List of conditions for each system, in the same order as `nodes`. + conditions: SecondaryMap>, + /// Systems and their conditions that have not been initialized yet. + uninit: Vec, +} + +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> { + self.conditions.get_mut(key) + } + + /// Returns an iterator over all systems and their conditions in this + /// container. + pub fn iter( + &self, + ) -> impl Iterator + '_ { + 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>>, + ) -> 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 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 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, + /// List of conditions for each system set, in the same order as `sets`. + conditions: SecondaryMap>, + /// Map from system sets to their keys. + ids: HashMap, + /// System sets that have not been initialized yet. + uninit: Vec, +} + +/// 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, +} + +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> { + 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 { + 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>>, + ) -> 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 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()); + } +} diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 0544956678..848763dfad 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -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, -} - -/// 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, -} - -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, -} - -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, - }, -} - -/// Metadata for system sets in a schedule. -#[derive(Default)] -struct SystemSets { - /// List of system sets in the schedule - sets: SlotMap, - /// List of conditions for each system set, in the same order as `system_sets` - conditions: SecondaryMap>, - /// Map from system set to node id - ids: HashMap, -} - -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, - /// List of conditions for each system, in the same order as `systems` - pub system_conditions: SecondaryMap>, - /// 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, + /// 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 { - 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 { - 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) -> 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) -> 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) -> 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>, ) { let mut set_systems: HashMap> = - 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> = - 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, 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>, ) -> 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 { let mut sets = >::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() diff --git a/release-content/migration-guides/schedule_cleanup.md b/release-content/migration-guides/schedule_cleanup.md new file mode 100644 index 0000000000..86aa9cf56e --- /dev/null +++ b/release-content/migration-guides/schedule_cleanup.md @@ -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`. diff --git a/release-content/migration-guides/schedule_slotmaps.md b/release-content/migration-guides/schedule_slotmaps.md deleted file mode 100644 index 8be72a23fa..0000000000 --- a/release-content/migration-guides/schedule_slotmaps.md +++ /dev/null @@ -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.