diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index 27a8c9532e660..f78283cba0bb9 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -336,11 +336,7 @@ pub struct FilteredAccess { impl Default for FilteredAccess { fn default() -> Self { - Self { - access: Access::default(), - required: FixedBitSet::default(), - filter_sets: vec![AccessFilters::default()], - } + Self::matches_everything() } } @@ -353,6 +349,26 @@ impl From> for FilteredAccessSet { } impl FilteredAccess { + /// 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 { diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 08bcbbe27bf25..b761661bb6df3 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/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) { - let mut _new_access = _access.clone(); - + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { // 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 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 updates the `access` but not the `filter_sets`. + <($(Option<$name>,)*)>::update_component_access(state, access); } #[allow(unused_variables)] diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 316aed862bc44..0e4d9cc405097 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/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) { 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,)>` 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; } diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index d2cf03a96eda0..071e02f9d3ca4 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/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,)>>) {} + + 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>)>) {}