From 5b599d25034cdee9bb2478838b2441c02d7dc57d Mon Sep 17 00:00:00 2001 From: Jerome Humbert Date: Sun, 7 Nov 2021 20:53:07 +0000 Subject: [PATCH 1/4] Fix `Changed` docs with advantages and drawbacks Fix the documentation of the `Changed` filter to detail its mutating detection functioning, and explain the advantages and drawbacks of using it. Bug: #3082 --- crates/bevy_ecs/src/query/filter.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index bbd8ec567a985..dce0d69c6b38c 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -616,18 +616,27 @@ impl_tick_filter!( ); impl_tick_filter!( - /// Filter that retrieves components of type `T` that have been changed since the last - /// execution of this system. + /// Filter that retrieves components of type `T` that have been changed since the last tick. /// - /// This filter is useful for synchronizing components, and as a performance optimization as it - /// means that the query contains fewer items for a system to iterate over. + /// All components internally remember the last tick they were added at, and the last tick they + /// were mutated at. The mutation detection is powered by `DerefMut`, such that any dereferencing + /// will mark the component as changed. Note that there is no deep value inspection on the actual + /// data of the component, which would be prohibilively expensive. This means false positives can + /// occur if dereferencing via `DerefMut` but not changing any component data. /// - /// Because the ordering of systems can change and this filter is only effective on changes - /// before the query executes you need to use explicit dependency ordering or ordered - /// stages to avoid frame delays. + /// This filter is useful for synchronizing components. It can also be used as a performance + /// optimization in case of expensive operations, by filtering unchanged components out and + /// returning via the query only the components which changed. However, note that like all + /// filterings, applying this filtering has a small cost, which must be balanced against the + /// cost of the operation on the changed components. /// - /// If instead behavior is meant to change on whether the component changed or not - /// [`ChangeTrackers`](crate::query::ChangeTrackers) may be used. + /// Because by default the ordering of systems can change, and this filter is only effective + /// on changes detected before the query executes, to avoid frame delays you need to use + /// explicit dependency ordering or ordered stages to ensure the detecting system where the + /// query is used runs after the system(s) which mutate the component. + /// + /// To instead retrieve all components without filtering but allow querying if they changed + /// or not since last tick, you can use [`ChangeTrackers`](crate::query::ChangeTrackers). /// /// # Examples /// From f71aac030791ff6377f10720fad3fb104b91fe43 Mon Sep 17 00:00:00 2001 From: Jerome Humbert Date: Sun, 7 Nov 2021 21:03:40 +0000 Subject: [PATCH 2/4] Fix typo --- crates/bevy_ecs/src/query/filter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index dce0d69c6b38c..88a5548d6d748 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -621,7 +621,7 @@ impl_tick_filter!( /// All components internally remember the last tick they were added at, and the last tick they /// were mutated at. The mutation detection is powered by `DerefMut`, such that any dereferencing /// will mark the component as changed. Note that there is no deep value inspection on the actual - /// data of the component, which would be prohibilively expensive. This means false positives can + /// data of the component, which would be prohibitively expensive. This means false positives can /// occur if dereferencing via `DerefMut` but not changing any component data. /// /// This filter is useful for synchronizing components. It can also be used as a performance From 1dcebc3e10945d3f95e53d8fe68167c4e7f424b4 Mon Sep 17 00:00:00 2001 From: Jerome Humbert Date: Sun, 7 Nov 2021 23:08:21 +0000 Subject: [PATCH 3/4] Apply suggestion Co-authored-by: Alice Cecile --- crates/bevy_ecs/src/query/filter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 88a5548d6d748..22602e32cf305 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -630,7 +630,7 @@ impl_tick_filter!( /// filterings, applying this filtering has a small cost, which must be balanced against the /// cost of the operation on the changed components. /// - /// Because by default the ordering of systems can change, and this filter is only effective + /// Because by default the ordering of systems within the same stage is nondeterministic, and this filter is only effective /// on changes detected before the query executes, to avoid frame delays you need to use /// explicit dependency ordering or ordered stages to ensure the detecting system where the /// query is used runs after the system(s) which mutate the component. From cdf4bf0433fca20e093cd29865fc971b079eaf5b Mon Sep 17 00:00:00 2001 From: Jerome Humbert Date: Sun, 7 Nov 2021 23:09:22 +0000 Subject: [PATCH 4/4] Apply suggestion Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- crates/bevy_ecs/src/query/filter.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 22602e32cf305..2cd1b6ec17bc5 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -619,10 +619,11 @@ impl_tick_filter!( /// Filter that retrieves components of type `T` that have been changed since the last tick. /// /// All components internally remember the last tick they were added at, and the last tick they - /// were mutated at. The mutation detection is powered by `DerefMut`, such that any dereferencing + /// were mutated at. The mutation detection is powered by [`DerefMut`](std::ops::DerefMut), such that any mutable dereferencing /// will mark the component as changed. Note that there is no deep value inspection on the actual /// data of the component, which would be prohibitively expensive. This means false positives can - /// occur if dereferencing via `DerefMut` but not changing any component data. + /// occur if dereferencing via [`DerefMut`](std::ops::DerefMut) but not changing any component data. + /// Just reading the value of a component will not mark it as changed, as [`Deref`](std::ops::Deref) will be used instead. /// /// This filter is useful for synchronizing components. It can also be used as a performance /// optimization in case of expensive operations, by filtering unchanged components out and