diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index be73d6c0638abd..d999401fa719ff 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -13,7 +13,7 @@ pub use system::{Query, *}; pub mod prelude { pub use crate::{ core::WorldBuilderSource, - resource::{ChangedRes, FromResources, Local, NonSend, Res, ResMut, Resource, Resources}, + resource::{FromResources, Local, NonSend, Res, ResMut, Resource, Resources}, schedule::{ ExclusiveSystemDescriptorCoercion, ParallelSystemDescriptorCoercion, ReportExecutionOrderAmbiguities, RunOnce, Schedule, Stage, State, StateStage, diff --git a/crates/bevy_ecs/src/resource/resource_query.rs b/crates/bevy_ecs/src/resource/resource_query.rs index b06c9a75e8a8fd..27c7029ca3e81b 100644 --- a/crates/bevy_ecs/src/resource/resource_query.rs +++ b/crates/bevy_ecs/src/resource/resource_query.rs @@ -8,37 +8,41 @@ use std::{ // TODO: align TypeAccess api with Query::Fetch -/// A shared borrow of a Resource -/// that will only return in a query if the Resource has been changed +/// Shared borrow of a Resource #[derive(Debug)] -pub struct ChangedRes<'a, T: Resource> { +pub struct Res<'a, T: Resource> { value: &'a T, } -impl<'a, T: Resource> ChangedRes<'a, T> { - /// Creates a reference cell to a Resource from a pointer - /// - /// # Safety - /// The pointer must have correct lifetime / storage - pub unsafe fn new(value: NonNull) -> Self { +pub struct ResFlags<'a, T: Resource> { + added: bool, + mutated: bool, + _marker: PhantomData<&'a T>, +} + +impl<'a, T: Resource> ResFlags<'a, T> { + pub fn new(added: bool, mutated: bool) -> Self { Self { - value: &*value.as_ptr(), + added, + mutated, + _marker: Default::default(), } } -} -impl<'a, T: Resource> Deref for ChangedRes<'a, T> { - type Target = T; + #[inline(always)] + pub fn added(&self) -> bool { + self.added + } - fn deref(&self) -> &T { - self.value + #[inline(always)] + pub fn mutated(&self) -> bool { + self.mutated } -} -/// Shared borrow of a Resource -#[derive(Debug)] -pub struct Res<'a, T: Resource> { - value: &'a T, + #[inline(always)] + pub fn changed(&self) -> bool { + self.added || self.mutated + } } impl<'a, T: Resource> Res<'a, T> { @@ -66,6 +70,7 @@ impl<'a, T: Resource> Deref for Res<'a, T> { pub struct ResMut<'a, T: Resource> { _marker: PhantomData<&'a T>, value: *mut T, + added: bool, mutated: *mut bool, } @@ -74,10 +79,11 @@ impl<'a, T: Resource> ResMut<'a, T> { /// /// # Safety /// The pointer must have correct lifetime / storage / ownership - pub unsafe fn new(value: NonNull, mutated: NonNull) -> Self { + pub unsafe fn new(value: NonNull, added: bool, mutated: NonNull) -> Self { Self { value: value.as_ptr(), mutated: mutated.as_ptr(), + added, _marker: Default::default(), } } @@ -100,6 +106,23 @@ impl<'a, T: Resource> DerefMut for ResMut<'a, T> { } } +impl<'a, T: Resource> ResMut<'a, T> { + #[inline(always)] + pub fn added(this: &Self) -> bool { + this.added + } + + #[inline(always)] + pub fn mutated(this: &Self) -> bool { + unsafe { *this.mutated } + } + + #[inline(always)] + pub fn changed(this: &Self) -> bool { + this.added || Self::mutated(this) + } +} + /// Local resources are unique per-system. Two instances of the same system will each have their own resource. /// Local resources are automatically initialized using the FromResources trait. #[derive(Debug)] diff --git a/crates/bevy_ecs/src/system/into_system.rs b/crates/bevy_ecs/src/system/into_system.rs index a97903e8f06aba..8d7ef5f9702a8a 100644 --- a/crates/bevy_ecs/src/system/into_system.rs +++ b/crates/bevy_ecs/src/system/into_system.rs @@ -340,8 +340,8 @@ mod tests { clear_trackers_system, resource::{Res, ResMut, Resources}, schedule::Schedule, - ChangedRes, Entity, IntoExclusiveSystem, Local, Or, Query, QuerySet, Stage, System, - SystemStage, With, World, + Entity, IntoExclusiveSystem, Local, Query, QuerySet, ResFlags, Stage, System, SystemStage, + With, World, }; #[derive(Debug, Eq, PartialEq, Default)] @@ -444,43 +444,11 @@ mod tests { #[test] fn changed_resource_system() { - fn incr_e_on_flip(_run_on_flip: ChangedRes, mut query: Query<&mut i32>) { - for mut i in query.iter_mut() { - *i += 1; + fn incr_e_on_flip(run_on_flip: ResFlags, mut query: Query<&mut i32>) { + if !run_on_flip.changed() { + return; } - } - - let mut world = World::default(); - let mut resources = Resources::default(); - resources.insert(false); - let ent = world.spawn((0,)); - - let mut schedule = Schedule::default(); - let mut update = SystemStage::parallel(); - update.add_system(incr_e_on_flip.system()); - schedule.add_stage("update", update); - schedule.add_stage( - "clear_trackers", - SystemStage::single(clear_trackers_system.exclusive_system()), - ); - - schedule.run(&mut world, &mut resources); - assert_eq!(*(world.get::(ent).unwrap()), 1); - schedule.run(&mut world, &mut resources); - assert_eq!(*(world.get::(ent).unwrap()), 1); - - *resources.get_mut::().unwrap() = true; - schedule.run(&mut world, &mut resources); - assert_eq!(*(world.get::(ent).unwrap()), 2); - } - - #[test] - fn changed_resource_or_system() { - fn incr_e_on_flip( - _or: Or<(Option>, Option>)>, - mut query: Query<&mut i32>, - ) { for mut i in query.iter_mut() { *i += 1; } @@ -489,7 +457,6 @@ mod tests { let mut world = World::default(); let mut resources = Resources::default(); resources.insert(false); - resources.insert::(10); let ent = world.spawn((0,)); let mut schedule = Schedule::default(); @@ -510,13 +477,6 @@ mod tests { *resources.get_mut::().unwrap() = true; schedule.run(&mut world, &mut resources); assert_eq!(*(world.get::(ent).unwrap()), 2); - - schedule.run(&mut world, &mut resources); - assert_eq!(*(world.get::(ent).unwrap()), 2); - - *resources.get_mut::().unwrap() = 20; - schedule.run(&mut world, &mut resources); - assert_eq!(*(world.get::(ent).unwrap()), 3); } #[test] @@ -624,14 +584,6 @@ mod tests { test_for_conflicting_resources(sys.system()) } - #[test] - #[should_panic] - fn conflicting_changed_and_mutable_resource() { - // A tempting pattern, but unsound if allowed. - fn sys(_: ResMut, _: ChangedRes) {} - test_for_conflicting_resources(sys.system()) - } - #[test] #[should_panic] fn conflicting_system_local_resources() { diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 603699be1b0fc8..c92de2a6de4fb7 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1,7 +1,7 @@ use crate::{ - ArchetypeComponent, ChangedRes, Commands, Fetch, FromResources, Local, NonSend, Or, Query, - QueryAccess, QueryFilter, QuerySet, QueryTuple, Res, ResMut, Resource, ResourceIndex, - Resources, SystemState, TypeAccess, World, WorldQuery, + ArchetypeComponent, Commands, Fetch, FromResources, Local, NonSend, Or, Query, QueryAccess, + QueryFilter, QuerySet, QueryTuple, Res, ResFlags, ResMut, Resource, ResourceIndex, Resources, + SystemState, TypeAccess, World, WorldQuery, }; use parking_lot::Mutex; use std::{any::TypeId, marker::PhantomData, sync::Arc}; @@ -179,29 +179,25 @@ impl<'a, T: Resource> FetchSystemParam<'a> for FetchRes { } } -pub struct FetchResMut(PhantomData); +pub struct FetchResFlags(PhantomData); -impl<'a, T: Resource> SystemParam for ResMut<'a, T> { - type Fetch = FetchResMut; +impl<'a, T: Resource> SystemParam for ResFlags<'a, T> { + type Fetch = FetchResFlags; } -impl<'a, T: Resource> FetchSystemParam<'a> for FetchResMut { - type Item = ResMut<'a, T>; + +impl<'a, T: Resource> FetchSystemParam<'a> for FetchResFlags { + type Item = ResFlags<'a, T>; fn init(system_state: &mut SystemState, _world: &World, _resources: &mut Resources) { - // If a system already has access to the resource in another parameter, then we fail early. - // e.g. `fn(Res, ResMut)` or `fn(ResMut, ResMut)` must not be allowed. - if system_state - .resource_access - .is_read_or_write(&TypeId::of::()) - { + if system_state.resource_access.is_write(&TypeId::of::()) { panic!( - "System `{}` has a `ResMut<{res}>` parameter that conflicts with \ - another parameter to the same `{res}` resource. `ResMut` must have unique access.", + "System `{}` has a `Res<{res}>` parameter that conflicts with \ + another parameter with mutable access to the same `{res}` resource.", system_state.name, res = std::any::type_name::() ); } - system_state.resource_access.add_write(TypeId::of::()); + system_state.resource_access.add_read(TypeId::of::()); } #[inline] @@ -210,31 +206,35 @@ impl<'a, T: Resource> FetchSystemParam<'a> for FetchResMut { _world: &'a World, resources: &'a Resources, ) -> Option { - let (value, _added, mutated) = + let (_, added, mutated) = resources.get_unsafe_ref_with_added_and_mutated::(ResourceIndex::Global); - Some(ResMut::new(value, mutated)) + Some(ResFlags::new(*added.as_ptr(), *mutated.as_ptr())) } } -pub struct FetchChangedRes(PhantomData); +pub struct FetchResMut(PhantomData); -impl<'a, T: Resource> SystemParam for ChangedRes<'a, T> { - type Fetch = FetchChangedRes; +impl<'a, T: Resource> SystemParam for ResMut<'a, T> { + type Fetch = FetchResMut; } - -impl<'a, T: Resource> FetchSystemParam<'a> for FetchChangedRes { - type Item = ChangedRes<'a, T>; +impl<'a, T: Resource> FetchSystemParam<'a> for FetchResMut { + type Item = ResMut<'a, T>; fn init(system_state: &mut SystemState, _world: &World, _resources: &mut Resources) { - if system_state.resource_access.is_write(&TypeId::of::()) { + // If a system already has access to the resource in another parameter, then we fail early. + // e.g. `fn(Res, ResMut)` or `fn(ResMut, ResMut)` must not be allowed. + if system_state + .resource_access + .is_read_or_write(&TypeId::of::()) + { panic!( - "System `{}` has a `ChangedRes<{res}>` parameter that conflicts with \ - another parameter with mutable access to the same `{res}` resource.", + "System `{}` has a `ResMut<{res}>` parameter that conflicts with \ + another parameter to the same `{res}` resource. `ResMut` must have unique access.", system_state.name, res = std::any::type_name::() ); } - system_state.resource_access.add_read(TypeId::of::()); + system_state.resource_access.add_write(TypeId::of::()); } #[inline] @@ -245,11 +245,7 @@ impl<'a, T: Resource> FetchSystemParam<'a> for FetchChangedRes { ) -> Option { let (value, added, mutated) = resources.get_unsafe_ref_with_added_and_mutated::(ResourceIndex::Global); - if *added.as_ptr() || *mutated.as_ptr() { - Some(ChangedRes::new(value)) - } else { - None - } + Some(ResMut::new(value, *added.as_ptr(), mutated)) } }