Skip to content

Commit

Permalink
add MutUntyped::map_unchanged (#9194)
Browse files Browse the repository at this point in the history
### **Adopted #6430**

# Objective

`MutUntyped` is the untyped variant of `Mut<T>` that stores a `PtrMut`
instead of a `&mut T`. Working with a `MutUntyped` is a bit annoying,
because as soon you want to use the ptr e.g. as a `&mut dyn Reflect` you
cannot use a type like `Mut<dyn Reflect>` but instead need to carry
around a `&mut dyn Reflect` and a `impl FnMut()` to mark the value as
changed.
## Solution

* Provide a method `map_unchanged` to turn a `MutUntyped` into a
`Mut<T>` by mapping the `PtrMut<'a>` to a `&'a mut T`
      This can be used like this:


```rust
// SAFETY: ptr is of type `u8`
let val: Mut<u8> = mut_untyped.map_unchanged(|ptr| unsafe { ptr.deref_mut::<u8>() });

// SAFETY: from the context it is known that `ReflectFromPtr` was made for the type of the `MutUntyped`
let val: Mut<dyn Reflect> = mut_untyped.map_unchanged(|ptr| unsafe { reflect_from_ptr.as_reflect_ptr_mut(ptr) });
```

Note that nothing prevents you from doing

```rust
mut_untyped.map_unchanged(|ptr| &mut ());
```

or using any other mutable reference you can get, but IMO that is fine
since that will only result in a `Mut` that will dereference to that
value and mark the original value as changed. The lifetimes here prevent
anything bad from happening.
## Alternatives

1. Make `Ticks` public and provide a method to get construct a `Mut`
from `Ticks` and `&mut T`. More powerful and more easy to misuse.
2. Do nothing. People can still do everything they want, but they need
to pass (`&mut dyn Reflect, impl FnMut() + '_)` around instead of
`Mut<dyn Reflect>`

## Changelog

- add `MutUntyped::map_unchanged` to turn a `MutUntyped` into its typed
counterpart

---------

Co-authored-by: Jakob Hellermann <jakob.hellermann@protonmail.com>
Co-authored-by: JoJoJet <21144246+JoJoJet@users.noreply.github.com>
  • Loading branch information
3 people committed Jul 23, 2023
1 parent 630958a commit 3b1b60e
Showing 1 changed file with 68 additions and 2 deletions.
70 changes: 68 additions & 2 deletions crates/bevy_ecs/src/change_detection.rs
Expand Up @@ -735,6 +735,35 @@ impl<'a> MutUntyped<'a> {
self.value.as_ref()
}

/// Turn this [`MutUntyped`] into a [`Mut`] by mapping the inner [`PtrMut`] to another value,
/// without flagging a change.
/// This function is the untyped equivalent of [`Mut::map_unchanged`].
///
/// You should never modify the argument passed to the closure – if you want to modify the data without flagging a change, consider using [`bypass_change_detection`](DetectChangesMut::bypass_change_detection) to make your intent explicit.
///
/// If you know the type of the value you can do
/// ```no_run
/// # use bevy_ecs::change_detection::{Mut, MutUntyped};
/// # let mut_untyped: MutUntyped = unimplemented!();
/// // SAFETY: ptr is of type `u8`
/// mut_untyped.map_unchanged(|ptr| unsafe { ptr.deref_mut::<u8>() });
/// ```
/// If you have a [`ReflectFromPtr`](bevy_reflect::ReflectFromPtr) that you know belongs to this [`MutUntyped`],
/// you can do
/// ```no_run
/// # use bevy_ecs::change_detection::{Mut, MutUntyped};
/// # let mut_untyped: MutUntyped = unimplemented!();
/// # let reflect_from_ptr: bevy_reflect::ReflectFromPtr = unimplemented!();
/// // SAFETY: from the context it is known that `ReflectFromPtr` was made for the type of the `MutUntyped`
/// mut_untyped.map_unchanged(|ptr| unsafe { reflect_from_ptr.as_reflect_ptr_mut(ptr) });
/// ```
pub fn map_unchanged<T: ?Sized>(self, f: impl FnOnce(PtrMut<'a>) -> &'a mut T) -> Mut<'a, T> {
Mut {
value: f(self.value),
ticks: self.ticks,
}
}

/// Transforms this [`MutUntyped`] into a [`Mut<T>`] with the same lifetime.
///
/// # Safety
Expand Down Expand Up @@ -798,6 +827,8 @@ impl std::fmt::Debug for MutUntyped<'_> {
#[cfg(test)]
mod tests {
use bevy_ecs_macros::Resource;
use bevy_ptr::PtrMut;
use bevy_reflect::{FromType, ReflectFromPtr};

use crate::{
self as bevy_ecs,
Expand All @@ -809,8 +840,7 @@ mod tests {
world::World,
};

use super::DetectChanges;
use super::DetectChangesMut;
use super::{DetectChanges, DetectChangesMut, MutUntyped};

#[derive(Component, PartialEq)]
struct C;
Expand Down Expand Up @@ -1034,4 +1064,40 @@ mod tests {
"Resource must be changed after setting to a different value."
);
}

#[test]
fn mut_untyped_to_reflect() {
let last_run = Tick::new(2);
let this_run = Tick::new(3);
let mut component_ticks = ComponentTicks {
added: Tick::new(1),
changed: Tick::new(2),
};
let ticks = TicksMut {
added: &mut component_ticks.added,
changed: &mut component_ticks.changed,
last_run,
this_run,
};

let mut value: i32 = 5;
let value = MutUntyped {
value: PtrMut::from(&mut value),
ticks,
};

let reflect_from_ptr = <ReflectFromPtr as FromType<i32>>::from_type();

let mut new = value.map_unchanged(|ptr| {
// SAFETY: The underlying type of `ptr` matches `reflect_from_ptr`.
let value = unsafe { reflect_from_ptr.as_reflect_ptr_mut(ptr) };
value
});

assert!(!new.is_changed());

new.reflect_mut();

assert!(new.is_changed());
}
}

0 comments on commit 3b1b60e

Please sign in to comment.