From 227b87ed4167721e8bee1df3707b68f04e550c1e Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Tue, 3 Dec 2024 21:25:43 -0600 Subject: [PATCH 01/10] turn apply_deferred into a ZST system --- crates/bevy_ecs/src/schedule/executor/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 8b549a15ea6d3..718ff114aab70 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -3,9 +3,11 @@ mod multi_threaded; mod simple; mod single_threaded; -use alloc::{borrow::Cow, vec, vec::Vec}; +use alloc::borrow::Cow; use core::any::TypeId; +use alloc::{vec, vec::Vec}; + pub use self::{simple::SimpleExecutor, single_threaded::SingleThreadedExecutor}; #[cfg(feature = "std")] From 4983987c479e9085e829adc0e280a03150f23b0c Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Tue, 3 Dec 2024 02:44:22 -0600 Subject: [PATCH 02/10] unify exclusive system params with normal system params --- crates/bevy_ecs/macros/src/lib.rs | 130 +++++++ crates/bevy_ecs/src/schedule/set.rs | 19 +- .../src/system/exclusive_function_system.rs | 342 ------------------ .../src/system/exclusive_system_param.rs | 169 --------- crates/bevy_ecs/src/system/function_system.rs | 2 +- crates/bevy_ecs/src/system/mod.rs | 4 - crates/bevy_ecs/src/system/system_name.rs | 15 +- crates/bevy_ecs/src/system/system_param.rs | 128 ++++++- crates/bevy_ecs/src/world/identifier.rs | 15 +- 9 files changed, 259 insertions(+), 565 deletions(-) delete mode 100644 crates/bevy_ecs/src/system/exclusive_function_system.rs delete mode 100644 crates/bevy_ecs/src/system/exclusive_system_param.rs diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index e4cca41a4d7aa..84e853483c11f 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -283,6 +283,136 @@ pub fn derive_visit_entities(input: TokenStream) -> TokenStream { }) } +fn get_idents(fmt_string: fn(usize) -> String, count: usize) -> Vec { + (0..count) + .map(|i| Ident::new(&fmt_string(i), Span::call_site())) + .collect::>() +} + +#[proc_macro] +pub fn impl_param_set(_input: TokenStream) -> TokenStream { + let mut tokens = TokenStream::new(); + let max_params = 8; + let params = get_idents(|i| format!("P{i}"), max_params); + let metas = get_idents(|i| format!("m{i}"), max_params); + let mut param_fn_muts = Vec::new(); + for (i, param) in params.iter().enumerate() { + let fn_name = Ident::new(&format!("p{i}"), Span::call_site()); + let index = Index::from(i); + let ordinal = match i { + 1 => "1st".to_owned(), + 2 => "2nd".to_owned(), + 3 => "3rd".to_owned(), + x => format!("{x}th"), + }; + let comment = + format!("Gets exclusive access to the {ordinal} parameter in this [`ParamSet`]."); + param_fn_muts.push(quote! { + #[doc = #comment] + /// No other parameters may be accessed while this one is active. + pub fn #fn_name<'a>(&'a mut self) -> SystemParamItem<'a, 'a, #param> { + // SAFETY: systems run without conflicts with other systems. + // Conflicting params in ParamSet are not accessible at the same time + // ParamSets are guaranteed to not conflict with other SystemParams + unsafe { + #param::get_param(&mut self.param_states.#index, &self.system_meta, self.world, self.change_tick) + } + } + }); + } + + for param_count in 1..=max_params { + let param = ¶ms[0..param_count]; + let meta = &metas[0..param_count]; + let param_fn_mut = ¶m_fn_muts[0..param_count]; + tokens.extend(TokenStream::from(quote! { + // SAFETY: All parameters are constrained to ReadOnlySystemParam, so World is only read + unsafe impl<'w, 's, #(#param,)*> ReadOnlySystemParam for ParamSet<'w, 's, (#(#param,)*)> + where #(#param: ReadOnlySystemParam,)* + { } + + // SAFETY: Relevant parameter ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any ParamState conflicts + // with any prior access, a panic will occur. + unsafe impl<'_w, '_s, #(#param: SystemParam,)*> SystemParam for ParamSet<'_w, '_s, (#(#param,)*)> + { + type State = (#(#param::State,)*); + type Item<'w, 's> = ParamSet<'w, 's, (#(#param,)*)>; + + // Note: We allow non snake case so the compiler don't complain about the creation of non_snake_case variables + #[allow(non_snake_case)] + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + #( + // Pretend to add each param to the system alone, see if it conflicts + let mut #meta = system_meta.clone(); + #meta.component_access_set.clear(); + #meta.archetype_component_access.clear(); + #param::init_state(world, &mut #meta); + // The variable is being defined with non_snake_case here + let #param = #param::init_state(world, &mut system_meta.clone()); + )* + // Make the ParamSet non-send if any of its parameters are non-send. + if false #(|| !#meta.is_send())* { + system_meta.set_non_send(); + } + #( + system_meta + .component_access_set + .extend(#meta.component_access_set); + system_meta + .archetype_component_access + .extend(&#meta.archetype_component_access); + )* + (#(#param,)*) + } + + unsafe fn new_archetype(state: &mut Self::State, archetype: &Archetype, system_meta: &mut SystemMeta) { + // SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`. + unsafe { <(#(#param,)*) as SystemParam>::new_archetype(state, archetype, system_meta); } + } + + fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { + <(#(#param,)*) as SystemParam>::apply(state, system_meta, world); + } + + fn queue(state: &mut Self::State, system_meta: &SystemMeta, mut world: DeferredWorld) { + <(#(#param,)*) as SystemParam>::queue(state, system_meta, world.reborrow()); + } + + #[inline] + unsafe fn validate_param<'w, 's>( + state: &'s Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + ) -> bool { + <(#(#param,)*) as SystemParam>::validate_param(state, system_meta, world) + } + + #[inline] + unsafe fn get_param<'w, 's>( + state: &'s mut Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, + ) -> Self::Item<'w, 's> { + ParamSet { + param_states: state, + system_meta: system_meta.clone(), + world, + change_tick, + } + } + } + + impl<'w, 's, #(#param: SystemParam,)*> ParamSet<'w, 's, (#(#param,)*)> + { + #(#param_fn_mut)* + } + })); + } + + tokens +} + /// Implement `SystemParam` to use a struct as a parameter in a system #[proc_macro_derive(SystemParam, attributes(system_param))] pub fn derive_system_param(input: TokenStream) -> TokenStream { diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 010e9b5303848..1c498d1ee8b54 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -12,10 +12,7 @@ pub use bevy_ecs_macros::{ScheduleLabel, SystemSet}; use crate::{ define_label, intern::Interned, - system::{ - ExclusiveFunctionSystem, ExclusiveSystemParamFunction, FunctionSystem, - IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction, - }, + system::{FunctionSystem, IsFunctionSystem, SystemParamFunction}, }; define_label!( @@ -200,20 +197,6 @@ where } } -// exclusive systems -impl IntoSystemSet<(IsExclusiveFunctionSystem, Marker)> for F -where - Marker: 'static, - F: ExclusiveSystemParamFunction, -{ - type Set = SystemTypeSet>; - - #[inline] - fn into_system_set(self) -> Self::Set { - SystemTypeSet::>::new() - } -} - #[cfg(test)] mod tests { use crate::{ diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs deleted file mode 100644 index 2b1c081d51db0..0000000000000 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ /dev/null @@ -1,342 +0,0 @@ -use crate::{ - archetype::ArchetypeComponentId, - component::{ComponentId, Tick}, - query::Access, - schedule::{InternedSystemSet, SystemSet}, - system::{ - check_system_change_tick, ExclusiveSystemParam, ExclusiveSystemParamItem, IntoSystem, - System, SystemIn, SystemInput, SystemMeta, - }, - world::{unsafe_world_cell::UnsafeWorldCell, World}, -}; - -use alloc::{borrow::Cow, vec, vec::Vec}; -use core::marker::PhantomData; -use variadics_please::all_tuples; - -/// A function system that runs with exclusive [`World`] access. -/// -/// You get this by calling [`IntoSystem::into_system`] on a function that only accepts -/// [`ExclusiveSystemParam`]s. -/// -/// [`ExclusiveFunctionSystem`] must be `.initialized` before they can be run. -pub struct ExclusiveFunctionSystem -where - F: ExclusiveSystemParamFunction, -{ - func: F, - param_state: Option<::State>, - system_meta: SystemMeta, - // NOTE: PhantomData T> gives this safe Send/Sync impls - marker: PhantomData Marker>, -} - -impl ExclusiveFunctionSystem -where - F: ExclusiveSystemParamFunction, -{ - /// Return this system with a new name. - /// - /// Useful to give closure systems more readable and unique names for debugging and tracing. - pub fn with_name(mut self, new_name: impl Into>) -> Self { - self.system_meta.set_name(new_name.into()); - self - } -} - -/// A marker type used to distinguish exclusive function systems from regular function systems. -#[doc(hidden)] -pub struct IsExclusiveFunctionSystem; - -impl IntoSystem for F -where - Marker: 'static, - F: ExclusiveSystemParamFunction, -{ - type System = ExclusiveFunctionSystem; - fn into_system(func: Self) -> Self::System { - ExclusiveFunctionSystem { - func, - param_state: None, - system_meta: SystemMeta::new::(), - marker: PhantomData, - } - } -} - -const PARAM_MESSAGE: &str = "System's param_state was not found. Did you forget to initialize this system before running it?"; - -impl System for ExclusiveFunctionSystem -where - Marker: 'static, - F: ExclusiveSystemParamFunction, -{ - type In = F::In; - type Out = F::Out; - - #[inline] - fn name(&self) -> Cow<'static, str> { - self.system_meta.name.clone() - } - - #[inline] - fn component_access(&self) -> &Access { - self.system_meta.component_access_set.combined_access() - } - - #[inline] - fn archetype_component_access(&self) -> &Access { - &self.system_meta.archetype_component_access - } - - #[inline] - fn is_send(&self) -> bool { - // exclusive systems should have access to non-send resources - // the executor runs exclusive systems on the main thread, so this - // field reflects that constraint - false - } - - #[inline] - fn is_exclusive(&self) -> bool { - true - } - - #[inline] - fn has_deferred(&self) -> bool { - // exclusive systems have no deferred system params - false - } - - #[inline] - unsafe fn run_unsafe( - &mut self, - _input: SystemIn<'_, Self>, - _world: UnsafeWorldCell, - ) -> Self::Out { - panic!("Cannot run exclusive systems with a shared World reference"); - } - - fn run(&mut self, input: SystemIn<'_, Self>, world: &mut World) -> Self::Out { - world.last_change_tick_scope(self.system_meta.last_run, |world| { - #[cfg(feature = "trace")] - let _span_guard = self.system_meta.system_span.enter(); - - let params = F::Param::get_param( - self.param_state.as_mut().expect(PARAM_MESSAGE), - &self.system_meta, - ); - let out = self.func.run(world, input, params); - - world.flush(); - self.system_meta.last_run = world.increment_change_tick(); - - out - }) - } - - #[inline] - fn apply_deferred(&mut self, _world: &mut World) { - // "pure" exclusive systems do not have any buffers to apply. - // Systems made by piping a normal system with an exclusive system - // might have buffers to apply, but this is handled by `PipeSystem`. - } - - #[inline] - fn queue_deferred(&mut self, _world: crate::world::DeferredWorld) { - // "pure" exclusive systems do not have any buffers to apply. - // Systems made by piping a normal system with an exclusive system - // might have buffers to apply, but this is handled by `PipeSystem`. - } - - #[inline] - unsafe fn validate_param_unsafe(&mut self, _world: UnsafeWorldCell) -> bool { - // All exclusive system params are always available. - true - } - - #[inline] - fn initialize(&mut self, world: &mut World) { - self.system_meta.last_run = world.change_tick().relative_to(Tick::MAX); - self.param_state = Some(F::Param::init(world, &mut self.system_meta)); - } - - fn update_archetype_component_access(&mut self, _world: UnsafeWorldCell) {} - - #[inline] - fn check_change_tick(&mut self, change_tick: Tick) { - check_system_change_tick( - &mut self.system_meta.last_run, - change_tick, - self.system_meta.name.as_ref(), - ); - } - - fn default_system_sets(&self) -> Vec { - let set = crate::schedule::SystemTypeSet::::new(); - vec![set.intern()] - } - - fn get_last_run(&self) -> Tick { - self.system_meta.last_run - } - - fn set_last_run(&mut self, last_run: Tick) { - self.system_meta.last_run = last_run; - } -} - -/// A trait implemented for all exclusive system functions that can be used as [`System`]s. -/// -/// This trait can be useful for making your own systems which accept other systems, -/// sometimes called higher order systems. -#[diagnostic::on_unimplemented( - message = "`{Self}` is not an exclusive system", - label = "invalid system" -)] -pub trait ExclusiveSystemParamFunction: Send + Sync + 'static { - /// The input type to this system. See [`System::In`]. - type In: SystemInput; - - /// The return type of this system. See [`System::Out`]. - type Out; - - /// The [`ExclusiveSystemParam`]'s defined by this system's `fn` parameters. - type Param: ExclusiveSystemParam; - - /// Executes this system once. See [`System::run`]. - fn run( - &mut self, - world: &mut World, - input: ::Inner<'_>, - param_value: ExclusiveSystemParamItem, - ) -> Self::Out; -} - -/// A marker type used to distinguish exclusive function systems with and without input. -#[doc(hidden)] -pub struct HasExclusiveSystemInput; - -macro_rules! impl_exclusive_system_function { - ($($param: ident),*) => { - #[expect( - clippy::allow_attributes, - reason = "This is within a macro, and as such, the below lints may not always apply." - )] - #[allow( - non_snake_case, - reason = "Certain variable names are provided by the caller, not by us." - )] - impl ExclusiveSystemParamFunction Out> for Func - where - Func: Send + Sync + 'static, - for <'a> &'a mut Func: - FnMut(&mut World, $($param),*) -> Out + - FnMut(&mut World, $(ExclusiveSystemParamItem<$param>),*) -> Out, - Out: 'static, - { - type In = (); - type Out = Out; - type Param = ($($param,)*); - #[inline] - fn run(&mut self, world: &mut World, _in: (), param_value: ExclusiveSystemParamItem< ($($param,)*)>) -> Out { - // Yes, this is strange, but `rustc` fails to compile this impl - // without using this function. It fails to recognize that `func` - // is a function, potentially because of the multiple impls of `FnMut` - fn call_inner( - mut f: impl FnMut(&mut World, $($param,)*) -> Out, - world: &mut World, - $($param: $param,)* - ) -> Out { - f(world, $($param,)*) - } - let ($($param,)*) = param_value; - call_inner(self, world, $($param),*) - } - } - - #[expect( - clippy::allow_attributes, - reason = "This is within a macro, and as such, the below lints may not always apply." - )] - #[allow( - non_snake_case, - reason = "Certain variable names are provided by the caller, not by us." - )] - impl ExclusiveSystemParamFunction<(HasExclusiveSystemInput, fn(In, $($param,)*) -> Out)> for Func - where - Func: Send + Sync + 'static, - for <'a> &'a mut Func: - FnMut(In, &mut World, $($param),*) -> Out + - FnMut(In::Param<'_>, &mut World, $(ExclusiveSystemParamItem<$param>),*) -> Out, - In: SystemInput + 'static, - Out: 'static, - { - type In = In; - type Out = Out; - type Param = ($($param,)*); - #[inline] - fn run(&mut self, world: &mut World, input: In::Inner<'_>, param_value: ExclusiveSystemParamItem< ($($param,)*)>) -> Out { - // Yes, this is strange, but `rustc` fails to compile this impl - // without using this function. It fails to recognize that `func` - // is a function, potentially because of the multiple impls of `FnMut` - fn call_inner( - mut f: impl FnMut(In::Param<'_>, &mut World, $($param,)*) -> Out, - input: In::Inner<'_>, - world: &mut World, - $($param: $param,)* - ) -> Out { - f(In::wrap(input), world, $($param,)*) - } - let ($($param,)*) = param_value; - call_inner(self, input, world, $($param),*) - } - } - }; -} -// Note that we rely on the highest impl to be <= the highest order of the tuple impls -// of `SystemParam` created. -all_tuples!(impl_exclusive_system_function, 0, 16, F); - -#[cfg(test)] -mod tests { - use crate::system::input::SystemInput; - - use super::*; - - #[test] - fn into_system_type_id_consistency() { - fn test(function: T) - where - T: IntoSystem + Copy, - { - fn reference_system(_world: &mut World) {} - - use core::any::TypeId; - - let system = IntoSystem::into_system(function); - - assert_eq!( - system.type_id(), - function.system_type_id(), - "System::type_id should be consistent with IntoSystem::system_type_id" - ); - - assert_eq!( - system.type_id(), - TypeId::of::(), - "System::type_id should be consistent with TypeId::of::()" - ); - - assert_ne!( - system.type_id(), - IntoSystem::into_system(reference_system).type_id(), - "Different systems should have different TypeIds" - ); - } - - fn exclusive_function_system(_world: &mut World) {} - - test(exclusive_function_system); - } -} diff --git a/crates/bevy_ecs/src/system/exclusive_system_param.rs b/crates/bevy_ecs/src/system/exclusive_system_param.rs deleted file mode 100644 index e36b7ea759bdd..0000000000000 --- a/crates/bevy_ecs/src/system/exclusive_system_param.rs +++ /dev/null @@ -1,169 +0,0 @@ -use crate::{ - prelude::{FromWorld, QueryState}, - query::{QueryData, QueryFilter}, - system::{Local, SystemMeta, SystemParam, SystemState}, - world::World, -}; -use bevy_utils::synccell::SyncCell; -use core::marker::PhantomData; -use variadics_please::all_tuples; - -/// A parameter that can be used in an exclusive system (a system with an `&mut World` parameter). -/// Any parameters implementing this trait must come after the `&mut World` parameter. -#[diagnostic::on_unimplemented( - message = "`{Self}` can not be used as a parameter for an exclusive system", - label = "invalid system parameter" -)] -pub trait ExclusiveSystemParam: Sized { - /// Used to store data which persists across invocations of a system. - type State: Send + Sync + 'static; - /// The item type returned when constructing this system param. - /// See [`SystemParam::Item`]. - type Item<'s>: ExclusiveSystemParam; - - /// Creates a new instance of this param's [`State`](Self::State). - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self::State; - - /// Creates a parameter to be passed into an [`ExclusiveSystemParamFunction`]. - /// - /// [`ExclusiveSystemParamFunction`]: super::ExclusiveSystemParamFunction - fn get_param<'s>(state: &'s mut Self::State, system_meta: &SystemMeta) -> Self::Item<'s>; -} - -/// Shorthand way of accessing the associated type [`ExclusiveSystemParam::Item`] -/// for a given [`ExclusiveSystemParam`]. -pub type ExclusiveSystemParamItem<'s, P> =

::Item<'s>; - -impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> ExclusiveSystemParam - for &'a mut QueryState -{ - type State = QueryState; - type Item<'s> = &'s mut QueryState; - - fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { - QueryState::new(world) - } - - fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - state - } -} - -impl<'a, P: SystemParam + 'static> ExclusiveSystemParam for &'a mut SystemState

{ - type State = SystemState

; - type Item<'s> = &'s mut SystemState

; - - fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { - SystemState::new(world) - } - - fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - state - } -} - -impl<'_s, T: FromWorld + Send + 'static> ExclusiveSystemParam for Local<'_s, T> { - type State = SyncCell; - type Item<'s> = Local<'s, T>; - - fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { - SyncCell::new(T::from_world(world)) - } - - fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - Local(state.get()) - } -} - -impl ExclusiveSystemParam for PhantomData { - type State = (); - type Item<'s> = PhantomData; - - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} - - fn get_param<'s>(_state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - PhantomData - } -} - -macro_rules! impl_exclusive_system_param_tuple { - ($(#[$meta:meta])* $($param: ident),*) => { - #[expect( - clippy::allow_attributes, - reason = "This is within a macro, and as such, the below lints may not always apply." - )] - #[allow( - non_snake_case, - reason = "Certain variable names are provided by the caller, not by us." - )] - #[allow( - unused_variables, - reason = "Zero-length tuples won't use any of the parameters." - )] - $(#[$meta])* - impl<$($param: ExclusiveSystemParam),*> ExclusiveSystemParam for ($($param,)*) { - type State = ($($param::State,)*); - type Item<'s> = ($($param::Item<'s>,)*); - - #[inline] - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { - (($($param::init(world, system_meta),)*)) - } - - #[inline] - fn get_param<'s>( - state: &'s mut Self::State, - system_meta: &SystemMeta, - ) -> Self::Item<'s> { - let ($($param,)*) = state; - #[allow( - clippy::unused_unit, - reason = "Zero-length tuples won't have any params to get." - )] - ($($param::get_param($param, system_meta),)*) - } - } - }; -} - -all_tuples!( - #[doc(fake_variadic)] - impl_exclusive_system_param_tuple, - 0, - 16, - P -); - -#[cfg(test)] -mod tests { - use crate as bevy_ecs; - use crate::{schedule::Schedule, system::Local, world::World}; - use alloc::vec::Vec; - use bevy_ecs_macros::Resource; - use core::marker::PhantomData; - - #[test] - fn test_exclusive_system_params() { - #[derive(Resource, Default)] - struct Res { - test_value: u32, - } - - fn my_system(world: &mut World, mut local: Local, _phantom: PhantomData>) { - assert_eq!(world.resource::().test_value, *local); - *local += 1; - world.resource_mut::().test_value += 1; - } - - let mut schedule = Schedule::default(); - schedule.add_systems(my_system); - - let mut world = World::default(); - world.init_resource::(); - - schedule.run(&mut world); - schedule.run(&mut world); - - assert_eq!(2, world.get_resource::().unwrap().test_value); - } -} diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 0f3950d1d4ba6..bb60a3d082fcc 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -802,7 +802,7 @@ where #[inline] fn is_exclusive(&self) -> bool { - false + F::Param::is_exclusive() } #[inline] diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index d5cbd5ffb1571..66d2099af788c 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -123,8 +123,6 @@ mod adapter_system; mod builder; mod combinator; mod commands; -mod exclusive_function_system; -mod exclusive_system_param; mod function_system; mod input; mod observer_system; @@ -141,8 +139,6 @@ pub use adapter_system::*; pub use builder::*; pub use combinator::*; pub use commands::*; -pub use exclusive_function_system::*; -pub use exclusive_system_param::*; pub use function_system::*; pub use input::*; pub use observer_system::*; diff --git a/crates/bevy_ecs/src/system/system_name.rs b/crates/bevy_ecs/src/system/system_name.rs index b28ddd89f6658..31420caeb9d59 100644 --- a/crates/bevy_ecs/src/system/system_name.rs +++ b/crates/bevy_ecs/src/system/system_name.rs @@ -1,7 +1,7 @@ use crate::{ component::Tick, prelude::World, - system::{ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam}, + system::{ReadOnlySystemParam, SystemMeta, SystemParam}, world::unsafe_world_cell::UnsafeWorldCell, }; use alloc::borrow::Cow; @@ -75,19 +75,6 @@ unsafe impl SystemParam for SystemName<'_> { // SAFETY: Only reads internal system state unsafe impl<'s> ReadOnlySystemParam for SystemName<'s> {} -impl ExclusiveSystemParam for SystemName<'_> { - type State = Cow<'static, str>; - type Item<'s> = SystemName<'s>; - - fn init(_world: &mut World, system_meta: &mut SystemMeta) -> Self::State { - system_meta.name.clone() - } - - fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - SystemName(state) - } -} - #[cfg(test)] mod tests { use crate::{ diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 5859b48346d89..b5a7d41bf557b 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -11,7 +11,7 @@ use crate::{ }, resource::Resource, storage::ResourceData, - system::{Query, Single, SystemMeta}, + system::{Query, Single, SystemMeta, SystemState}, world::{ unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FilteredResources, FilteredResourcesMut, FromWorld, World, @@ -287,6 +287,15 @@ pub unsafe trait SystemParam: Sized { world: UnsafeWorldCell<'world>, change_tick: Tick, ) -> Self::Item<'world, 'state>; + + /// Returns true if this [`SystemParam`] requires exclusive access to the + /// entire [`World`]. [`System`]s with exclusive parameters cannot run in + /// parallel with other systems. + /// + /// [`System`]: crate::system::System + fn is_exclusive() -> bool { + false + } } /// A [`SystemParam`] that only reads a given [`World`]. @@ -1022,7 +1031,7 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { } } -/// SAFETY: only reads world +// SAFETY: only reads world unsafe impl<'w> ReadOnlySystemParam for &'w World {} // SAFETY: `read_all` access is set and conflicts result in a panic @@ -1066,7 +1075,11 @@ unsafe impl SystemParam for &'_ World { } } -/// SAFETY: `DeferredWorld` can read all components and resources but cannot be used to gain any other mutable references. +const MUT_DEFERRED_WORLD_ERROR: &str = "DeferredWorld requires exclusive access + to the entire world, but a previous system parameter already registered \ + access to a part of it. Allowing this would break Rust's mutability rules"; + +// SAFETY: `DeferredWorld` registers exclusive access to the entire world. unsafe impl<'w> SystemParam for DeferredWorld<'w> { type State = (); type Item<'world, 'state> = DeferredWorld<'world>; @@ -1092,6 +1105,111 @@ unsafe impl<'w> SystemParam for DeferredWorld<'w> { ) -> Self::Item<'world, 'state> { world.into_deferred() } + + fn is_exclusive() -> bool { + true + } +} + +const MUT_WORLD_ERROR: &str = "&mut World requires exclusive access to the \ + entire world, but a previous system parameter already registered access to \ + a part of it. Allowing this would break Rust's mutability rules"; + +// SAFETY: `&mut World` registers exclusive access to the entire world. +unsafe impl SystemParam for &mut World { + type State = (); + type Item<'w, 's> = &'w mut World; + + fn init_state(_world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + let mut access = Access::default(); + access.read_all(); + if !system_meta + .archetype_component_access + .is_compatible(&access) + { + panic!("{}", MUT_WORLD_ERROR); + } + system_meta.archetype_component_access.extend(&access); + + let mut filtered_access = FilteredAccess::default(); + + filtered_access.read_all(); + if !system_meta + .component_access_set + .get_conflicts_single(&filtered_access) + .is_empty() + { + panic!("{}", MUT_WORLD_ERROR); + } + system_meta.component_access_set.add(filtered_access); + system_meta.set_has_deferred(); + } + + unsafe fn get_param<'world, 'state>( + _state: &'state mut Self::State, + _system_meta: &SystemMeta, + world: UnsafeWorldCell<'world>, + _change_tick: Tick, + ) -> Self::Item<'world, 'state> { + // SAFETY: Exclusive access to the entire world was registered in `init_state`. + unsafe { world.world_mut() } + } + + fn is_exclusive() -> bool { + true + } +} + +// SAFETY: `&mut QueryState` only accesses internal state +unsafe impl SystemParam + for &mut QueryState +{ + type State = QueryState; + type Item<'w, 's> = &'s mut QueryState; + + fn init_state(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { + QueryState::new(world) + } + + unsafe fn new_archetype( + state: &mut Self::State, + archetype: &Archetype, + system_meta: &mut SystemMeta, + ) { + state.new_archetype(archetype, &mut system_meta.archetype_component_access); + } + + unsafe fn get_param<'world, 'state>( + state: &'state mut Self::State, + _system_meta: &SystemMeta, + _world: UnsafeWorldCell<'world>, + _change_tick: Tick, + ) -> Self::Item<'world, 'state> { + state + } +} + +// SAFETY: `&mut SystemState` only accesses internal state +unsafe impl SystemParam for &mut SystemState

{ + type State = SystemState

; + type Item<'w, 's> = &'s mut SystemState

; + + fn init_state(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { + SystemState::new(world) + } + + fn apply(state: &mut Self::State, _system_meta: &SystemMeta, world: &mut World) { + state.apply(world); + } + + unsafe fn get_param<'world, 'state>( + state: &'state mut Self::State, + _system_meta: &SystemMeta, + _world: UnsafeWorldCell<'world>, + _change_tick: Tick, + ) -> Self::Item<'world, 'state> { + state + } } /// A system local [`SystemParam`]. @@ -2040,6 +2158,10 @@ macro_rules! impl_system_param_tuple { )] ($($param::get_param($param, system_meta, world, change_tick),)*) } + + fn is_exclusive() -> bool { + false $(|| $param::is_exclusive())* + } } }; } diff --git a/crates/bevy_ecs/src/world/identifier.rs b/crates/bevy_ecs/src/world/identifier.rs index 221ddd8210919..810ca191826b9 100644 --- a/crates/bevy_ecs/src/world/identifier.rs +++ b/crates/bevy_ecs/src/world/identifier.rs @@ -1,7 +1,7 @@ use crate::{ component::Tick, storage::SparseSetIndex, - system::{ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam}, + system::{ReadOnlySystemParam, SystemMeta, SystemParam}, world::{FromWorld, World}, }; use bevy_platform_support::sync::atomic::{AtomicUsize, Ordering}; @@ -66,19 +66,6 @@ unsafe impl SystemParam for WorldId { } } -impl ExclusiveSystemParam for WorldId { - type State = WorldId; - type Item<'s> = WorldId; - - fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { - world.id() - } - - fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { - *state - } -} - impl SparseSetIndex for WorldId { #[inline] fn sparse_set_index(&self) -> usize { From 9c6bf55ba20ed11a99ab84faadb7bb1081ff9e26 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Wed, 4 Dec 2024 01:27:21 -0600 Subject: [PATCH 03/10] make mutable world access params register write_all --- crates/bevy_ecs/src/system/system_param.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index b5a7d41bf557b..3f4b4e6b81419 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1075,7 +1075,7 @@ unsafe impl SystemParam for &'_ World { } } -const MUT_DEFERRED_WORLD_ERROR: &str = "DeferredWorld requires exclusive access +const MUT_DEFERRED_WORLD_ERROR: &str = "DeferredWorld requires exclusive access \ to the entire world, but a previous system parameter already registered \ access to a part of it. Allowing this would break Rust's mutability rules"; @@ -1123,6 +1123,7 @@ unsafe impl SystemParam for &mut World { fn init_state(_world: &mut World, system_meta: &mut SystemMeta) -> Self::State { let mut access = Access::default(); access.read_all(); + access.write_all(); if !system_meta .archetype_component_access .is_compatible(&access) @@ -1134,6 +1135,7 @@ unsafe impl SystemParam for &mut World { let mut filtered_access = FilteredAccess::default(); filtered_access.read_all(); + filtered_access.write_all(); if !system_meta .component_access_set .get_conflicts_single(&filtered_access) From 61d338e3734dfe511c0f591c81815a8fdc13a79e Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Wed, 4 Dec 2024 02:39:14 -0600 Subject: [PATCH 04/10] add some conflict tests --- crates/bevy_ecs/src/system/system_param.rs | 133 ++++++++++++++++++++- 1 file changed, 131 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 3f4b4e6b81419..64e4d89d3ec7c 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -2703,8 +2703,9 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { mod tests { use super::*; use crate::{ - self as bevy_ecs, // Necessary for the `SystemParam` Derive when used inside `bevy_ecs`. - system::assert_is_system, + self as bevy_ecs, + prelude::Component, + system::{assert_is_system, IntoSystem, System}, }; use core::cell::RefCell; @@ -2935,4 +2936,132 @@ mod tests { let _query: Query<()> = p.downcast_mut_inner().unwrap(); let _query: Query<()> = p.downcast().unwrap(); } + + #[derive(Component, Resource)] + struct Foo; + + fn check_conflict(system: impl IntoSystem<(), (), M>) { + let mut world = World::new(); + let mut system = IntoSystem::into_system(system); + system.initialize(&mut world); + } + + #[test] + #[should_panic] + fn conflict_mut_world_multiple() { + fn system(_: &mut World, _: &mut World) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_mut_world_deferred_world() { + fn system(_: &mut World, _: DeferredWorld) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_mut_world_query() { + fn system(_: &mut World, _: Query<&Foo>) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_query_mut_world() { + fn system(_: Query<&Foo>, _: &mut World) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_mut_world_resource() { + fn system(_: &mut World, _: Res) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_resource_mut_world() { + fn system(_: Res, _: &mut World) {} + check_conflict(system); + } + + #[test] + fn no_conflict_mut_world_local() { + fn system(_: &mut World, _: Local) {} + check_conflict(system); + } + + #[test] + fn no_conflict_mut_world_query_state() { + fn system(_: &mut World, _: &mut QueryState<&Foo>) {} + check_conflict(system); + } + + #[test] + fn no_conflict_mut_world_system_state() { + fn system(_: &mut World, _: &mut SystemState>) {} + check_conflict(system); + } + + #[test] + fn no_conflict_mut_world_system_state_recursive() { + fn system(_: &mut World, _: &mut SystemState<&mut SystemState>>) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_deferred_world_multiple() { + fn system(_: DeferredWorld, _: DeferredWorld) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_deferred_world_query() { + fn system(_: DeferredWorld, _: Query<&Foo>) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_query_deferred_world() { + fn system(_: Query<&Foo>, _: DeferredWorld) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_deferred_world_resource() { + fn system(_: DeferredWorld, _: Res) {} + check_conflict(system); + } + + #[test] + #[should_panic] + fn conflict_resource_deferred_world() { + fn system(_: Res, _: DeferredWorld) {} + check_conflict(system); + } + + #[test] + fn no_conflict_deferred_world_local() { + fn system(_: DeferredWorld, _: Local) {} + check_conflict(system); + } + + #[test] + fn no_conflict_deferred_world_query_state() { + fn system(_: DeferredWorld, _: &mut QueryState<&Foo>) {} + check_conflict(system); + } + + #[test] + fn no_conflict_deferred_world_system_state() { + fn system(_: DeferredWorld, _: &mut SystemState>) {} + check_conflict(system); + } } From 2d67495878eed4c4cc38dd257a5a4c2ba4ed3d36 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Wed, 4 Dec 2024 10:37:58 -0600 Subject: [PATCH 05/10] mark &mut World and DeferredWorld system params as non-send --- crates/bevy_ecs/src/system/system_param.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 64e4d89d3ec7c..2c4c516da2db9 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1051,7 +1051,6 @@ unsafe impl SystemParam for &'_ World { system_meta.archetype_component_access.extend(&access); let mut filtered_access = FilteredAccess::default(); - filtered_access.read_all(); if !system_meta .component_access_set @@ -1133,7 +1132,6 @@ unsafe impl SystemParam for &mut World { system_meta.archetype_component_access.extend(&access); let mut filtered_access = FilteredAccess::default(); - filtered_access.read_all(); filtered_access.write_all(); if !system_meta @@ -1144,7 +1142,9 @@ unsafe impl SystemParam for &mut World { panic!("{}", MUT_WORLD_ERROR); } system_meta.component_access_set.add(filtered_access); + system_meta.set_has_deferred(); + system_meta.set_non_send(); } unsafe fn get_param<'world, 'state>( From 7d1218c3bc4b7d38fd27eddbca2540242f12bf0f Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Wed, 29 Jan 2025 21:18:05 -0600 Subject: [PATCH 06/10] fix rebase issues --- crates/bevy_ecs/macros/src/lib.rs | 130 ------------------- crates/bevy_ecs/src/schedule/executor/mod.rs | 4 +- 2 files changed, 1 insertion(+), 133 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 84e853483c11f..e4cca41a4d7aa 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -283,136 +283,6 @@ pub fn derive_visit_entities(input: TokenStream) -> TokenStream { }) } -fn get_idents(fmt_string: fn(usize) -> String, count: usize) -> Vec { - (0..count) - .map(|i| Ident::new(&fmt_string(i), Span::call_site())) - .collect::>() -} - -#[proc_macro] -pub fn impl_param_set(_input: TokenStream) -> TokenStream { - let mut tokens = TokenStream::new(); - let max_params = 8; - let params = get_idents(|i| format!("P{i}"), max_params); - let metas = get_idents(|i| format!("m{i}"), max_params); - let mut param_fn_muts = Vec::new(); - for (i, param) in params.iter().enumerate() { - let fn_name = Ident::new(&format!("p{i}"), Span::call_site()); - let index = Index::from(i); - let ordinal = match i { - 1 => "1st".to_owned(), - 2 => "2nd".to_owned(), - 3 => "3rd".to_owned(), - x => format!("{x}th"), - }; - let comment = - format!("Gets exclusive access to the {ordinal} parameter in this [`ParamSet`]."); - param_fn_muts.push(quote! { - #[doc = #comment] - /// No other parameters may be accessed while this one is active. - pub fn #fn_name<'a>(&'a mut self) -> SystemParamItem<'a, 'a, #param> { - // SAFETY: systems run without conflicts with other systems. - // Conflicting params in ParamSet are not accessible at the same time - // ParamSets are guaranteed to not conflict with other SystemParams - unsafe { - #param::get_param(&mut self.param_states.#index, &self.system_meta, self.world, self.change_tick) - } - } - }); - } - - for param_count in 1..=max_params { - let param = ¶ms[0..param_count]; - let meta = &metas[0..param_count]; - let param_fn_mut = ¶m_fn_muts[0..param_count]; - tokens.extend(TokenStream::from(quote! { - // SAFETY: All parameters are constrained to ReadOnlySystemParam, so World is only read - unsafe impl<'w, 's, #(#param,)*> ReadOnlySystemParam for ParamSet<'w, 's, (#(#param,)*)> - where #(#param: ReadOnlySystemParam,)* - { } - - // SAFETY: Relevant parameter ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any ParamState conflicts - // with any prior access, a panic will occur. - unsafe impl<'_w, '_s, #(#param: SystemParam,)*> SystemParam for ParamSet<'_w, '_s, (#(#param,)*)> - { - type State = (#(#param::State,)*); - type Item<'w, 's> = ParamSet<'w, 's, (#(#param,)*)>; - - // Note: We allow non snake case so the compiler don't complain about the creation of non_snake_case variables - #[allow(non_snake_case)] - fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { - #( - // Pretend to add each param to the system alone, see if it conflicts - let mut #meta = system_meta.clone(); - #meta.component_access_set.clear(); - #meta.archetype_component_access.clear(); - #param::init_state(world, &mut #meta); - // The variable is being defined with non_snake_case here - let #param = #param::init_state(world, &mut system_meta.clone()); - )* - // Make the ParamSet non-send if any of its parameters are non-send. - if false #(|| !#meta.is_send())* { - system_meta.set_non_send(); - } - #( - system_meta - .component_access_set - .extend(#meta.component_access_set); - system_meta - .archetype_component_access - .extend(&#meta.archetype_component_access); - )* - (#(#param,)*) - } - - unsafe fn new_archetype(state: &mut Self::State, archetype: &Archetype, system_meta: &mut SystemMeta) { - // SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`. - unsafe { <(#(#param,)*) as SystemParam>::new_archetype(state, archetype, system_meta); } - } - - fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { - <(#(#param,)*) as SystemParam>::apply(state, system_meta, world); - } - - fn queue(state: &mut Self::State, system_meta: &SystemMeta, mut world: DeferredWorld) { - <(#(#param,)*) as SystemParam>::queue(state, system_meta, world.reborrow()); - } - - #[inline] - unsafe fn validate_param<'w, 's>( - state: &'s Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell<'w>, - ) -> bool { - <(#(#param,)*) as SystemParam>::validate_param(state, system_meta, world) - } - - #[inline] - unsafe fn get_param<'w, 's>( - state: &'s mut Self::State, - system_meta: &SystemMeta, - world: UnsafeWorldCell<'w>, - change_tick: Tick, - ) -> Self::Item<'w, 's> { - ParamSet { - param_states: state, - system_meta: system_meta.clone(), - world, - change_tick, - } - } - } - - impl<'w, 's, #(#param: SystemParam,)*> ParamSet<'w, 's, (#(#param,)*)> - { - #(#param_fn_mut)* - } - })); - } - - tokens -} - /// Implement `SystemParam` to use a struct as a parameter in a system #[proc_macro_derive(SystemParam, attributes(system_param))] pub fn derive_system_param(input: TokenStream) -> TokenStream { diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 718ff114aab70..8b549a15ea6d3 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -3,11 +3,9 @@ mod multi_threaded; mod simple; mod single_threaded; -use alloc::borrow::Cow; +use alloc::{borrow::Cow, vec, vec::Vec}; use core::any::TypeId; -use alloc::{vec, vec::Vec}; - pub use self::{simple::SimpleExecutor, single_threaded::SingleThreadedExecutor}; #[cfg(feature = "std")] From 9850d1ebf6b423bb96ac4b03e36662538c9fcf61 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Wed, 29 Jan 2025 23:53:27 -0600 Subject: [PATCH 07/10] replace SystemParam::is_exclusive with world_access_level --- crates/bevy_ecs/macros/src/lib.rs | 4 + crates/bevy_ecs/src/removal_detection.rs | 6 +- crates/bevy_ecs/src/system/commands/mod.rs | 6 +- crates/bevy_ecs/src/system/function_system.rs | 8 +- crates/bevy_ecs/src/system/system_name.rs | 6 + crates/bevy_ecs/src/system/system_param.rs | 241 +++++++++++++++++- crates/bevy_ecs/src/world/identifier.rs | 6 +- crates/bevy_gizmos/src/gizmos.rs | 8 +- crates/bevy_render/src/extract_param.rs | 9 +- 9 files changed, 272 insertions(+), 22 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index e4cca41a4d7aa..25bc966372af9 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -521,6 +521,10 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { #(#fields: #field_locals,)* } } + + fn world_access_level() -> #path::system::WorldAccessLevel { + <#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::world_access_level() + } } // Safety: Each field is `ReadOnlySystemParam`, so this can only read from the `World` diff --git a/crates/bevy_ecs/src/removal_detection.rs b/crates/bevy_ecs/src/removal_detection.rs index c81a5cfa1eb2c..dbe22ce6bdf3f 100644 --- a/crates/bevy_ecs/src/removal_detection.rs +++ b/crates/bevy_ecs/src/removal_detection.rs @@ -7,7 +7,7 @@ use crate::{ event::{Event, EventCursor, EventId, EventIterator, EventIteratorWithId, Events}, prelude::Local, storage::SparseSet, - system::{ReadOnlySystemParam, SystemMeta, SystemParam}, + system::{ReadOnlySystemParam, SystemMeta, SystemParam, WorldAccessLevel}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; @@ -266,4 +266,8 @@ unsafe impl<'a> SystemParam for &'a RemovedComponentEvents { ) -> Self::Item<'w, 's> { world.removed_components() } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 5694df4a49451..cf116d19b4b3f 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -29,7 +29,7 @@ use crate::{ schedule::ScheduleLabel, system::{ command::HandleError, entity_command::CommandWithEntity, input::SystemInput, Deferred, - IntoObserverSystem, IntoSystem, RegisteredSystem, SystemId, + IntoObserverSystem, IntoSystem, RegisteredSystem, SystemId, WorldAccessLevel, }, world::{ command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, CommandQueue, @@ -202,6 +202,10 @@ const _: () = { entities: f1, } } + + fn world_access_level() -> WorldAccessLevel { + <(Deferred, &Entities) as bevy_ecs::system::SystemParam>::world_access_level() + } } // SAFETY: Only reads Entities unsafe impl<'w, 's> bevy_ecs::system::ReadOnlySystemParam for Commands<'w, 's> diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index bb60a3d082fcc..1282e81d03540 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -5,8 +5,8 @@ use crate::{ query::{Access, FilteredAccessSet}, schedule::{InternedSystemSet, SystemSet}, system::{ - check_system_change_tick, ReadOnlySystemParam, System, SystemIn, SystemInput, SystemParam, - SystemParamItem, + assert_valid_world_access_level, check_system_change_tick, ReadOnlySystemParam, System, + SystemIn, SystemInput, SystemParam, SystemParamItem, WorldAccessLevel, }, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World, WorldId}, }; @@ -425,6 +425,7 @@ impl SystemState { /// `new` does not cache any of the world's archetypes, so you must call [`SystemState::update_archetypes`] /// manually before calling `get_manual{_mut}`. pub fn new(world: &mut World) -> Self { + assert_valid_world_access_level::(); let mut meta = SystemMeta::new::(); meta.last_run = world.change_tick().relative_to(Tick::MAX); let param_state = Param::init_state(world, &mut meta); @@ -751,6 +752,7 @@ where { type System = FunctionSystem; fn into_system(func: Self) -> Self::System { + assert_valid_world_access_level::(); FunctionSystem { func, state: None, @@ -802,7 +804,7 @@ where #[inline] fn is_exclusive(&self) -> bool { - F::Param::is_exclusive() + F::Param::world_access_level() == WorldAccessLevel::Exclusive } #[inline] diff --git a/crates/bevy_ecs/src/system/system_name.rs b/crates/bevy_ecs/src/system/system_name.rs index 31420caeb9d59..64a99877e33eb 100644 --- a/crates/bevy_ecs/src/system/system_name.rs +++ b/crates/bevy_ecs/src/system/system_name.rs @@ -8,6 +8,8 @@ use alloc::borrow::Cow; use core::ops::Deref; use derive_more::derive::{AsRef, Display, Into}; +use super::WorldAccessLevel; + /// [`SystemParam`] that returns the name of the system which it is used in. /// /// This is not a reliable identifier, it is more so useful for debugging or logging. @@ -70,6 +72,10 @@ unsafe impl SystemParam for SystemName<'_> { ) -> Self::Item<'w, 's> { SystemName(name) } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::None + } } // SAFETY: Only reads internal system state diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 2c4c516da2db9..c787f3bcee5b7 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -288,14 +288,9 @@ pub unsafe trait SystemParam: Sized { change_tick: Tick, ) -> Self::Item<'world, 'state>; - /// Returns true if this [`SystemParam`] requires exclusive access to the - /// entire [`World`]. [`System`]s with exclusive parameters cannot run in - /// parallel with other systems. - /// - /// [`System`]: crate::system::System - fn is_exclusive() -> bool { - false - } + /// Returns the access level required by this [`SystemParam`] to access the + /// [`World`] via [`SystemParam::get_param`]. + fn world_access_level() -> WorldAccessLevel; } /// A [`SystemParam`] that only reads a given [`World`]. @@ -307,6 +302,31 @@ pub unsafe trait ReadOnlySystemParam: SystemParam {} /// Shorthand way of accessing the associated type [`SystemParam::Item`] for a given [`SystemParam`]. pub type SystemParamItem<'w, 's, P> =

::Item<'w, 's>; +/// The level of access a [`SystemParam`] needs from the [`World`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum WorldAccessLevel { + /// The [`SystemParam`] does not need access to the [`World`] beyond + /// [`SystemParam::init_state`]. + None, + /// The [`SystemParam`] needs access to the [`World`] for + /// [`SystemParam::get_param`] and registers its required access to the + /// [`SystemMeta`]. All other [`SystemParam`]s must be [`WorldAccessLevel::Shared`] + /// and register their access to the [`SystemMeta`] accordingly, or + /// [`WorldAccessLevel::None`]. + Shared, + /// The [`SystemParam`] needs exclusive access to the [`World`] for + /// [`SystemParam::get_param`] which requires all other [`SystemParam`]s to + /// be [`WorldAccessLevel::None`]. + Exclusive, +} + +/// Asserts that the given [`SystemParam`] has a valid [`WorldAccessLevel`]. +pub fn assert_valid_world_access_level() { + // This will panic on params/tuples whose fields/members have conflicting + // access levels. + let _ = P::world_access_level(); +} + // SAFETY: QueryState is constrained to read-only fetches, so it only reads World. unsafe impl<'w, 's, D: ReadOnlyQueryData + 'static, F: QueryFilter + 'static> ReadOnlySystemParam for Query<'w, 's, D, F> @@ -345,6 +365,10 @@ unsafe impl SystemParam for Qu // world data that the query needs. unsafe { Query::new(world, state, system_meta.last_run, change_tick) } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } pub(crate) fn init_query_param( @@ -445,6 +469,10 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam fo } is_valid } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If @@ -511,6 +539,10 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam } is_valid } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: QueryState is constrained to read-only fetches, so it only reads World. @@ -572,6 +604,10 @@ unsafe impl SystemParam state.is_empty_unsafe_world_cell(world, system_meta.last_run, world.change_tick()) } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: QueryState is constrained to read-only fetches, so it only reads World. @@ -783,6 +819,27 @@ macro_rules! impl_param_set { change_tick, } } + + fn world_access_level() -> WorldAccessLevel { + let mut shared = false; + $( + match $param::world_access_level() { + WorldAccessLevel::Exclusive => { + return WorldAccessLevel::Exclusive; + } + WorldAccessLevel::Shared => { + shared = true; + } + WorldAccessLevel::None => {} + } + )* + + if shared { + WorldAccessLevel::Shared + } else { + WorldAccessLevel::None + } + } } impl<'w, 's, $($param: SystemParam,)*> ParamSet<'w, 's, ($($param,)*)> @@ -884,6 +941,10 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { changed_by: _caller.deref(), } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: Only reads a single World resource @@ -919,6 +980,10 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { changed_by: _caller.deref(), }) } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res @@ -997,6 +1062,10 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { changed_by: value.changed_by, } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: this impl defers to `ResMut`, which initializes and validates the correct world access. @@ -1029,6 +1098,10 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { changed_by: value.changed_by, }) } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: only reads world @@ -1072,6 +1145,10 @@ unsafe impl SystemParam for &'_ World { // SAFETY: Read-only access to the entire world was registered in `init_state`. unsafe { world.world() } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } const MUT_DEFERRED_WORLD_ERROR: &str = "DeferredWorld requires exclusive access \ @@ -1105,8 +1182,8 @@ unsafe impl<'w> SystemParam for DeferredWorld<'w> { world.into_deferred() } - fn is_exclusive() -> bool { - true + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Exclusive } } @@ -1157,8 +1234,8 @@ unsafe impl SystemParam for &mut World { unsafe { world.world_mut() } } - fn is_exclusive() -> bool { - true + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Exclusive } } @@ -1189,6 +1266,10 @@ unsafe impl SystemParam ) -> Self::Item<'world, 'state> { state } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::None + } } // SAFETY: `&mut SystemState` only accesses internal state @@ -1212,6 +1293,10 @@ unsafe impl SystemParam for &mut SystemState

{ ) -> Self::Item<'world, 'state> { state } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::None + } } /// A system local [`SystemParam`]. @@ -1325,6 +1410,10 @@ unsafe impl<'a, T: FromWorld + Send + 'static> SystemParam for Local<'a, T> { ) -> Self::Item<'w, 's> { Local(state.get()) } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::None + } } /// Types that can be used with [`Deferred`] in systems. @@ -1514,6 +1603,10 @@ unsafe impl SystemParam for Deferred<'_, T> { ) -> Self::Item<'w, 's> { Deferred(state.get()) } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::None + } } /// Shared borrow of a non-[`Send`] resource. @@ -1663,6 +1756,10 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { changed_by: _caller.deref(), } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: Only reads a single World non-send resource @@ -1695,6 +1792,10 @@ unsafe impl SystemParam for Option> { changed_by: _caller.deref(), }) } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: NonSendMut ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this @@ -1771,6 +1872,10 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { changed_by: _caller.deref_mut(), } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: this impl defers to `NonSendMut`, which initializes and validates the correct world access. @@ -1798,6 +1903,10 @@ unsafe impl<'a, T: 'static> SystemParam for Option> { changed_by: _caller.deref_mut(), }) } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: Only reads World archetypes @@ -1819,6 +1928,10 @@ unsafe impl<'a> SystemParam for &'a Archetypes { ) -> Self::Item<'w, 's> { world.archetypes() } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: Only reads World components @@ -1840,6 +1953,10 @@ unsafe impl<'a> SystemParam for &'a Components { ) -> Self::Item<'w, 's> { world.components() } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: Only reads World entities @@ -1861,6 +1978,10 @@ unsafe impl<'a> SystemParam for &'a Entities { ) -> Self::Item<'w, 's> { world.entities() } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: Only reads World bundles @@ -1882,6 +2003,10 @@ unsafe impl<'a> SystemParam for &'a Bundles { ) -> Self::Item<'w, 's> { world.bundles() } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } /// A [`SystemParam`] that reads the previous and current change ticks of the system. @@ -1935,6 +2060,10 @@ unsafe impl SystemParam for SystemChangeTick { this_run: change_tick, } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::None + } } // SAFETY: When initialized with `init_state`, `get_param` returns an empty `Vec` and does no access. @@ -1998,6 +2127,10 @@ unsafe impl SystemParam for Vec { T::queue(state, system_meta, world.reborrow()); } } + + fn world_access_level() -> WorldAccessLevel { + T::world_access_level() + } } // SAFETY: When initialized with `init_state`, `get_param` returns an empty `Vec` and does no access. @@ -2049,6 +2182,10 @@ unsafe impl SystemParam for ParamSet<'_, '_, Vec> { T::queue(state, system_meta, world.reborrow()); } } + + fn world_access_level() -> WorldAccessLevel { + T::world_access_level() + } } impl ParamSet<'_, '_, Vec> { @@ -2161,8 +2298,40 @@ macro_rules! impl_system_param_tuple { ($($param::get_param($param, system_meta, world, change_tick),)*) } - fn is_exclusive() -> bool { - false $(|| $param::is_exclusive())* + fn world_access_level() -> WorldAccessLevel { + #[allow(unused_mut, reason = "tuple might be zero-length")] + let mut exclusive = false; + #[allow(unused_mut, reason = "tuple might be zero-length")] + let mut shared = false; + $( + match $param::world_access_level() { + WorldAccessLevel::Exclusive => { + if shared || exclusive { + let type_name = core::any::type_name::<$param>(); + panic!("error[B0002]: {type_name} conflicts with a previous system parameter. {type_name} is exclusive, and so cannot be combined with any system parameter that accesses the World."); + } else { + exclusive = true; + } + } + WorldAccessLevel::Shared => { + if exclusive { + let type_name = core::any::type_name::<$param>(); + panic!("error[B0002]: {type_name} conflicts with a previous system parameter which is exclusive. {type_name} is shared, and so cannot be combined with any system parameter that accesses the World exclusively."); + } else { + shared = true; + } + } + WorldAccessLevel::None => {} + } + )* + + if exclusive { + WorldAccessLevel::Exclusive + } else if shared { + WorldAccessLevel::Shared + } else { + WorldAccessLevel::None + } } } }; @@ -2328,6 +2497,10 @@ unsafe impl SystemParam for StaticSystemParam<'_, '_, // SAFETY: Defer to the safety of P::SystemParam StaticSystemParam(unsafe { P::get_param(state, system_meta, world, change_tick) }) } + + fn world_access_level() -> WorldAccessLevel { + P::world_access_level() + } } // SAFETY: No world access. @@ -2346,6 +2519,10 @@ unsafe impl SystemParam for PhantomData { ) -> Self::Item<'world, 'state> { PhantomData } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::None + } } // SAFETY: No world access. @@ -2646,6 +2823,10 @@ unsafe impl SystemParam for DynSystemParam<'_, '_> { fn queue(state: &mut Self::State, system_meta: &SystemMeta, world: DeferredWorld) { state.0.queue(system_meta, world); } + + fn world_access_level() -> WorldAccessLevel { + todo!() + } } // SAFETY: When initialized with `init_state`, `get_param` returns a `FilteredResources` with no access. @@ -2670,6 +2851,10 @@ unsafe impl SystemParam for FilteredResources<'_, '_> { // and the builder registers `access` in `build`. unsafe { FilteredResources::new(world, state, system_meta.last_run, change_tick) } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } // SAFETY: FilteredResources only reads resources. @@ -2697,6 +2882,10 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { // and the builder registers `access` in `build`. unsafe { FilteredResourcesMut::new(world, state, system_meta.last_run, change_tick) } } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::Shared + } } #[cfg(test)] @@ -2946,6 +3135,13 @@ mod tests { system.initialize(&mut world); } + #[test] + #[should_panic] + fn conflict_world_archetypes() { + fn system(_: &mut World, _: &Archetypes, _: &Components, _: &Entities, _: &Bundles) {} + check_conflict(system); + } + #[test] #[should_panic] fn conflict_mut_world_multiple() { @@ -3064,4 +3260,21 @@ mod tests { fn system(_: DeferredWorld, _: &mut SystemState>) {} check_conflict(system); } + + #[test] + fn param_set_no_conflicts() { + fn system(_: ParamSet<(&mut World, DeferredWorld, &World, Query<&Foo>)>) {} + check_conflict(system); + } + + #[test] + fn derived_deferred_world_system_state() { + #[derive(SystemParam)] + struct Test<'w, 's> { + _dw: DeferredWorld<'w>, + _ss: &'s mut SystemState>, + } + fn system(_: Test) {} + check_conflict(system); + } } diff --git a/crates/bevy_ecs/src/world/identifier.rs b/crates/bevy_ecs/src/world/identifier.rs index 810ca191826b9..dcd5308aa2880 100644 --- a/crates/bevy_ecs/src/world/identifier.rs +++ b/crates/bevy_ecs/src/world/identifier.rs @@ -1,7 +1,7 @@ use crate::{ component::Tick, storage::SparseSetIndex, - system::{ReadOnlySystemParam, SystemMeta, SystemParam}, + system::{ReadOnlySystemParam, SystemMeta, SystemParam, WorldAccessLevel}, world::{FromWorld, World}, }; use bevy_platform_support::sync::atomic::{AtomicUsize, Ordering}; @@ -64,6 +64,10 @@ unsafe impl SystemParam for WorldId { ) -> Self::Item<'world, 'state> { world.id() } + + fn world_access_level() -> WorldAccessLevel { + WorldAccessLevel::None + } } impl SparseSetIndex for WorldId { diff --git a/crates/bevy_gizmos/src/gizmos.rs b/crates/bevy_gizmos/src/gizmos.rs index b1fe363f510cb..c09b65012d9b3 100644 --- a/crates/bevy_gizmos/src/gizmos.rs +++ b/crates/bevy_gizmos/src/gizmos.rs @@ -11,7 +11,9 @@ use bevy_color::{Color, LinearRgba}; use bevy_ecs::{ component::Tick, resource::Resource, - system::{Deferred, ReadOnlySystemParam, Res, SystemBuffer, SystemMeta, SystemParam}, + system::{ + Deferred, ReadOnlySystemParam, Res, SystemBuffer, SystemMeta, SystemParam, WorldAccessLevel, + }, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; use bevy_math::{Isometry2d, Isometry3d, Vec2, Vec3}; @@ -256,6 +258,10 @@ where config_ext, } } + + fn world_access_level() -> WorldAccessLevel { + GizmosState::::world_access_level() + } } #[expect( diff --git a/crates/bevy_render/src/extract_param.rs b/crates/bevy_render/src/extract_param.rs index 6ac7079bc5f49..d647fd6bf8030 100644 --- a/crates/bevy_render/src/extract_param.rs +++ b/crates/bevy_render/src/extract_param.rs @@ -2,7 +2,10 @@ use crate::MainWorld; use bevy_ecs::{ component::Tick, prelude::*, - system::{ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamItem, SystemState}, + system::{ + ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamItem, SystemState, + WorldAccessLevel, + }, world::unsafe_world_cell::UnsafeWorldCell, }; use core::ops::{Deref, DerefMut}; @@ -120,6 +123,10 @@ where let item = state.state.get(main_world.into_inner()); Extract { item } } + + fn world_access_level() -> WorldAccessLevel { + P::world_access_level() + } } impl<'w, 's, P> Deref for Extract<'w, 's, P> From 2dbfdc88cdad1931c209d1e6d3a922f1a991b028 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Mon, 3 Feb 2025 13:43:26 -0600 Subject: [PATCH 08/10] replace todo with assumed exclusive --- crates/bevy_ecs/src/system/system_param.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index c787f3bcee5b7..ce61ac751790a 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -2825,7 +2825,8 @@ unsafe impl SystemParam for DynSystemParam<'_, '_> { } fn world_access_level() -> WorldAccessLevel { - todo!() + // TODO: Don't assume exclusive access + WorldAccessLevel::Exclusive } } From ed7762e3083c49191f8d6ca263b8c42360a94a60 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Mon, 3 Feb 2025 20:15:03 -0600 Subject: [PATCH 09/10] switch dynsystemparam to shared and block it from being used with exclusive parameters --- crates/bevy_ecs/src/system/builder.rs | 6 ++++++ crates/bevy_ecs/src/system/system_param.rs | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/builder.rs b/crates/bevy_ecs/src/system/builder.rs index c7605764e98f7..355d8fa1814cd 100644 --- a/crates/bevy_ecs/src/system/builder.rs +++ b/crates/bevy_ecs/src/system/builder.rs @@ -8,6 +8,7 @@ use crate::{ resource::Resource, system::{ DynSystemParam, DynSystemParamState, Local, ParamSet, Query, SystemMeta, SystemParam, + WorldAccessLevel, }, world::{ FilteredResources, FilteredResourcesBuilder, FilteredResourcesMut, @@ -513,6 +514,11 @@ impl<'a> DynParamBuilder<'a> { /// Creates a new [`DynParamBuilder`] by wrapping a [`SystemParamBuilder`] of any type. /// The built [`DynSystemParam`] can be downcast to `T`. pub fn new(builder: impl SystemParamBuilder + 'a) -> Self { + assert_ne!( + T::world_access_level(), + WorldAccessLevel::Exclusive, + "DynSystemParam cannot hold exclusive system parameters" + ); Self(Box::new(|world, meta| { DynSystemParamState::new::(builder.build(world, meta)) })) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index ce61ac751790a..10ea91bb8c40e 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -2825,8 +2825,11 @@ unsafe impl SystemParam for DynSystemParam<'_, '_> { } fn world_access_level() -> WorldAccessLevel { - // TODO: Don't assume exclusive access - WorldAccessLevel::Exclusive + // We don't allow users to create DynSystemParam's that hold exclusive + // params, so we can safely return Shared here. This is technically + // over-restrictive when storing `WorldAccessLevel::None` params, but we + // can't conditionally return based on the held state. + WorldAccessLevel::Shared } } From de0f644c5bd7d84293822d5cee188514ce2188d4 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Mon, 3 Feb 2025 20:22:37 -0600 Subject: [PATCH 10/10] remove unused constant --- crates/bevy_ecs/src/system/system_param.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 10ea91bb8c40e..231180b288edc 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1151,10 +1151,6 @@ unsafe impl SystemParam for &'_ World { } } -const MUT_DEFERRED_WORLD_ERROR: &str = "DeferredWorld requires exclusive access \ - to the entire world, but a previous system parameter already registered \ - access to a part of it. Allowing this would break Rust's mutability rules"; - // SAFETY: `DeferredWorld` registers exclusive access to the entire world. unsafe impl<'w> SystemParam for DeferredWorld<'w> { type State = ();