[Merged by Bors] - ParamSet for conflicting SystemParam:s

.github/contributing/example_style_guide.md

@@ -36,7 +36,7 @@ For more advice on writing examples, see the [relevant section](../../CONTRIBUTI
 2. Prefer `for` loops over `.for_each`. The latter is faster (for now), but it is less clear for beginners, less idiomatic, and less flexible.
 3. Use `.single` and `.single_mut` where appropriate.
 4. In Queries, prefer `With<T>` filters over actually fetching unused data with `&T`.
-5. Prefer disjoint queries using `With` and `Without` over query sets when you need more than one query in a single system.
+5. Prefer disjoint queries using `With` and `Without` over param sets when you need more than one query in a single system.
 6. Prefer structs with named fields over tuple structs except in the case of single-field wrapper types.
 7. Use enum-labels over string-labels for system / stage / etc. labels.
 

crates/bevy_ecs/macros/src/lib.rs

@@ -176,91 +176,105 @@ fn get_idents(fmt_string: fn(usize) -> String, count: usize) -> Vec<Ident> {
 }
 
 #[proc_macro]
-pub fn impl_query_set(_input: TokenStream) -> TokenStream {
+pub fn impl_param_set(_input: TokenStream) -> TokenStream {
     let mut tokens = TokenStream::new();
-    let max_queries = 4;
-    let queries = get_idents(|i| format!("Q{}", i), max_queries);
-    let filters = get_idents(|i| format!("F{}", i), max_queries);
-    let mut query_fn_muts = Vec::new();
-    for i in 0..max_queries {
-        let query = &queries[i];
-        let filter = &filters[i];
-        let fn_name = Ident::new(&format!("q{}", i), Span::call_site());
+    let max_params = 8;
+    let params = get_idents(|i| format!("P{}", i), max_params);
+    let params_fetch = get_idents(|i| format!("PF{}", i), max_params);
+    let values = get_idents(|i| format!("p{}", i), max_params);
+    let mut param_fn_muts = Vec::new();
+    for (i, param) in params.iter().enumerate() {
+        let fn_name = Ident::new(&format!("p{}", i), Span::call_site());
         let index = Index::from(i);
-        query_fn_muts.push(quote! {
-            pub fn #fn_name(&mut self) -> Query<'_, '_, #query, #filter> {
+        param_fn_muts.push(quote! {
+            pub fn #fn_name<'a>(&'a mut self) -> <#param::Fetch as SystemParamFetch<'a, 'a>>::Item {
                 // SAFE: systems run without conflicts with other systems.
-                // Conflicting queries in QuerySet are not accessible at the same time
-                // QuerySets are guaranteed to not conflict with other SystemParams
+                // Conflicting params in ParamSet are not accessible at the same time
+                // ParamSets are guaranteed to not conflict with other SystemParams
                 unsafe {
-                    Query::new(self.world, &self.query_states.#index, self.last_change_tick, self.change_tick)
+                    <#param::Fetch as SystemParamFetch<'a, 'a>>::get_param(&mut self.param_states.#index, &self.system_meta, self.world, self.change_tick)
                 }
             }
         });
     }
 
-    for query_count in 1..=max_queries {
-        let query = &queries[0..query_count];
-        let filter = &filters[0..query_count];
-        let query_fn_mut = &query_fn_muts[0..query_count];
+    for param_count in 1..=max_params {
+        let param = &params[0..param_count];
+        let param_fetch = &params_fetch[0..param_count];
+        let value = &values[0..param_count];
+        let param_fn_mut = &param_fn_muts[0..param_count];
         tokens.extend(TokenStream::from(quote! {
-            impl<'w, 's, #(#query: WorldQuery + 'static,)* #(#filter: WorldQuery + 'static,)*> SystemParam for QuerySet<'w, 's, (#(QueryState<#query, #filter>,)*)>
-                where #(#filter::Fetch: FilterFetch,)*
+            impl<'w, 's, #(#param: SystemParam,)*> SystemParam for ParamSet<'w, 's, (#(#param,)*)>
             {
-                type Fetch = QuerySetState<(#(QueryState<#query, #filter>,)*)>;
+                type Fetch = ParamSetState<(#(#param::Fetch,)*)>;
             }
 
-            // SAFE: All Queries are constrained to ReadOnlyFetch, so World is only read
-            unsafe impl<#(#query: WorldQuery + 'static,)* #(#filter: WorldQuery + 'static,)*> ReadOnlySystemParamFetch for QuerySetState<(#(QueryState<#query, #filter>,)*)>
-            where #(#query::Fetch: ReadOnlyFetch,)* #(#filter::Fetch: FilterFetch,)*
+            // SAFE: All parameters are constrained to ReadOnlyFetch, so World is only read
+
+            unsafe impl<#(#param_fetch: for<'w1, 's1> SystemParamFetch<'w1, 's1>,)*> ReadOnlySystemParamFetch for ParamSetState<(#(#param_fetch,)*)>
+            where #(#param_fetch: ReadOnlySystemParamFetch,)*
             { }
 
-            // SAFE: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any QueryState conflicts
+            // SAFE: Relevant parameter ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any ParamState conflicts
             // with any prior access, a panic will occur.
-            unsafe impl<#(#query: WorldQuery + 'static,)* #(#filter: WorldQuery + 'static,)*> SystemParamState for QuerySetState<(#(QueryState<#query, #filter>,)*)>
-                where #(#filter::Fetch: FilterFetch,)*
+
+            unsafe impl<#(#param_fetch: for<'w1, 's1> SystemParamFetch<'w1, 's1>,)*> SystemParamState for ParamSetState<(#(#param_fetch,)*)>
             {
-                type Config = ();
+                type Config = (#(#param_fetch::Config,)*);
+
+
                 fn init(world: &mut World, system_meta: &mut SystemMeta, config: Self::Config) -> Self {
+                    let (#(#value,)*) = config;
                     #(
-                        let mut #query = QueryState::<#query, #filter>::new(world);
-                        assert_component_access_compatibility(
-                            &system_meta.name,
-                            std::any::type_name::<#query>(),
-                            std::any::type_name::<#filter>(),
-                            &system_meta.component_access_set,
-                            &#query.component_access,
-                            world,
-                        );
+                        // Pretend to add each param to the system alone, see if it conflicts
+                        let #param = #param_fetch::init(world, &mut system_meta.clone(), #value);
                     )*
                     #(
                         system_meta
                             .component_access_set
-                            .add(#query.component_access.clone());
+                            .extend(#param.component_access_set());
                         system_meta
                             .archetype_component_access
-                            .extend(&#query.archetype_component_access);
+                            .extend(&#param.archetype_component_access());
                     )*
-                    QuerySetState((#(#query,)*))
+                    ParamSetState((#(#param,)*))
                 }
 
                 fn new_archetype(&mut self, archetype: &Archetype, system_meta: &mut SystemMeta) {
-                    let (#(#query,)*) = &mut self.0;
+                    let (#(#param,)*) = &mut self.0;
                     #(
-                        #query.new_archetype(archetype);
-                        system_meta
-                            .archetype_component_access
-                            .extend(&#query.archetype_component_access);
+                        #param.new_archetype(archetype, system_meta);
                     )*
                 }
 
-                fn default_config() {}
+                fn archetype_component_access(&self) -> Access<ArchetypeComponentId> {
+                    let (#(#param,)*) = &self.0;
+                    let mut combined_access = Access::<ArchetypeComponentId>::default();
+                    #({
+                        combined_access.extend(&#param.archetype_component_access());
+                    })*;
+                    combined_access
+                }
+
+                fn component_access_set(&self) -> FilteredAccessSet<ComponentId> {
+                    let (#(#param,)*) = &self.0;
+                    let mut combined_access = FilteredAccessSet::<ComponentId>::default();
+                    #({
+                        combined_access.extend(#param.component_access_set());
+                    })*;
+                    combined_access
+                }
+
+                fn default_config() -> Self::Config {
+                    (#(#param_fetch::default_config(),)*)
+                }
             }
 
-            impl<'w, 's, #(#query: WorldQuery + 'static,)* #(#filter: WorldQuery + 'static,)*> SystemParamFetch<'w, 's> for QuerySetState<(#(QueryState<#query, #filter>,)*)>
-                where #(#filter::Fetch: FilterFetch,)*
+
+
+            impl<'w, 's, #(#param_fetch: for<'w1, 's1> SystemParamFetch<'w1, 's1>,)*> SystemParamFetch<'w, 's> for ParamSetState<(#(#param_fetch,)*)>
             {
-                type Item = QuerySet<'w, 's, (#(QueryState<#query, #filter>,)*)>;
+                type Item = ParamSet<'w, 's, (#(<#param_fetch as SystemParamFetch<'w, 's>>::Item,)*)>;
 
                 #[inline]
                 unsafe fn get_param(
@@ -269,19 +283,19 @@ pub fn impl_query_set(_input: TokenStream) -> TokenStream {
                     world: &'w World,
                     change_tick: u32,
                 ) -> Self::Item {
-                    QuerySet {
-                        query_states: &state.0,
+                    ParamSet {
+                        param_states: &mut state.0,
+                        system_meta: system_meta.clone(),
                         world,
-                        last_change_tick: system_meta.last_change_tick,
                         change_tick,
                     }
                 }
             }
 
-            impl<'w, 's, #(#query: WorldQuery,)* #(#filter: WorldQuery,)*> QuerySet<'w, 's, (#(QueryState<#query, #filter>,)*)>
-                where #(#filter::Fetch: FilterFetch,)*
+            impl<'w, 's, #(#param: SystemParam,)*> ParamSet<'w, 's, (#(#param,)*)>
             {
-                #(#query_fn_mut)*
+
+                #(#param_fn_mut)*
             }
         }));
     }
@@ -395,6 +409,14 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
                 }
             }
 
+            fn archetype_component_access(&self) -> #path::query::Access<#path::archetype::ArchetypeComponentId> {
+                self.state.archetype_component_access()
+            }
+
+            fn component_access_set(&self) -> #path::query::FilteredAccessSet<#path::component::ComponentId> {
+                self.state.component_access_set()
+            }
+
             fn new_archetype(&mut self, archetype: &#path::archetype::Archetype, system_meta: &mut #path::system::SystemMeta) {
                 self.state.new_archetype(archetype, system_meta)
             }

crates/bevy_ecs/src/lib.rs

@@ -34,7 +34,7 @@ pub mod prelude {
         },
         system::{
             Commands, ConfigurableSystem, In, IntoChainSystem, IntoExclusiveSystem, IntoSystem,
-            Local, NonSend, NonSendMut, Query, QuerySet, RemovedComponents, Res, ResMut, System,
+            Local, NonSend, NonSendMut, ParamSet, Query, RemovedComponents, Res, ResMut, System,
         },
         world::{FromWorld, Mut, World},
     };

crates/bevy_ecs/src/query/access.rs

@@ -1,4 +1,5 @@
 use crate::storage::SparseSetIndex;
+use bevy_utils::HashSet;
 use fixedbitset::FixedBitSet;
 use std::marker::PhantomData;
 
@@ -129,7 +130,7 @@ impl<T: SparseSetIndex> Access<T> {
     }
 }
 
-#[derive(Clone, Eq, PartialEq)]
+#[derive(Clone, Eq, PartialEq, Debug)]
 pub struct FilteredAccess<T: SparseSetIndex> {
     access: Access<T>,
     with: FixedBitSet,
@@ -146,6 +147,14 @@ impl<T: SparseSetIndex> Default for FilteredAccess<T> {
     }
 }
 
+impl<T: SparseSetIndex> From<FilteredAccess<T>> for FilteredAccessSet<T> {
+    fn from(filtered_access: FilteredAccess<T>) -> Self {
+        let mut base = FilteredAccessSet::<T>::default();
+        base.add(filtered_access);
+        base
+    }
+}
+
 impl<T: SparseSetIndex> FilteredAccess<T> {
     #[inline]
     pub fn access(&self) -> &Access<T> {
@@ -191,7 +200,7 @@ impl<T: SparseSetIndex> FilteredAccess<T> {
         self.access.read_all();
     }
 }
-
+#[derive(Clone, Debug)]
 pub struct FilteredAccessSet<T: SparseSetIndex> {
     combined_access: Access<T>,
     filtered_accesses: Vec<FilteredAccess<T>>,
@@ -211,22 +220,59 @@ impl<T: SparseSetIndex> FilteredAccessSet<T> {
     pub fn get_conflicts(&self, filtered_access: &FilteredAccess<T>) -> Vec<T> {
         // if combined unfiltered access is incompatible, check each filtered access for
         // compatibility
+        let mut conflicts = HashSet::<usize>::default();
         if !filtered_access.access.is_compatible(&self.combined_access) {
             for current_filtered_access in self.filtered_accesses.iter() {
                 if !current_filtered_access.is_compatible(filtered_access) {
-                    return current_filtered_access
-                        .access
-                        .get_conflicts(&filtered_access.access);
+                    conflicts.extend(
+                        current_filtered_access
+                            .access
+                            .get_conflicts(&filtered_access.access)
+                            .iter()
+                            .map(|ind| ind.sparse_set_index()),
+                    );
                 }
             }
         }
-        Vec::new()
+        conflicts
+            .iter()
+            .map(|ind| T::get_sparse_set_index(*ind))
+            .collect()
+    }
+
+    pub fn get_conflicts_set(&self, filtered_access_set: &FilteredAccessSet<T>) -> Vec<T> {
+        // if combined unfiltered access is incompatible, check each filtered access for
+        // compatibility with the set
+        let mut conflicts = HashSet::<usize>::default();
+        if !filtered_access_set
+            .combined_access
+            .is_compatible(&self.combined_access)
+        {
+            for current_filtered_access in filtered_access_set.filtered_accesses.iter() {
+                conflicts.extend(
+                    self.get_conflicts(current_filtered_access)
+                        .iter()
+                        .map(|ind| ind.sparse_set_index()),
+                );
+            }
+        }
+        conflicts
+            .iter()
+            .map(|ind| T::get_sparse_set_index(*ind))
+            .collect()
     }
 
     pub fn add(&mut self, filtered_access: FilteredAccess<T>) {
         self.combined_access.extend(&filtered_access.access);
         self.filtered_accesses.push(filtered_access);
     }
+
+    pub fn extend(&mut self, filtered_access_set: FilteredAccessSet<T>) {
+        self.combined_access
+            .extend(&filtered_access_set.combined_access);
+        self.filtered_accesses
+            .extend(filtered_access_set.filtered_accesses);
+    }
 }
 
 impl<T: SparseSetIndex> Default for FilteredAccessSet<T> {

crates/bevy_ecs/src/system/function_system.rs

@@ -12,6 +12,7 @@ use bevy_ecs_macros::all_tuples;
 use std::{borrow::Cow, marker::PhantomData};
 
 /// The metadata of a [`System`].
+#[derive(Clone)]
 pub struct SystemMeta {
     pub(crate) name: Cow<'static, str>,
     pub(crate) component_access_set: FilteredAccessSet<ComponentId>,