Update naming and the docs

vladbat00 · vladbat00 · commit 0d1dbe368f84 · 2021-08-24T13:22:24.000+03:00
diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs
@@ -23,9 +23,9 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream {
         impl_generics,
         ty_generics,
         where_clause,
-        struct_has_world_lt,
-        world_lt,
-        state_lt,
+        struct_has_world_lifetime,
+        world_lifetime,
+        state_lifetime,
     } = fetch_impl_tokens(&ast);
 
     // Fetch's HRTBs require this hack to make the implementation compile. I don't fully understand
@@ -34,9 +34,9 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream {
     // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails)
     // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles)
     let mut fetch_generics = ast.generics.clone();
-    fetch_generics.params.insert(0, state_lt);
-    if !struct_has_world_lt {
-        fetch_generics.params.insert(0, world_lt);
+    fetch_generics.params.insert(0, state_lifetime);
+    if !struct_has_world_lifetime {
+        fetch_generics.params.insert(0, world_lifetime);
     }
     fetch_generics
         .params
@@ -46,7 +46,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream {
         ))));
     let (fetch_impl_generics, _, _) = fetch_generics.split_for_impl();
     let mut fetch_generics = ast.generics.clone();
-    if struct_has_world_lt {
+    if struct_has_world_lifetime {
         *fetch_generics.params.first_mut().unwrap() =
             GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site())));
     }
@@ -142,9 +142,9 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream {
             /// SAFETY: each item in the struct is read only
             unsafe impl #impl_generics #path::query::ReadOnlyFetch for #fetch_struct_name #ty_generics #where_clause {}
 
-            // Statically checks that the safety guarantee holds true indeed. We need this to make
-            // sure that we don't compile ReadOnlyFetch if our struct contains nested WorldQuery
-            // that don't implement it.
+            // Statically checks that the safety guarantee actually holds true. We need this to make
+            // sure that we don't compile `ReadOnlyFetch` if our struct contains nested `WorldQuery`
+            // members that don't implement it.
             #[allow(dead_code)]
             const _: () = {
                 fn assert_readonly<T: #path::query::ReadOnlyFetch>() {}
@@ -183,16 +183,20 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream {
                 true #(&& self.#field_idents.is_dense())*
             }
 
+            /// SAFETY: we call `set_archetype` for each member that implements `Fetch`
             #[inline]
             unsafe fn set_archetype(&mut self, _state: &Self::State, _archetype: &#path::archetype::Archetype, _tables: &#path::storage::Tables) {
                 #(self.#field_idents.set_archetype(&_state.#field_idents, _archetype, _tables);)*
             }
 
+            /// SAFETY: we call `set_table` for each member that implements `Fetch`
             #[inline]
             unsafe fn set_table(&mut self, _state: &Self::State, _table: &#path::storage::Table) {
                 #(self.#field_idents.set_table(&_state.#field_idents, _table);)*
             }
 
+            /// SAFETY: we call `table_fetch` for each member that implements `Fetch` or
+            /// `table_filter_fetch` if it also implements `FilterFetch`.
             #[inline]
             unsafe fn table_fetch(&mut self, _table_row: usize) -> Self::Item {
                 use #path::query::FilterFetch;
@@ -203,6 +207,8 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream {
                 }
             }
 
+            /// SAFETY: we call `archetype_fetch` for each member that implements `Fetch` or
+            /// `archetype_filter_fetch` if it also implements `FilterFetch`.
             #[inline]
             unsafe fn archetype_fetch(&mut self, _archetype_index: usize) -> Self::Item {
                 use #path::query::FilterFetch;
@@ -214,7 +220,7 @@ pub fn derive_fetch_impl(input: TokenStream) -> TokenStream {
             }
         }
 
-        // SAFETY: update_component_access and update_archetype_component_access are called for each item in the struct
+        // SAFETY: `update_component_access` and `update_archetype_component_access` are called for each item in the struct
         unsafe impl #impl_generics #path::query::FetchState for #state_struct_name #ty_generics #where_clause {
             fn init(world: &mut #path::world::World) -> Self {
                 #state_struct_name {
@@ -260,9 +266,9 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream {
         impl_generics,
         ty_generics,
         where_clause,
-        struct_has_world_lt,
-        world_lt,
-        state_lt,
+        struct_has_world_lifetime,
+        world_lifetime,
+        state_lifetime,
     } = fetch_impl_tokens(&ast);
 
     // Fetch's HRTBs require this hack to make the implementation compile. I don't fully understand
@@ -271,9 +277,9 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream {
     // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=da5e260a5c2f3e774142d60a199e854a (this fails)
     // - https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=802517bb3d8f83c45ee8c0be360bb250 (this compiles)
     let mut fetch_generics = ast.generics.clone();
-    fetch_generics.params.insert(0, state_lt);
-    if !struct_has_world_lt {
-        fetch_generics.params.insert(0, world_lt);
+    fetch_generics.params.insert(0, state_lifetime);
+    if !struct_has_world_lifetime {
+        fetch_generics.params.insert(0, world_lifetime);
     }
     fetch_generics
         .params
@@ -283,7 +289,7 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream {
         ))));
     let (fetch_impl_generics, _, _) = fetch_generics.split_for_impl();
     let mut fetch_generics = ast.generics.clone();
-    if struct_has_world_lt {
+    if struct_has_world_lifetime {
         *fetch_generics.params.first_mut().unwrap() =
             GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'fetch", Span::call_site())));
     }
@@ -410,6 +416,7 @@ pub fn derive_filter_fetch_impl(input: TokenStream) -> TokenStream {
     tokens
 }
 
+// This struct is used to share common tokens between `Fetch` and `FilterFetch` implementations.
 struct FetchImplTokens<'a> {
     struct_name: Ident,
     fetch_struct_name: Ident,
@@ -418,26 +425,26 @@ struct FetchImplTokens<'a> {
     impl_generics: ImplGenerics<'a>,
     ty_generics: TypeGenerics<'a>,
     where_clause: Option<&'a WhereClause>,
-    struct_has_world_lt: bool,
-    world_lt: GenericParam,
-    state_lt: GenericParam,
+    struct_has_world_lifetime: bool,
+    world_lifetime: GenericParam,
+    state_lifetime: GenericParam,
 }
 
 fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens {
-    let world_lt = ast.generics.params.first().and_then(|param| match param {
+    let world_lifetime = ast.generics.params.first().and_then(|param| match param {
         lt @ GenericParam::Lifetime(_) => Some(lt.clone()),
         _ => None,
     });
-    let struct_has_world_lt = world_lt.is_some();
-    let world_lt = world_lt.unwrap_or_else(|| {
+    let struct_has_world_lifetime = world_lifetime.is_some();
+    let world_lifetime = world_lifetime.unwrap_or_else(|| {
         GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'world", Span::call_site())))
     });
-    let state_lt =
+    let state_lifetime =
         GenericParam::Lifetime(LifetimeDef::new(Lifetime::new("'state", Span::call_site())));
 
     let mut fetch_trait_punctuated_lifetimes = Punctuated::<_, Token![,]>::new();
-    fetch_trait_punctuated_lifetimes.push(world_lt.clone());
-    fetch_trait_punctuated_lifetimes.push(state_lt.clone());
+    fetch_trait_punctuated_lifetimes.push(world_lifetime.clone());
+    fetch_trait_punctuated_lifetimes.push(state_lifetime.clone());
 
     let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl();
 
@@ -453,9 +460,9 @@ fn fetch_impl_tokens(ast: &DeriveInput) -> FetchImplTokens {
         impl_generics,
         ty_generics,
         where_clause,
-        struct_has_world_lt,
-        world_lt,
-        state_lt,
+        struct_has_world_lifetime,
+        world_lifetime,
+        state_lifetime,
     }
 }
 
diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs
@@ -22,6 +22,10 @@ use std::{
 ///
 /// See [`Query`](crate::system::Query) for a primer on queries.
 ///
+/// If you want to implement a custom query, see [`Fetch`] trait documentation.
+///
+/// If you want to implement a custom query filter, see [`FilterFetch`] trait documentation.
+///
 /// # Basic WorldQueries
 ///
 /// Here is a small list of the most important world queries to know about where `C` stands for a
@@ -46,19 +50,47 @@ pub trait WorldQuery {
     type State: FetchState;
 }
 
+/// Types that implement this trait are responsible for fetching query items from tables or
+/// archetypes.
+///
+/// Every type that implements [`WorldQuery`] have their associated [`WorldQuery::Fetch`] and
+/// [`WorldQuery::State`] types that are essential for fetching component data. If you want to
+/// implement a custom query type, you'll need to implement [`Fetch`] and [`FetchState`] for
+/// those associated types.
+///
+/// You may want to implement a custom query for the following reasons:
+/// - Named structs can be easier to use than complex query tuples. Access via struct fields
+///   is more convenient than destructuring tuples or accessing them via `q.0, q.1, ...` pattern
+///   and saves a lot of maintenance burden when adding or removing components.
+/// - Nested queries enable the composition pattern and makes query types easier to re-use.
+/// - It allows to go over the limit of 15 components that exists for query tuples.
+///
 /// # Derive
 ///
 /// This trait can be derived with the [`derive@super::Fetch`] macro.
 /// To do so, all fields in the struct must themselves impl [`WorldQuery`].
 ///
+/// The derive macro implements [`WorldQuery`] for your type and declares two structs that
+/// implement [`Fetch`] and [`FetchState`] and are used as [`WorldQuery::Fetch`] and
+/// [`WorldQuery::State`] associated types respectively.
+///
+/// **Note:** currently, the macro only supports named structs.
+///
 /// ```
 /// # use bevy_ecs::prelude::*;
 /// use bevy_ecs::query::Fetch;
 ///
+/// struct Foo;
+/// struct Bar;
+/// struct OptionalFoo;
+/// struct OptionalBar;
+///
 /// #[derive(Fetch)]
 /// struct MyQuery<'w> {
-///     foo: &'w u32,
-///     bar: Mut<'w, i32>,
+///     foo: &'w Foo,
+///     bar: Mut<'w, Bar>,
+///     optional_foo: Option<&'w OptionalFoo>,
+///     optional_bar: Option<Mut<'w, OptionalBar>>,
 /// }
 ///
 /// fn my_system(mut query: Query<MyQuery>) {
@@ -74,19 +106,55 @@ pub trait WorldQuery {
 ///
 /// All filter members must be marked with `filter` attribute and have `bool` type.
 ///
+/// **Note:** values of filter members will always be `true`, which means that the entities that
+/// don't meet filter requirements, won't be accessed. If you want to check, for instance, whether
+/// a component exists or not, use `Option<&T>` instead of filters.
+///
 /// ```
 /// # use bevy_ecs::prelude::*;
 /// use bevy_ecs::query::Fetch;
 ///
+/// struct Foo;
+/// struct Bar;
+///
 /// #[derive(Fetch)]
 /// struct MyQuery<'w> {
-///     foo: &'w u32,
-///     bar: Mut<'w, i32>,
-///     #[filter(Changed<u32>)]
+///     foo: &'w Foo,
+///     bar: Mut<'w, Bar>,
+///     #[filter(Changed<Foo>)]
 ///     foo_is_changed: bool,
 /// }
 /// ```
 ///
+/// ## Nested queries
+///
+/// Using nested queries enable the composition pattern, which makes it possible to re-use other
+/// query types. All types that implement [`WorldQuery`] (including the ones that use this derive
+/// macro) are supported.
+///
+/// ```
+/// # use bevy_ecs::prelude::*;
+/// use bevy_ecs::query::Fetch;
+///
+/// struct Foo;
+/// struct Bar;
+/// struct OptionalFoo;
+/// struct OptionalBar;
+///
+/// #[derive(Fetch)]
+/// struct MyQuery<'w> {
+///     foo: FooQuery<'w>,
+///     bar: (&'w Bar, Option<&'w OptionalBar>)
+/// }
+///
+/// #[derive(Fetch)]
+/// struct FooQuery<'w> {
+///     foo: &'w Foo,
+///     optional_foo: Option<&'w OptionalFoo>,
+/// }
+///
+/// ```
+///
 /// ## Read-only queries
 ///
 /// All queries that access components non-mutably are read-only by default, with the exception
@@ -98,41 +166,47 @@ pub trait WorldQuery {
 /// # use bevy_ecs::prelude::*;
 /// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery};
 ///
+/// struct Foo;
+/// struct Bar;
+///
 /// #[derive(Fetch)]
 /// struct FooQuery<'w> {
-///     foo: &'w u32,
+///     foo: &'w Foo,
 ///     #[readonly]
 ///     bar_query: BarQuery<'w>,
 /// }
 ///
 /// #[derive(Fetch)]
 /// struct BarQuery<'w> {
-///     bar: &'w u32,
+///     bar: &'w Bar,
 /// }
 ///
 /// fn assert_readonly<T: ReadOnlyFetch>() {}
 ///
 /// assert_readonly::<<FooQuery as WorldQuery>::Fetch>();
 /// ```
 ///
-/// **Note** that if you mark a field that doesn't implement `ReadOnlyFetch` as `readonly`, the
+/// **Note:** if you mark a field that doesn't implement `ReadOnlyFetch` as `readonly`,
 /// compilation will fail. We insert static checks as in the example above for every nested query
 /// marked as `readonly`. (They neither affect the runtime, nor pollute your local namespace.)
 ///
 /// ```compile_fail
 /// # use bevy_ecs::prelude::*;
 /// use bevy_ecs::query::{Fetch, ReadOnlyFetch, WorldQuery};
 ///
+/// struct Foo;
+/// struct Bar;
+///
 /// #[derive(Fetch)]
 /// struct FooQuery<'w> {
-///     foo: &'w u32,
+///     foo: &'w Foo,
 ///     #[readonly]
 ///     bar_query: BarQuery<'w>,
 /// }
 ///
 /// #[derive(Fetch)]
 /// struct BarQuery<'w> {
-///     bar: Mut<'w, u32>,
+///     bar: Mut<'w, Bar>,
 /// }
 /// ```
 pub trait Fetch<'world, 'state>: Sized {
diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs
@@ -12,26 +12,42 @@ use std::{cell::UnsafeCell, marker::PhantomData, ptr};
 /// Extension trait for [`Fetch`] containing methods used by query filters.
 /// This trait exists to allow "short circuit" behaviors for relevant query filter fetches.
 ///
+/// This trait is automatically implemented for every type that implements [`Fetch`] trait and
+/// specifies `bool` as the associated type for [`Fetch::Item`].
+///
+/// Using [`derive@super::FilterFetch`] macro allows creating custom query filters.
+/// You may want to implement a custom query filter for the following reasons:
+/// - Nested query filters enable the composition pattern and makes them easier to re-use.
+/// - It allows to go over the limit of 15 components that exists for query filters declared as
+///   tuples.
+///
 /// ## Derive
 ///
 /// This trait can be derived with the [`derive@super::FilterFetch`] macro.
 /// To do so, all fields in the struct must be filters themselves (their [`WorldQuery::Fetch`]
 /// associated types should implement [`FilterFetch`]).
 ///
+/// **Note:** currently, the macro only supports named structs.
+///
 /// ```
 /// # use bevy_ecs::prelude::*;
 /// use bevy_ecs::{query::FilterFetch, component::Component};
 ///
+/// struct Foo;
+/// struct Bar;
+/// struct Baz;
+/// struct Qux;
+///
 /// #[derive(FilterFetch)]
 /// struct MyFilter<T: Component, P: Component> {
-///     _u_16: With<u16>,
-///     _u_32: With<u32>,
-///     _or: Or<(With<i16>, Changed<u16>, Added<u32>)>,
+///     _foo: With<Foo>,
+///     _bar: With<Bar>,
+///     _or: Or<(With<Baz>, Changed<Foo>, Added<Bar>)>,
 ///     _generic_tuple: (With<T>, Without<P>),
 ///     _tp: std::marker::PhantomData<(T, P)>,
 /// }
 ///
-/// fn my_system(query: Query<Entity, MyFilter<u16, i16>>) {
+/// fn my_system(query: Query<Entity, MyFilter<Foo, Qux>>) {
 ///     for _ in query.iter() {}
 /// }
 ///
