diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs index 0e8cbb6a49675..7312a5fcf659f 100644 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -552,7 +552,7 @@ impl Plugin for AnimationPlugin { .register_type::() .add_system( animation_player - .in_set(CoreSet::PostUpdate) + .in_base_set(CoreSet::PostUpdate) .before(TransformSystem::TransformPropagate), ); } diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 893abb5cc4d92..2aaf527c36d17 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -328,14 +328,14 @@ impl App { apply_state_transition::, ) .chain() - .in_set(CoreSet::StateTransitions), + .in_base_set(CoreSet::StateTransitions), ); let main_schedule = self.get_schedule_mut(CoreSchedule::Main).unwrap(); for variant in S::variants() { main_schedule.configure_set( OnUpdate(variant.clone()) - .in_set(CoreSet::StateTransitions) + .in_base_set(CoreSet::StateTransitions) .run_if(state_equals(variant)) .after(apply_state_transition::), ); @@ -580,7 +580,7 @@ impl App { { if !self.world.contains_resource::>() { self.init_resource::>() - .add_system(Events::::update_system.in_set(CoreSet::First)); + .add_system(Events::::update_system.in_base_set(CoreSet::First)); } self } diff --git a/crates/bevy_app/src/lib.rs b/crates/bevy_app/src/lib.rs index 2556c844cc28c..97df5ba5beade 100644 --- a/crates/bevy_app/src/lib.rs +++ b/crates/bevy_app/src/lib.rs @@ -29,8 +29,8 @@ pub mod prelude { use bevy_ecs::{ schedule_v3::{ - apply_system_buffers, IntoSystemConfig, IntoSystemSetConfig, Schedule, ScheduleLabel, - SystemSet, + apply_system_buffers, IntoSystemConfig, IntoSystemSetConfig, IntoSystemSetConfigs, + Schedule, ScheduleLabel, SystemSet, }, system::Local, world::World, @@ -90,6 +90,7 @@ impl CoreSchedule { /// that runs immediately after the matching system set. /// These can be useful for ordering, but you almost never want to add your systems to these sets. #[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)] +#[system_set(base)] pub enum CoreSet { /// Runs before all other members of this set. First, @@ -129,20 +130,30 @@ impl CoreSet { let mut schedule = Schedule::new(); // Create "stage-like" structure using buffer flushes + ordering - schedule.add_system(apply_system_buffers.in_set(FirstFlush)); - schedule.add_system(apply_system_buffers.in_set(PreUpdateFlush)); - schedule.add_system(apply_system_buffers.in_set(UpdateFlush)); - schedule.add_system(apply_system_buffers.in_set(PostUpdateFlush)); - schedule.add_system(apply_system_buffers.in_set(LastFlush)); - - schedule.configure_set(First.before(FirstFlush)); - schedule.configure_set(PreUpdate.after(FirstFlush).before(PreUpdateFlush)); - schedule.configure_set(StateTransitions.after(PreUpdateFlush).before(FixedUpdate)); - schedule.configure_set(FixedUpdate.after(StateTransitions).before(Update)); - schedule.configure_set(Update.after(FixedUpdate).before(UpdateFlush)); - schedule.configure_set(PostUpdate.after(UpdateFlush).before(PostUpdateFlush)); - schedule.configure_set(Last.after(PostUpdateFlush).before(LastFlush)); - + schedule + .set_default_base_set(Update) + .add_system(apply_system_buffers.in_base_set(FirstFlush)) + .add_system(apply_system_buffers.in_base_set(PreUpdateFlush)) + .add_system(apply_system_buffers.in_base_set(UpdateFlush)) + .add_system(apply_system_buffers.in_base_set(PostUpdateFlush)) + .add_system(apply_system_buffers.in_base_set(LastFlush)) + .configure_sets( + ( + First, + FirstFlush, + PreUpdate, + PreUpdateFlush, + StateTransitions, + FixedUpdate, + Update, + UpdateFlush, + PostUpdate, + PostUpdateFlush, + Last, + LastFlush, + ) + .chain(), + ); schedule } } diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index 7f29d3ee99499..c1be48384484c 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -644,7 +644,7 @@ pub fn free_unused_assets_system(asset_server: Res) { mod test { use super::*; use crate::{loader::LoadedAsset, update_asset_storage_system}; - use bevy_app::{App, CoreSet}; + use bevy_app::App; use bevy_ecs::prelude::*; use bevy_reflect::TypeUuid; use bevy_utils::BoxedFuture; @@ -852,16 +852,8 @@ mod test { let mut app = App::new(); app.insert_resource(assets); app.insert_resource(asset_server); - app.add_system( - free_unused_assets_system - .in_set(FreeUnusedAssets) - .in_set(CoreSet::Update), - ); - app.add_system( - update_asset_storage_system:: - .after(FreeUnusedAssets) - .in_set(CoreSet::Update), - ); + app.add_system(free_unused_assets_system.in_set(FreeUnusedAssets)); + app.add_system(update_asset_storage_system::.after(FreeUnusedAssets)); fn load_asset(path: AssetPath, world: &World) -> HandleUntyped { let asset_server = world.resource::(); diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index a359eff7487a5..3d70f44f2efaa 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -331,8 +331,8 @@ impl AddAsset for App { }; self.insert_resource(assets) - .add_system(Assets::::asset_event_system.in_set(AssetSet::AssetEvents)) - .add_system(update_asset_storage_system::.in_set(AssetSet::LoadAssets)) + .add_system(Assets::::asset_event_system.in_base_set(AssetSet::AssetEvents)) + .add_system(update_asset_storage_system::.in_base_set(AssetSet::LoadAssets)) .register_type::>() .add_event::>() } diff --git a/crates/bevy_asset/src/debug_asset_server.rs b/crates/bevy_asset/src/debug_asset_server.rs index 47fd13422d8b6..718820adfd97e 100644 --- a/crates/bevy_asset/src/debug_asset_server.rs +++ b/crates/bevy_asset/src/debug_asset_server.rs @@ -75,7 +75,7 @@ impl Plugin for DebugAssetServerPlugin { watch_for_changes: true, }); app.insert_non_send_resource(DebugAssetApp(debug_asset_app)); - app.add_system(run_debug_asset_app.in_set(CoreSet::Update)); + app.add_system(run_debug_asset_app); } } diff --git a/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs b/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs index bbcb712ab1d8b..1603e40382469 100644 --- a/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs +++ b/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs @@ -19,7 +19,7 @@ impl Default for AssetCountDiagnosticsPlugin { impl Plugin for AssetCountDiagnosticsPlugin { fn build(&self, app: &mut App) { app.add_startup_system(Self::setup_system.in_set(StartupSet::Startup)) - .add_system(Self::diagnostic_system.in_set(CoreSet::Update)); + .add_system(Self::diagnostic_system); } } diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index c1fe5bef836e6..59b0b2cb327f3 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -51,6 +51,7 @@ use bevy_ecs::prelude::*; /// [`SystemSet`]s for asset loading in an [`App`] schedule. #[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)] +#[system_set(base)] pub enum AssetSet { /// Asset storages are updated. LoadAssets, @@ -109,22 +110,20 @@ impl Plugin for AssetPlugin { app.configure_set( AssetSet::LoadAssets - .no_default_set() .before(CoreSet::PreUpdate) .after(CoreSet::First), ) .configure_set( AssetSet::AssetEvents - .no_default_set() .after(CoreSet::PostUpdate) .before(CoreSet::Last), ) - .add_system(asset_server::free_unused_assets_system.in_set(CoreSet::PreUpdate)); + .add_system(asset_server::free_unused_assets_system.in_base_set(CoreSet::PreUpdate)); #[cfg(all( feature = "filesystem_watcher", all(not(target_arch = "wasm32"), not(target_os = "android")) ))] - app.add_system(io::filesystem_watcher_system.in_set(AssetSet::LoadAssets)); + app.add_system(io::filesystem_watcher_system.in_base_set(AssetSet::LoadAssets)); } } diff --git a/crates/bevy_audio/src/lib.rs b/crates/bevy_audio/src/lib.rs index 03aac688a3c4f..14b410135df52 100644 --- a/crates/bevy_audio/src/lib.rs +++ b/crates/bevy_audio/src/lib.rs @@ -56,7 +56,7 @@ impl Plugin for AudioPlugin { .add_asset::() .add_asset::() .init_resource::>() - .add_system(play_queued_audio_system::.in_set(CoreSet::PostUpdate)); + .add_system(play_queued_audio_system::.in_base_set(CoreSet::PostUpdate)); #[cfg(any(feature = "mp3", feature = "flac", feature = "wav", feature = "vorbis"))] app.init_asset_loader::(); @@ -71,6 +71,6 @@ impl AddAudioSource for App { self.add_asset::() .init_resource::>() .init_resource::>() - .add_system(play_queued_audio_system::.in_set(CoreSet::PostUpdate)) + .add_system(play_queued_audio_system::.in_base_set(CoreSet::PostUpdate)) } } diff --git a/crates/bevy_core/src/lib.rs b/crates/bevy_core/src/lib.rs index caa60d4b9f2c6..1a7d13d63756b 100644 --- a/crates/bevy_core/src/lib.rs +++ b/crates/bevy_core/src/lib.rs @@ -107,7 +107,7 @@ impl Plugin for TaskPoolPlugin { self.task_pool_options.create_default_pools(); #[cfg(not(target_arch = "wasm32"))] - app.add_system(tick_global_task_pools.in_set(bevy_app::CoreSet::Last)); + app.add_system(tick_global_task_pools.in_base_set(bevy_app::CoreSet::Last)); } } /// A dummy type that is [`!Send`](Send), to force systems to run on the main thread. @@ -142,7 +142,7 @@ pub struct FrameCountPlugin; impl Plugin for FrameCountPlugin { fn build(&self, app: &mut App) { app.init_resource::(); - app.add_system(update_frame_count.in_set(CoreSet::Last)); + app.add_system(update_frame_count.in_base_set(CoreSet::Last)); } } diff --git a/crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs index d8206fa4ad580..eff6ae58fc598 100644 --- a/crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs +++ b/crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs @@ -10,7 +10,7 @@ pub struct EntityCountDiagnosticsPlugin; impl Plugin for EntityCountDiagnosticsPlugin { fn build(&self, app: &mut App) { app.add_startup_system(Self::setup_system.in_set(StartupSet::Startup)) - .add_system(Self::diagnostic_system.in_set(CoreSet::Update)); + .add_system(Self::diagnostic_system); } } diff --git a/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs index 9f82d64727e28..85b7825e5f0d9 100644 --- a/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs +++ b/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs @@ -11,7 +11,7 @@ pub struct FrameTimeDiagnosticsPlugin; impl Plugin for FrameTimeDiagnosticsPlugin { fn build(&self, app: &mut bevy_app::App) { app.add_startup_system(Self::setup_system.in_set(StartupSet::Startup)) - .add_system(Self::diagnostic_system.in_set(CoreSet::Update)); + .add_system(Self::diagnostic_system); } } diff --git a/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs index dea8cd7676d8b..dd80c8c8f2024 100644 --- a/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs +++ b/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs @@ -37,9 +37,9 @@ impl Plugin for LogDiagnosticsPlugin { }); if self.debug { - app.add_system(Self::log_diagnostics_debug_system.in_set(CoreSet::PostUpdate)); + app.add_system(Self::log_diagnostics_debug_system.in_base_set(CoreSet::PostUpdate)); } else { - app.add_system(Self::log_diagnostics_system.in_set(CoreSet::PostUpdate)); + app.add_system(Self::log_diagnostics_system.in_base_set(CoreSet::PostUpdate)); } } } diff --git a/crates/bevy_diagnostic/src/system_information_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/system_information_diagnostics_plugin.rs index 9bc56418de845..e82277a9317d1 100644 --- a/crates/bevy_diagnostic/src/system_information_diagnostics_plugin.rs +++ b/crates/bevy_diagnostic/src/system_information_diagnostics_plugin.rs @@ -16,7 +16,7 @@ pub struct SystemInformationDiagnosticsPlugin; impl Plugin for SystemInformationDiagnosticsPlugin { fn build(&self, app: &mut App) { app.add_startup_system(internal::setup_system.in_set(StartupSet::Startup)) - .add_system(internal::diagnostic_system.in_set(CoreSet::Update)); + .add_system(internal::diagnostic_system); } } diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 86e40a8144a01..be45ebe6f3b93 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -2,9 +2,10 @@ extern crate proc_macro; mod component; mod fetch; +mod set; -use crate::fetch::derive_world_query_impl; -use bevy_macro_utils::{derive_boxed_label, derive_set, get_named_struct_fields, BevyManifest}; +use crate::{fetch::derive_world_query_impl, set::derive_set}; +use bevy_macro_utils::{derive_boxed_label, get_named_struct_fields, BevyManifest}; use proc_macro::TokenStream; use proc_macro2::Span; use quote::{format_ident, quote}; @@ -537,7 +538,7 @@ pub fn derive_schedule_label(input: TokenStream) -> TokenStream { } /// Derive macro generating an impl of the trait `SystemSet`. -#[proc_macro_derive(SystemSet)] +#[proc_macro_derive(SystemSet, attributes(system_set))] pub fn derive_system_set(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); let mut trait_path = bevy_ecs_path(); diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs new file mode 100644 index 0000000000000..7c1b0c66b34a0 --- /dev/null +++ b/crates/bevy_ecs/macros/src/set.rs @@ -0,0 +1,84 @@ +use proc_macro::TokenStream; +use quote::{quote, ToTokens}; +use syn::parse::{Parse, ParseStream}; + +pub static SYSTEM_SET_ATTRIBUTE_NAME: &str = "system_set"; +pub static BASE_ATTRIBUTE_NAME: &str = "base"; + +/// Derive a label trait +/// +/// # Args +/// +/// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait +/// - `trait_path`: The path [`syn::Path`] to the label trait +pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { + let ident = input.ident; + + let mut is_base = false; + for attr in &input.attrs { + if !attr + .path + .get_ident() + .map_or(false, |ident| ident == SYSTEM_SET_ATTRIBUTE_NAME) + { + continue; + } + + attr.parse_args_with(|input: ParseStream| { + let meta = input.parse_terminated::(syn::Meta::parse)?; + for meta in meta { + let ident = meta.path().get_ident().unwrap_or_else(|| { + panic!( + "Unrecognized attribute: `{}`", + meta.path().to_token_stream() + ) + }); + if ident == BASE_ATTRIBUTE_NAME { + if let syn::Meta::Path(_) = meta { + is_base = true; + } else { + panic!( + "The `{BASE_ATTRIBUTE_NAME}` attribute is expected to have no value or arguments", + ); + } + } else { + panic!( + "Unrecognized attribute: `{}`", + meta.path().to_token_stream() + ); + } + } + Ok(()) + }) + .unwrap_or_else(|_| panic!("Invalid `{SYSTEM_SET_ATTRIBUTE_NAME}` attribute format")); + } + + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause { + where_token: Default::default(), + predicates: Default::default(), + }); + where_clause.predicates.push( + syn::parse2(quote! { + Self: 'static + Send + Sync + Clone + Eq + ::std::fmt::Debug + ::std::hash::Hash + }) + .unwrap(), + ); + + (quote! { + impl #impl_generics #trait_path for #ident #ty_generics #where_clause { + fn is_system_type(&self) -> bool { + false + } + + fn is_base(&self) -> bool { + #is_base + } + + fn dyn_clone(&self) -> std::boxed::Box { + std::boxed::Box::new(std::clone::Clone::clone(self)) + } + } + }) + .into() +} diff --git a/crates/bevy_ecs/src/schedule_v3/config.rs b/crates/bevy_ecs/src/schedule_v3/config.rs index c72d0983f5866..bb171871f0462 100644 --- a/crates/bevy_ecs/src/schedule_v3/config.rs +++ b/crates/bevy_ecs/src/schedule_v3/config.rs @@ -28,7 +28,7 @@ impl SystemSetConfig { Self { set, - graph_info: GraphInfo::default(), + graph_info: GraphInfo::system_set(), conditions: Vec::new(), } } @@ -45,12 +45,11 @@ impl SystemConfig { fn new(system: BoxedSystem) -> Self { // include system in its default sets let sets = system.default_system_sets().into_iter().collect(); + let mut graph_info = GraphInfo::system(); + graph_info.sets = sets; Self { system, - graph_info: GraphInfo { - sets, - ..Default::default() - }, + graph_info, conditions: Vec::new(), } } @@ -87,9 +86,13 @@ pub trait IntoSystemSetConfig: sealed::IntoSystemSetConfig { #[doc(hidden)] fn into_config(self) -> SystemSetConfig; /// Add to the provided `set`. + #[track_caller] fn in_set(self, set: impl SystemSet) -> SystemSetConfig; - /// Don't add this set to the schedules's default set. - fn no_default_set(self) -> SystemSetConfig; + /// Add to the provided "base" `set`. For more information on base sets, see [`SystemSet::is_base`]. + #[track_caller] + fn in_base_set(self, set: impl SystemSet) -> SystemSetConfig; + /// Add this set to the schedules's default base set. + fn in_default_base_set(self) -> SystemSetConfig; /// Run before all systems in `set`. fn before(self, set: impl IntoSystemSet) -> SystemSetConfig; /// Run after all systems in `set`. @@ -117,12 +120,18 @@ where SystemSetConfig::new(Box::new(self)) } + #[track_caller] fn in_set(self, set: impl SystemSet) -> SystemSetConfig { self.into_config().in_set(set) } - fn no_default_set(self) -> SystemSetConfig { - self.into_config().no_default_set() + #[track_caller] + fn in_base_set(self, set: impl SystemSet) -> SystemSetConfig { + self.into_config().in_base_set(set) + } + + fn in_default_base_set(self) -> SystemSetConfig { + self.into_config().in_default_base_set() } fn before(self, set: impl IntoSystemSet) -> SystemSetConfig { @@ -155,12 +164,18 @@ impl IntoSystemSetConfig for BoxedSystemSet { SystemSetConfig::new(self) } + #[track_caller] fn in_set(self, set: impl SystemSet) -> SystemSetConfig { self.into_config().in_set(set) } - fn no_default_set(self) -> SystemSetConfig { - self.into_config().no_default_set() + #[track_caller] + fn in_base_set(self, set: impl SystemSet) -> SystemSetConfig { + self.into_config().in_base_set(set) + } + + fn in_default_base_set(self) -> SystemSetConfig { + self.into_config().in_default_base_set() } fn before(self, set: impl IntoSystemSet) -> SystemSetConfig { @@ -193,17 +208,44 @@ impl IntoSystemSetConfig for SystemSetConfig { self } + #[track_caller] fn in_set(mut self, set: impl SystemSet) -> Self { assert!( !set.is_system_type(), "adding arbitrary systems to a system type set is not allowed" ); + assert!( + !set.is_base(), + "Sets cannot be added to 'base' system sets using 'in_set'. Use 'in_base_set' instead." + ); + assert!( + !self.set.is_base(), + "Base system sets cannot be added to other sets." + ); self.graph_info.sets.push(Box::new(set)); self } - fn no_default_set(mut self) -> SystemSetConfig { - self.graph_info.add_default_set = false; + #[track_caller] + fn in_base_set(mut self, set: impl SystemSet) -> Self { + assert!( + !set.is_system_type(), + "System type sets cannot be base sets." + ); + assert!( + set.is_base(), + "Sets cannot be added to normal sets using 'in_base_set'. Use 'in_set' instead." + ); + assert!( + !self.set.is_base(), + "Base system sets cannot be added to other sets." + ); + self.graph_info.set_base_set(Box::new(set)); + self + } + + fn in_default_base_set(mut self) -> SystemSetConfig { + self.graph_info.add_default_base_set = true; self } @@ -252,9 +294,13 @@ pub trait IntoSystemConfig: sealed::IntoSystemConfig { #[doc(hidden)] fn into_config(self) -> SystemConfig; /// Add to `set` membership. + #[track_caller] fn in_set(self, set: impl SystemSet) -> SystemConfig; + /// Add to the provided "base" `set`. For more information on base sets, see [`SystemSet::is_base`]. + #[track_caller] + fn in_base_set(self, set: impl SystemSet) -> SystemConfig; /// Don't add this system to the schedules's default set. - fn no_default_set(self) -> SystemConfig; + fn no_default_base_set(self) -> SystemConfig; /// Run before all systems in `set`. fn before(self, set: impl IntoSystemSet) -> SystemConfig; /// Run after all systems in `set`. @@ -282,12 +328,18 @@ where SystemConfig::new(Box::new(IntoSystem::into_system(self))) } + #[track_caller] fn in_set(self, set: impl SystemSet) -> SystemConfig { self.into_config().in_set(set) } - fn no_default_set(self) -> SystemConfig { - self.into_config().no_default_set() + #[track_caller] + fn in_base_set(self, set: impl SystemSet) -> SystemConfig { + self.into_config().in_base_set(set) + } + + fn no_default_base_set(self) -> SystemConfig { + self.into_config().no_default_base_set() } fn before(self, set: impl IntoSystemSet) -> SystemConfig { @@ -320,12 +372,18 @@ impl IntoSystemConfig<()> for BoxedSystem<(), ()> { SystemConfig::new(self) } + #[track_caller] fn in_set(self, set: impl SystemSet) -> SystemConfig { self.into_config().in_set(set) } - fn no_default_set(self) -> SystemConfig { - self.into_config().no_default_set() + #[track_caller] + fn in_base_set(self, set: impl SystemSet) -> SystemConfig { + self.into_config().in_base_set(set) + } + + fn no_default_base_set(self) -> SystemConfig { + self.into_config().no_default_base_set() } fn before(self, set: impl IntoSystemSet) -> SystemConfig { @@ -358,17 +416,36 @@ impl IntoSystemConfig<()> for SystemConfig { self } + #[track_caller] fn in_set(mut self, set: impl SystemSet) -> Self { assert!( !set.is_system_type(), "adding arbitrary systems to a system type set is not allowed" ); + assert!( + !set.is_base(), + "Systems cannot be added to 'base' system sets using 'in_set'. Use 'in_base_set' instead." + ); self.graph_info.sets.push(Box::new(set)); self } - fn no_default_set(mut self) -> SystemConfig { - self.graph_info.add_default_set = false; + #[track_caller] + fn in_base_set(mut self, set: impl SystemSet) -> Self { + assert!( + !set.is_system_type(), + "System type sets cannot be base sets." + ); + assert!( + set.is_base(), + "Systems cannot be added to normal sets using 'in_base_set'. Use 'in_set' instead." + ); + self.graph_info.set_base_set(Box::new(set)); + self + } + + fn no_default_base_set(mut self) -> SystemConfig { + self.graph_info.add_default_base_set = false; self } @@ -451,10 +528,17 @@ where fn into_configs(self) -> SystemConfigs; /// Add these systems to the provided `set`. + #[track_caller] fn in_set(self, set: impl SystemSet) -> SystemConfigs { self.into_configs().in_set(set) } + /// Add these systems to the provided "base" `set`. For more information on base sets, see [`SystemSet::is_base`]. + #[track_caller] + fn in_base_set(self, set: impl SystemSet) -> SystemConfigs { + self.into_configs().in_base_set(set) + } + /// Run before all systems in `set`. fn before(self, set: impl IntoSystemSet) -> SystemConfigs { self.into_configs().before(set) @@ -495,11 +579,16 @@ impl IntoSystemConfigs<()> for SystemConfigs { self } + #[track_caller] fn in_set(mut self, set: impl SystemSet) -> Self { assert!( !set.is_system_type(), "adding arbitrary systems to a system type set is not allowed" ); + assert!( + !set.is_base(), + "Systems cannot be added to 'base' system sets using 'in_set'. Use 'in_base_set' instead." + ); for config in &mut self.systems { config.graph_info.sets.push(set.dyn_clone()); } @@ -507,6 +596,23 @@ impl IntoSystemConfigs<()> for SystemConfigs { self } + #[track_caller] + fn in_base_set(mut self, set: impl SystemSet) -> Self { + assert!( + !set.is_system_type(), + "System type sets cannot be base sets." + ); + assert!( + set.is_base(), + "Systems cannot be added to normal sets using 'in_base_set'. Use 'in_set' instead." + ); + for config in &mut self.systems { + config.graph_info.set_base_set(set.dyn_clone()); + } + + self + } + fn before(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); for config in &mut self.systems { @@ -575,10 +681,17 @@ where fn into_configs(self) -> SystemSetConfigs; /// Add these system sets to the provided `set`. + #[track_caller] fn in_set(self, set: impl SystemSet) -> SystemSetConfigs { self.into_configs().in_set(set) } + /// Add these system sets to the provided "base" `set`. For more information on base sets, see [`SystemSet::is_base`]. + #[track_caller] + fn in_base_set(self, set: impl SystemSet) -> SystemSetConfigs { + self.into_configs().in_base_set(set) + } + /// Run before all systems in `set`. fn before(self, set: impl IntoSystemSet) -> SystemSetConfigs { self.into_configs().before(set) @@ -614,18 +727,48 @@ impl IntoSystemSetConfigs for SystemSetConfigs { self } + #[track_caller] fn in_set(mut self, set: impl SystemSet) -> Self { assert!( !set.is_system_type(), "adding arbitrary systems to a system type set is not allowed" ); + assert!( + !set.is_base(), + "Sets cannot be added to 'base' system sets using 'in_set'. Use 'in_base_set' instead." + ); for config in &mut self.sets { + assert!( + !config.set.is_base(), + "Base system sets cannot be added to other sets." + ); config.graph_info.sets.push(set.dyn_clone()); } self } + #[track_caller] + fn in_base_set(mut self, set: impl SystemSet) -> Self { + assert!( + !set.is_system_type(), + "System type sets cannot be base sets." + ); + assert!( + set.is_base(), + "Sets cannot be added to normal sets using 'in_base_set'. Use 'in_set' instead." + ); + for config in &mut self.sets { + assert!( + !config.set.is_base(), + "Base system sets cannot be added to other sets." + ); + config.graph_info.set_base_set(set.dyn_clone()); + } + + self + } + fn before(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); for config in &mut self.sets { diff --git a/crates/bevy_ecs/src/schedule_v3/executor/mod.rs b/crates/bevy_ecs/src/schedule_v3/executor/mod.rs index b26dd181562e9..9ba5fdc8f0274 100644 --- a/crates/bevy_ecs/src/schedule_v3/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule_v3/executor/mod.rs @@ -54,8 +54,8 @@ pub(super) struct SystemSchedule { pub(super) set_ids: Vec, pub(super) system_dependencies: Vec, pub(super) system_dependents: Vec>, - pub(super) sets_of_systems: Vec, - pub(super) systems_in_sets: Vec, + pub(super) sets_with_conditions_of_systems: Vec, + pub(super) systems_in_sets_with_conditions: Vec, } impl SystemSchedule { @@ -68,8 +68,8 @@ impl SystemSchedule { set_ids: Vec::new(), system_dependencies: Vec::new(), system_dependents: Vec::new(), - sets_of_systems: Vec::new(), - systems_in_sets: Vec::new(), + sets_with_conditions_of_systems: Vec::new(), + systems_in_sets_with_conditions: Vec::new(), } } } 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 dadd75a5dff35..66bcc3c638523 100644 --- a/crates/bevy_ecs/src/schedule_v3/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule_v3/executor/multi_threaded.rs @@ -32,8 +32,8 @@ struct SyncUnsafeSchedule<'a> { struct Conditions<'a> { system_conditions: &'a mut [Vec], set_conditions: &'a mut [Vec], - sets_of_systems: &'a [FixedBitSet], - systems_in_sets: &'a [FixedBitSet], + sets_with_conditions_of_systems: &'a [FixedBitSet], + systems_in_sets_with_conditions: &'a [FixedBitSet], } impl SyncUnsafeSchedule<'_> { @@ -43,8 +43,8 @@ impl SyncUnsafeSchedule<'_> { conditions: Conditions { system_conditions: &mut schedule.system_conditions, set_conditions: &mut schedule.set_conditions, - sets_of_systems: &schedule.sets_of_systems, - systems_in_sets: &schedule.systems_in_sets, + sets_with_conditions_of_systems: &schedule.sets_with_conditions_of_systems, + systems_in_sets_with_conditions: &schedule.systems_in_sets_with_conditions, }, } } @@ -333,7 +333,9 @@ impl MultiThreadedExecutor { } // TODO: an earlier out if world's archetypes did not change - for set_idx in conditions.sets_of_systems[system_index].difference(&self.evaluated_sets) { + for set_idx in conditions.sets_with_conditions_of_systems[system_index] + .difference(&self.evaluated_sets) + { for condition in &mut conditions.set_conditions[set_idx] { condition.update_archetype_component_access(world); if !condition @@ -382,7 +384,7 @@ impl MultiThreadedExecutor { world: &World, ) -> bool { let mut should_run = !self.skipped_systems.contains(system_index); - for set_idx in conditions.sets_of_systems[system_index].ones() { + for set_idx in conditions.sets_with_conditions_of_systems[system_index].ones() { if self.evaluated_sets.contains(set_idx) { continue; } @@ -393,7 +395,7 @@ impl MultiThreadedExecutor { if !set_conditions_met { self.skipped_systems - .union_with(&conditions.systems_in_sets[set_idx]); + .union_with(&conditions.systems_in_sets_with_conditions[set_idx]); } should_run &= set_conditions_met; diff --git a/crates/bevy_ecs/src/schedule_v3/executor/simple.rs b/crates/bevy_ecs/src/schedule_v3/executor/simple.rs index c4ad1e2282f72..1e78e3655a86a 100644 --- a/crates/bevy_ecs/src/schedule_v3/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule_v3/executor/simple.rs @@ -41,7 +41,7 @@ impl SystemExecutor for SimpleExecutor { let should_run_span = info_span!("check_conditions", name = &*name).entered(); let mut should_run = !self.completed_systems.contains(system_index); - for set_idx in schedule.sets_of_systems[system_index].ones() { + for set_idx in schedule.sets_with_conditions_of_systems[system_index].ones() { if self.evaluated_sets.contains(set_idx) { continue; } @@ -52,7 +52,7 @@ impl SystemExecutor for SimpleExecutor { if !set_conditions_met { self.completed_systems - .union_with(&schedule.systems_in_sets[set_idx]); + .union_with(&schedule.systems_in_sets_with_conditions[set_idx]); } should_run &= set_conditions_met; diff --git a/crates/bevy_ecs/src/schedule_v3/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule_v3/executor/single_threaded.rs index 14dfb984d1584..a54f3a6378a4b 100644 --- a/crates/bevy_ecs/src/schedule_v3/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule_v3/executor/single_threaded.rs @@ -51,7 +51,7 @@ impl SystemExecutor for SingleThreadedExecutor { let should_run_span = info_span!("check_conditions", name = &*name).entered(); let mut should_run = !self.completed_systems.contains(system_index); - for set_idx in schedule.sets_of_systems[system_index].ones() { + for set_idx in schedule.sets_with_conditions_of_systems[system_index].ones() { if self.evaluated_sets.contains(set_idx) { continue; } @@ -62,7 +62,7 @@ impl SystemExecutor for SingleThreadedExecutor { if !set_conditions_met { self.completed_systems - .union_with(&schedule.systems_in_sets[set_idx]); + .union_with(&schedule.systems_in_sets_with_conditions[set_idx]); } should_run &= set_conditions_met; diff --git a/crates/bevy_ecs/src/schedule_v3/graph_utils.rs b/crates/bevy_ecs/src/schedule_v3/graph_utils.rs index 27092acda9333..9a29d504de730 100644 --- a/crates/bevy_ecs/src/schedule_v3/graph_utils.rs +++ b/crates/bevy_ecs/src/schedule_v3/graph_utils.rs @@ -72,16 +72,48 @@ pub(crate) struct GraphInfo { pub(crate) sets: Vec, pub(crate) dependencies: Vec, pub(crate) ambiguous_with: Ambiguity, - pub(crate) add_default_set: bool, + pub(crate) add_default_base_set: bool, + pub(crate) base_set: Option, } impl Default for GraphInfo { fn default() -> Self { GraphInfo { sets: Vec::new(), + base_set: None, dependencies: Vec::new(), ambiguous_with: Ambiguity::default(), - add_default_set: true, + add_default_base_set: true, + } + } +} + +impl GraphInfo { + pub(crate) fn system() -> GraphInfo { + GraphInfo { + // systems get the default base set automatically + add_default_base_set: true, + ..Default::default() + } + } + + pub(crate) fn system_set() -> GraphInfo { + GraphInfo { + // sets do not get the default base set automatically + add_default_base_set: false, + ..Default::default() + } + } + + #[track_caller] + pub(crate) fn set_base_set(&mut self, set: BoxedSystemSet) { + if let Some(current) = &self.base_set { + panic!( + "Cannot set the base set because base set {:?} has already been configured.", + current + ); + } else { + self.base_set = Some(set); } } } diff --git a/crates/bevy_ecs/src/schedule_v3/mod.rs b/crates/bevy_ecs/src/schedule_v3/mod.rs index 185dae4d753bd..05408305c125e 100644 --- a/crates/bevy_ecs/src/schedule_v3/mod.rs +++ b/crates/bevy_ecs/src/schedule_v3/mod.rs @@ -644,4 +644,173 @@ mod tests { assert!(matches!(result, Err(ScheduleBuildError::Ambiguity))); } } + + mod base_sets { + use super::*; + + #[derive(SystemSet, Hash, Debug, Eq, PartialEq, Clone)] + #[system_set(base)] + enum Base { + A, + B, + } + + #[derive(SystemSet, Hash, Debug, Eq, PartialEq, Clone)] + enum Normal { + X, + Y, + Z, + } + + #[test] + #[should_panic] + fn disallow_adding_base_sets_to_system_with_in_set() { + let mut schedule = Schedule::new(); + schedule.add_system(named_system.in_set(Base::A)); + } + + #[test] + #[should_panic] + fn disallow_adding_sets_to_system_with_in_base_set() { + let mut schedule = Schedule::new(); + schedule.add_system(named_system.in_base_set(Normal::X)); + } + + #[test] + #[should_panic] + fn disallow_adding_base_sets_to_systems_with_in_set() { + let mut schedule = Schedule::new(); + schedule.add_systems((named_system, named_system).in_set(Base::A)); + } + + #[test] + #[should_panic] + fn disallow_adding_sets_to_systems_with_in_base_set() { + let mut schedule = Schedule::new(); + schedule.add_systems((named_system, named_system).in_base_set(Normal::X)); + } + + #[test] + #[should_panic] + fn disallow_adding_base_sets_to_set_with_in_set() { + let mut schedule = Schedule::new(); + schedule.configure_set(Normal::Y.in_set(Base::A)); + } + + #[test] + #[should_panic] + fn disallow_adding_sets_to_set_with_in_base_set() { + let mut schedule = Schedule::new(); + schedule.configure_set(Normal::Y.in_base_set(Normal::X)); + } + + #[test] + #[should_panic] + fn disallow_adding_base_sets_to_sets_with_in_set() { + let mut schedule = Schedule::new(); + schedule.configure_sets((Normal::X, Normal::Y).in_set(Base::A)); + } + + #[test] + #[should_panic] + fn disallow_adding_sets_to_sets_with_in_base_set() { + let mut schedule = Schedule::new(); + schedule.configure_sets((Normal::X, Normal::Y).in_base_set(Normal::Z)); + } + + #[test] + #[should_panic] + fn disallow_adding_base_sets_to_sets() { + let mut schedule = Schedule::new(); + schedule.configure_set(Base::A.in_set(Normal::X)); + } + + #[test] + #[should_panic] + fn disallow_adding_base_sets_to_base_sets() { + let mut schedule = Schedule::new(); + schedule.configure_set(Base::A.in_base_set(Base::B)); + } + + #[test] + #[should_panic] + fn disallow_adding_set_to_multiple_base_sets() { + let mut schedule = Schedule::new(); + schedule.configure_set(Normal::X.in_base_set(Base::A).in_base_set(Base::B)); + } + + #[test] + #[should_panic] + fn disallow_adding_sets_to_multiple_base_sets() { + let mut schedule = Schedule::new(); + schedule.configure_sets( + (Normal::X, Normal::Y) + .in_base_set(Base::A) + .in_base_set(Base::B), + ); + } + + #[test] + #[should_panic] + fn disallow_adding_system_to_multiple_base_sets() { + let mut schedule = Schedule::new(); + schedule.add_system(named_system.in_base_set(Base::A).in_base_set(Base::B)); + } + + #[test] + #[should_panic] + fn disallow_adding_systems_to_multiple_base_sets() { + let mut schedule = Schedule::new(); + schedule.add_systems( + (make_function_system(0), make_function_system(1)) + .in_base_set(Base::A) + .in_base_set(Base::B), + ); + } + + #[test] + fn disallow_multiple_base_sets() { + let mut world = World::new(); + + let mut schedule = Schedule::new(); + schedule + .configure_set(Normal::X.in_base_set(Base::A)) + .configure_set(Normal::Y.in_base_set(Base::B)) + .add_system(named_system.in_set(Normal::X).in_set(Normal::Y)); + + let result = schedule.initialize(&mut world); + assert!(matches!( + result, + Err(ScheduleBuildError::SystemInMultipleBaseSets { .. }) + )); + + let mut schedule = Schedule::new(); + schedule + .configure_set(Normal::X.in_base_set(Base::A)) + .configure_set(Normal::Y.in_base_set(Base::B).in_set(Normal::X)); + + let result = schedule.initialize(&mut world); + assert!(matches!( + result, + Err(ScheduleBuildError::SetInMultipleBaseSets { .. }) + )); + } + + #[test] + fn default_base_set_ordering() { + let mut world = World::default(); + let mut schedule = Schedule::default(); + + world.init_resource::(); + + schedule + .set_default_base_set(Base::A) + .configure_set(Base::A.before(Base::B)) + .add_system(make_function_system(0).in_base_set(Base::B)) + .add_system(make_function_system(1)); + schedule.run(&mut world); + + assert_eq!(world.resource::().0, vec![1, 0]); + } + } } diff --git a/crates/bevy_ecs/src/schedule_v3/schedule.rs b/crates/bevy_ecs/src/schedule_v3/schedule.rs index 6488d357a61fb..e8e0ba5c994d0 100644 --- a/crates/bevy_ecs/src/schedule_v3/schedule.rs +++ b/crates/bevy_ecs/src/schedule_v3/schedule.rs @@ -127,6 +127,12 @@ impl Schedule { } } + pub fn set_default_base_set(&mut self, default_base_set: impl SystemSet) -> &mut Self { + self.graph + .set_default_base_set(Some(Box::new(default_base_set))); + self + } + /// Add a system to the schedule. pub fn add_system

(&mut self, system: impl IntoSystemConfig

) -> &mut Self { self.graph.add_system(system); @@ -264,14 +270,25 @@ impl Dag { } } +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +enum BaseSetMembership { + Uncalculated, + None, + Some(NodeId), +} + /// A [`SystemSet`] with metadata, stored in a [`ScheduleGraph`]. struct SystemSetNode { inner: BoxedSystemSet, + base_set_membership: BaseSetMembership, } impl SystemSetNode { pub fn new(set: BoxedSystemSet) -> Self { - Self { inner: set } + Self { + inner: set, + base_set_membership: BaseSetMembership::Uncalculated, + } } pub fn name(&self) -> String { @@ -283,15 +300,43 @@ impl SystemSetNode { } } +/// A [`BoxedSystem`] with metadata, stored in a [`ScheduleGraph`]. +struct SystemNode { + inner: Option, + base_set_membership: BaseSetMembership, +} + +impl SystemNode { + pub fn new(system: BoxedSystem) -> Self { + Self { + inner: Some(system), + base_set_membership: BaseSetMembership::Uncalculated, + } + } + + pub fn get(&self) -> Option<&BoxedSystem> { + self.inner.as_ref() + } + + pub fn get_mut(&mut self) -> Option<&mut BoxedSystem> { + self.inner.as_mut() + } + + pub fn name(&self) -> String { + format!("{:?}", &self.inner) + } +} + /// Metadata for a [`Schedule`]. #[derive(Default)] struct ScheduleGraph { - systems: Vec>, + systems: Vec, system_conditions: Vec>>, system_sets: Vec, system_set_conditions: Vec>>, system_set_ids: HashMap, uninit: Vec<(NodeId, usize)>, + maybe_default_base_set: Vec, hierarchy: Dag, dependency: Dag, dependency_flattened: Dag, @@ -300,6 +345,7 @@ struct ScheduleGraph { ambiguous_with_all: HashSet, changed: bool, settings: ScheduleBuildSettings, + default_base_set: Option, } impl ScheduleGraph { @@ -310,6 +356,7 @@ impl ScheduleGraph { system_sets: Vec::new(), system_set_conditions: Vec::new(), system_set_ids: HashMap::new(), + maybe_default_base_set: Vec::new(), uninit: Vec::new(), hierarchy: Dag::new(), dependency: Dag::new(), @@ -319,6 +366,7 @@ impl ScheduleGraph { ambiguous_with_all: HashSet::new(), changed: false, settings: default(), + default_base_set: None, } } @@ -357,11 +405,11 @@ impl ScheduleGraph { let id = NodeId::System(self.systems.len()); // graph updates are immediate - self.update_graphs(id, graph_info)?; + self.update_graphs(id, graph_info, false)?; // system init has to be deferred (need `&mut World`) self.uninit.push((id, 0)); - self.systems.push(Some(system)); + self.systems.push(SystemNode::new(system)); self.system_conditions.push(Some(conditions)); Ok(id) @@ -405,7 +453,7 @@ impl ScheduleGraph { }; // graph updates are immediate - self.update_graphs(id, graph_info)?; + self.update_graphs(id, graph_info, set.is_base())?; // system init has to be deferred (need `&mut World`) let system_set_conditions = @@ -424,23 +472,37 @@ impl ScheduleGraph { id } + fn check_set( + &mut self, + id: &NodeId, + set: &Box, + ) -> Result<(), ScheduleBuildError> { + match self.system_set_ids.get(set) { + Some(set_id) => { + if id == set_id { + let string = format!("{:?}", &set); + return Err(ScheduleBuildError::HierarchyLoop(string)); + } + } + None => { + self.add_set(set.dyn_clone()); + } + } + + Ok(()) + } + fn check_sets( &mut self, id: &NodeId, graph_info: &GraphInfo, ) -> Result<(), ScheduleBuildError> { for set in &graph_info.sets { - match self.system_set_ids.get(set) { - Some(set_id) => { - if id == set_id { - let string = format!("{:?}", &set); - return Err(ScheduleBuildError::HierarchyLoop(string)); - } - } - None => { - self.add_set(set.dyn_clone()); - } - } + self.check_set(id, set)?; + } + + if let Some(base_set) = &graph_info.base_set { + self.check_set(id, base_set)?; } Ok(()) @@ -480,6 +542,7 @@ impl ScheduleGraph { &mut self, id: NodeId, graph_info: GraphInfo, + is_base_set: bool, ) -> Result<(), ScheduleBuildError> { self.check_sets(&id, &graph_info)?; self.check_edges(&id, &graph_info)?; @@ -489,6 +552,8 @@ impl ScheduleGraph { sets, dependencies, ambiguous_with, + base_set, + add_default_base_set, .. } = graph_info; @@ -502,6 +567,34 @@ impl ScheduleGraph { self.dependency.graph.add_node(set); } + // If the current node is not a base set, set the base set if it was configured + if !is_base_set { + if let Some(base_set) = base_set { + let set_id = self.system_set_ids[&base_set]; + self.hierarchy.graph.add_edge(set_id, id, ()); + } else if let Some(default_base_set) = &self.default_base_set { + if add_default_base_set { + match id { + NodeId::System(_) => { + // Queue the default base set. We queue systems instead of adding directly to allow + // sets to define base sets, which will override the default inheritance behavior + self.maybe_default_base_set.push(id); + } + NodeId::Set(_) => { + // Sets should be added automatically because developers explicitly called + // in_default_base_set() + let set_id = self.system_set_ids[default_base_set]; + self.hierarchy.graph.add_edge(set_id, id, ()); + } + } + } + } + } + + if !self.dependency.graph.contains_node(id) { + self.dependency.graph.add_node(id); + } + for (kind, set) in dependencies .into_iter() .map(|Dependency { kind, set }| (kind, self.system_set_ids[&set])) @@ -538,7 +631,7 @@ impl ScheduleGraph { for (id, i) in self.uninit.drain(..) { match id { NodeId::System(index) => { - self.systems[index].as_mut().unwrap().initialize(world); + self.systems[index].get_mut().unwrap().initialize(world); if let Some(v) = self.system_conditions[index].as_mut() { for condition in v.iter_mut() { condition.initialize(world); @@ -556,12 +649,141 @@ impl ScheduleGraph { } } + /// Calculates the base set for each node and caches the results on the node + fn calculate_base_sets_and_detect_cycles(&mut self) -> Result<(), ScheduleBuildError> { + let set_ids = (0..self.system_sets.len()).map(|i| NodeId::Set(i)); + let system_ids = (0..self.systems.len()).map(|i| NodeId::System(i)); + let mut visited_sets = vec![false; self.system_sets.len()]; + // reset base set membership, as this can change when the schedule updates + for system in self.systems.iter_mut() { + system.base_set_membership = BaseSetMembership::Uncalculated; + } + for system_set in self.system_sets.iter_mut() { + system_set.base_set_membership = BaseSetMembership::Uncalculated; + } + for node_id in set_ids.chain(system_ids) { + Self::calculate_base_set( + &self.hierarchy, + &mut self.system_sets, + &mut self.systems, + &mut visited_sets, + node_id, + )?; + } + Ok(()) + } + + fn calculate_base_set( + hierarchy: &Dag, + system_sets: &mut [SystemSetNode], + systems: &mut [SystemNode], + visited_sets: &mut [bool], + node_id: NodeId, + ) -> Result, ScheduleBuildError> { + let base_set_membership = match node_id { + // systems only have + NodeId::System(_) => BaseSetMembership::Uncalculated, + NodeId::Set(index) => { + let set_node = &mut system_sets[index]; + if set_node.inner.is_base() { + set_node.base_set_membership = BaseSetMembership::Some(node_id); + } + set_node.base_set_membership + } + }; + let base_set = match base_set_membership { + BaseSetMembership::None => None, + BaseSetMembership::Some(node_id) => Some(node_id), + BaseSetMembership::Uncalculated => { + let mut base_set: Option = None; + if let NodeId::Set(index) = node_id { + if visited_sets[index] { + return Err(ScheduleBuildError::HierarchyCycle); + } + visited_sets[index] = true; + } + for neighbor in hierarchy + .graph + .neighbors_directed(node_id, Direction::Incoming) + { + if let Some(calculated_base_set) = Self::calculate_base_set( + hierarchy, + system_sets, + systems, + visited_sets, + neighbor, + )? { + if let Some(first_set) = base_set { + return Err(match node_id { + NodeId::System(index) => { + ScheduleBuildError::SystemInMultipleBaseSets { + system: systems[index].name(), + first_set: system_sets[first_set.index()].name(), + second_set: system_sets[calculated_base_set.index()].name(), + } + } + NodeId::Set(index) => ScheduleBuildError::SetInMultipleBaseSets { + set: system_sets[index].name(), + first_set: system_sets[first_set.index()].name(), + second_set: system_sets[calculated_base_set.index()].name(), + }, + }); + } + base_set = Some(calculated_base_set); + } + } + + match node_id { + NodeId::System(index) => { + systems[index].base_set_membership = if let Some(base_set) = base_set { + BaseSetMembership::Some(base_set) + } else { + BaseSetMembership::None + }; + } + NodeId::Set(index) => { + system_sets[index].base_set_membership = if let Some(base_set) = base_set { + BaseSetMembership::Some(base_set) + } else { + BaseSetMembership::None + }; + } + } + base_set + } + }; + Ok(base_set) + } + fn build_schedule( &mut self, components: &Components, ) -> Result { + self.calculate_base_sets_and_detect_cycles()?; + + // Add missing base set membership to systems that defaulted to using the + // default base set and weren't added to a set that belongs to a base set. + if let Some(default_base_set) = &self.default_base_set { + let default_set_id = self.system_set_ids[default_base_set]; + for system_id in std::mem::take(&mut self.maybe_default_base_set) { + let system_node = &mut self.systems[system_id.index()]; + if system_node.base_set_membership == BaseSetMembership::None { + self.hierarchy.graph.add_edge(default_set_id, system_id, ()); + system_node.base_set_membership = BaseSetMembership::Some(default_set_id); + } + + debug_assert_ne!( + system_node.base_set_membership, + BaseSetMembership::Uncalculated, + "base set membership should have been calculated" + ); + } + } + // check hierarchy for cycles let hier_scc = tarjan_scc(&self.hierarchy.graph); + // PERF: in theory we can skip this contains_cycles because we've already detected cycles + // using calculate_base_sets_and_detect_cycles if self.contains_cycles(&hier_scc) { self.report_cycles(&hier_scc); return Err(ScheduleBuildError::HierarchyCycle); @@ -767,8 +989,8 @@ impl ScheduleGraph { continue; } - let system_a = self.systems[a.index()].as_ref().unwrap(); - let system_b = self.systems[b.index()].as_ref().unwrap(); + let system_a = self.systems[a.index()].get().unwrap(); + let system_b = self.systems[b.index()].get().unwrap(); if system_a.is_exclusive() || system_b.is_exclusive() { conflicting_systems.push((a, b, Vec::new())); } else { @@ -808,7 +1030,7 @@ impl ScheduleGraph { .filter(|&(_i, id)| id.is_system()) .collect::>(); - let (hg_set_idxs, hg_set_ids): (Vec<_>, Vec<_>) = self + let (hg_set_with_conditions_idxs, hg_set_ids): (Vec<_>, Vec<_>) = self .hierarchy .topsort .iter() @@ -826,7 +1048,7 @@ impl ScheduleGraph { .unzip(); let sys_count = self.systems.len(); - let set_count = hg_set_ids.len(); + let set_with_conditions_count = hg_set_ids.len(); let node_count = self.systems.len() + self.system_sets.len(); // get the number of dependencies and the immediate dependents of each system @@ -853,9 +1075,10 @@ impl ScheduleGraph { // get the rows and columns of the hierarchy graph's reachability matrix // (needed to we can evaluate conditions in the correct order) - let mut systems_in_sets = vec![FixedBitSet::with_capacity(sys_count); set_count]; - for (i, &row) in hg_set_idxs.iter().enumerate() { - let bitset = &mut systems_in_sets[i]; + let mut systems_in_sets_with_conditions = + vec![FixedBitSet::with_capacity(sys_count); set_with_conditions_count]; + for (i, &row) in hg_set_with_conditions_idxs.iter().enumerate() { + let bitset = &mut systems_in_sets_with_conditions[i]; for &(col, sys_id) in &hg_systems { let idx = dg_system_idx_map[&sys_id]; let is_descendant = hier_results.reachable[index(row, col, node_count)]; @@ -863,11 +1086,12 @@ impl ScheduleGraph { } } - let mut sets_of_systems = vec![FixedBitSet::with_capacity(set_count); sys_count]; + let mut sets_with_conditions_of_systems = + vec![FixedBitSet::with_capacity(set_with_conditions_count); sys_count]; for &(col, sys_id) in &hg_systems { let i = dg_system_idx_map[&sys_id]; - let bitset = &mut sets_of_systems[i]; - for (idx, &row) in hg_set_idxs + let bitset = &mut sets_with_conditions_of_systems[i]; + for (idx, &row) in hg_set_with_conditions_idxs .iter() .enumerate() .take_while(|&(_idx, &row)| row < col) @@ -880,13 +1104,13 @@ impl ScheduleGraph { Ok(SystemSchedule { systems: Vec::with_capacity(sys_count), system_conditions: Vec::with_capacity(sys_count), - set_conditions: Vec::with_capacity(set_count), + set_conditions: Vec::with_capacity(set_with_conditions_count), system_ids: dg_system_ids, set_ids: hg_set_ids, system_dependencies, system_dependents, - sets_of_systems, - systems_in_sets, + sets_with_conditions_of_systems, + systems_in_sets_with_conditions, }) } @@ -906,7 +1130,7 @@ impl ScheduleGraph { .zip(schedule.systems.drain(..)) .zip(schedule.system_conditions.drain(..)) { - self.systems[id.index()] = Some(system); + self.systems[id.index()].inner = Some(system); self.system_conditions[id.index()] = Some(conditions); } @@ -922,7 +1146,7 @@ impl ScheduleGraph { // move systems into new schedule for &id in &schedule.system_ids { - let system = self.systems[id.index()].take().unwrap(); + let system = self.systems[id.index()].inner.take().unwrap(); let conditions = self.system_conditions[id.index()].take().unwrap(); schedule.systems.push(system); schedule.system_conditions.push(conditions); @@ -935,17 +1159,24 @@ impl ScheduleGraph { Ok(()) } + + fn set_default_base_set(&mut self, set: Option) { + if let Some(set) = set { + self.default_base_set = Some(set.dyn_clone()); + if self.system_set_ids.get(&set).is_none() { + self.add_set(set); + } + } else { + self.default_base_set = None; + } + } } // methods for reporting errors impl ScheduleGraph { fn get_node_name(&self, id: &NodeId) -> String { match id { - NodeId::System(_) => self.systems[id.index()] - .as_ref() - .unwrap() - .name() - .to_string(), + NodeId::System(_) => self.systems[id.index()].get().unwrap().name().to_string(), NodeId::Set(_) => self.system_sets[id.index()].name(), } } @@ -1100,6 +1331,20 @@ pub enum ScheduleBuildError { /// Tried to run a schedule before all of its systems have been initialized. #[error("Systems in schedule have not been initialized.")] Uninitialized, + /// Tried to add a system to multiple base sets. + #[error("System `{system:?}` is in the base sets {first_set:?} and {second_set:?}, but systems can only belong to one base set.")] + SystemInMultipleBaseSets { + system: String, + first_set: String, + second_set: String, + }, + /// Tried to add a set to multiple base sets. + #[error("Set `{set:?}` is in the base sets {first_set:?} and {second_set:?}, but sets can only belong to one base set.")] + SetInMultipleBaseSets { + set: String, + first_set: String, + second_set: String, + }, } /// Specifies how schedule construction should respond to detecting a certain kind of issue. diff --git a/crates/bevy_ecs/src/schedule_v3/set.rs b/crates/bevy_ecs/src/schedule_v3/set.rs index 096b66e25a662..c5f62f9259d00 100644 --- a/crates/bevy_ecs/src/schedule_v3/set.rs +++ b/crates/bevy_ecs/src/schedule_v3/set.rs @@ -23,6 +23,16 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { false } + /// Returns `true` if this set is a "base system set". Systems + /// can only belong to one base set at a time. Systems and Sets + /// can only be added to base sets using specialized `in_base_set` + /// APIs. This enables "mutually exclusive" behaviors. It also + /// enables schedules to have a "default base set", which can be used + /// to apply default configuration to systems. + fn is_base(&self) -> bool { + false + } + /// Creates a boxed clone of the label corresponding to this system set. fn dyn_clone(&self) -> Box; } diff --git a/crates/bevy_gilrs/src/lib.rs b/crates/bevy_gilrs/src/lib.rs index 958dec3809fd4..34b560be1f865 100644 --- a/crates/bevy_gilrs/src/lib.rs +++ b/crates/bevy_gilrs/src/lib.rs @@ -24,7 +24,7 @@ impl Plugin for GilrsPlugin { .add_system( gilrs_event_system .before(InputSystem) - .in_set(CoreSet::PreUpdate), + .in_base_set(CoreSet::PreUpdate), ); } Err(err) => error!("Failed to start Gilrs. {}", err), diff --git a/crates/bevy_hierarchy/src/valid_parent_check_plugin.rs b/crates/bevy_hierarchy/src/valid_parent_check_plugin.rs index ddd07f19fbec1..557450f9af39d 100644 --- a/crates/bevy_hierarchy/src/valid_parent_check_plugin.rs +++ b/crates/bevy_hierarchy/src/valid_parent_check_plugin.rs @@ -99,7 +99,7 @@ impl Plugin for ValidParentCheckPlugin { app.init_resource::>().add_system( check_hierarchy_component_has_valid_parent:: .run_if(resource_equals(ReportHierarchyIssue::::new(true))) - .in_set(CoreSet::Last), + .in_base_set(CoreSet::Last), ); } } diff --git a/crates/bevy_input/src/lib.rs b/crates/bevy_input/src/lib.rs index 6cbb4f262ea6b..cbf4bf0e7c660 100644 --- a/crates/bevy_input/src/lib.rs +++ b/crates/bevy_input/src/lib.rs @@ -51,7 +51,7 @@ pub struct InputSystem; impl Plugin for InputPlugin { fn build(&self, app: &mut App) { - app.configure_set(InputSystem.in_set(CoreSet::PreUpdate)) + app.configure_set(InputSystem.in_base_set(CoreSet::PreUpdate)) // keyboard .add_event::() .init_resource::>() diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 0f38a27d8403f..3fd275ceb9f48 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -138,40 +138,6 @@ pub fn derive_boxed_label(input: syn::DeriveInput, trait_path: &syn::Path) -> To .into() } -/// Derive a label trait -/// -/// # Args -/// -/// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait -/// - `trait_path`: The path [`syn::Path`] to the label trait -pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { - let ident = input.ident; - let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause { - where_token: Default::default(), - predicates: Default::default(), - }); - where_clause.predicates.push( - syn::parse2(quote! { - Self: 'static + Send + Sync + Clone + Eq + ::std::fmt::Debug + ::std::hash::Hash - }) - .unwrap(), - ); - - (quote! { - impl #impl_generics #trait_path for #ident #ty_generics #where_clause { - fn is_system_type(&self) -> bool { - false - } - - fn dyn_clone(&self) -> std::boxed::Box { - std::boxed::Box::new(std::clone::Clone::clone(self)) - } - } - }) - .into() -} - /// Derive a label trait /// /// # Args diff --git a/crates/bevy_pbr/src/lib.rs b/crates/bevy_pbr/src/lib.rs index 8f22b7b2dbac4..2b47ae1e621a7 100644 --- a/crates/bevy_pbr/src/lib.rs +++ b/crates/bevy_pbr/src/lib.rs @@ -181,14 +181,10 @@ impl Plugin for PbrPlugin { SimulationLightSystems::UpdateDirectionalLightCascades, SimulationLightSystems::UpdateLightFrusta, ) - .in_set(CoreSet::PostUpdate), + .in_base_set(CoreSet::PostUpdate), ) .add_plugin(FogPlugin) - .add_system( - // NOTE: Clusters need to have been added before update_clusters is run so - // add as an exclusive system - add_clusters.in_set(SimulationLightSystems::AddClusters), - ) + .add_system(add_clusters.in_set(SimulationLightSystems::AddClusters)) .add_system(apply_system_buffers.in_set(SimulationLightSystems::AddClustersFlush)) .add_system( assign_lights_to_clusters diff --git a/crates/bevy_render/src/camera/projection.rs b/crates/bevy_render/src/camera/projection.rs index 00df1c0b9a190..4228db69b8f03 100644 --- a/crates/bevy_render/src/camera/projection.rs +++ b/crates/bevy_render/src/camera/projection.rs @@ -31,7 +31,7 @@ impl Plugin for CameraPro .edit_schedule(CoreSchedule::Startup, |schedule| { schedule.configure_set(CameraUpdateSystem.in_set(StartupSet::PostStartup)); }) - .configure_set(CameraUpdateSystem.in_set(CoreSet::PostUpdate)) + .configure_set(CameraUpdateSystem.in_base_set(CoreSet::PostUpdate)) .add_startup_system( crate::camera::camera_system:: .in_set(CameraUpdateSystem) diff --git a/crates/bevy_render/src/view/visibility/mod.rs b/crates/bevy_render/src/view/visibility/mod.rs index 48569b0010d14..b4cccb1319c1c 100644 --- a/crates/bevy_render/src/view/visibility/mod.rs +++ b/crates/bevy_render/src/view/visibility/mod.rs @@ -210,19 +210,19 @@ impl Plugin for VisibilityPlugin { fn build(&self, app: &mut bevy_app::App) { use VisibilitySystems::*; - app.configure_set(CalculateBounds.in_set(CoreSet::PostUpdate)) + app.configure_set(CalculateBounds.in_base_set(CoreSet::PostUpdate)) // We add an AABB component in CaclulateBounds, which must be ready on the same frame. .add_system(apply_system_buffers.in_set(CalculateBoundsFlush)) .configure_set( CalculateBoundsFlush .after(CalculateBounds) - .in_set(CoreSet::PostUpdate), + .in_base_set(CoreSet::PostUpdate), ) - .configure_set(UpdateOrthographicFrusta.in_set(CoreSet::PostUpdate)) - .configure_set(UpdatePerspectiveFrusta.in_set(CoreSet::PostUpdate)) - .configure_set(UpdateProjectionFrusta.in_set(CoreSet::PostUpdate)) - .configure_set(CheckVisibility.in_set(CoreSet::PostUpdate)) - .configure_set(VisibilityPropagate.in_set(CoreSet::PostUpdate)) + .configure_set(UpdateOrthographicFrusta.in_base_set(CoreSet::PostUpdate)) + .configure_set(UpdatePerspectiveFrusta.in_base_set(CoreSet::PostUpdate)) + .configure_set(UpdateProjectionFrusta.in_base_set(CoreSet::PostUpdate)) + .configure_set(CheckVisibility.in_base_set(CoreSet::PostUpdate)) + .configure_set(VisibilityPropagate.in_base_set(CoreSet::PostUpdate)) .add_system(calculate_bounds.in_set(CalculateBounds)) .add_system( update_frusta:: @@ -247,7 +247,7 @@ impl Plugin for VisibilityPlugin { ) .add_system( update_frusta:: - .in_set(CoreSet::PostUpdate) + .in_base_set(CoreSet::PostUpdate) .after(camera_system::) .after(TransformSystem::TransformPropagate), ) diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs index 8256b2fc77545..2d7ef33951b30 100644 --- a/crates/bevy_scene/src/lib.rs +++ b/crates/bevy_scene/src/lib.rs @@ -36,9 +36,9 @@ impl Plugin for ScenePlugin { .add_asset::() .init_asset_loader::() .init_resource::() - .add_system(scene_spawner_system.in_set(CoreSet::Update)) + .add_system(scene_spawner_system) // Systems `*_bundle_spawner` must run before `scene_spawner_system` - .add_system(scene_spawner.in_set(CoreSet::PreUpdate)); + .add_system(scene_spawner.in_base_set(CoreSet::PreUpdate)); } } diff --git a/crates/bevy_text/src/lib.rs b/crates/bevy_text/src/lib.rs index f5ec217440a41..85757cd7af7f8 100644 --- a/crates/bevy_text/src/lib.rs +++ b/crates/bevy_text/src/lib.rs @@ -82,7 +82,7 @@ impl Plugin for TextPlugin { .insert_resource(TextPipeline::default()) .add_system( update_text2d_layout - .in_set(CoreSet::PostUpdate) + .in_base_set(CoreSet::PostUpdate) .after(ModifiesWindows) // Potential conflict: `Assets` // In practice, they run independently since `bevy_render::camera_update_system` diff --git a/crates/bevy_time/src/lib.rs b/crates/bevy_time/src/lib.rs index 4ed03e3225ce0..4481ccac918cc 100644 --- a/crates/bevy_time/src/lib.rs +++ b/crates/bevy_time/src/lib.rs @@ -39,9 +39,9 @@ impl Plugin for TimePlugin { .register_type::