Skip to content

Commit

Permalink
merge matches_archetype and matches_table (#4807)
Browse files Browse the repository at this point in the history
# Objective

the code in these fns are always identical so stop having two functions

## Solution

make them the same function

---

## Changelog

change `matches_archetype` and `matches_table` to `fn matches_component_set(&self, &SparseArray<ComponentId, usize>) -> bool` then do extremely boring updating of all `FetchState` impls

## Migration Guide

- move logic of `matches_archetype` and `matches_table` into `matches_component_set` in any manual `FetchState` impls
  • Loading branch information
BoxyUwU committed May 30, 2022
1 parent 2f5591f commit e528b63
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 88 deletions.
7 changes: 2 additions & 5 deletions crates/bevy_ecs/macros/src/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,9 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
#(self.#field_idents.update_archetype_component_access(_archetype, _access);)*
}

fn matches_archetype(&self, _archetype: &#path::archetype::Archetype) -> bool {
true #(&& self.#field_idents.matches_archetype(_archetype))*
}
fn matches_component_set(&self, _set_contains_id: &impl Fn(#path::component::ComponentId) -> bool) -> bool {
true #(&& self.#field_idents.matches_component_set(_set_contains_id))*

fn matches_table(&self, _table: &#path::storage::Table) -> bool {
true #(&& self.#field_idents.matches_table(_table))*
}
}
};
Expand Down
80 changes: 27 additions & 53 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ pub trait Fetch<'world>: Sized {
///
/// Implementor must ensure that [`FetchState::update_component_access`] and
/// [`FetchState::update_archetype_component_access`] exactly reflects the results of
/// [`FetchState::matches_archetype`], [`FetchState::matches_table`], [`Fetch::archetype_fetch`], and
/// [`FetchState::matches_component_set`], [`Fetch::archetype_fetch`], and
/// [`Fetch::table_fetch`].
pub unsafe trait FetchState: Send + Sync + Sized {
fn init(world: &mut World) -> Self;
Expand All @@ -431,8 +431,7 @@ pub unsafe trait FetchState: Send + Sync + Sized {
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
);
fn matches_archetype(&self, archetype: &Archetype) -> bool;
fn matches_table(&self, table: &Table) -> bool;
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool;
}

/// A fetch that is read only.
Expand Down Expand Up @@ -480,12 +479,7 @@ unsafe impl FetchState for EntityState {
}

#[inline]
fn matches_archetype(&self, _archetype: &Archetype) -> bool {
true
}

#[inline]
fn matches_table(&self, _table: &Table) -> bool {
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
true
}
}
Expand Down Expand Up @@ -588,12 +582,8 @@ unsafe impl<T: Component> FetchState for ReadState<T> {
}
}

fn matches_archetype(&self, archetype: &Archetype) -> bool {
archetype.contains(self.component_id)
}

fn matches_table(&self, table: &Table) -> bool {
table.has_column(self.component_id)
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
}

Expand Down Expand Up @@ -825,12 +815,8 @@ unsafe impl<T: Component> FetchState for WriteState<T> {
}
}

fn matches_archetype(&self, archetype: &Archetype) -> bool {
archetype.contains(self.component_id)
}

fn matches_table(&self, table: &Table) -> bool {
table.has_column(self.component_id)
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
}

Expand Down Expand Up @@ -1105,17 +1091,16 @@ unsafe impl<T: FetchState> FetchState for OptionState<T> {
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if self.state.matches_archetype(archetype) {
if self
.state
.matches_component_set(&|id| archetype.contains(id))
{
self.state
.update_archetype_component_access(archetype, access);
}
}

fn matches_archetype(&self, _archetype: &Archetype) -> bool {
true
}

fn matches_table(&self, _table: &Table) -> bool {
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
true
}
}
Expand Down Expand Up @@ -1153,15 +1138,19 @@ impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch<T> {
archetype: &'w Archetype,
tables: &'w Tables,
) {
self.matches = state.state.matches_archetype(archetype);
self.matches = state
.state
.matches_component_set(&|id| archetype.contains(id));
if self.matches {
self.fetch.set_archetype(&state.state, archetype, tables);
}
}

#[inline]
unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) {
self.matches = state.state.matches_table(table);
self.matches = state
.state
.matches_component_set(&|id| table.has_column(id));
if self.matches {
self.fetch.set_table(&state.state, table);
}
Expand Down Expand Up @@ -1297,12 +1286,8 @@ unsafe impl<T: Component> FetchState for ChangeTrackersState<T> {
}
}

fn matches_archetype(&self, archetype: &Archetype) -> bool {
archetype.contains(self.component_id)
}

fn matches_table(&self, table: &Table) -> bool {
table.has_column(self.component_id)
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
}

Expand Down Expand Up @@ -1551,14 +1536,9 @@ macro_rules! impl_tuple_fetch {
$($name.update_archetype_component_access(_archetype, _access);)*
}

fn matches_archetype(&self, _archetype: &Archetype) -> bool {
let ($($name,)*) = self;
true $(&& $name.matches_archetype(_archetype))*
}

fn matches_table(&self, _table: &Table) -> bool {
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
let ($($name,)*) = self;
true $(&& $name.matches_table(_table))*
true $(&& $name.matches_component_set(_set_contains_id))*
}
}

Expand Down Expand Up @@ -1618,7 +1598,7 @@ macro_rules! impl_anytuple_fetch {
let ($($name,)*) = &mut self.0;
let ($($state,)*) = &_state.0;
$(
$name.1 = $state.matches_archetype(_archetype);
$name.1 = $state.matches_component_set(&|id| _archetype.contains(id));
if $name.1 {
$name.0.set_archetype($state, _archetype, _tables);
}
Expand All @@ -1630,7 +1610,7 @@ macro_rules! impl_anytuple_fetch {
let ($($name,)*) = &mut self.0;
let ($($state,)*) = &_state.0;
$(
$name.1 = $state.matches_table(_table);
$name.1 = $state.matches_component_set(&|id| _table.has_column(id));
if $name.1 {
$name.0.set_table($state, _table);
}
Expand Down Expand Up @@ -1699,20 +1679,14 @@ macro_rules! impl_anytuple_fetch {
fn update_archetype_component_access(&self, _archetype: &Archetype, _access: &mut Access<ArchetypeComponentId>) {
let ($($name,)*) = &self.0;
$(
if $name.matches_archetype(_archetype) {
if $name.matches_component_set(&|id| _archetype.contains(id)) {
$name.update_archetype_component_access(_archetype, _access);
}
)*
}

fn matches_archetype(&self, _archetype: &Archetype) -> bool {
let ($($name,)*) = &self.0;
false $(|| $name.matches_archetype(_archetype))*
}

fn matches_table(&self, _table: &Table) -> bool {
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
let ($($name,)*) = &self.0;
false $(|| $name.matches_table(_table))*
false $(|| $name.matches_component_set(_set_contains_id))*
}
}

Expand Down
38 changes: 10 additions & 28 deletions crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,8 @@ unsafe impl<T: Component> FetchState for WithState<T> {
) {
}

fn matches_archetype(&self, archetype: &Archetype) -> bool {
archetype.contains(self.component_id)
}

fn matches_table(&self, table: &Table) -> bool {
table.has_column(self.component_id)
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
}

Expand Down Expand Up @@ -232,13 +228,8 @@ unsafe impl<T: Component> FetchState for WithoutState<T> {
_access: &mut Access<ArchetypeComponentId>,
) {
}

fn matches_archetype(&self, archetype: &Archetype) -> bool {
!archetype.contains(self.component_id)
}

fn matches_table(&self, table: &Table) -> bool {
!table.has_column(self.component_id)
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
!set_contains_id(self.component_id)
}
}

Expand Down Expand Up @@ -390,7 +381,7 @@ macro_rules! impl_query_filter_tuple {
let ($($filter,)*) = &mut self.0;
let ($($state,)*) = &state.0;
$(
$filter.matches = $state.matches_table(table);
$filter.matches = $state.matches_component_set(&|id| table.has_column(id));
if $filter.matches {
$filter.fetch.set_table($state, table);
}
Expand All @@ -402,7 +393,7 @@ macro_rules! impl_query_filter_tuple {
let ($($filter,)*) = &mut self.0;
let ($($state,)*) = &state.0;
$(
$filter.matches = $state.matches_archetype(archetype);
$filter.matches = $state.matches_component_set(&|id| archetype.contains(id));
if $filter.matches {
$filter.fetch.set_archetype($state, archetype, tables);
}
Expand Down Expand Up @@ -477,14 +468,9 @@ macro_rules! impl_query_filter_tuple {
$($filter.update_archetype_component_access(archetype, access);)*
}

fn matches_archetype(&self, archetype: &Archetype) -> bool {
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
let ($($filter,)*) = &self.0;
false $(|| $filter.matches_archetype(archetype))*
}

fn matches_table(&self, table: &Table) -> bool {
let ($($filter,)*) = &self.0;
false $(|| $filter.matches_table(table))*
false $(|| $filter.matches_component_set(_set_contains_id))*
}
}

Expand Down Expand Up @@ -564,12 +550,8 @@ macro_rules! impl_tick_filter {
}
}

fn matches_archetype(&self, archetype: &Archetype) -> bool {
archetype.contains(self.component_id)
}

fn matches_table(&self, table: &Table) -> bool {
table.has_column(self.component_id)
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
}

Expand Down
8 changes: 6 additions & 2 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,12 @@ impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {

/// Creates a new [`Archetype`].
pub fn new_archetype(&mut self, archetype: &Archetype) {
if self.fetch_state.matches_archetype(archetype)
&& self.filter_state.matches_archetype(archetype)
if self
.fetch_state
.matches_component_set(&|id| archetype.contains(id))
&& self
.filter_state
.matches_component_set(&|id| archetype.contains(id))
{
self.fetch_state
.update_archetype_component_access(archetype, &mut self.archetype_component_access);
Expand Down

0 comments on commit e528b63

Please sign in to comment.