Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FilteredAccess::empty and simplify the implementatin of update_component_access for AnyOf/Or #14352

Merged
merged 8 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions crates/bevy_ecs/src/query/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand All @@ -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.
SkiFire13 marked this conversation as resolved.
Show resolved Hide resolved
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> {
Expand Down
36 changes: 18 additions & 18 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
25 changes: 13 additions & 12 deletions crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
SkiFire13 marked this conversation as resolved.
Show resolved Hide resolved
)*

// The required components remain the same as the original `access`.
_new_access.required = std::mem::take(&mut access.required);

*access = _new_access;
}

Expand Down
9 changes: 9 additions & 0 deletions crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>)>) {}
Expand Down