Skip to content

Commit

Permalink
remove ReflectMut in favor of Mut<dyn Reflect> (bevyengine#5630)
Browse files Browse the repository at this point in the history
# Objective

- `ReflectMut` served no purpose that wasn't met by `Mut<dyn Reflect>` which is easier to understand since you have to deal with fewer types
- there is another `ReflectMut` type that could be confused with this one

## Solution/Changelog

- relax `T: ?Sized` bound in `Mut<T>`
- replace all instances of `ReflectMut` with `Mut<dyn Reflect>`
  • Loading branch information
jakobhellermann authored and james7132 committed Oct 28, 2022
1 parent 28cef96 commit 6c8fbd2
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 28 deletions.
15 changes: 1 addition & 14 deletions crates/bevy_ecs/src/change_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
use crate::{component::ComponentTicks, ptr::PtrMut, system::Resource};
#[cfg(feature = "bevy_reflect")]
use bevy_reflect::Reflect;
use std::ops::{Deref, DerefMut};

/// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans.
Expand Down Expand Up @@ -230,7 +229,7 @@ impl<'a, T: 'static> From<NonSendMut<'a, T>> for Mut<'a, T> {
}

/// Unique mutable borrow of an entity's component
pub struct Mut<'a, T> {
pub struct Mut<'a, T: ?Sized> {
pub(crate) value: &'a mut T,
pub(crate) ticks: Ticks<'a>,
}
Expand All @@ -239,18 +238,6 @@ 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 or resource
#[cfg(feature = "bevy_reflect")]
pub struct ReflectMut<'a> {
pub(crate) value: &'a mut dyn Reflect,
pub(crate) ticks: Ticks<'a>,
}

#[cfg(feature = "bevy_reflect")]
change_detection_impl!(ReflectMut<'a>, dyn Reflect,);
#[cfg(feature = "bevy_reflect")]
impl_into_inner!(ReflectMut<'a>, dyn Reflect,);

/// Unique mutable borrow of resources or an entity's component.
///
/// Similar to [`Mut`], but not generic over the component type, instead
Expand Down
33 changes: 19 additions & 14 deletions crates/bevy_ecs/src/reflect.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Types that enable reflection support.
pub use crate::change_detection::ReflectMut;
use crate::{
change_detection::Mut,
component::Component,
entity::{Entity, EntityMap, MapEntities, MapEntitiesError},
system::Resource,
Expand All @@ -23,7 +23,7 @@ pub struct ReflectComponent {
apply_or_insert: fn(&mut World, Entity, &dyn Reflect),
remove: fn(&mut World, Entity),
reflect: fn(&World, Entity) -> Option<&dyn Reflect>,
reflect_mut: unsafe fn(&World, Entity) -> Option<ReflectMut>,
reflect_mut: unsafe fn(&World, Entity) -> Option<Mut<dyn Reflect>>,
copy: fn(&World, &mut World, Entity, Entity),
}

Expand Down Expand Up @@ -70,7 +70,11 @@ impl ReflectComponent {
}

/// Gets the value of this [`Component`] type from the entity as a mutable reflected reference.
pub fn reflect_mut<'a>(&self, world: &'a mut World, entity: Entity) -> Option<ReflectMut<'a>> {
pub fn reflect_mut<'a>(
&self,
world: &'a mut World,
entity: Entity,
) -> Option<Mut<'a, dyn Reflect>> {
// SAFETY: unique world access
unsafe { (self.reflect_mut)(world, entity) }
}
Expand All @@ -85,7 +89,7 @@ impl ReflectComponent {
&self,
world: &'a World,
entity: Entity,
) -> Option<ReflectMut<'a>> {
) -> Option<Mut<'a, dyn Reflect>> {
(self.reflect_mut)(world, entity)
}

Expand Down Expand Up @@ -155,7 +159,7 @@ impl<C: Component + Reflect + FromWorld> FromType<C> for ReflectComponent {
world
.get_entity(entity)?
.get_unchecked_mut::<C>(world.last_change_tick(), world.read_change_tick())
.map(|c| ReflectMut {
.map(|c| Mut {
value: c.value as &mut dyn Reflect,
ticks: c.ticks,
})
Expand All @@ -176,7 +180,7 @@ pub struct ReflectResource {
apply_or_insert: fn(&mut World, &dyn Reflect),
remove: fn(&mut World),
reflect: fn(&World) -> Option<&dyn Reflect>,
reflect_unchecked_mut: unsafe fn(&World) -> Option<ReflectMut>,
reflect_unchecked_mut: unsafe fn(&World) -> Option<Mut<dyn Reflect>>,
copy: fn(&World, &mut World),
}

Expand Down Expand Up @@ -211,7 +215,7 @@ impl ReflectResource {
}

/// Gets the value of this [`Resource`] type from the world as a mutable reflected reference.
pub fn reflect_mut<'a>(&self, world: &'a mut World) -> Option<ReflectMut<'a>> {
pub fn reflect_mut<'a>(&self, world: &'a mut World) -> Option<Mut<'a, dyn Reflect>> {
// SAFETY: unique world access
unsafe { (self.reflect_unchecked_mut)(world) }
}
Expand All @@ -222,7 +226,10 @@ impl ReflectResource {
/// * Only call this method in an exclusive system to avoid sharing across threads (or use a
/// scheduler that enforces safe memory access).
/// * Don't call this method more than once in the same scope for a given [`Resource`].
pub unsafe fn reflect_unchecked_mut<'a>(&self, world: &'a World) -> Option<ReflectMut<'a>> {
pub unsafe fn reflect_unchecked_mut<'a>(
&self,
world: &'a World,
) -> Option<Mut<'a, dyn Reflect>> {
// SAFETY: caller promises to uphold uniqueness guarantees
(self.reflect_unchecked_mut)(world)
}
Expand Down Expand Up @@ -266,12 +273,10 @@ impl<C: Resource + Reflect + FromWorld> FromType<C> for ReflectResource {
// SAFETY: all usages of `reflect_unchecked_mut` guarantee that there is either a single mutable
// reference or multiple immutable ones alive at any given point
unsafe {
world
.get_resource_unchecked_mut::<C>()
.map(|res| ReflectMut {
value: res.value as &mut dyn Reflect,
ticks: res.ticks,
})
world.get_resource_unchecked_mut::<C>().map(|res| Mut {
value: res.value as &mut dyn Reflect,
ticks: res.ticks,
})
}
},
copy: |source_world, destination_world| {
Expand Down

0 comments on commit 6c8fbd2

Please sign in to comment.