Skip to content

Commit

Permalink
Refactor ResMut/Mut/ReflectMut to remove duplicated code (#2217)
Browse files Browse the repository at this point in the history
`ResMut`, `Mut` and `ReflectMut` all share very similar code for change detection.
This PR is a first pass at refactoring these implementation and removing a lot of the duplicated code.

Note, this introduces a new trait `ChangeDetectable`.

Please feel free to comment away and let me know what you think!
  • Loading branch information
NathanSWard committed May 30, 2021
1 parent 08e5939 commit 173bb48
Show file tree
Hide file tree
Showing 9 changed files with 231 additions and 234 deletions.
169 changes: 169 additions & 0 deletions crates/bevy_ecs/src/change_detection.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
use crate::component::{Component, ComponentTicks};
use bevy_reflect::Reflect;
use std::ops::{Deref, DerefMut};

/// Types that implement reliable change detection.
///
/// ## Example
/// Using types that implement [`DetectChanges`], such as [`ResMut`], provide
/// a way to query if a value has been mutated in another system.
/// Normally change detecting is triggered by either [`DerefMut`] or [`AsMut`], however
/// it can be manually triggered via [`DetectChanges::set_changed`].
///
/// ```
/// use bevy_ecs::prelude::*;
///
/// struct MyResource(u32);
///
/// fn my_system(mut resource: ResMut<MyResource>) {
/// if resource.is_changed() {
/// println!("My resource was mutated!");
/// }
///
/// resource.0 = 42; // triggers change detection via [`DerefMut`]
/// }
/// ```
///
pub trait DetectChanges {
/// Returns true if (and only if) this value been added since the last execution of this
/// system.
fn is_added(&self) -> bool;

/// Returns true if (and only if) this value been changed since the last execution of this
/// system.
fn is_changed(&self) -> bool;

/// Manually flags this value as having been changed. This normally isn't
/// required because accessing this pointer mutably automatically flags this
/// value as "changed".
///
/// **Note**: This operation is irreversible.
fn set_changed(&mut self);
}

macro_rules! change_detection_impl {
($name:ident < $( $generics:tt ),+ >, $target:ty, $($traits:ident)?) => {
impl<$($generics),* $(: $traits)?> DetectChanges for $name<$($generics),*> {
#[inline]
fn is_added(&self) -> bool {
self.ticks
.component_ticks
.is_added(self.ticks.last_change_tick, self.ticks.change_tick)
}

#[inline]
fn is_changed(&self) -> bool {
self.ticks
.component_ticks
.is_changed(self.ticks.last_change_tick, self.ticks.change_tick)
}

#[inline]
fn set_changed(&mut self) {
self.ticks
.component_ticks
.set_changed(self.ticks.change_tick);
}
}

impl<$($generics),* $(: $traits)?> Deref for $name<$($generics),*> {
type Target = $target;

#[inline]
fn deref(&self) -> &Self::Target {
self.value
}
}

impl<$($generics),* $(: $traits)?> DerefMut for $name<$($generics),*> {
#[inline]
fn deref_mut(&mut self) -> &mut Self::Target {
self.set_changed();
self.value
}
}

impl<$($generics),* $(: $traits)?> AsRef<$target> for $name<$($generics),*> {
#[inline]
fn as_ref(&self) -> &$target {
self.deref()
}
}

impl<$($generics),* $(: $traits)?> AsMut<$target> for $name<$($generics),*> {
#[inline]
fn as_mut(&mut self) -> &mut $target {
self.deref_mut()
}
}
};
}

macro_rules! impl_into_inner {
($name:ident < $( $generics:tt ),+ >, $target:ty, $($traits:ident)?) => {
impl<$($generics),* $(: $traits)?> $name<$($generics),*> {
/// Consume `self` and return a mutable reference to the
/// contained value while marking `self` as "changed".
#[inline]
pub fn into_inner(mut self) -> &'a mut $target {
self.set_changed();
self.value
}
}
};
}

macro_rules! impl_debug {
($name:ident < $( $generics:tt ),+ >, $($traits:ident)?) => {
impl<$($generics),* $(: $traits)?> std::fmt::Debug for $name<$($generics),*>
where T: std::fmt::Debug
{
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_tuple(stringify!($name))
.field(self.value)
.finish()
}
}

};
}

pub(crate) struct Ticks<'a> {
pub(crate) component_ticks: &'a mut ComponentTicks,
pub(crate) last_change_tick: u32,
pub(crate) change_tick: u32,
}

/// Unique mutable borrow of a resource.
///
/// # Panics
///
/// Panics when used as a [`SystemParameter`](crate::system::SystemParam) if the resource does not exist.
///
/// Use `Option<ResMut<T>>` instead if the resource might not always exist.
pub struct ResMut<'a, T: Component> {
pub(crate) value: &'a mut T,
pub(crate) ticks: Ticks<'a>,
}

change_detection_impl!(ResMut<'a, T>, T, Component);
impl_into_inner!(ResMut<'a, T>, T, Component);
impl_debug!(ResMut<'a, T>, Component);

/// Unique mutable borrow of an entity's component
pub struct Mut<'a, T> {
pub(crate) value: &'a mut T,
pub(crate) ticks: Ticks<'a>,
}

change_detection_impl!(Mut<'a, T>, T,);
impl_into_inner!(Mut<'a, T>, T,);
impl_debug!(Mut<'a, T>,);

/// Unique mutable borrow of a Reflected component
pub struct ReflectMut<'a> {
pub(crate) value: &'a mut dyn Reflect,
pub(crate) ticks: Ticks<'a>,
}

change_detection_impl!(ReflectMut<'a>, dyn Reflect,);
2 changes: 2 additions & 0 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod archetype;
pub mod bundle;
pub mod change_detection;
pub mod component;
pub mod entity;
pub mod event;
Expand All @@ -18,6 +19,7 @@ pub mod prelude {
#[doc(hidden)]
pub use crate::{
bundle::Bundle,
change_detection::DetectChanges,
entity::Entity,
event::{EventReader, EventWriter},
query::{Added, ChangeTrackers, Changed, Or, QueryState, With, WithBundle, Without},
Expand Down
25 changes: 16 additions & 9 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
archetype::{Archetype, ArchetypeComponentId},
change_detection::Ticks,
component::{Component, ComponentId, ComponentTicks, StorageType},
entity::Entity,
query::{Access, FilteredAccess},
Expand Down Expand Up @@ -529,9 +530,11 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch<T> {
let table_row = *self.entity_table_rows.add(archetype_index);
Mut {
value: &mut *self.table_components.as_ptr().add(table_row),
component_ticks: &mut *self.table_ticks.add(table_row),
change_tick: self.change_tick,
last_change_tick: self.last_change_tick,
ticks: Ticks {
component_ticks: &mut *self.table_ticks.add(table_row),
change_tick: self.change_tick,
last_change_tick: self.last_change_tick,
},
}
}
StorageType::SparseSet => {
Expand All @@ -540,9 +543,11 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch<T> {
(*self.sparse_set).get_with_ticks(entity).unwrap();
Mut {
value: &mut *component.cast::<T>(),
component_ticks: &mut *component_ticks,
change_tick: self.change_tick,
last_change_tick: self.last_change_tick,
ticks: Ticks {
component_ticks: &mut *component_ticks,
change_tick: self.change_tick,
last_change_tick: self.last_change_tick,
},
}
}
}
Expand All @@ -552,9 +557,11 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch<T> {
unsafe fn table_fetch(&mut self, table_row: usize) -> Self::Item {
Mut {
value: &mut *self.table_components.as_ptr().add(table_row),
component_ticks: &mut *self.table_ticks.add(table_row),
change_tick: self.change_tick,
last_change_tick: self.last_change_tick,
ticks: Ticks {
component_ticks: &mut *self.table_ticks.add(table_row),
change_tick: self.change_tick,
last_change_tick: self.last_change_tick,
},
}
}
}
Expand Down
50 changes: 3 additions & 47 deletions crates/bevy_ecs/src/reflect.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::ops::{Deref, DerefMut};

pub use crate::change_detection::ReflectMut;
use crate::{
component::{Component, ComponentTicks},
component::Component,
entity::{Entity, EntityMap, MapEntities, MapEntitiesError},
world::{FromWorld, World},
};
Expand Down Expand Up @@ -104,56 +103,13 @@ impl<C: Component + Reflect + FromWorld> FromType<C> for ReflectComponent {
.get_unchecked_mut::<C>(world.last_change_tick(), world.read_change_tick())
.map(|c| ReflectMut {
value: c.value as &mut dyn Reflect,
component_ticks: c.component_ticks,
last_change_tick: c.last_change_tick,
change_tick: c.change_tick,
ticks: c.ticks,
})
},
}
}
}

/// Unique borrow of a Reflected component
pub struct ReflectMut<'a> {
pub(crate) value: &'a mut dyn Reflect,
pub(crate) component_ticks: &'a mut ComponentTicks,
pub(crate) last_change_tick: u32,
pub(crate) change_tick: u32,
}

impl<'a> Deref for ReflectMut<'a> {
type Target = dyn Reflect;

#[inline]
fn deref(&self) -> &dyn Reflect {
self.value
}
}

impl<'a> DerefMut for ReflectMut<'a> {
#[inline]
fn deref_mut(&mut self) -> &mut dyn Reflect {
self.component_ticks.set_changed(self.change_tick);
self.value
}
}

impl<'a> ReflectMut<'a> {
/// Returns true if (and only if) this component been added since the last execution of this
/// system.
pub fn is_added(&self) -> bool {
self.component_ticks
.is_added(self.last_change_tick, self.change_tick)
}

/// Returns true if (and only if) this component been changed since the last execution of this
/// system.
pub fn is_changed(&self) -> bool {
self.component_ticks
.is_changed(self.last_change_tick, self.change_tick)
}
}

impl_reflect_value!(Entity(Hash, PartialEq, Serialize, Deserialize));

#[derive(Clone)]
Expand Down
Loading

0 comments on commit 173bb48

Please sign in to comment.