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 https://github.com/bevyengine/bevy/pull/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.
This commit is contained in:
Giacomo Stevanato 2024-07-30 01:20:06 +02:00 committed by GitHub
parent 71c5f1e3e4
commit 7de271f992
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 61 additions and 35 deletions

View File

@ -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> {

View File

@ -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)]

View File

@ -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;
}

View File

@ -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>)>) {}