forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Get rid of ChangedRes (#1313)

Browse Source 
This replaces `ChangedRes` with simple associated methods that return the same info, but don't block execution. Also, since ChangedRes was infectious and was the only reason `FetchSystemParam::get_params` and `System::run_unsafe` returned `Option`s, their implementation could be simplified after this PR is merged, or as part of it with a future commit.
This commit is contained in:
TheRawMeatball 2021-03-03 01:59:40 +00:00
parent e035ce1f2a
commit 87ada5b589
4 changed files with 56 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/bevy_ecs/src/lib.rs
View File

@ -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,

71
crates/bevy_ecs/src/resource/resource_query.rs
View File

@ -8,37 +8,12 @@ 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
#[derive(Debug)]
pub struct ChangedRes<'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<T>) -> Self {
Self {
value: &*value.as_ptr(),
}
}
}
impl<'a, T: Resource> Deref for ChangedRes<'a, T> {
type Target = T;
fn deref(&self) -> &T {
self.value
}
}
/// Shared borrow of a Resource
#[derive(Debug)]
pub struct Res<'a, T: Resource> {
value: &'a T,
added: bool,
mutated: bool,
}
impl<'a, T: Resource> Res<'a, T> {
@ -46,9 +21,11 @@ impl<'a, T: Resource> Res<'a, T> {
///
/// # Safety
/// The pointer must have correct lifetime / storage
pub unsafe fn new(value: NonNull<T>) -> Self {
pub unsafe fn new(value: NonNull<T>, added: bool, changed: bool) -> Self {
Self {
value: &*value.as_ptr(),
added,
mutated: changed,
}
}
}
@ -61,11 +38,29 @@ impl<'a, T: Resource> Deref for Res<'a, T> {
}
}
impl<'a, T: Resource> Res<'a, T> {
#[inline(always)]
pub fn added(this: &Self) -> bool {
this.added
}
#[inline(always)]
pub fn mutated(this: &Self) -> bool {
this.mutated
}
#[inline(always)]
pub fn changed(this: &Self) -> bool {
this.added || this.mutated
}
}
/// Unique borrow of a Resource
#[derive(Debug)]
pub struct ResMut<'a, T: Resource> {
_marker: PhantomData<&'a T>,
value: *mut T,
added: bool,
mutated: *mut bool,
}
@ -74,10 +69,11 @@ impl<'a, T: Resource> ResMut<'a, T> {
///
/// # Safety
/// The pointer must have correct lifetime / storage / ownership
pub unsafe fn new(value: NonNull<T>, mutated: NonNull<bool>) -> Self {
pub unsafe fn new(value: NonNull<T>, added: bool, mutated: NonNull<bool>) -> Self {
Self {
value: value.as_ptr(),
mutated: mutated.as_ptr(),
added,
_marker: Default::default(),
}
}
@ -100,6 +96,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<T> 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)]

62
crates/bevy_ecs/src/system/into_system.rs
View File

@ -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, Stage, System, SystemStage, With,
World,
};
#[derive(Debug, Eq, PartialEq, Default)]
@ -444,7 +444,11 @@ mod tests {
#[test]
fn changed_resource_system() {
fn incr_e_on_flip(_run_on_flip: ChangedRes<bool>, mut query: Query<&mut i32>) {
fn incr_e_on_flip(run_on_flip: Res<bool>, mut query: Query<&mut i32>) {
if !Res::changed(&run_on_flip) {
return;
}
for mut i in query.iter_mut() {
*i += 1;
}
@ -475,50 +479,6 @@ mod tests {
assert_eq!(*(world.get::<i32>(ent).unwrap()), 2);
}
#[test]
fn changed_resource_or_system() {
fn incr_e_on_flip(
_or: Or<(Option<ChangedRes<bool>>, Option<ChangedRes<i32>>)>,
mut query: Query<&mut i32>,
) {
for mut i in query.iter_mut() {
*i += 1;
}
}
let mut world = World::default();
let mut resources = Resources::default();
resources.insert(false);
resources.insert::<i32>(10);
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::<i32>(ent).unwrap()), 1);
schedule.run(&mut world, &mut resources);
assert_eq!(*(world.get::<i32>(ent).unwrap()), 1);
*resources.get_mut::<bool>().unwrap() = true;
schedule.run(&mut world, &mut resources);
assert_eq!(*(world.get::<i32>(ent).unwrap()), 2);
schedule.run(&mut world, &mut resources);
assert_eq!(*(world.get::<i32>(ent).unwrap()), 2);
*resources.get_mut::<i32>().unwrap() = 20;
schedule.run(&mut world, &mut resources);
assert_eq!(*(world.get::<i32>(ent).unwrap()), 3);
}
#[test]
#[should_panic]
fn conflicting_query_mut_system() {
@ -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<BufferRes>, _: ChangedRes<BufferRes>) {}
test_for_conflicting_resources(sys.system())
}
#[test]
#[should_panic]
fn conflicting_system_local_resources() {

50
crates/bevy_ecs/src/system/system_param.rs
View File

@ -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, ResMut, Resource, ResourceIndex, Resources,
SystemState, TypeAccess, World, WorldQuery,
};
use parking_lot::Mutex;
use std::{any::TypeId, marker::PhantomData, sync::Arc};
@ -173,9 +173,8 @@ impl<'a, T: Resource> FetchSystemParam<'a> for FetchRes<T> {
_world: &'a World,
resources: &'a Resources,
) -> Option<Self::Item> {
Some(Res::new(
resources.get_unsafe_ref::<T>(ResourceIndex::Global),
))
let result = resources.get_unsafe_ref_with_added_and_mutated::<T>(ResourceIndex::Global);
Some(Res::new(result.0, *result.1.as_ptr(), *result.2.as_ptr()))
}
}
@ -204,39 +203,6 @@ impl<'a, T: Resource> FetchSystemParam<'a> for FetchResMut<T> {
system_state.resource_access.add_write(TypeId::of::<T>());
}
#[inline]
unsafe fn get_param(
_system_state: &'a SystemState,
_world: &'a World,
resources: &'a Resources,
) -> Option<Self::Item> {
let (value, _added, mutated) =
resources.get_unsafe_ref_with_added_and_mutated::<T>(ResourceIndex::Global);
Some(ResMut::new(value, mutated))
}
}
pub struct FetchChangedRes<T>(PhantomData<T>);
impl<'a, T: Resource> SystemParam for ChangedRes<'a, T> {
type Fetch = FetchChangedRes<T>;
}
impl<'a, T: Resource> FetchSystemParam<'a> for FetchChangedRes<T> {
type Item = ChangedRes<'a, T>;
fn init(system_state: &mut SystemState, _world: &World, _resources: &mut Resources) {
if system_state.resource_access.is_write(&TypeId::of::<T>()) {
panic!(
"System `{}` has a `ChangedRes<{res}>` parameter that conflicts with \
another parameter with mutable access to the same `{res}` resource.",
system_state.name,
res = std::any::type_name::<T>()
);
}
system_state.resource_access.add_read(TypeId::of::<T>());
}
#[inline]
unsafe fn get_param(
_system_state: &'a SystemState,
@ -245,11 +211,7 @@ impl<'a, T: Resource> FetchSystemParam<'a> for FetchChangedRes<T> {
) -> Option<Self::Item> {
let (value, added, mutated) =
resources.get_unsafe_ref_with_added_and_mutated::<T>(ResourceIndex::Global);
if *added.as_ptr() || *mutated.as_ptr() {
Some(ChangedRes::new(value))
} else {
None
}
Some(ResMut::new(value, *added.as_ptr(), mutated))
}
}