Add FilteredAccess::empty and simplify the implementatin of `update…

…_component_access` for `AnyOf`/`Or` (#14352)

# Objective

- The implementation of `update_component_access` for `AnyOf`/`Or` is
kinda weird due to special casing the first filter, let's simplify it;
- Fundamentally we want to fold/reduce the various filters using an OR
operation, however in order to do a proper fold we need a neutral
element for the initial accumulator, which for OR is FALSE. However we
didn't have a way to create a `FilteredAccess` value corresponding to
FALSE and thus the only option was reducing, which special cases the
first element as being the initial accumulator.

This is an alternative to #14026

## Solution

- Introduce `FilteredAccess::empty` as a way to create a
`FilteredAccess` corresponding to the logical proposition FALSE;
- Use it as the initial accumulator for the above operations, allowing
to handle all the elements to fold in the same way.

---

## Migration Guide

- The behaviour of `AnyOf<()>` and `Or<()>` has been changed to match no
archetypes rather than all archetypes to naturally match the
corresponding logical operation. Consider replacing them with `()`
instead.

crates/bevy_ecs/src/query/access.rs

@@ -336,11 +336,7 @@ pub struct FilteredAccess<T: SparseSetIndex> {
 
 impl<T: SparseSetIndex> Default for FilteredAccess<T> {
     fn default() -> Self {
-        Self {
-            access: Access::default(),
-            required: FixedBitSet::default(),
-            filter_sets: vec![AccessFilters::default()],
-        }
+        Self::matches_everything()
     }
 }
 
@@ -353,6 +349,26 @@ impl<T: SparseSetIndex> From<FilteredAccess<T>> for FilteredAccessSet<T> {
 }
 
 impl<T: SparseSetIndex> FilteredAccess<T> {
+    /// Returns a `FilteredAccess` which has no access and matches everything.
+    /// This is the equivalent of a `TRUE` logic atom.
+    pub fn matches_everything() -> Self {
+        Self {
+            access: Access::default(),
+            required: FixedBitSet::default(),
+            filter_sets: vec![AccessFilters::default()],
+        }
+    }
+
+    /// Returns a `FilteredAccess` which has no access and matches nothing.
+    /// This is the equivalent of a `FALSE` logic atom.
+    pub fn matches_nothing() -> Self {
+        Self {
+            access: Access::default(),
+            required: FixedBitSet::default(),
+            filter_sets: Vec::new(),
+        }
+    }
+
     /// Returns a reference to the underlying unfiltered access.
     #[inline]
     pub fn access(&self) -> &Access<T> {

crates/bevy_ecs/src/query/fetch.rs

@@ -1861,30 +1861,30 @@ macro_rules! impl_anytuple_fetch {
                 )*)
             }
 
-            fn update_component_access(state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {
-                let mut _new_access = _access.clone();
-
+            fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
                 // update the filters (Or<(With<$name>,)>)
                 let ($($name,)*) = state;
-                let mut _not_first = false;
+
+                let mut _new_access = FilteredAccess::matches_nothing();
+
                 $(
-                    if _not_first {
-                        // we use an intermediate access because we only want to update the filter_sets, not the access
-                        let mut intermediate = _access.clone();
-                        $name::update_component_access($name, &mut intermediate);
-                        _new_access.append_or(&intermediate);
-                    } else {
-                        $name::update_component_access($name, &mut _new_access);
-                        _new_access.required = _access.required.clone();
-                        _not_first = true;
-                    }
+                    // Create an intermediate because `access`'s value needs to be preserved
+                    // for the next query data, and `_new_access` has to be modified only by `append_or` to it,
+                    // which only updates the `filter_sets`, not the `access`.
+                    let mut intermediate = access.clone();
+                    $name::update_component_access($name, &mut intermediate);
+                    _new_access.append_or(&intermediate);
                 )*
 
-                _access.filter_sets = _new_access.filter_sets;
+                // Of the accumulated `_new_access` we only care about the filter sets, not the access.
+                access.filter_sets = _new_access.filter_sets;
 
-                // update the access (add the read/writes)
-                // Option<T> updates the access but not the filter_sets
-                <($(Option<$name>,)*)>::update_component_access(state, _access);
+                // For the access we instead delegate to a tuple of `Option`s.
+                // This has essentially the same semantics of `AnyOf`, except that it doesn't
+                // require at least one of them to be `Some`.
+                // We however solve this by setting explicitly the `filter_sets` above.
+                // Also note that Option<T> updates the `access` but not the `filter_sets`.
+                <($(Option<$name>,)*)>::update_component_access(state, access);
 
             }
             #[allow(unused_variables)]

crates/bevy_ecs/src/query/filter.rs

@@ -443,21 +443,22 @@ macro_rules! impl_or_query_filter {
             fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
                 let ($($filter,)*) = state;
 
-                let mut _new_access = access.clone();
-                let mut _not_first = false;
+                let mut _new_access = FilteredAccess::matches_nothing();
+
                 $(
-                    if _not_first {
-                        let mut intermediate = access.clone();
-                        $filter::update_component_access($filter, &mut intermediate);
-                        _new_access.append_or(&intermediate);
-                        _new_access.extend_access(&intermediate);
-                    } else {
-                        $filter::update_component_access($filter, &mut _new_access);
-                        _new_access.required = access.required.clone();
-                        _not_first = true;
-                    }
+                    // Create an intermediate because `access`'s value needs to be preserved
+                    // for the next filter, and `_new_access` has to be modified only by `append_or` to it.
+                    let mut intermediate = access.clone();
+                    $filter::update_component_access($filter, &mut intermediate);
+                    _new_access.append_or(&intermediate);
+                    // Also extend the accesses required to compute the filter. This is required because
+                    // otherwise a `Query<(), Or<(Changed<Foo>,)>` won't conflict with `Query<&mut Foo>`.
+                    _new_access.extend_access(&intermediate);
                 )*
 
+                // The required components remain the same as the original `access`.
+                _new_access.required = std::mem::take(&mut access.required);
+
                 *access = _new_access;
             }
 

crates/bevy_ecs/src/system/mod.rs

@@ -808,6 +808,15 @@ mod tests {
         run_system(&mut world, sys);
     }
 
+    #[test]
+    #[should_panic]
+    fn changed_trackers_or_conflict() {
+        fn sys(_: Query<&mut A>, _: Query<(), Or<(Changed<A>,)>>) {}
+
+        let mut world = World::default();
+        run_system(&mut world, sys);
+    }
+
     #[test]
     fn query_set_system() {
         fn sys(mut _set: ParamSet<(Query<&mut A>, Query<&A>)>) {}