From e7ab65675da1ce8018b27054e8315c799dde922b Mon Sep 17 00:00:00 2001
From: Robert Walter <26892280+RobWalt@users.noreply.github.com>
Date: Mon, 15 Apr 2024 12:38:38 +0000
Subject: [PATCH] Update docs of `set_if_neq` and `replace_if_neq` (#12919)

# Objective

- ~~This PR adds more flexible versions of `set_if_neq` and
`replace_if_neq` to only compare and update certain fields of a
components which is not just a newtype~~
- https://github.com/bevyengine/bevy/pull/12919#issuecomment-2048049786
gave a good solution to the original problem, so let's update the docs
so that this is easier to find

## Solution

- ~~Add `set_if_neq_with` and `replace_if_neq_with` which take an
accessor closure to access the relevant field~~

---

In a recent project, a scenario emerged that required careful
consideration regarding change detection without compromising
performance. The context involves a component that maintains a
collection of `Vec<Vec2>` representing a horizontal surface, alongside a
height field. When the height is updated, there are a few approaches to
consider:

1. Clone the collection of points to utilize the existing `set_if_neq`
method.
2. Inline and adjust the `set_if_neq` code specifically for this
scenario.
3. (Consider splitting the component into more granular components.)

It's worth noting that the third option might be the most suitable in
most cases.

A similar situation arises with the Bevy internal Transform component,
which includes fields for translation, rotation, and scale. These fields
are relatively small (`Vec3` or `Quat` with 3 or 4 `f32` values), but
the creation of a single pointer (`usize`) might be more efficient than
copying the data of the other fields. This is speculative, and insights
from others could be valuable.

Questions remain:

- Is it feasible to develop a more flexible API, and what might that
entail?
- Is there general interest in this change?

There's no hard feelings if this idea or the PR is ultimately rejected.
I just wanted to put this idea out there and hope that this might be
beneficial to others and that feedback could be valuable before
abandoning the idea.
---
 crates/bevy_ecs/src/change_detection.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs
index f0b863fc86..d4cc1a73af 100644
--- a/crates/bevy_ecs/src/change_detection.rs
+++ b/crates/bevy_ecs/src/change_detection.rs
@@ -131,6 +131,12 @@ pub trait DetectChangesMut: DetectChanges {
     /// This is useful to ensure change detection is only triggered when the underlying value
     /// changes, instead of every time it is mutably accessed.
     ///
+    /// If you're dealing with non-trivial structs which have multiple fields of non-trivial size,
+    /// then consider applying a `map_unchanged` beforehand to allow changing only the relevant
+    /// field and prevent unnecessary copying and cloning.
+    /// See the docs of [`Mut::map_unchanged`], [`MutUntyped::map_unchanged`],
+    /// [`ResMut::map_unchanged`] or [`NonSendMut::map_unchanged`] for an example
+    ///
     /// If you need the previous value, use [`replace_if_neq`](DetectChangesMut::replace_if_neq).
     ///
     /// # Examples
@@ -181,6 +187,12 @@ pub trait DetectChangesMut: DetectChanges {
     /// This is useful to ensure change detection is only triggered when the underlying value
     /// changes, instead of every time it is mutably accessed.
     ///
+    /// If you're dealing with non-trivial structs which have multiple fields of non-trivial size,
+    /// then consider applying a [`map_unchanged`](Mut::map_unchanged) beforehand to allow
+    /// changing only the relevant field and prevent unnecessary copying and cloning.
+    /// See the docs of [`Mut::map_unchanged`], [`MutUntyped::map_unchanged`],
+    /// [`ResMut::map_unchanged`] or [`NonSendMut::map_unchanged`] for an example
+    ///
     /// If you don't need the previous value, use [`set_if_neq`](DetectChangesMut::set_if_neq).
     ///
     /// # Examples