Skip to content

Commit

Permalink
add more flexible version of set_if_neq
Browse files Browse the repository at this point in the history
In one of my personal projects I stumbled across a case where I wanted
to be cautious with the change detection while also not sacraficing any
performance.

My use case: I basically have a horizontal surface which is saved as a
collection of `Vec<Vec2>` in a component. The component also includes a
height field. Now when the height is updated there only have two (three)
options:

- The collection of points can be cloned so the existing `set_if_neq` is
  usable
- The `set_if_neq` code can be inlined and adjusted for this special case
- (The component can be split up into two more granular components)

I have to acknowledge that the third point here is probably the way to
go in most cases, but I was curious:

- if it would be possible to have a more flexible API and how that would
  look like
- if you are interested in this change in general

No hard feelings if we just end up rejecting this PR. I'm also totally
fine with this. But maybe it's useful to anyone and I just wanted to
hear your feedback first before tossing this back into the void and
forgetting about it.
  • Loading branch information
RobWalt committed Apr 10, 2024
1 parent 11817f4 commit 71b90f5
Showing 1 changed file with 154 additions and 0 deletions.
154 changes: 154 additions & 0 deletions crates/bevy_ecs/src/change_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,71 @@ pub trait DetectChangesMut: DetectChanges {
}
}

/// Overwrites this smart pointer with the given value, if and only if
/// `*accessor_f(self) != value`.
/// Returns `true` if the value was overwritten, and returns `false` if it was not.
///
/// This is basically the less restrictive version of
/// [`set_if_neq`](DetectChangesMut::set_if_neq) if you can't compare the value directly.
/// Nevertheless [`set_if_neq`](DetectChangesMut::set_if_neq) should be preferred if there is
/// no need for accessing fields of the component!
///
/// # Examples
///
/// The following example highlights the use case. It would be quiet wasteful to recreate or
/// clone the old `text` field if you just want to update `read_by` value.
///
/// ```
/// # use bevy_ecs::{prelude::*, schedule::common_conditions::resource_changed};
/// #[derive(Resource, PartialEq, Eq)]
/// pub struct Description {
/// text: String,
/// read_by: usize,
/// };
///
/// fn reset_description(mut description: ResMut<Description>) {
/// // Set the description to zero, unless it is already zero.
/// description.set_if_neq_with(|d| &mut d.read_by, 0);
/// }
/// # let mut world = World::new();
/// # world.insert_resource(Description {
/// # text: String::from("Hello, World!"),
/// # read_by: 2,
/// # });
/// # let mut description_changed = IntoSystem::into_system(resource_changed::<Description>);
/// # description_changed.initialize(&mut world);
/// # description_changed.run((), &mut world);
/// #
/// # let mut schedule = Schedule::default();
/// # schedule.add_systems(reset_description);
/// #
/// # // first time `reset_description` runs, the description is changed.
/// # schedule.run(&mut world);
/// # assert!(description_changed.run((), &mut world));
/// # // second time `reset_description` runs, the description is not changed.
/// # schedule.run(&mut world);
/// # assert!(!description_changed.run((), &mut world));
/// ```
#[inline]
fn set_if_neq_with<T>(
&mut self,
accessor_f: impl Fn(&mut Self::Inner) -> &mut T,
value: T,
) -> bool
where
T: PartialEq,
{
let old = self.bypass_change_detection();
let field = accessor_f(old);
if *field != value {
*field = value;
self.set_changed();
true
} else {
false
}
}

/// Overwrites this smart pointer with the given value, if and only if `*self != value`,
/// returning the previous value if this occurs.
///
Expand Down Expand Up @@ -245,6 +310,95 @@ pub trait DetectChangesMut: DetectChanges {
None
}
}

/// Overwrites this smart pointer with the given value, if and only if
/// `*accessor_f(self) != value`,
/// returning the previous value if this occurs.
///
/// This is basically the less restrictive version of
/// [`replace_if_neq`](DetectChangesMut::replace_if_neq) if you can't compare the value
/// directly. Nevertheless [`replace_if_neq`](DetectChangesMut::replace_if_neq) should be
/// preferred if there is no need for accessing fields of the component!
///
/// If you don't need the previous value, use [`set_if_neq_with`](DetectChangesMut::set_if_neq_with).
///
/// # Examples
///
/// ```
/// # use bevy_ecs::{prelude::*, schedule::common_conditions::{resource_changed, on_event}};
/// #[derive(Resource, PartialEq, Eq)]
/// pub struct Description {
/// text: String,
/// read_by: usize,
/// };
///
/// #[derive(Event, PartialEq, Eq)]
/// pub struct DescriptionChanged {
/// current_read_by: usize,
/// previous_read_by: usize,
/// }
///
/// fn reset_description(mut description: ResMut<Description>, mut description_changed: EventWriter<DescriptionChanged>) {
/// // Set the description read_by to zero, unless it is already zero.
/// let new_read_by = 0;
/// if let Some(previous_read_by) = description.replace_if_neq_with(
/// |d| &mut d.read_by,
/// new_read_by
/// ) {
/// // If `description` change, emit a `DescriptionChanged` event.
/// description_changed.send(DescriptionChanged {
/// current_read_by: new_read_by,
/// previous_read_by: previous_read_by,
/// });
/// }
/// }
/// # let mut world = World::new();
/// # world.insert_resource(Events::<DescriptionChanged>::default());
/// # world.insert_resource(Description {
/// # text: String::from("Hello, World!"),
/// # read_by: 3,
/// # });
/// # let mut description_changed = IntoSystem::into_system(resource_changed::<Description>);
/// # description_changed.initialize(&mut world);
/// # description_changed.run((), &mut world);
/// #
/// # let mut description_changed_event = IntoSystem::into_system(on_event::<DescriptionChanged>());
/// # description_changed_event.initialize(&mut world);
/// # description_changed_event.run((), &mut world);
/// #
/// # let mut schedule = Schedule::default();
/// # schedule.add_systems(reset_description);
/// #
/// # // first time `reset_description` runs, the description is changed.
/// # schedule.run(&mut world);
/// # assert!(description_changed.run((), &mut world));
/// # assert!(description_changed_event.run((), &mut world));
/// # // second time `reset_description` runs, the description is not changed.
/// # schedule.run(&mut world);
/// # assert!(!description_changed.run((), &mut world));
/// # assert!(!description_changed_event.run((), &mut world));
/// ```
#[inline]
#[must_use = "If you don't need to handle the previous value, use `set_if_neq_with` instead."]
fn replace_if_neq_with<T>(
&mut self,
accessor_f: impl Fn(&mut Self::Inner) -> &mut T,
value: T,
) -> Option<T>
where
T: PartialEq,
Self::Inner: Sized,
{
let old = self.bypass_change_detection();
let field = accessor_f(old);
if *field != value {
let previous = mem::replace(field, value);
self.set_changed();
Some(previous)
} else {
None
}
}
}

macro_rules! change_detection_impl {
Expand Down

0 comments on commit 71b90f5

Please sign in to comment.