Skip to content

Commit fa2ffa6

Browse files
BoxyUwUBoxyUwU
committed
rebase
1 parent fef318c commit fa2ffa6

File tree

2 files changed

+65
-103
lines changed

2 files changed

+65
-103
lines changed

‎crates/bevy_ecs/src/query/fetch.rs

+37-65
Original file line numberDiff line numberDiff line change
@@ -920,15 +920,21 @@ unsafe impl<T: WorldQuery> WorldQuery for Option<T> {
920920
}
921921

922922
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
923-
T::update_component_access(&state.state, access);
923+
// We don't want to add the `with`/`without` of `T` as `Option<T>` will match things regardless of
924+
// `T`'s filters. for example `Query<(Option<&U>, &mut V)>` will match every entity with a `V` component
925+
// regardless of whether it has a `U` component. If we dont do this the query will not conflict with
926+
// `Query<&mut V, Without<U>>` which would be unsound.
927+
let mut intermediate = access.clone();
928+
T::update_component_access(&state.state, &mut intermediate);
929+
access.extend_access(&intermediate);
924930
}
925931

926932
fn update_archetype_component_access(
927933
state: &Self::State,
928934
archetype: &Archetype,
929935
access: &mut Access<ArchetypeComponentId>,
930936
) {
931-
if state.state.matches_archetype(archetype) {
937+
if state.matches_archetype(archetype) {
932938
T::update_archetype_component_access(&state.state, archetype, access);
933939
}
934940
}
@@ -958,27 +964,6 @@ impl<T: FetchState> FetchState for OptionState<T> {
958964
}
959965
}
960966

961-
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
962-
// We don't want to add the `with`/`without` of `T` as `Option<T>` will match things regardless of
963-
// `T`'s filters. for example `Query<(Option<&U>, &mut V)>` will match every entity with a `V` component
964-
// regardless of whether it has a `U` component. If we dont do this the query will not conflict with
965-
// `Query<&mut V, Without<U>>` which would be unsound.
966-
let mut intermediate = access.clone();
967-
self.state.update_component_access(&mut intermediate);
968-
access.extend_access(&intermediate);
969-
}
970-
971-
fn update_archetype_component_access(
972-
&self,
973-
archetype: &Archetype,
974-
access: &mut Access<ArchetypeComponentId>,
975-
) {
976-
if self.state.matches_archetype(archetype) {
977-
self.state
978-
.update_archetype_component_access(archetype, access);
979-
}
980-
}
981-
982967
fn matches_archetype(&self, _archetype: &Archetype) -> bool {
983968
true
984969
}
@@ -1529,47 +1514,6 @@ macro_rules! impl_anytuple_fetch {
15291514
AnyOf(($($name::init(_world),)*))
15301515
}
15311516

1532-
fn update_component_access(&self, _access: &mut FilteredAccess<ComponentId>) {
1533-
let ($($name,)*) = &self.0;
1534-
1535-
// We do not unconditionally add `$name`'s `with`/`without` accesses to `_access`
1536-
// as this would be unsound. For example the following two queries should conflict:
1537-
// - Query<(AnyOf<(&A, ())>, &mut B)>
1538-
// - Query<&mut B, Without<A>>
1539-
//
1540-
// If we were to unconditionally add `$name`'s `with`/`without` accesses then `AnyOf<(&A, ())>`
1541-
// would have a `With<A>` access which is incorrect as this `WorldQuery` will match entities that
1542-
// do not have the `A` component. This is the same logic as the `Or<...>: WorldQuery` impl.
1543-
//
1544-
// The correct thing to do here is to only add a `with`/`without` access to `_access` if all
1545-
// `$name` params have that `with`/`without` access. More jargony put- we add the intersection
1546-
// of all `with`/`without` accesses of the `$name` params to `_access`.
1547-
let mut _intersected_access = _access.clone();
1548-
let mut _not_first = false;
1549-
$(
1550-
if _not_first {
1551-
let mut intermediate = _access.clone();
1552-
$name.update_component_access(&mut intermediate);
1553-
_intersected_access.extend_intersect_filter(&intermediate);
1554-
_intersected_access.extend_access(&intermediate);
1555-
} else {
1556-
$name.update_component_access(&mut _intersected_access);
1557-
_not_first = true;
1558-
}
1559-
)*
1560-
1561-
*_access = _intersected_access;
1562-
}
1563-
1564-
fn update_archetype_component_access(&self, _archetype: &Archetype, _access: &mut Access<ArchetypeComponentId>) {
1565-
let ($($name,)*) = &self.0;
1566-
$(
1567-
if $name.matches_archetype(_archetype) {
1568-
$name.update_archetype_component_access(_archetype, _access);
1569-
}
1570-
)*
1571-
}
1572-
15731517
fn matches_archetype(&self, _archetype: &Archetype) -> bool {
15741518
let ($($name,)*) = &self.0;
15751519
false $(|| $name.matches_archetype(_archetype))*
@@ -1597,7 +1541,35 @@ macro_rules! impl_anytuple_fetch {
15971541

15981542
fn update_component_access(state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {
15991543
let ($($name,)*) = &state.0;
1600-
$($name::update_component_access($name, _access);)*
1544+
1545+
// We do not unconditionally add `$name`'s `with`/`without` accesses to `_access`
1546+
// as this would be unsound. For example the following two queries should conflict:
1547+
// - Query<(AnyOf<(&A, ())>, &mut B)>
1548+
// - Query<&mut B, Without<A>>
1549+
//
1550+
// If we were to unconditionally add `$name`'s `with`/`without` accesses then `AnyOf<(&A, ())>`
1551+
// would have a `With<A>` access which is incorrect as this `WorldQuery` will match entities that
1552+
// do not have the `A` component. This is the same logic as the `Or<...>: WorldQuery` impl.
1553+
//
1554+
// The correct thing to do here is to only add a `with`/`without` access to `_access` if all
1555+
// `$name` params have that `with`/`without` access. More jargony put- we add the intersection
1556+
// of all `with`/`without` accesses of the `$name` params to `_access`.
1557+
let mut _intersected_access = _access.clone();
1558+
let mut _not_first = false;
1559+
$(
1560+
if _not_first {
1561+
let mut intermediate = _access.clone();
1562+
$name::update_component_access($name, &mut intermediate);
1563+
_intersected_access.extend_intersect_filter(&intermediate);
1564+
_intersected_access.extend_access(&intermediate);
1565+
} else {
1566+
1567+
$name::update_component_access($name, &mut _intersected_access);
1568+
_not_first = true;
1569+
}
1570+
)*
1571+
1572+
*_access = _intersected_access;
16011573
}
16021574

16031575
fn update_archetype_component_access(state: &Self::State, _archetype: &Archetype, _access: &mut Access<ArchetypeComponentId>) {

‎crates/bevy_ecs/src/query/filter.rs

+28-38
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,34 @@ macro_rules! impl_query_filter_tuple {
361361

362362
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
363363
let ($($filter,)*) = &state.0;
364-
$($filter::update_component_access($filter, access);)*
364+
365+
// We do not unconditionally add `$filter`'s `with`/`without` accesses to `access`
366+
// as this would be unsound. For example the following two queries should conflict:
367+
// - Query<&mut B, Or<(With<A>, ())>>
368+
// - Query<&mut B, Without<A>>
369+
//
370+
// If we were to unconditionally add `$name`'s `with`/`without` accesses then `Or<(With<A>, ())>`
371+
// would have a `With<A>` access which is incorrect as this `WorldQuery` will match entities that
372+
// do not have the `A` component. This is the same logic as the `AnyOf<...>: WorldQuery` impl.
373+
//
374+
// The correct thing to do here is to only add a `with`/`without` access to `_access` if all
375+
// `$filter` params have that `with`/`without` access. More jargony put- we add the intersection
376+
// of all `with`/`without` accesses of the `$filter` params to `access`.
377+
let mut _intersected_access = access.clone();
378+
let mut _not_first = false;
379+
$(
380+
if _not_first {
381+
let mut intermediate = access.clone();
382+
$filter::update_component_access($filter, &mut intermediate);
383+
_intersected_access.extend_intersect_filter(&intermediate);
384+
_intersected_access.extend_access(&intermediate);
385+
} else {
386+
$filter::update_component_access($filter, &mut _intersected_access);
387+
_not_first = true;
388+
}
389+
)*
390+
391+
*access = _intersected_access;
365392
}
366393

367394
fn update_archetype_component_access(state: &Self::State, archetype: &Archetype, access: &mut Access<ArchetypeComponentId>) {
@@ -450,43 +477,6 @@ macro_rules! impl_query_filter_tuple {
450477
Or(($($filter::init(world),)*))
451478
}
452479

453-
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
454-
let ($($filter,)*) = &self.0;
455-
456-
// We do not unconditionally add `$filter`'s `with`/`without` accesses to `access`
457-
// as this would be unsound. For example the following two queries should conflict:
458-
// - Query<&mut B, Or<(With<A>, ())>>
459-
// - Query<&mut B, Without<A>>
460-
//
461-
// If we were to unconditionally add `$name`'s `with`/`without` accesses then `Or<(With<A>, ())>`
462-
// would have a `With<A>` access which is incorrect as this `WorldQuery` will match entities that
463-
// do not have the `A` component. This is the same logic as the `AnyOf<...>: WorldQuery` impl.
464-
//
465-
// The correct thing to do here is to only add a `with`/`without` access to `_access` if all
466-
// `$filter` params have that `with`/`without` access. More jargony put- we add the intersection
467-
// of all `with`/`without` accesses of the `$filter` params to `access`.
468-
let mut _intersected_access = access.clone();
469-
let mut _not_first = false;
470-
$(
471-
if _not_first {
472-
let mut intermediate = access.clone();
473-
$filter.update_component_access(&mut intermediate);
474-
_intersected_access.extend_intersect_filter(&intermediate);
475-
_intersected_access.extend_access(&intermediate);
476-
} else {
477-
$filter.update_component_access(&mut _intersected_access);
478-
_not_first = true;
479-
}
480-
)*
481-
482-
*access = _intersected_access;
483-
}
484-
485-
fn update_archetype_component_access(&self, archetype: &Archetype, access: &mut Access<ArchetypeComponentId>) {
486-
let ($($filter,)*) = &self.0;
487-
$($filter.update_archetype_component_access(archetype, access);)*
488-
}
489-
490480
fn matches_archetype(&self, archetype: &Archetype) -> bool {
491481
let ($($filter,)*) = &self.0;
492482
false $(|| $filter.matches_archetype(archetype))*

0 commit comments

Comments
 (0)