Add set_if_neq method to DetectChanges trait (#5373)

# Objective

Change detection can be spuriously triggered by setting a field to the same value as before.
As a result, a common pattern is to write:

```rust
if *foo != value {
  *foo = value;
}
```

This is confusing to read, and heavy on boilerplate.

## Solution

1. Add a method to the `DetectChanges` trait that implements this boilerplate when the appropriate trait bounds are met.
2. Document this minor footgun, and point users to it.

## Changelog

- added the `set_if_neq` method to avoid triggering change detection when the new and previous values are equal. This will work on both components and resources.

## Migration Guide

If you are manually checking if a component or resource's value is equal to its new value before setting it to avoid triggering change detection, migrate to the clearer and more convenient `set_if_neq` method.

## Context

Related to #2363 as it avoids triggering change detection, but not a complete solution (as it still requires triggering it when real changes are made).

Author: alice-i-cecile

crates/bevy_ecs/src/change_detection.rs

@@ -31,6 +31,13 @@ pub const MAX_CHANGE_AGE: u32 = u32::MAX - (2 * CHECK_TICK_THRESHOLD - 1);
 /// Normally change detecting is triggered by either [`DerefMut`] or [`AsMut`], however
 /// it can be manually triggered via [`DetectChanges::set_changed`].
 ///
+/// To ensure that changes are only triggered when the value actually differs,
+/// check if the value would change before assignment, such as by checking that `new != old`.
+/// You must be *sure* that you are not mutably derefencing in this process.
+///
+/// [`set_if_neq`](DetectChanges::set_if_neq) is a helper
+/// method for this common functionality.
+///
 /// ```
 /// use bevy_ecs::prelude::*;
 ///
@@ -75,6 +82,24 @@ pub trait DetectChanges {
     /// [`SystemParam`](crate::system::SystemParam).
     fn last_changed(&self) -> u32;
 
+    /// Sets `self` to `value`, if and only if `*self != *value`
+    ///
+    /// `T` is the type stored within the smart pointer (e.g. [`Mut`] or [`ResMut`]).
+    ///
+    /// This is useful to ensure change detection is only triggered when the underlying value
+    /// changes, instead of every time [`DerefMut`] is used.
+    #[inline]
+    fn set_if_neq<T>(&mut self, value: T)
+    where
+        Self: Deref<Target = T> + DerefMut<Target = T>,
+        T: PartialEq,
+    {
+        // This dereference is immutable, so does not trigger change detection
+        if **self != value {
+            **self = value;
+        }
+    }
+
     /// Manually sets the change tick recording the previous time this data was mutated.
     ///
     /// # Warning
@@ -458,11 +483,13 @@ mod tests {
         world::World,
     };
 
-    #[derive(Component)]
+    use super::DetectChanges;
+
+    #[derive(Component, PartialEq)]
     struct C;
 
-    #[derive(Resource)]
-    struct R;
+    #[derive(PartialEq, Resource)]
+    struct R(u8);
 
     #[test]
     fn change_expiration() {
@@ -551,6 +578,32 @@ mod tests {
         }
     }
 
+    #[test]
+    fn set_if_neq() {
+        let mut world = World::new();
+
+        world.insert_resource(R(0));
+        // Resources are Changed when first added
+        world.increment_change_tick();
+        // This is required to update world::last_change_tick
+        world.clear_trackers();
+
+        let mut r = world.resource_mut::<R>();
+        assert!(!r.is_changed(), "Resource must begin unchanged.");
+
+        r.set_if_neq(R(0));
+        assert!(
+            !r.is_changed(),
+            "Resource must not be changed after setting to the same value."
+        );
+
+        r.set_if_neq(R(3));
+        assert!(
+            r.is_changed(),
+            "Resource must be changed after setting to a different value."
+        );
+    }
+
     #[test]
     fn mut_from_res_mut() {
         let mut component_ticks = ComponentTicks {
@@ -563,7 +616,7 @@ mod tests {
             last_change_tick: 3,
             change_tick: 4,
         };
-        let mut res = R {};
+        let mut res = R(0);
         let res_mut = ResMut {
             value: &mut res,
             ticks,
@@ -588,7 +641,7 @@ mod tests {
             last_change_tick: 3,
             change_tick: 4,
         };
-        let mut res = R {};
+        let mut res = R(0);
         let non_send_mut = NonSendMut {
             value: &mut res,
             ticks,

crates/bevy_ui/src/focus.rs

@@ -1,5 +1,6 @@
 use crate::{camera_config::UiCameraConfig, CalculatedClip, Node, UiStack};
 use bevy_ecs::{
+    change_detection::DetectChanges,
     entity::Entity,
     prelude::Component,
     query::WorldQuery,
@@ -179,10 +180,8 @@ pub fn ui_focus_system(
                     Some(*entity)
                 } else {
                     if let Some(mut interaction) = node.interaction {
-                        if *interaction == Interaction::Hovered
-                            || (cursor_position.is_none() && *interaction != Interaction::None)
-                        {
-                            *interaction = Interaction::None;
+                        if *interaction == Interaction::Hovered || (cursor_position.is_none()) {
+                            interaction.set_if_neq(Interaction::None);
                         }
                     }
                     None
@@ -227,8 +226,8 @@ pub fn ui_focus_system(
     while let Some(node) = iter.fetch_next() {
         if let Some(mut interaction) = node.interaction {
             // don't reset clicked nodes because they're handled separately
-            if *interaction != Interaction::Clicked && *interaction != Interaction::None {
-                *interaction = Interaction::None;
+            if *interaction != Interaction::Clicked {
+                interaction.set_if_neq(Interaction::None);
             }
         }
     }

examples/ecs/component_change_detection.rs

@@ -13,7 +13,7 @@ fn main() {
         .run();
 }
 
-#[derive(Component, Debug)]
+#[derive(Component, PartialEq, Debug)]
 struct MyComponent(f32);
 
 fn setup(mut commands: Commands) {
@@ -25,7 +25,12 @@ fn change_component(time: Res<Time>, mut query: Query<(Entity, &mut MyComponent)
     for (entity, mut component) in &mut query {
         if rand::thread_rng().gen_bool(0.1) {
             info!("changing component {:?}", entity);
-            component.0 = time.elapsed_seconds();
+            let new_component = MyComponent(time.seconds_since_startup().round());
+            // Change detection occurs on mutable derefence,
+            // and does not consider whether or not a value is actually equal.
+            // To avoid triggering change detection when nothing has actually changed,
+            // you can use the `set_if_neq` method on any component or resource that implements PartialEq
+            component.set_if_neq(new_component);
         }
     }
 }