Replace ReadOnlyFetch with ReadOnlyWorldQuery (#4626)

# Objective

- Fix a type inference regression introduced by #3001
- Make read only bounds on world queries more user friendly

ptrification required you to write `Q::Fetch: ReadOnlyFetch` as `for<'w> QueryFetch<'w, Q>: ReadOnlyFetch` which has the same type inference problem as `for<'w> QueryFetch<'w, Q>: FilterFetch<'w>` had, i.e. the following code would error:
```rust
#[derive(Component)]
struct Foo;

fn bar(a: Query<(&Foo, Without<Foo>)>) {
    foo(a);
}

fn foo<Q: WorldQuery>(a: Query<Q, ()>)
where
    for<'w> QueryFetch<'w, Q>: ReadOnlyFetch,
{
}
```
`for<..>` bounds are also rather user unfriendly..

## Solution

Remove the `ReadOnlyFetch` trait in favour of a `ReadOnlyWorldQuery` trait, and remove `WorldQueryGats::ReadOnlyFetch` in favor of `WorldQuery::ReadOnly` allowing the previous code snippet to be written as:
```rust
#[derive(Component)]
struct Foo;

fn bar(a: Query<(&Foo, Without<Foo>)>) {
    foo(a);
}

fn foo<Q: ReadOnlyWorldQuery>(a: Query<Q, ()>) {}
``` 
This avoids the `for<...>` bound which makes the code simpler and also fixes the type inference issue.

The reason for moving the two functions out of `FetchState` and into `WorldQuery` is to allow the world query `&mut T` to share a `State` with the `&T` world query so that it can have `type ReadOnly = &T`. Presumably it would be possible to instead have a `ReadOnlyRefMut<T>` world query and then do `type ReadOnly = ReadOnlyRefMut<T>` much like how (before this PR) we had a `ReadOnlyWriteFetch<T>`. A side benefit of the current solution in this PR is that it will likely make it easier in the future to support an API such as `Query<&mut T> -> Query<&T>`. The primary benefit IMO is just that `ReadOnlyRefMut<T>` and its associated fetch would have to reimplement all of the logic that the `&T` world query impl does but this solution avoids that :)

---

## Changelog/Migration Guide

The trait `ReadOnlyFetch` has been replaced with `ReadOnlyWorldQuery` along with the `WorldQueryGats::ReadOnlyFetch` assoc type which has been replaced with `<WorldQuery::ReadOnly as WorldQueryGats>::Fetch`
- Any where clauses such as `QueryFetch<Q>: ReadOnlyFetch` should be replaced with `Q: ReadOnlyWorldQuery`.
- Any custom world query impls should implement `ReadOnlyWorldQuery` insead of `ReadOnlyFetch`

Functions `update_component_access` and `update_archetype_component_access` have been moved from the `FetchState` trait to `WorldQuery`
- Any callers should now call `Q::update_component_access(state` instead of `state.update_component_access` (and `update_archetype_component_access` respectively)
- Any custom world query impls should move the functions from the `FetchState` impl to `WorldQuery` impl

`WorldQuery` has been made an `unsafe trait`, `FetchState` has been made a safe `trait`. (I think this is how it should have always been, but regardless this is _definitely_ necessary now that the two functions have been moved to `WorldQuery`)
- If you have a custom `FetchState` impl make it a normal `impl` instead of `unsafe impl`
- If you have a custom `WorldQuery` impl make it an `unsafe impl`, if your code was sound before it is going to still be sound
This commit is contained in:
Boxy 2022-06-13 23:35:54 +00:00
parent 4050c8aa31
commit 407c080e59
9 changed files with 442 additions and 480 deletions

View file

@ -90,6 +90,11 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
user_generics_with_world.split_for_impl();
let struct_name = ast.ident.clone();
let read_only_struct_name = if fetch_struct_attributes.is_mutable {
Ident::new(&format!("{}ReadOnly", struct_name), Span::call_site())
} else {
struct_name.clone()
};
let item_struct_name = Ident::new(&format!("{}Item", struct_name), Span::call_site());
let read_only_item_struct_name = if fetch_struct_attributes.is_mutable {
@ -178,7 +183,8 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
#(#ignored_field_idents: #ignored_field_types,)*
}
impl #user_impl_generics_with_world #path::query::Fetch<'__w>
// SAFETY: `update_component_access` and `update_archetype_component_access` are called on every field
unsafe impl #user_impl_generics_with_world #path::query::Fetch<'__w>
for #fetch_struct_name #user_ty_generics_with_world #user_where_clauses_with_world {
type Item = #item_struct_name #user_ty_generics_with_world;
@ -248,6 +254,17 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
unsafe fn archetype_filter_fetch(&mut self, _archetype_index: usize) -> bool {
true #(&& self.#field_idents.archetype_filter_fetch(_archetype_index))*
}
fn update_component_access(state: &Self::State, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) {
#( #path::query::#fetch_type_alias::<'static, #field_types> :: update_component_access(&state.#field_idents, _access); )*
}
fn update_archetype_component_access(state: &Self::State, _archetype: &#path::archetype::Archetype, _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>) {
#(
#path::query::#fetch_type_alias::<'static, #field_types>
:: update_archetype_component_access(&state.#field_idents, _archetype, _access);
)*
}
}
}
};
@ -262,8 +279,7 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
#(#ignored_field_idents: #ignored_field_types,)*
}
// SAFETY: `update_component_access` and `update_archetype_component_access` are called for each item in the struct
unsafe impl #user_impl_generics #path::query::FetchState for #state_struct_name #user_ty_generics #user_where_clauses {
impl #user_impl_generics #path::query::FetchState for #state_struct_name #user_ty_generics #user_where_clauses {
fn init(world: &mut #path::world::World) -> Self {
#state_struct_name {
#(#field_idents: <<#field_types as #path::query::WorldQuery>::State as #path::query::FetchState>::init(world),)*
@ -271,14 +287,6 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
}
}
fn update_component_access(&self, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) {
#(self.#field_idents.update_component_access(_access);)*
}
fn update_archetype_component_access(&self, _archetype: &#path::archetype::Archetype, _access: &mut #path::query::Access<#path::archetype::ArchetypeComponentId>) {
#(self.#field_idents.update_archetype_component_access(_archetype, _access);)*
}
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))*
@ -290,29 +298,66 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
impl_fetch(
true,
read_only_fetch_struct_name.clone(),
read_only_item_struct_name,
read_only_item_struct_name.clone(),
)
} else {
quote! {}
};
let read_only_world_query_impl = if fetch_struct_attributes.is_mutable {
quote! {
#[automatically_derived]
#visibility struct #read_only_struct_name #user_impl_generics #user_where_clauses {
#( #field_idents: < #field_types as #path::query::WorldQuery >::ReadOnly, )*
#(#(#ignored_field_attrs)* #ignored_field_visibilities #ignored_field_idents: #ignored_field_types,)*
}
// SAFETY: `ROQueryFetch<Self>` is the same as `QueryFetch<Self>`
unsafe impl #user_impl_generics #path::query::WorldQuery for #read_only_struct_name #user_ty_generics #user_where_clauses {
type ReadOnly = Self;
type State = #state_struct_name #user_ty_generics;
fn shrink<'__wlong: '__wshort, '__wshort>(item: #path::query::#item_type_alias<'__wlong, Self>)
-> #path::query::#item_type_alias<'__wshort, Self> {
#read_only_item_struct_name {
#(
#field_idents : <
< #field_types as #path::query::WorldQuery >::ReadOnly as #path::query::WorldQuery
> :: shrink( item.#field_idents ),
)*
#(
#ignored_field_idents: item.#ignored_field_idents,
)*
}
}
}
impl #user_impl_generics_with_world #path::query::WorldQueryGats<'__w> for #read_only_struct_name #user_ty_generics #user_where_clauses {
type Fetch = #read_only_fetch_struct_name #user_ty_generics_with_world;
type _State = #state_struct_name #user_ty_generics;
}
}
} else {
quote! {}
};
let read_only_asserts = if fetch_struct_attributes.is_mutable {
quote! {
// Double-check that the data fetched by `ROQueryFetch` is read-only.
// This is technically unnecessary as `<_ as WorldQueryGats<'world>>::ReadOnlyFetch: ReadOnlyFetch`
// but to protect against future mistakes we assert the assoc type implements `ReadOnlyFetch` anyway
#( assert_readonly::<#path::query::ROQueryFetch<'__w, #field_types>>(); )*
// Double-check that the data fetched by `<_ as WorldQuery>::ReadOnly` is read-only.
// This is technically unnecessary as `<_ as WorldQuery>::ReadOnly: ReadOnlyWorldQuery`
// but to protect against future mistakes we assert the assoc type implements `ReadOnlyWorldQuery` anyway
#( assert_readonly::< < #field_types as #path::query::WorldQuery > :: ReadOnly >(); )*
}
} else {
quote! {
// Statically checks that the safety guarantee of `ReadOnlyFetch` for `$fetch_struct_name` actually holds true.
// We need this to make sure that we don't compile `ReadOnlyFetch` if our struct contains nested `WorldQuery`
// Statically checks that the safety guarantee of `ReadOnlyWorldQuery` for `$fetch_struct_name` actually holds true.
// We need this to make sure that we don't compile `ReadOnlyWorldQuery` if our struct contains nested `WorldQuery`
// members that don't implement it. I.e.:
// ```
// #[derive(WorldQuery)]
// pub struct Foo { a: &'static mut MyComponent }
// ```
#( assert_readonly::<#path::query::QueryFetch<'__w, #field_types>>(); )*
#( assert_readonly::<#field_types>(); )*
}
};
@ -323,7 +368,12 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
#read_only_fetch_impl
impl #user_impl_generics #path::query::WorldQuery for #struct_name #user_ty_generics #user_where_clauses {
#read_only_world_query_impl
// SAFETY: if the worldquery is mutable this defers to soundness of the `#field_types: WorldQuery` impl, otherwise
// if the world query is immutable then `#read_only_struct_name #user_ty_generics` is the same type as `#struct_name #user_ty_generics`
unsafe impl #user_impl_generics #path::query::WorldQuery for #struct_name #user_ty_generics #user_where_clauses {
type ReadOnly = #read_only_struct_name #user_ty_generics;
type State = #state_struct_name #user_ty_generics;
fn shrink<'__wlong: '__wshort, '__wshort>(item: #path::query::#item_type_alias<'__wlong, Self>)
-> #path::query::#item_type_alias<'__wshort, Self> {
@ -340,19 +390,18 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
impl #user_impl_generics_with_world #path::query::WorldQueryGats<'__w> for #struct_name #user_ty_generics #user_where_clauses {
type Fetch = #fetch_struct_name #user_ty_generics_with_world;
type ReadOnlyFetch = #read_only_fetch_struct_name #user_ty_generics_with_world;
type _State = #state_struct_name #user_ty_generics;
}
/// SAFETY: each item in the struct is read only
unsafe impl #user_impl_generics_with_world #path::query::ReadOnlyFetch
for #read_only_fetch_struct_name #user_ty_generics_with_world #user_where_clauses_with_world {}
unsafe impl #user_impl_generics #path::query::ReadOnlyWorldQuery
for #read_only_struct_name #user_ty_generics #user_where_clauses {}
#[allow(dead_code)]
const _: () = {
fn assert_readonly<T>()
where
T: #path::query::ReadOnlyFetch,
T: #path::query::ReadOnlyWorldQuery,
{
}

View file

@ -143,6 +143,30 @@ use std::{cell::UnsafeCell, marker::PhantomData};
/// # bevy_ecs::system::assert_is_system(my_system);
/// ```
///
/// Mutable queries will also have a read only version derived:
/// ```rust
/// # use bevy_ecs::prelude::*;
/// use bevy_ecs::query::WorldQuery;
///
/// #[derive(Component)]
/// pub struct MyComponent;
///
/// #[derive(WorldQuery)]
/// #[world_query(mutable)]
/// pub struct Foo {
/// my_component_yay: &'static mut MyComponent,
/// }
///
/// fn my_system(mut my_query: Query<(FooReadOnly, FooReadOnly)>) {
/// for (i1, i2) in my_query.iter_mut() {
/// let _: FooReadOnlyItem<'_> = i1;
/// let _: FooReadOnlyItem<'_> = i2;
/// }
/// }
///
/// # bevy_ecs::system::assert_is_system(my_system);
/// ```
///
/// **Note:** if you omit the `mutable` attribute for a query that doesn't implement
/// `ReadOnlyFetch`, compilation will fail. We insert static checks as in the example above for
/// every query component and a nested query.
@ -292,26 +316,36 @@ use std::{cell::UnsafeCell, marker::PhantomData};
///
/// # bevy_ecs::system::assert_is_system(my_system);
/// ```
pub trait WorldQuery: for<'w> WorldQueryGats<'w, _State = Self::State> {
/// # Safety
///
/// component access of `ROQueryFetch<Self>` should be a subset of `QueryFetch<Self>`
pub unsafe trait WorldQuery: for<'w> WorldQueryGats<'w, _State = Self::State> {
type ReadOnly: ReadOnlyWorldQuery<State = Self::State>;
type State: FetchState;
/// This function manually implements variance for the query items.
fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self>;
}
/// A world query that is read only.
///
/// # Safety
///
/// This must only be implemented for read-only [`WorldQuery`]'s.
pub unsafe trait ReadOnlyWorldQuery: WorldQuery {}
/// The [`Fetch`] of a [`WorldQuery`], which declares which data it needs access to
pub type QueryFetch<'w, Q> = <Q as WorldQueryGats<'w>>::Fetch;
/// The item type returned when a [`WorldQuery`] is iterated over
pub type QueryItem<'w, Q> = <<Q as WorldQueryGats<'w>>::Fetch as Fetch<'w>>::Item;
/// The read-only [`Fetch`] of a [`WorldQuery`], which declares which data it needs access to when accessed immutably
pub type ROQueryFetch<'w, Q> = <Q as WorldQueryGats<'w>>::ReadOnlyFetch;
pub type ROQueryFetch<'w, Q> = QueryFetch<'w, <Q as WorldQuery>::ReadOnly>;
/// The read-only variant of the item type returned when a [`WorldQuery`] is iterated over immutably
pub type ROQueryItem<'w, Q> = <<Q as WorldQueryGats<'w>>::ReadOnlyFetch as Fetch<'w>>::Item;
pub type ROQueryItem<'w, Q> = QueryItem<'w, <Q as WorldQuery>::ReadOnly>;
/// A helper trait for [`WorldQuery`] that works around Rust's lack of Generic Associated Types
pub trait WorldQueryGats<'world> {
type Fetch: Fetch<'world, State = Self::_State>;
type ReadOnlyFetch: Fetch<'world, State = Self::_State> + ReadOnlyFetch;
type _State: FetchState;
}
@ -320,7 +354,14 @@ pub trait WorldQueryGats<'world> {
///
/// Every type that implements [`WorldQuery`] have their associated [`WorldQuery::Fetch`](WorldQueryGats::Fetch) and
/// [`WorldQuery::State`] types that are essential for fetching component data.
pub trait Fetch<'world>: Sized {
///
/// # Safety
///
/// Implementor must ensure that [`Fetch::update_component_access`] and
/// [`Fetch::update_archetype_component_access`] exactly reflects the results of
/// [`FetchState::matches_component_set`], [`Fetch::archetype_fetch`], and
/// [`Fetch::table_fetch`].
pub unsafe trait Fetch<'world>: Sized {
type Item;
type State: FetchState;
@ -411,37 +452,30 @@ pub trait Fetch<'world>: Sized {
unsafe fn table_filter_fetch(&mut self, table_row: usize) -> bool {
true
}
// This does not have a default body of `{}` because 99% of cases need to add accesses
// and forgetting to do so would be unsound.
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>);
// This does not have a default body of `{}` becaues 99% of cases need to add accesses
// and forgetting to do so would be unsound.
fn update_archetype_component_access(
state: &Self::State,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
);
}
/// State used to construct a Fetch. This will be cached inside [`QueryState`](crate::query::QueryState),
/// so it is best to move as much data / computation here as possible to reduce the cost of
/// constructing Fetch.
///
/// # Safety
///
/// Implementor must ensure that [`FetchState::update_component_access`] and
/// [`FetchState::update_archetype_component_access`] exactly reflects the results of
/// [`FetchState::matches_component_set`], [`Fetch::archetype_fetch`], and
/// [`Fetch::table_fetch`].
pub unsafe trait FetchState: Send + Sync + Sized {
pub trait FetchState: Send + Sync + Sized {
fn init(world: &mut World) -> Self;
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>);
fn update_archetype_component_access(
&self,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
);
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool;
}
/// A fetch that is read only.
///
/// # Safety
///
/// This must only be implemented for read-only fetches.
pub unsafe trait ReadOnlyFetch {}
impl WorldQuery for Entity {
/// SAFETY: no component or archetype access
unsafe impl WorldQuery for Entity {
type ReadOnly = Self;
type State = EntityState;
fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
@ -457,27 +491,17 @@ pub struct EntityFetch<'w> {
}
/// SAFETY: access is read only
unsafe impl<'w> ReadOnlyFetch for EntityFetch<'w> {}
unsafe impl ReadOnlyWorldQuery for Entity {}
/// The [`FetchState`] of [`Entity`].
#[doc(hidden)]
pub struct EntityState;
// SAFETY: no component or archetype access
unsafe impl FetchState for EntityState {
impl FetchState for EntityState {
fn init(_world: &mut World) -> Self {
Self
}
fn update_component_access(&self, _access: &mut FilteredAccess<ComponentId>) {}
fn update_archetype_component_access(
&self,
_archetype: &Archetype,
_access: &mut Access<ArchetypeComponentId>,
) {
}
#[inline]
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
true
@ -486,11 +510,11 @@ unsafe impl FetchState for EntityState {
impl<'w> WorldQueryGats<'w> for Entity {
type Fetch = EntityFetch<'w>;
type ReadOnlyFetch = EntityFetch<'w>;
type _State = EntityState;
}
impl<'w> Fetch<'w> for EntityFetch<'w> {
/// SAFETY: no component or archetype access
unsafe impl<'w> Fetch<'w> for EntityFetch<'w> {
type Item = Entity;
type State = EntityState;
@ -533,10 +557,21 @@ impl<'w> Fetch<'w> for EntityFetch<'w> {
let entities = self.entities.unwrap_or_else(|| debug_checked_unreachable());
*entities.get(archetype_index)
}
fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {}
fn update_archetype_component_access(
_state: &Self::State,
_archetype: &Archetype,
_access: &mut Access<ArchetypeComponentId>,
) {
}
}
impl<T: Component> WorldQuery for &T {
type State = ReadState<T>;
/// SAFETY: `ROQueryFetch<Self>` is the same as `QueryFetch<Self>`
unsafe impl<T: Component> WorldQuery for &T {
type ReadOnly = Self;
type State = ComponentIdState<T>;
fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
item
@ -545,43 +580,20 @@ impl<T: Component> WorldQuery for &T {
/// The [`FetchState`] of `&T`.
#[doc(hidden)]
pub struct ReadState<T> {
pub struct ComponentIdState<T> {
component_id: ComponentId,
marker: PhantomData<T>,
}
// SAFETY: component access and archetype component access are properly updated to reflect that T is
// read
unsafe impl<T: Component> FetchState for ReadState<T> {
impl<T: Component> FetchState for ComponentIdState<T> {
fn init(world: &mut World) -> Self {
let component_id = world.init_component::<T>();
ReadState {
ComponentIdState {
component_id,
marker: PhantomData,
}
}
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
assert!(
!access.access().has_write(self.component_id),
"&{} conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",
std::any::type_name::<T>(),
);
access.add_read(self.component_id);
}
fn update_archetype_component_access(
&self,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if let Some(archetype_component_id) =
archetype.get_archetype_component_id(self.component_id)
{
access.add_read(archetype_component_id);
}
}
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
@ -610,17 +622,18 @@ impl<T> Clone for ReadFetch<'_, T> {
}
/// SAFETY: access is read only
unsafe impl<'w, T: Component> ReadOnlyFetch for ReadFetch<'w, T> {}
unsafe impl<T: Component> ReadOnlyWorldQuery for &T {}
impl<'w, T: Component> WorldQueryGats<'w> for &T {
type Fetch = ReadFetch<'w, T>;
type ReadOnlyFetch = ReadFetch<'w, T>;
type _State = ReadState<T>;
type _State = ComponentIdState<T>;
}
impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> {
// SAFETY: component access and archetype component access are properly updated to reflect that T is
// read
unsafe impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> {
type Item = &'w T;
type State = ReadState<T>;
type State = ComponentIdState<T>;
const IS_DENSE: bool = {
match T::Storage::STORAGE_TYPE {
@ -633,7 +646,7 @@ impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> {
unsafe fn init(
world: &'w World,
state: &ReadState<T>,
state: &ComponentIdState<T>,
_last_change_tick: u32,
_change_tick: u32,
) -> ReadFetch<'w, T> {
@ -713,10 +726,33 @@ impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> {
.unwrap_or_else(|| debug_checked_unreachable());
components.get(table_row).deref()
}
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
assert!(
!access.access().has_write(state.component_id),
"&{} conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",
std::any::type_name::<T>(),
);
access.add_read(state.component_id);
}
fn update_archetype_component_access(
state: &Self::State,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if let Some(archetype_component_id) =
archetype.get_archetype_component_id(state.component_id)
{
access.add_read(archetype_component_id);
}
}
}
impl<T: Component> WorldQuery for &mut T {
type State = WriteState<T>;
/// SAFETY: access of `&T` is a subset of `&mut T`
unsafe impl<'w, T: Component> WorldQuery for &'w mut T {
type ReadOnly = &'w T;
type State = ComponentIdState<T>;
fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
item
@ -752,83 +788,16 @@ impl<T> Clone for WriteFetch<'_, T> {
}
}
/// The [`ReadOnlyFetch`] of `&mut T`.
pub struct ReadOnlyWriteFetch<'w, T> {
// T::Storage = TableStorage
table_components: Option<ThinSlicePtr<'w, UnsafeCell<T>>>,
entity_table_rows: Option<ThinSlicePtr<'w, usize>>,
// T::Storage = SparseStorage
entities: Option<ThinSlicePtr<'w, Entity>>,
sparse_set: Option<&'w ComponentSparseSet>,
}
/// SAFETY: access is read only
unsafe impl<'w, T: Component> ReadOnlyFetch for ReadOnlyWriteFetch<'w, T> {}
impl<T> Clone for ReadOnlyWriteFetch<'_, T> {
fn clone(&self) -> Self {
Self {
table_components: self.table_components,
entities: self.entities,
entity_table_rows: self.entity_table_rows,
sparse_set: self.sparse_set,
}
}
}
/// The [`FetchState`] of `&mut T`.
#[doc(hidden)]
pub struct WriteState<T> {
component_id: ComponentId,
marker: PhantomData<T>,
}
// SAFETY: component access and archetype component access are properly updated to reflect that T is
// written
unsafe impl<T: Component> FetchState for WriteState<T> {
fn init(world: &mut World) -> Self {
let component_id = world.init_component::<T>();
WriteState {
component_id,
marker: PhantomData,
}
}
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
assert!(
!access.access().has_read(self.component_id),
"&mut {} conflicts with a previous access in this query. Mutable component access must be unique.",
std::any::type_name::<T>(),
);
access.add_write(self.component_id);
}
fn update_archetype_component_access(
&self,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if let Some(archetype_component_id) =
archetype.get_archetype_component_id(self.component_id)
{
access.add_write(archetype_component_id);
}
}
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
}
impl<'w, T: Component> WorldQueryGats<'w> for &mut T {
type Fetch = WriteFetch<'w, T>;
type ReadOnlyFetch = ReadOnlyWriteFetch<'w, T>;
type _State = WriteState<T>;
type _State = ComponentIdState<T>;
}
impl<'w, T: Component> Fetch<'w> for WriteFetch<'w, T> {
/// SAFETY: component access and archetype component access are properly updated to reflect that T is
/// read and write
unsafe impl<'w, T: Component> Fetch<'w> for WriteFetch<'w, T> {
type Item = Mut<'w, T>;
type State = WriteState<T>;
type State = ComponentIdState<T>;
const IS_DENSE: bool = {
match T::Storage::STORAGE_TYPE {
@ -841,7 +810,7 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch<'w, T> {
unsafe fn init(
world: &'w World,
state: &WriteState<T>,
state: &ComponentIdState<T>,
last_change_tick: u32,
change_tick: u32,
) -> Self {
@ -943,106 +912,32 @@ impl<'w, T: Component> Fetch<'w> for WriteFetch<'w, T> {
},
}
}
}
impl<'w, T: Component> Fetch<'w> for ReadOnlyWriteFetch<'w, T> {
type Item = &'w T;
type State = WriteState<T>;
const IS_DENSE: bool = {
match T::Storage::STORAGE_TYPE {
StorageType::Table => true,
StorageType::SparseSet => false,
}
};
const IS_ARCHETYPAL: bool = true;
unsafe fn init(
world: &'w World,
state: &WriteState<T>,
_last_change_tick: u32,
_change_tick: u32,
) -> Self {
Self {
table_components: None,
entities: None,
entity_table_rows: None,
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
world
.storages()
.sparse_sets
.get(state.component_id)
.unwrap()
}),
}
}
#[inline]
unsafe fn set_archetype(
&mut self,
state: &Self::State,
archetype: &'w Archetype,
tables: &'w Tables,
) {
match T::Storage::STORAGE_TYPE {
StorageType::Table => {
self.entity_table_rows = Some(archetype.entity_table_rows().into());
let column = tables[archetype.table_id()]
.get_column(state.component_id)
.unwrap();
self.table_components = Some(column.get_data_slice().into());
}
StorageType::SparseSet => self.entities = Some(archetype.entities().into()),
}
}
#[inline]
unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) {
self.table_components = Some(
table
.get_column(state.component_id)
.unwrap()
.get_data_slice()
.into(),
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
assert!(
!access.access().has_read(state.component_id),
"&mut {} conflicts with a previous access in this query. Mutable component access must be unique.",
std::any::type_name::<T>(),
);
access.add_write(state.component_id);
}
#[inline]
unsafe fn archetype_fetch(&mut self, archetype_index: usize) -> Self::Item {
match T::Storage::STORAGE_TYPE {
StorageType::Table => {
let (entity_table_rows, table_components) = self
.entity_table_rows
.zip(self.table_components)
.unwrap_or_else(|| debug_checked_unreachable());
let table_row = *entity_table_rows.get(archetype_index);
table_components.get(table_row).deref()
}
StorageType::SparseSet => {
let (entities, sparse_set) = self
.entities
.zip(self.sparse_set)
.unwrap_or_else(|| debug_checked_unreachable());
let entity = *entities.get(archetype_index);
sparse_set
.get(entity)
.unwrap_or_else(|| debug_checked_unreachable())
.deref::<T>()
}
fn update_archetype_component_access(
state: &Self::State,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if let Some(archetype_component_id) =
archetype.get_archetype_component_id(state.component_id)
{
access.add_write(archetype_component_id);
}
}
#[inline]
unsafe fn table_fetch(&mut self, table_row: usize) -> Self::Item {
let components = self
.table_components
.unwrap_or_else(|| debug_checked_unreachable());
components.get(table_row).deref()
}
}
impl<T: WorldQuery> WorldQuery for Option<T> {
// SAFETY: defers to soundness of `T: WorldQuery` impl
unsafe impl<T: WorldQuery> WorldQuery for Option<T> {
type ReadOnly = Option<T::ReadOnly>;
type State = OptionState<T::State>;
fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
@ -1059,7 +954,7 @@ pub struct OptionFetch<T> {
}
/// SAFETY: [`OptionFetch`] is read only because `T` is read only
unsafe impl<T: ReadOnlyFetch> ReadOnlyFetch for OptionFetch<T> {}
unsafe impl<T: ReadOnlyWorldQuery> ReadOnlyWorldQuery for Option<T> {}
/// The [`FetchState`] of `Option<T>`.
#[doc(hidden)]
@ -1067,39 +962,13 @@ pub struct OptionState<T: FetchState> {
state: T,
}
// SAFETY: component access and archetype component access are properly updated according to the
// internal Fetch
unsafe impl<T: FetchState> FetchState for OptionState<T> {
impl<T: FetchState> FetchState for OptionState<T> {
fn init(world: &mut World) -> Self {
Self {
state: T::init(world),
}
}
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
// We don't want to add the `with`/`without` of `T` as `Option<T>` will match things regardless of
// `T`'s filters. for example `Query<(Option<&U>, &mut V)>` will match every entity with a `V` component
// regardless of whether it has a `U` component. If we dont do this the query will not conflict with
// `Query<&mut V, Without<U>>` which would be unsound.
let mut intermediate = access.clone();
self.state.update_component_access(&mut intermediate);
access.extend_access(&intermediate);
}
fn update_archetype_component_access(
&self,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if self
.state
.matches_component_set(&|id| archetype.contains(id))
{
self.state
.update_archetype_component_access(archetype, access);
}
}
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
true
}
@ -1107,11 +976,12 @@ unsafe impl<T: FetchState> FetchState for OptionState<T> {
impl<'w, T: WorldQueryGats<'w>> WorldQueryGats<'w> for Option<T> {
type Fetch = OptionFetch<T::Fetch>;
type ReadOnlyFetch = OptionFetch<T::ReadOnlyFetch>;
type _State = OptionState<T::_State>;
}
impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch<T> {
// SAFETY: component access and archetype component access are properly updated according to the
// internal Fetch
unsafe impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch<T> {
type Item = Option<T::Item>;
type State = OptionState<T::State>;
@ -1173,6 +1043,26 @@ impl<'w, T: Fetch<'w>> Fetch<'w> for OptionFetch<T> {
None
}
}
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
// We don't want to add the `with`/`without` of `T` as `Option<T>` will match things regardless of
// `T`'s filters. for example `Query<(Option<&U>, &mut V)>` will match every entity with a `V` component
// regardless of whether it has a `U` component. If we dont do this the query will not conflict with
// `Query<&mut V, Without<U>>` which would be unsound.
let mut intermediate = access.clone();
T::update_component_access(&state.state, &mut intermediate);
access.extend_access(&intermediate);
}
fn update_archetype_component_access(
state: &Self::State,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if state.matches_component_set(&|id| archetype.contains(id)) {
T::update_archetype_component_access(&state.state, archetype, access);
}
}
}
/// [`WorldQuery`] that tracks changes and additions for component `T`.
@ -1239,7 +1129,9 @@ impl<T: Component> ChangeTrackers<T> {
}
}
impl<T: Component> WorldQuery for ChangeTrackers<T> {
// SAFETY: `ROQueryFetch<Self>` is the same as `QueryFetch<Self>`
unsafe impl<T: Component> WorldQuery for ChangeTrackers<T> {
type ReadOnly = Self;
type State = ChangeTrackersState<T>;
fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
@ -1254,9 +1146,7 @@ pub struct ChangeTrackersState<T> {
marker: PhantomData<T>,
}
// SAFETY: component access and archetype component access are properly updated to reflect that T is
// read
unsafe impl<T: Component> FetchState for ChangeTrackersState<T> {
impl<T: Component> FetchState for ChangeTrackersState<T> {
fn init(world: &mut World) -> Self {
let component_id = world.init_component::<T>();
Self {
@ -1265,27 +1155,6 @@ unsafe impl<T: Component> FetchState for ChangeTrackersState<T> {
}
}
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
assert!(
!access.access().has_write(self.component_id),
"ChangeTrackers<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",
std::any::type_name::<T>()
);
access.add_read(self.component_id);
}
fn update_archetype_component_access(
&self,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if let Some(archetype_component_id) =
archetype.get_archetype_component_id(self.component_id)
{
access.add_read(archetype_component_id);
}
}
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
@ -1321,15 +1190,16 @@ impl<T> Clone for ChangeTrackersFetch<'_, T> {
}
/// SAFETY: access is read only
unsafe impl<'w, T: Component> ReadOnlyFetch for ChangeTrackersFetch<'w, T> {}
unsafe impl<T: Component> ReadOnlyWorldQuery for ChangeTrackers<T> {}
impl<'w, T: Component> WorldQueryGats<'w> for ChangeTrackers<T> {
type Fetch = ChangeTrackersFetch<'w, T>;
type ReadOnlyFetch = ChangeTrackersFetch<'w, T>;
type _State = ChangeTrackersState<T>;
}
impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> {
// SAFETY: component access and archetype component access are properly updated to reflect that T is
// read
unsafe impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> {
type Item = ChangeTrackers<T>;
type State = ChangeTrackersState<T>;
@ -1448,6 +1318,27 @@ impl<'w, T: Component> Fetch<'w> for ChangeTrackersFetch<'w, T> {
change_tick: self.change_tick,
}
}
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
assert!(
!access.access().has_write(state.component_id),
"ChangeTrackers<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",
std::any::type_name::<T>()
);
access.add_read(state.component_id);
}
fn update_archetype_component_access(
state: &Self::State,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if let Some(archetype_component_id) =
archetype.get_archetype_component_id(state.component_id)
{
access.add_read(archetype_component_id);
}
}
}
macro_rules! impl_tuple_fetch {
@ -1456,12 +1347,12 @@ macro_rules! impl_tuple_fetch {
#[allow(non_snake_case)]
impl<'w, $($name: WorldQueryGats<'w>),*> WorldQueryGats<'w> for ($($name,)*) {
type Fetch = ($($name::Fetch,)*);
type ReadOnlyFetch = ($($name::ReadOnlyFetch,)*);
type _State = ($($name::_State,)*);
}
#[allow(non_snake_case)]
impl<'w, $($name: Fetch<'w>),*> Fetch<'w> for ($($name,)*) {
// SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple
unsafe impl<'w, $($name: Fetch<'w>),*> Fetch<'w> for ($($name,)*) {
type Item = ($($name::Item,)*);
type State = ($($name::State,)*);
@ -1516,26 +1407,25 @@ macro_rules! impl_tuple_fetch {
let ($($name,)*) = self;
true $(&& $name.archetype_filter_fetch(archetype_index))*
}
fn update_component_access(state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {
let ($($name,)*) = state;
$($name::update_component_access($name, _access);)*
}
fn update_archetype_component_access(state: &Self::State, _archetype: &Archetype, _access: &mut Access<ArchetypeComponentId>) {
let ($($name,)*) = state;
$($name::update_archetype_component_access($name, _archetype, _access);)*
}
}
// SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple
#[allow(non_snake_case)]
#[allow(clippy::unused_unit)]
unsafe impl<$($name: FetchState),*> FetchState for ($($name,)*) {
impl<$($name: FetchState),*> FetchState for ($($name,)*) {
fn init(_world: &mut World) -> Self {
($($name::init(_world),)*)
}
fn update_component_access(&self, _access: &mut FilteredAccess<ComponentId>) {
let ($($name,)*) = self;
$($name.update_component_access(_access);)*
}
fn update_archetype_component_access(&self, _archetype: &Archetype, _access: &mut Access<ArchetypeComponentId>) {
let ($($name,)*) = self;
$($name.update_archetype_component_access(_archetype, _access);)*
}
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
let ($($name,)*) = self;
true $(&& $name.matches_component_set(_set_contains_id))*
@ -1544,7 +1434,9 @@ macro_rules! impl_tuple_fetch {
#[allow(non_snake_case)]
#[allow(clippy::unused_unit)]
impl<$($name: WorldQuery),*> WorldQuery for ($($name,)*) {
// SAFETY: defers to soundness `$name: WorldQuery` impl
unsafe impl<$($name: WorldQuery),*> WorldQuery for ($($name,)*) {
type ReadOnly = ($($name::ReadOnly,)*);
type State = ($($name::State,)*);
fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
@ -1556,7 +1448,7 @@ macro_rules! impl_tuple_fetch {
}
/// SAFETY: each item in the tuple is read only
unsafe impl<'w, $($name: ReadOnlyFetch),*> ReadOnlyFetch for ($($name,)*) {}
unsafe impl<$($name: ReadOnlyWorldQuery),*> ReadOnlyWorldQuery for ($($name,)*) {}
};
}
@ -1574,12 +1466,12 @@ macro_rules! impl_anytuple_fetch {
#[allow(non_snake_case)]
impl<'w, $($name: WorldQueryGats<'w>),*> WorldQueryGats<'w> for AnyOf<($($name,)*)> {
type Fetch = AnyOf<($(($name::Fetch, bool),)*)>;
type ReadOnlyFetch = AnyOf<($(($name::ReadOnlyFetch, bool),)*)>;
type _State = AnyOf<($($name::_State,)*)>;
}
#[allow(non_snake_case)]
impl<'w, $($name: Fetch<'w>),*> Fetch<'w> for AnyOf<($(($name, bool),)*)> {
// SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple
unsafe impl<'w, $($name: Fetch<'w>),*> Fetch<'w> for AnyOf<($(($name, bool),)*)> {
type Item = ($(Option<$name::Item>,)*);
type State = AnyOf<($($name::State,)*)>;
@ -1634,18 +1526,9 @@ macro_rules! impl_anytuple_fetch {
$name.1.then(|| $name.0.archetype_fetch(_archetype_index)),
)*)
}
}
// SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple
#[allow(non_snake_case)]
#[allow(clippy::unused_unit)]
unsafe impl<$($name: FetchState),*> FetchState for AnyOf<($($name,)*)> {
fn init(_world: &mut World) -> Self {
AnyOf(($($name::init(_world),)*))
}
fn update_component_access(&self, _access: &mut FilteredAccess<ComponentId>) {
let ($($name,)*) = &self.0;
fn update_component_access(state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {
let ($($name,)*) = &state.0;
// We do not unconditionally add `$name`'s `with`/`without` accesses to `_access`
// as this would be unsound. For example the following two queries should conflict:
@ -1664,11 +1547,12 @@ macro_rules! impl_anytuple_fetch {
$(
if _not_first {
let mut intermediate = _access.clone();
$name.update_component_access(&mut intermediate);
$name::update_component_access($name, &mut intermediate);
_intersected_access.extend_intersect_filter(&intermediate);
_intersected_access.extend_access(&intermediate);
} else {
$name.update_component_access(&mut _intersected_access);
$name::update_component_access($name, &mut _intersected_access);
_not_first = true;
}
)*
@ -1676,14 +1560,23 @@ macro_rules! impl_anytuple_fetch {
*_access = _intersected_access;
}
fn update_archetype_component_access(&self, _archetype: &Archetype, _access: &mut Access<ArchetypeComponentId>) {
let ($($name,)*) = &self.0;
fn update_archetype_component_access(state: &Self::State, _archetype: &Archetype, _access: &mut Access<ArchetypeComponentId>) {
let ($($name,)*) = &state.0;
$(
if $name.matches_component_set(&|id| _archetype.contains(id)) {
$name.update_archetype_component_access(_archetype, _access);
$name::update_archetype_component_access($name, _archetype, _access);
}
)*
}
}
#[allow(non_snake_case)]
#[allow(clippy::unused_unit)]
impl<$($name: FetchState),*> FetchState for AnyOf<($($name,)*)> {
fn init(_world: &mut World) -> Self {
AnyOf(($($name::init(_world),)*))
}
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
let ($($name,)*) = &self.0;
false $(|| $name.matches_component_set(_set_contains_id))*
@ -1692,7 +1585,9 @@ macro_rules! impl_anytuple_fetch {
#[allow(non_snake_case)]
#[allow(clippy::unused_unit)]
impl<$($name: WorldQuery),*> WorldQuery for AnyOf<($($name,)*)> {
// SAFETY: defers to soundness of `$name: WorldQuery` impl
unsafe impl<$($name: WorldQuery),*> WorldQuery for AnyOf<($($name,)*)> {
type ReadOnly = AnyOf<($($name::ReadOnly,)*)>;
type State = AnyOf<($($name::State,)*)>;
fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
@ -1704,7 +1599,7 @@ macro_rules! impl_anytuple_fetch {
}
/// SAFETY: each item in the tuple is read only
unsafe impl<'w, $($name: ReadOnlyFetch),*> ReadOnlyFetch for AnyOf<($(($name, bool),)*)> {}
unsafe impl<$($name: ReadOnlyWorldQuery),*> ReadOnlyWorldQuery for AnyOf<($($name,)*)> {}
};
}
@ -1719,7 +1614,8 @@ pub struct NopFetch<State> {
state: PhantomData<State>,
}
impl<'w, State: FetchState> Fetch<'w> for NopFetch<State> {
// SAFETY: NopFetch doesnt access anything
unsafe impl<'w, State: FetchState> Fetch<'w> for NopFetch<State> {
type Item = ();
type State = State;
@ -1754,4 +1650,13 @@ impl<'w, State: FetchState> Fetch<'w> for NopFetch<State> {
#[inline(always)]
unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item {}
fn update_component_access(_state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {}
fn update_archetype_component_access(
_state: &Self::State,
_archetype: &Archetype,
_access: &mut Access<ArchetypeComponentId>,
) {
}
}

View file

@ -4,7 +4,7 @@ use crate::{
entity::Entity,
query::{
debug_checked_unreachable, Access, Fetch, FetchState, FilteredAccess, QueryFetch,
ROQueryFetch, WorldQuery, WorldQueryGats,
WorldQuery, WorldQueryGats,
},
storage::{ComponentSparseSet, Table, Tables},
world::World,
@ -13,7 +13,7 @@ use bevy_ecs_macros::all_tuples;
use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref};
use std::{cell::UnsafeCell, marker::PhantomData};
use super::ReadOnlyFetch;
use super::ReadOnlyWorldQuery;
/// Filter that selects entities with a component `T`.
///
@ -44,7 +44,9 @@ use super::ReadOnlyFetch;
/// ```
pub struct With<T>(PhantomData<T>);
impl<T: Component> WorldQuery for With<T> {
// SAFETY: `ROQueryFetch<Self>` is the same as `QueryFetch<Self>`
unsafe impl<T: Component> WorldQuery for With<T> {
type ReadOnly = Self;
type State = WithState<T>;
#[allow(clippy::semicolon_if_nothing_returned)]
@ -68,8 +70,7 @@ pub struct WithState<T> {
marker: PhantomData<T>,
}
// SAFETY: no component access or archetype component access
unsafe impl<T: Component> FetchState for WithState<T> {
impl<T: Component> FetchState for WithState<T> {
fn init(world: &mut World) -> Self {
let component_id = world.init_component::<T>();
Self {
@ -78,19 +79,6 @@ unsafe impl<T: Component> FetchState for WithState<T> {
}
}
#[inline]
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
access.add_with(self.component_id);
}
#[inline]
fn update_archetype_component_access(
&self,
_archetype: &Archetype,
_access: &mut Access<ArchetypeComponentId>,
) {
}
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
@ -98,11 +86,11 @@ unsafe impl<T: Component> FetchState for WithState<T> {
impl<T: Component> WorldQueryGats<'_> for With<T> {
type Fetch = WithFetch<T>;
type ReadOnlyFetch = WithFetch<T>;
type _State = WithState<T>;
}
impl<'w, T: Component> Fetch<'w> for WithFetch<T> {
// SAFETY: no component access or archetype component access
unsafe impl<'w, T: Component> Fetch<'w> for WithFetch<T> {
type Item = ();
type State = WithState<T>;
@ -143,10 +131,23 @@ impl<'w, T: Component> Fetch<'w> for WithFetch<T> {
#[inline]
unsafe fn table_fetch(&mut self, _table_row: usize) {}
#[inline]
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
access.add_with(state.component_id);
}
#[inline]
fn update_archetype_component_access(
_state: &Self::State,
_archetype: &Archetype,
_access: &mut Access<ArchetypeComponentId>,
) {
}
}
// SAFETY: no component access or archetype component access
unsafe impl<T: Component> ReadOnlyFetch for WithFetch<T> {}
unsafe impl<T: Component> ReadOnlyWorldQuery for With<T> {}
impl<T> Clone for WithFetch<T> {
fn clone(&self) -> Self {
@ -184,7 +185,9 @@ impl<T> Copy for WithFetch<T> {}
/// ```
pub struct Without<T>(PhantomData<T>);
impl<T: Component> WorldQuery for Without<T> {
// SAFETY: `ROQueryFetch<Self>` is the same as `QueryFetch<Self>`
unsafe impl<T: Component> WorldQuery for Without<T> {
type ReadOnly = Self;
type State = WithoutState<T>;
#[allow(clippy::semicolon_if_nothing_returned)]
@ -208,8 +211,7 @@ pub struct WithoutState<T> {
marker: PhantomData<T>,
}
// SAFETY: no component access or archetype component access
unsafe impl<T: Component> FetchState for WithoutState<T> {
impl<T: Component> FetchState for WithoutState<T> {
fn init(world: &mut World) -> Self {
let component_id = world.init_component::<T>();
Self {
@ -218,18 +220,6 @@ unsafe impl<T: Component> FetchState for WithoutState<T> {
}
}
#[inline]
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
access.add_without(self.component_id);
}
#[inline]
fn update_archetype_component_access(
&self,
_archetype: &Archetype,
_access: &mut Access<ArchetypeComponentId>,
) {
}
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
!set_contains_id(self.component_id)
}
@ -237,11 +227,11 @@ unsafe impl<T: Component> FetchState for WithoutState<T> {
impl<T: Component> WorldQueryGats<'_> for Without<T> {
type Fetch = WithoutFetch<T>;
type ReadOnlyFetch = WithoutFetch<T>;
type _State = WithoutState<T>;
}
impl<'w, T: Component> Fetch<'w> for WithoutFetch<T> {
// SAFETY: no component access or archetype component access
unsafe impl<'w, T: Component> Fetch<'w> for WithoutFetch<T> {
type Item = ();
type State = WithoutState<T>;
@ -282,10 +272,23 @@ impl<'w, T: Component> Fetch<'w> for WithoutFetch<T> {
#[inline]
unsafe fn table_fetch(&mut self, _table_row: usize) {}
#[inline]
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
access.add_without(state.component_id);
}
#[inline]
fn update_archetype_component_access(
_state: &Self::State,
_archetype: &Archetype,
_access: &mut Access<ArchetypeComponentId>,
) {
}
}
// SAFETY: no component access or archetype component access
unsafe impl<T: Component> ReadOnlyFetch for WithoutFetch<T> {}
unsafe impl<T: Component> ReadOnlyWorldQuery for Without<T> {}
impl<T> Clone for WithoutFetch<T> {
fn clone(&self) -> Self {
@ -343,7 +346,9 @@ macro_rules! impl_query_filter_tuple {
($(($filter: ident, $state: ident)),*) => {
#[allow(unused_variables)]
#[allow(non_snake_case)]
impl<$($filter: WorldQuery),*> WorldQuery for Or<($($filter,)*)> {
// SAFETY: defers to soundness of `$filter: WorldQuery` impl
unsafe impl<$($filter: WorldQuery),*> WorldQuery for Or<($($filter,)*)> {
type ReadOnly = Or<($($filter::ReadOnly,)*)>;
type State = Or<($($filter::State,)*)>;
fn shrink<'wlong: 'wshort, 'wshort>(item: super::QueryItem<'wlong, Self>) -> super::QueryItem<'wshort, Self> {
@ -355,13 +360,13 @@ macro_rules! impl_query_filter_tuple {
#[allow(non_snake_case)]
impl<'w, $($filter: WorldQueryGats<'w>),*> WorldQueryGats<'w> for Or<($($filter,)*)> {
type Fetch = Or<($(OrFetch<'w, QueryFetch<'w, $filter>>,)*)>;
type ReadOnlyFetch = Or<($(OrFetch<'w, ROQueryFetch<'w, $filter>>,)*)>;
type _State = Or<($($filter::_State,)*)>;
}
#[allow(unused_variables)]
#[allow(non_snake_case)]
impl<'w, $($filter: Fetch<'w>),*> Fetch<'w> for Or<($(OrFetch<'w, $filter>,)*)> {
// SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple
unsafe impl<'w, $($filter: Fetch<'w>),*> Fetch<'w> for Or<($(OrFetch<'w, $filter>,)*)> {
type State = Or<($(<$filter as Fetch<'w>>::State,)*)>;
type Item = bool;
@ -423,18 +428,9 @@ macro_rules! impl_query_filter_tuple {
unsafe fn archetype_filter_fetch(&mut self, archetype_index: usize) -> bool {
self.archetype_fetch(archetype_index)
}
}
// SAFETY: update_component_access and update_archetype_component_access are called for each item in the tuple
#[allow(unused_variables)]
#[allow(non_snake_case)]
unsafe impl<$($filter: FetchState),*> FetchState for Or<($($filter,)*)> {
fn init(world: &mut World) -> Self {
Or(($($filter::init(world),)*))
}
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
let ($($filter,)*) = &self.0;
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
let ($($filter,)*) = &state.0;
// We do not unconditionally add `$filter`'s `with`/`without` accesses to `access`
// as this would be unsound. For example the following two queries should conflict:
@ -453,11 +449,11 @@ macro_rules! impl_query_filter_tuple {
$(
if _not_first {
let mut intermediate = access.clone();
$filter.update_component_access(&mut intermediate);
$filter::update_component_access($filter, &mut intermediate);
_intersected_access.extend_intersect_filter(&intermediate);
_intersected_access.extend_access(&intermediate);
} else {
$filter.update_component_access(&mut _intersected_access);
$filter::update_component_access($filter, &mut _intersected_access);
_not_first = true;
}
)*
@ -465,9 +461,17 @@ macro_rules! impl_query_filter_tuple {
*access = _intersected_access;
}
fn update_archetype_component_access(&self, archetype: &Archetype, access: &mut Access<ArchetypeComponentId>) {
let ($($filter,)*) = &self.0;
$($filter.update_archetype_component_access(archetype, access);)*
fn update_archetype_component_access(state: &Self::State, archetype: &Archetype, access: &mut Access<ArchetypeComponentId>) {
let ($($filter,)*) = &state.0;
$($filter::update_archetype_component_access($filter, archetype, access);)*
}
}
#[allow(unused_variables)]
#[allow(non_snake_case)]
impl<$($filter: FetchState),*> FetchState for Or<($($filter,)*)> {
fn init(world: &mut World) -> Self {
Or(($($filter::init(world),)*))
}
fn matches_component_set(&self, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
@ -477,7 +481,7 @@ macro_rules! impl_query_filter_tuple {
}
// SAFE: filters are read only
unsafe impl<'w, $($filter: Fetch<'w> + ReadOnlyFetch),*> ReadOnlyFetch for Or<($(OrFetch<'w, $filter>,)*)> {}
unsafe impl<$($filter: ReadOnlyWorldQuery),*> ReadOnlyWorldQuery for Or<($($filter,)*)> {}
};
}
@ -515,7 +519,9 @@ macro_rules! impl_tick_filter {
marker: PhantomData<T>,
}
impl<T: Component> WorldQuery for $name<T> {
// SAFETY: `ROQueryFetch<Self>` is the same as `QueryFetch<Self>`
unsafe impl<T: Component> WorldQuery for $name<T> {
type ReadOnly = Self;
type State = $state_name<T>;
fn shrink<'wlong: 'wshort, 'wshort>(item: super::QueryItem<'wlong, Self>) -> super::QueryItem<'wshort, Self> {
@ -523,8 +529,7 @@ macro_rules! impl_tick_filter {
}
}
// SAFETY: this reads the T component. archetype component access and component access are updated to reflect that
unsafe impl<T: Component> FetchState for $state_name<T> {
impl<T: Component> FetchState for $state_name<T> {
fn init(world: &mut World) -> Self {
Self {
component_id: world.init_component::<T>(),
@ -532,26 +537,6 @@ macro_rules! impl_tick_filter {
}
}
#[inline]
fn update_component_access(&self, access: &mut FilteredAccess<ComponentId>) {
if access.access().has_write(self.component_id) {
panic!("$state_name<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",
std::any::type_name::<T>());
}
access.add_read(self.component_id);
}
#[inline]
fn update_archetype_component_access(
&self,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if let Some(archetype_component_id) = archetype.get_archetype_component_id(self.component_id) {
access.add_read(archetype_component_id);
}
}
fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
set_contains_id(self.component_id)
}
@ -559,11 +544,11 @@ macro_rules! impl_tick_filter {
impl<'w, T: Component> WorldQueryGats<'w> for $name<T> {
type Fetch = $fetch_name<'w, T>;
type ReadOnlyFetch = $fetch_name<'w, T>;
type _State = $state_name<T>;
}
impl<'w, T: Component> Fetch<'w> for $fetch_name<'w, T> {
// SAFETY: this reads the T component. archetype component access and component access are updated to reflect that
unsafe impl<'w, T: Component> Fetch<'w> for $fetch_name<'w, T> {
type State = $state_name<T>;
type Item = bool;
@ -637,10 +622,30 @@ macro_rules! impl_tick_filter {
unsafe fn archetype_filter_fetch(&mut self, archetype_index: usize) -> bool {
self.archetype_fetch(archetype_index)
}
#[inline]
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
if access.access().has_write(state.component_id) {
panic!("$state_name<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",
std::any::type_name::<T>());
}
access.add_read(state.component_id);
}
#[inline]
fn update_archetype_component_access(
state: &Self::State,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
if let Some(archetype_component_id) = archetype.get_archetype_component_id(state.component_id) {
access.add_read(archetype_component_id);
}
}
}
/// SAFETY: read-only access
unsafe impl<'w, T: Component> ReadOnlyFetch for $fetch_name<'w, T> {}
unsafe impl<T: Component> ReadOnlyWorldQuery for $name<T> {}
impl<T> Clone for $fetch_name<'_, T> {
fn clone(&self) -> Self {

View file

@ -7,7 +7,7 @@ use crate::{
};
use std::{borrow::Borrow, iter::FusedIterator, marker::PhantomData, mem::MaybeUninit};
use super::{QueryFetch, QueryItem, ReadOnlyFetch};
use super::{QueryFetch, QueryItem, ReadOnlyWorldQuery};
/// An [`Iterator`] over query results of a [`Query`](crate::system::Query).
///
@ -302,11 +302,11 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> QueryCombinationIter<
// Iterator type is intentionally implemented only for read-only access.
// Doing so for mutable references would be unsound, because calling `next`
// multiple times would allow multiple owned references to the same data to exist.
impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> Iterator
impl<'w, 's, Q: ReadOnlyWorldQuery, F: ReadOnlyWorldQuery, const K: usize> Iterator
for QueryCombinationIter<'w, 's, Q, F, K>
where
QueryFetch<'w, Q>: Clone + ReadOnlyFetch,
QueryFetch<'w, F>: Clone + ReadOnlyFetch,
QueryFetch<'w, Q>: Clone,
QueryFetch<'w, F>: Clone,
{
type Item = [QueryItem<'w, Q>; K];
@ -366,11 +366,11 @@ where
}
// This is correct as [`QueryCombinationIter`] always returns `None` once exhausted.
impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> FusedIterator
impl<'w, 's, Q: ReadOnlyWorldQuery, F: ReadOnlyWorldQuery, const K: usize> FusedIterator
for QueryCombinationIter<'w, 's, Q, F, K>
where
QueryFetch<'w, Q>: Clone + ReadOnlyFetch,
QueryFetch<'w, F>: Clone + ReadOnlyFetch,
QueryFetch<'w, Q>: Clone,
QueryFetch<'w, F>: Clone,
{
}

View file

@ -47,13 +47,16 @@ impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {
let filter_state = <F::State as FetchState>::init(world);
let mut component_access = FilteredAccess::default();
fetch_state.update_component_access(&mut component_access);
QueryFetch::<'static, Q>::update_component_access(&fetch_state, &mut component_access);
// Use a temporary empty FilteredAccess for filters. This prevents them from conflicting with the
// main Query's `fetch_state` access. Filters are allowed to conflict with the main query fetch
// because they are evaluated *before* a specific reference is constructed.
let mut filter_component_access = FilteredAccess::default();
filter_state.update_component_access(&mut filter_component_access);
QueryFetch::<'static, F>::update_component_access(
&filter_state,
&mut filter_component_access,
);
// Merge the temporary filter access with the main access. This ensures that filter access is
// properly considered in a global "cross-query" context (both within systems and across systems).
@ -122,10 +125,16 @@ impl<Q: WorldQuery, F: WorldQuery> QueryState<Q, F> {
.filter_state
.matches_component_set(&|id| archetype.contains(id))
{
self.fetch_state
.update_archetype_component_access(archetype, &mut self.archetype_component_access);
self.filter_state
.update_archetype_component_access(archetype, &mut self.archetype_component_access);
QueryFetch::<'static, Q>::update_archetype_component_access(
&self.fetch_state,
archetype,
&mut self.archetype_component_access,
);
QueryFetch::<'static, F>::update_archetype_component_access(
&self.filter_state,
archetype,
&mut self.archetype_component_access,
);
let archetype_index = archetype.id().index();
if !self.matched_archetypes.contains(archetype_index) {
self.matched_archetypes.grow(archetype_index + 1);

View file

@ -3,7 +3,7 @@ use crate::{
entity::Entity,
query::{
NopFetch, QueryCombinationIter, QueryEntityError, QueryFetch, QueryItem, QueryIter,
QueryManyIter, QuerySingleError, QueryState, ROQueryFetch, ROQueryItem, ReadOnlyFetch,
QueryManyIter, QuerySingleError, QueryState, ROQueryFetch, ROQueryItem, ReadOnlyWorldQuery,
WorldQuery,
},
world::{Mut, World},
@ -1306,10 +1306,7 @@ impl std::fmt::Display for QueryComponentError {
}
}
impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F>
where
QueryFetch<'w, Q>: ReadOnlyFetch,
{
impl<'w, 's, Q: ReadOnlyWorldQuery, F: WorldQuery> Query<'w, 's, Q, F> {
/// Returns the query result for the given [`Entity`], with the actual "inner" world lifetime.
///
/// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is

View file

@ -6,8 +6,7 @@ use crate::{
component::{Component, ComponentId, ComponentTicks, Components},
entity::{Entities, Entity},
query::{
Access, FilteredAccess, FilteredAccessSet, QueryFetch, QueryState, ReadOnlyFetch,
WorldQuery,
Access, FilteredAccess, FilteredAccessSet, QueryState, ReadOnlyWorldQuery, WorldQuery,
},
system::{CommandQueue, Commands, Query, SystemMeta},
world::{FromWorld, World},
@ -136,10 +135,7 @@ impl<'w, 's, Q: WorldQuery + 'static, F: WorldQuery + 'static> SystemParam for Q
}
// SAFE: QueryState is constrained to read-only fetches, so it only reads World.
unsafe impl<Q: WorldQuery, F: WorldQuery> ReadOnlySystemParamFetch for QueryState<Q, F> where
for<'x> QueryFetch<'x, Q>: ReadOnlyFetch
{
}
unsafe impl<Q: ReadOnlyWorldQuery, F: WorldQuery> ReadOnlySystemParamFetch for QueryState<Q, F> {}
// SAFE: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If
// this QueryState conflicts with any prior access, a panic will occur.

View file

@ -6,12 +6,13 @@ warning: unused import: `SystemState`
|
= note: `#[warn(unused_imports)]` on by default
error[E0277]: the trait bound `for<'x> WriteFetch<'x, Foo>: ReadOnlyFetch` is not satisfied
error[E0277]: the trait bound `&'static mut Foo: ReadOnlyWorldQuery` is not satisfied
--> tests/ui/system_param_derive_readonly.rs:18:5
|
18 | assert_readonly::<Mutable>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `for<'x> ReadOnlyFetch` is not implemented for `WriteFetch<'x, Foo>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `ReadOnlyWorldQuery` is not implemented for `&'static mut Foo`
|
= note: `ReadOnlyWorldQuery` is implemented for `&'static Foo`, but not for `&'static mut Foo`
= note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `QueryState<&'static mut Foo>`
= note: 2 redundant requirements hidden
= note: required because of the requirements on the impl of `ReadOnlySystemParamFetch` for `_::FetchState<(QueryState<&'static mut Foo>,)>`

View file

@ -1,8 +1,8 @@
error[E0277]: the trait bound `WriteFetch<'_, Foo>: ReadOnlyFetch` is not satisfied
--> tests/ui/world_query_derive.rs:7:10
error[E0277]: the trait bound `&'static mut Foo: ReadOnlyWorldQuery` is not satisfied
--> tests/ui/world_query_derive.rs:9:8
|
7 | #[derive(WorldQuery)]
| ^^^^^^^^^^ the trait `ReadOnlyFetch` is not implemented for `WriteFetch<'_, Foo>`
9 | a: &'static mut Foo,
| ^^^^^^^^^^^^^^^^ the trait `ReadOnlyWorldQuery` is not implemented for `&'static mut Foo`
|
note: required by a bound in `_::assert_readonly`
--> tests/ui/world_query_derive.rs:7:10
@ -11,11 +11,11 @@ note: required by a bound in `_::assert_readonly`
| ^^^^^^^^^^ required by this bound in `_::assert_readonly`
= note: this error originates in the derive macro `WorldQuery` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: the trait bound `MutableMarkedFetch<'_>: ReadOnlyFetch` is not satisfied
--> tests/ui/world_query_derive.rs:18:10
error[E0277]: the trait bound `MutableMarked: ReadOnlyWorldQuery` is not satisfied
--> tests/ui/world_query_derive.rs:20:8
|
18 | #[derive(WorldQuery)]
| ^^^^^^^^^^ the trait `ReadOnlyFetch` is not implemented for `MutableMarkedFetch<'_>`
20 | a: MutableMarked,
| ^^^^^^^^^^^^^ the trait `ReadOnlyWorldQuery` is not implemented for `MutableMarked`
|
note: required by a bound in `_::assert_readonly`
--> tests/ui/world_query_derive.rs:18:10