diff --git a/crates/bevy_ecs/src/schedule_v3/config.rs b/crates/bevy_ecs/src/schedule_v3/config.rs index ace2941f7ef54..860cc13f758c0 100644 --- a/crates/bevy_ecs/src/schedule_v3/config.rs +++ b/crates/bevy_ecs/src/schedule_v3/config.rs @@ -340,50 +340,51 @@ mod sealed { } /// A collection of [`SystemConfig`]. -pub struct SystemCollection { - inner: Vec, +pub struct SystemConfigs { + pub(super) systems: Vec, + pub(super) chained: bool, } -/// Methods that can configure multiple systems at once. -pub trait IntoSystemCollection +/// Types that can convert into a [`SystemConfigs`]. +pub trait IntoSystemConfigs where Self: Sized, { - /// Convert into a [`SystemCollection`]. + /// Convert into a [`SystemConfigs`]. #[doc(hidden)] - fn into_collection(self) -> SystemCollection; + fn into_configs(self) -> SystemConfigs; /// Add to `set` membership. - fn in_set(self, set: impl SystemSet) -> SystemCollection { - self.into_collection().in_set(set) + fn in_set(self, set: impl SystemSet) -> SystemConfigs { + self.into_configs().in_set(set) } /// Run before all members of `set`. - fn before(self, set: impl IntoSystemSet) -> SystemCollection { - self.into_collection().before(set) + fn before(self, set: impl IntoSystemSet) -> SystemConfigs { + self.into_configs().before(set) } /// Run after all members of `set`. - fn after(self, set: impl IntoSystemSet) -> SystemCollection { - self.into_collection().after(set) + fn after(self, set: impl IntoSystemSet) -> SystemConfigs { + self.into_configs().after(set) } /// Treat this collection as a sequence. /// /// Ordering constraints will be applied between the successive collection elements. - fn chain(self) -> SystemCollection { - self.into_collection().chain() + fn chain(self) -> SystemConfigs { + self.into_configs().chain() } } -impl IntoSystemCollection<()> for SystemCollection { - fn into_collection(self) -> Self { +impl IntoSystemConfigs<()> for SystemConfigs { + fn into_configs(self) -> Self { self } fn in_set(mut self, set: impl SystemSet) -> Self { assert!(!set.is_system_type(), "invalid use of system type set"); - for config in self.inner.iter_mut() { + for config in self.systems.iter_mut() { config.graph_info.sets.insert(set.dyn_clone()); } @@ -392,7 +393,7 @@ impl IntoSystemCollection<()> for SystemCollection { fn before(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - for config in self.inner.iter_mut() { + for config in self.systems.iter_mut() { config .graph_info .edges @@ -404,7 +405,7 @@ impl IntoSystemCollection<()> for SystemCollection { fn after(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - for config in self.inner.iter_mut() { + for config in self.systems.iter_mut() { config .graph_info .edges @@ -415,55 +416,57 @@ impl IntoSystemCollection<()> for SystemCollection { } fn chain(mut self) -> Self { - todo!() + self.chained = true; + self } } /// A collection of [`SystemSetConfig`]. -pub struct SystemSetCollection { - inner: Vec, +pub struct SystemSetConfigs { + pub(super) sets: Vec, + pub(super) chained: bool, } -/// Methods that can configure multiple system sets at once. -pub trait IntoSystemSetCollection +/// Types that can convert into a [`SystemSetConfigs`]. +pub trait IntoSystemSetConfigs where Self: Sized, { - /// Convert into a [`SystemSetCollection`]. + /// Convert into a [`SystemSetConfigs`]. #[doc(hidden)] - fn into_collection(self) -> SystemSetCollection; + fn into_configs(self) -> SystemSetConfigs; /// Add to `set` membership. - fn in_set(self, set: impl SystemSet) -> SystemSetCollection { - self.into_collection().in_set(set) + fn in_set(self, set: impl SystemSet) -> SystemSetConfigs { + self.into_configs().in_set(set) } /// Run before all members of `set`. - fn before(self, set: impl IntoSystemSet) -> SystemSetCollection { - self.into_collection().before(set) + fn before(self, set: impl IntoSystemSet) -> SystemSetConfigs { + self.into_configs().before(set) } /// Run after all members of `set`. - fn after(self, set: impl IntoSystemSet) -> SystemSetCollection { - self.into_collection().after(set) + fn after(self, set: impl IntoSystemSet) -> SystemSetConfigs { + self.into_configs().after(set) } /// Treat this collection as a sequence. /// /// Ordering constraints will be applied between the successive collection elements. - fn chain(self) -> SystemSetCollection { - self.into_collection().chain() + fn chain(self) -> SystemSetConfigs { + self.into_configs().chain() } } -impl IntoSystemSetCollection for SystemSetCollection { - fn into_collection(self) -> Self { +impl IntoSystemSetConfigs for SystemSetConfigs { + fn into_configs(self) -> Self { self } fn in_set(mut self, set: impl SystemSet) -> Self { assert!(!set.is_system_type(), "invalid use of system type set"); - for config in self.inner.iter_mut() { + for config in self.sets.iter_mut() { config.graph_info.sets.insert(set.dyn_clone()); } @@ -472,7 +475,7 @@ impl IntoSystemSetCollection for SystemSetCollection { fn before(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - for config in self.inner.iter_mut() { + for config in self.sets.iter_mut() { config .graph_info .edges @@ -484,7 +487,7 @@ impl IntoSystemSetCollection for SystemSetCollection { fn after(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); - for config in self.inner.iter_mut() { + for config in self.sets.iter_mut() { config .graph_info .edges @@ -495,21 +498,23 @@ impl IntoSystemSetCollection for SystemSetCollection { } fn chain(mut self) -> Self { - todo!() + self.chained = true; + self } } macro_rules! impl_system_collection { ($($param: ident, $sys: ident),*) => { - impl<$($param, $sys),*> IntoSystemCollection<($($param),*)> for ($($sys),*) + impl<$($param, $sys),*> IntoSystemConfigs<($($param),*)> for ($($sys),*) where $($sys: IntoSystemConfig<$param>),* { #[allow(non_snake_case)] - fn into_collection(self) -> SystemCollection { + fn into_configs(self) -> SystemConfigs { let ($($sys,)*) = self; - SystemCollection { - inner: vec![$($sys.into_config(),)*], + SystemConfigs { + systems: vec![$($sys.into_config(),)*], + chained: false, } } } @@ -518,13 +523,14 @@ macro_rules! impl_system_collection { macro_rules! impl_system_set_collection { ($($set: ident),*) => { - impl<$($set: IntoSystemSetConfig),*> IntoSystemSetCollection for ($($set),*) + impl<$($set: IntoSystemSetConfig),*> IntoSystemSetConfigs for ($($set),*) { #[allow(non_snake_case)] - fn into_collection(self) -> SystemSetCollection { + fn into_configs(self) -> SystemSetConfigs { let ($($set,)*) = self; - SystemSetCollection { - inner: vec![$($set.into_config(),)*], + SystemSetConfigs { + sets: vec![$($set.into_config(),)*], + chained: false, } } } diff --git a/crates/bevy_ecs/src/schedule_v3/executor/mod.rs b/crates/bevy_ecs/src/schedule_v3/executor/mod.rs index ae5cc07b35a5a..1d5830ab0a3d9 100644 --- a/crates/bevy_ecs/src/schedule_v3/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule_v3/executor/mod.rs @@ -83,14 +83,3 @@ pub(super) fn is_apply_system_buffers(system: &BoxedSystem) -> bool { let type_id = get_type_id(&IntoSystem::into_system(apply_system_buffers)); (&*system as &dyn Any).type_id() == type_id } - -#[cfg(test)] -mod tests { - - #[test] - fn executor_parity() { - // In the absence of ambiguities, the single-threaded and - // multi-threaded executors must return the same results. - todo!(); - } -} diff --git a/crates/bevy_ecs/src/schedule_v3/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule_v3/executor/multi_threaded.rs index b61e904945d49..5452c3fa9b610 100644 --- a/crates/bevy_ecs/src/schedule_v3/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule_v3/executor/multi_threaded.rs @@ -125,7 +125,7 @@ impl SystemExecutor for MultiThreadedExecutor { self.spawn_system_tasks(scope, schedule, world); } - if self.running_systems.count_ones(..) != 0 { + if self.running_systems.count_ones(..) > 0 { // wait for systems to complete let index = self .receiver @@ -271,7 +271,7 @@ impl MultiThreadedExecutor { .extend(&system_meta.archetype_component_access); self.ready_systems.set(system_index, false); - self.running_systems.set(system_index, true); + self.running_systems.insert(system_index); if system_meta.is_send { scope.spawn(task); @@ -411,7 +411,7 @@ impl MultiThreadedExecutor { unsafe { condition.run_unsafe((), world) } }); - self.completed_sets.set(set_idx, true); + self.completed_sets.insert(set_idx); if !set_conditions_met { // mark all members as completed @@ -470,14 +470,14 @@ impl MultiThreadedExecutor { } self.running_systems.set(system_index, false); - self.completed_systems.set(system_index, true); - self.unapplied_systems.set(system_index, true); + self.completed_systems.insert(system_index); + self.unapplied_systems.insert(system_index); self.signal_dependents(system_index); } fn skip_system_and_signal_dependents(&mut self, system_index: usize) { self.ready_systems.set(system_index, false); - self.completed_systems.set(system_index, true); + self.completed_systems.insert(system_index); self.signal_dependents(system_index); } @@ -493,7 +493,7 @@ impl MultiThreadedExecutor { if (dependent_meta.dependencies_remaining == 0) && !self.completed_systems.contains(dep_idx) { - self.ready_systems.set(dep_idx, true); + self.ready_systems.insert(dep_idx); } } diff --git a/crates/bevy_ecs/src/schedule_v3/schedule.rs b/crates/bevy_ecs/src/schedule_v3/schedule.rs index f2bba797b0189..ff21b17f873bd 100644 --- a/crates/bevy_ecs/src/schedule_v3/schedule.rs +++ b/crates/bevy_ecs/src/schedule_v3/schedule.rs @@ -11,6 +11,7 @@ use bevy_utils::{ }; use fixedbitset::FixedBitSet; +use petgraph::graph::Node; use petgraph::{algo::tarjan_scc, prelude::*}; use crate::{ @@ -135,10 +136,18 @@ impl Schedule { self.graph.add_system(system); } + pub fn add_systems

(&mut self, systems: impl IntoSystemConfigs

) { + self.graph.add_systems(systems); + } + pub fn configure_set(&mut self, set: impl IntoSystemSetConfig) { self.graph.configure_set(set); } + pub fn configure_sets(&mut self, sets: impl IntoSystemSetConfigs) { + self.graph.configure_sets(sets); + } + pub fn set_default_set(&mut self, set: impl SystemSet) { self.graph.set_default_set(set); } @@ -151,6 +160,11 @@ impl Schedule { }; } + pub fn run(&mut self, world: &mut World) { + self.initialize(world).unwrap(); + self.executor.run(&mut self.executable, world); + } + pub(crate) fn initialize(&mut self, world: &mut World) -> Result<(), BuildError> { if self.graph.changed { self.graph.initialize(world); @@ -161,11 +175,6 @@ impl Schedule { Ok(()) } - pub fn run(&mut self, world: &mut World) { - self.initialize(world).unwrap(); - self.executor.run(&mut self.executable, world); - } - /// Iterates all component change ticks and clamps any older than [`MAX_CHANGE_AGE`](crate::change_detection::MAX_CHANGE_AGE). /// This prevents overflow and thus prevents false positives. /// @@ -256,16 +265,40 @@ impl ScheduleMeta { id } + fn chain(&mut self, nodes: Vec) { + for pair in nodes.as_slice().windows(2) { + self.dependency.graph.add_edge(pair[0], pair[1], ()); + } + } + fn set_default_set(&mut self, set: impl SystemSet) { assert!(!set.is_system_type(), "invalid use of system type set"); self.default_set = Some(set.dyn_clone()); } + fn add_systems

(&mut self, systems: impl IntoSystemConfigs

) { + let SystemConfigs { systems, chained } = systems.into_configs(); + + let mut iter = systems + .into_iter() + .map(|system| self.add_system_inner(system).unwrap()); + + if chained { + let ids = iter.collect::>(); + self.chain(ids); + } else { + while iter.next().is_some() {} + } + } + fn add_system

(&mut self, system: impl IntoSystemConfig

) { self.add_system_inner(system).unwrap(); } - fn add_system_inner

(&mut self, system: impl IntoSystemConfig

) -> Result<(), BuildError> { + fn add_system_inner

( + &mut self, + system: impl IntoSystemConfig

, + ) -> Result { let SystemConfig { system, mut graph_info, @@ -288,14 +321,29 @@ impl ScheduleMeta { .push((id, UninitNode::System(system, conditions))); self.changed = true; - Ok(()) + Ok(id) + } + + fn configure_sets(&mut self, sets: impl IntoSystemSetConfigs) { + let SystemSetConfigs { sets, chained } = sets.into_configs(); + + let mut iter = sets + .into_iter() + .map(|set| self.configure_set_inner(set).unwrap()); + + if chained { + let ids = iter.collect::>(); + self.chain(ids); + } else { + while iter.next().is_some() {} + } } fn configure_set(&mut self, set: impl IntoSystemSetConfig) { self.configure_set_inner(set).unwrap(); } - fn configure_set_inner(&mut self, set: impl IntoSystemSetConfig) -> Result<(), BuildError> { + fn configure_set_inner(&mut self, set: impl IntoSystemSetConfig) -> Result { let SystemSetConfig { set, mut graph_info, @@ -304,12 +352,7 @@ impl ScheduleMeta { let id = match self.system_set_ids.get(&set) { Some(&id) => id, - None => { - let id = NodeId::Set(self.next_id()); - self.system_set_ids.insert(set.dyn_clone(), id); - self.system_sets.insert(id, SystemSetMeta::new(set)); - id - } + None => self.add_set(set), }; // TODO: only if this is the first time configure_set has been called for this set @@ -326,13 +369,14 @@ impl ScheduleMeta { self.uninit.push((id, UninitNode::SystemSet(conditions))); self.changed = true; - Ok(()) + Ok(id) } - fn add_set(&mut self, set: BoxedSystemSet) { + fn add_set(&mut self, set: BoxedSystemSet) -> NodeId { let id = NodeId::Set(self.next_id()); self.system_set_ids.insert(set.dyn_clone(), id); self.system_sets.insert(id, SystemSetMeta::new(set)); + id } fn check_sets(&mut self, id: &NodeId, graph_info: &GraphInfo) -> Result<(), BuildError> { @@ -343,7 +387,9 @@ impl ScheduleMeta { return Err(BuildError::HierarchyLoop(set.dyn_clone())); } } - None => self.add_set(set.dyn_clone()), + None => { + self.add_set(set.dyn_clone()); + } } } @@ -358,7 +404,9 @@ impl ScheduleMeta { return Err(BuildError::DependencyLoop(dep.dyn_clone())); } } - None => self.add_set(dep.dyn_clone()), + None => { + self.add_set(dep.dyn_clone()); + } } } @@ -376,13 +424,13 @@ impl ScheduleMeta { } self.dependency.graph.add_node(id); - for (kind, oid) in edges + for (edge_kind, other_id) in edges .into_iter() - .map(|(kind, set)| (kind, self.system_set_ids[&set])) + .map(|(edge_kind, set)| (edge_kind, self.system_set_ids[&set])) { - let (lhs, rhs) = match kind { - DependencyEdgeKind::Before => (id, oid), - DependencyEdgeKind::After => (oid, id), + let (lhs, rhs) = match edge_kind { + DependencyEdgeKind::Before => (id, other_id), + DependencyEdgeKind::After => (other_id, id), }; self.dependency.graph.add_edge(lhs, rhs, ()); } @@ -394,6 +442,7 @@ impl ScheduleMeta { for (id, uninit) in self.uninit.drain(..) { match uninit { UninitNode::System(mut system, mut conditions) => { + debug_assert!(id.is_system()); system.initialize(world); self.systems.insert(id, system); for condition in conditions.iter_mut() { @@ -402,6 +451,7 @@ impl ScheduleMeta { self.conditions.insert(id, conditions); } UninitNode::SystemSet(mut conditions) => { + debug_assert!(id.is_set()); for condition in conditions.iter_mut() { condition.initialize(world); } @@ -465,7 +515,6 @@ impl ScheduleMeta { // system type sets with multiple members cannot be used in dependencies for (&set, systems) in systems.iter() { if self.system_sets[&set].is_system_type() { - let member_count = systems.len(); let mut dep_count = 0; dep_count += self .dependency @@ -478,7 +527,7 @@ impl ScheduleMeta { .edges_directed(set, Direction::Outgoing) .count(); - if member_count > 1 && dep_count > 0 { + if systems.len() > 1 && dep_count > 0 { let type_set = self.system_sets[&set].0.dyn_clone(); return Err(BuildError::SystemTypeSetAmbiguity(type_set)); } @@ -571,6 +620,7 @@ impl ScheduleMeta { .topsort .iter() .filter(|&id| { + // ignore system type sets // ignore system sets that have no conditions id.is_set() && self.conditions.get(id).filter(|v| !v.is_empty()).is_some() }) @@ -816,45 +866,34 @@ mod tests { use std::sync::{Arc, Mutex}; #[derive(Resource)] - struct X(Arc>>); + struct X(Vec); - let x = X(Arc::new(Mutex::new(Vec::new()))); let mut world = World::new(); - world.insert_resource(x); + world.insert_resource(X(Vec::new())); - fn mark_as_completed(set: TestSet) -> impl FnMut(Res) { - move |x: Res| { - x.0.lock().unwrap().push(set.clone()); + fn run(set: TestSet) -> impl FnMut(ResMut) { + move |mut x: ResMut| { + x.0.push(set.clone()); } } let mut schedule = Schedule::new(); - schedule.add_system(mark_as_completed(TestSet::A).in_set(TestSet::A)); - schedule.add_system( - mark_as_completed(TestSet::B) - .in_set(TestSet::B) - .after(TestSet::A), - ); - schedule.add_system( - mark_as_completed(TestSet::C) - .in_set(TestSet::C) - .after(TestSet::A), - ); - schedule.add_system( - mark_as_completed(TestSet::D) - .in_set(TestSet::D) - .after(TestSet::B) - .after(TestSet::C), + schedule.add_systems( + ( + run(TestSet::A), + run(TestSet::B), + run(TestSet::C), + run(TestSet::D), + ) + .chain(), ); schedule.run(&mut world); - let x = world.remove_resource::().unwrap(); - let results = Arc::try_unwrap(x.0).unwrap().into_inner().unwrap(); - - // A -> B -> C -> D - // A -> C -> B -> D - - println!("{:?}", results); + let X(results) = world.remove_resource::().unwrap(); + assert_eq!( + results, + vec![TestSet::A, TestSet::B, TestSet::C, TestSet::D] + ); } #[test] @@ -931,6 +970,7 @@ mod tests { fn in_system_type_set() { fn foo() {} fn bar() {} + let mut schedule = Schedule::new(); schedule.add_system(foo.in_set(bar.into_system_set())); }