Add marker traits to distinguish base sets from regular system sets (b…

…evyengine#7863)

# Objective

Base sets, added in bevyengine#7466  are a special type of system set. Systems can only be added to base sets via `in_base_set`, while non-base sets can only be added via `in_set`. Unfortunately this is currently guarded by a runtime panic, which presents an unfortunate toe-stub when the wrong method is used. The delayed response between writing code and encountering the error (possibly hours) makes the distinction between base sets and other sets much more difficult to learn.

## Solution

Add the marker traits `BaseSystemSet` and `FreeSystemSet`. `in_base_set` and `in_set` now respectively accept these traits, which moves the runtime panic to a compile time error.

---

## Changelog

+ Added the marker trait `BaseSystemSet`, which is distinguished from a `FreeSystemSet`. These are both subtraits of `SystemSet`.

## Migration Guide

None if merged with 0.10

benches/benches/bevy_ecs/scheduling/schedule.rs

@@ -63,16 +63,11 @@ pub fn build_schedule(criterion: &mut Criterion) {
 
     // Use multiple different kinds of label to ensure that dynamic dispatch
     // doesn't somehow get optimized away.
-    #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+    #[derive(SystemSet, Debug, Clone, Copy, PartialEq, Eq, Hash)]
     struct NumSet(usize);
-    #[derive(Debug, Clone, Copy, SystemSet, PartialEq, Eq, Hash)]
-    struct DummySet;
 
-    impl SystemSet for NumSet {
-        fn dyn_clone(&self) -> Box<dyn SystemSet> {
-            Box::new(self.clone())
-        }
-    }
+    #[derive(SystemSet, Debug, Clone, Copy, PartialEq, Eq, Hash)]
+    struct DummySet;
 
     let mut group = criterion.benchmark_group("build_schedule");
     group.warm_up_time(std::time::Duration::from_millis(500));

crates/bevy_app/src/config.rs

@@ -1,8 +1,8 @@
 use bevy_ecs::{
     all_tuples,
     schedule::{
-        BoxedScheduleLabel, Condition, IntoSystemConfig, IntoSystemSet, ScheduleLabel,
-        SystemConfig, SystemConfigs, SystemSet,
+        BaseSystemSet, BoxedScheduleLabel, Condition, FreeSystemSet, IntoSystemConfig,
+        IntoSystemSet, ScheduleLabel, SystemConfig, SystemConfigs,
     },
 };
 
@@ -84,7 +84,7 @@ impl IntoSystemConfig<(), Self> for SystemAppConfig {
     }
 
     #[track_caller]
-    fn in_set(self, set: impl SystemSet) -> Self {
+    fn in_set(self, set: impl FreeSystemSet) -> Self {
         let Self { system, schedule } = self;
         Self {
             system: system.in_set(set),
@@ -93,7 +93,7 @@ impl IntoSystemConfig<(), Self> for SystemAppConfig {
     }
 
     #[track_caller]
-    fn in_base_set(self, set: impl SystemSet) -> Self {
+    fn in_base_set(self, set: impl BaseSystemSet) -> Self {
         let Self { system, schedule } = self;
         Self {
             system: system.in_base_set(set),

crates/bevy_ecs/macros/src/set.rs

@@ -1,5 +1,5 @@
 use proc_macro::TokenStream;
-use quote::{quote, ToTokens};
+use quote::{format_ident, quote, ToTokens};
 use syn::parse::{Parse, ParseStream};
 
 pub static SYSTEM_SET_ATTRIBUTE_NAME: &str = "system_set";
@@ -12,6 +12,14 @@ pub static BASE_ATTRIBUTE_NAME: &str = "base";
 /// - `input`: The [`syn::DeriveInput`] for the struct that we want to derive the set trait for
 /// - `trait_path`: The [`syn::Path`] to the set trait
 pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream {
+    let mut base_trait_path = trait_path.clone();
+    let ident = &mut base_trait_path.segments.last_mut().unwrap().ident;
+    *ident = format_ident!("Base{ident}");
+
+    let mut free_trait_path = trait_path.clone();
+    let ident = &mut free_trait_path.segments.last_mut().unwrap().ident;
+    *ident = format_ident!("Free{ident}");
+
     let ident = input.ident;
 
     let mut is_base = false;
@@ -65,6 +73,16 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea
         .unwrap(),
     );
 
+    let marker_impl = if is_base {
+        quote! {
+            impl #impl_generics #base_trait_path for #ident #ty_generics #where_clause {}
+        }
+    } else {
+        quote! {
+            impl #impl_generics #free_trait_path for #ident #ty_generics #where_clause {}
+        }
+    };
+
     (quote! {
         impl #impl_generics #trait_path for #ident #ty_generics #where_clause {
             fn is_base(&self) -> bool {
@@ -75,6 +93,8 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea
                 std::boxed::Box::new(std::clone::Clone::clone(self))
             }
         }
+
+        #marker_impl
     })
     .into()
 }

crates/bevy_ecs/src/schedule/config.rs

@@ -9,6 +9,8 @@ use crate::{
     system::{BoxedSystem, IntoSystem, System},
 };
 
+use super::{BaseSystemSet, FreeSystemSet};
+
 /// A [`SystemSet`] with scheduling metadata.
 pub struct SystemSetConfig {
     pub(super) set: BoxedSystemSet,
@@ -86,10 +88,10 @@ pub trait IntoSystemSetConfig {
     fn into_config(self) -> SystemSetConfig;
     /// Add to the provided `set`.
     #[track_caller]
-    fn in_set(self, set: impl SystemSet) -> SystemSetConfig;
+    fn in_set(self, set: impl FreeSystemSet) -> SystemSetConfig;
     /// Add to the provided "base" `set`. For more information on base sets, see [`SystemSet::is_base`].
     #[track_caller]
-    fn in_base_set(self, set: impl SystemSet) -> SystemSetConfig;
+    fn in_base_set(self, set: impl BaseSystemSet) -> SystemSetConfig;
     /// Add this set to the schedules's default base set.
     fn in_default_base_set(self) -> SystemSetConfig;
     /// Run before all systems in `set`.
@@ -115,12 +117,12 @@ impl<S: SystemSet> IntoSystemSetConfig for S {
     }
 
     #[track_caller]
-    fn in_set(self, set: impl SystemSet) -> SystemSetConfig {
+    fn in_set(self, set: impl FreeSystemSet) -> SystemSetConfig {
         self.into_config().in_set(set)
     }
 
     #[track_caller]
-    fn in_base_set(self, set: impl SystemSet) -> SystemSetConfig {
+    fn in_base_set(self, set: impl BaseSystemSet) -> SystemSetConfig {
         self.into_config().in_base_set(set)
     }
 
@@ -155,12 +157,12 @@ impl IntoSystemSetConfig for BoxedSystemSet {
     }
 
     #[track_caller]
-    fn in_set(self, set: impl SystemSet) -> SystemSetConfig {
+    fn in_set(self, set: impl FreeSystemSet) -> SystemSetConfig {
         self.into_config().in_set(set)
     }
 
     #[track_caller]
-    fn in_base_set(self, set: impl SystemSet) -> SystemSetConfig {
+    fn in_base_set(self, set: impl BaseSystemSet) -> SystemSetConfig {
         self.into_config().in_base_set(set)
     }
 
@@ -277,10 +279,10 @@ pub trait IntoSystemConfig<Marker, Config = SystemConfig> {
     fn into_config(self) -> Config;
     /// Add to `set` membership.
     #[track_caller]
-    fn in_set(self, set: impl SystemSet) -> Config;
+    fn in_set(self, set: impl FreeSystemSet) -> Config;
     /// Add to the provided "base" `set`. For more information on base sets, see [`SystemSet::is_base`].
     #[track_caller]
-    fn in_base_set(self, set: impl SystemSet) -> Config;
+    fn in_base_set(self, set: impl BaseSystemSet) -> Config;
     /// Don't add this system to the schedules's default set.
     fn no_default_base_set(self) -> Config;
     /// Run before all systems in `set`.
@@ -309,12 +311,12 @@ where
     }
 
     #[track_caller]
-    fn in_set(self, set: impl SystemSet) -> SystemConfig {
+    fn in_set(self, set: impl FreeSystemSet) -> SystemConfig {
         self.into_config().in_set(set)
     }
 
     #[track_caller]
-    fn in_base_set(self, set: impl SystemSet) -> SystemConfig {
+    fn in_base_set(self, set: impl BaseSystemSet) -> SystemConfig {
         self.into_config().in_base_set(set)
     }
 
@@ -349,12 +351,12 @@ impl IntoSystemConfig<()> for BoxedSystem<(), ()> {
     }
 
     #[track_caller]
-    fn in_set(self, set: impl SystemSet) -> SystemConfig {
+    fn in_set(self, set: impl FreeSystemSet) -> SystemConfig {
         self.into_config().in_set(set)
     }
 
     #[track_caller]
-    fn in_base_set(self, set: impl SystemSet) -> SystemConfig {
+    fn in_base_set(self, set: impl BaseSystemSet) -> SystemConfig {
         self.into_config().in_base_set(set)
     }
 
@@ -471,13 +473,13 @@ where
 
     /// Add these systems to the provided `set`.
     #[track_caller]
-    fn in_set(self, set: impl SystemSet) -> SystemConfigs {
+    fn in_set(self, set: impl FreeSystemSet) -> SystemConfigs {
         self.into_configs().in_set(set)
     }
 
     /// Add these systems to the provided "base" `set`. For more information on base sets, see [`SystemSet::is_base`].
     #[track_caller]
-    fn in_base_set(self, set: impl SystemSet) -> SystemConfigs {
+    fn in_base_set(self, set: impl BaseSystemSet) -> SystemConfigs {
         self.into_configs().in_base_set(set)
     }
 
@@ -657,13 +659,13 @@ where
 
     /// Add these system sets to the provided `set`.
     #[track_caller]
-    fn in_set(self, set: impl SystemSet) -> SystemSetConfigs {
+    fn in_set(self, set: impl FreeSystemSet) -> SystemSetConfigs {
         self.into_configs().in_set(set)
     }
 
     /// Add these system sets to the provided "base" `set`. For more information on base sets, see [`SystemSet::is_base`].
     #[track_caller]
-    fn in_base_set(self, set: impl SystemSet) -> SystemSetConfigs {
+    fn in_base_set(self, set: impl BaseSystemSet) -> SystemSetConfigs {
         self.into_configs().in_base_set(set)
     }
 

crates/bevy_ecs/src/schedule/mod.rs

@@ -567,16 +567,6 @@ mod tests {
             ));
         }
 
-        #[test]
-        #[should_panic]
-        fn in_system_type_set() {
-            fn foo() {}
-            fn bar() {}
-
-            let mut schedule = Schedule::new();
-            schedule.add_system(foo.in_set(bar.into_system_set()));
-        }
-
         #[test]
         #[should_panic]
         fn configure_system_type_set() {
@@ -688,63 +678,6 @@ mod tests {
         enum Normal {
             X,
             Y,
-            Z,
-        }
-
-        #[test]
-        #[should_panic]
-        fn disallow_adding_base_sets_to_system_with_in_set() {
-            let mut schedule = Schedule::new();
-            schedule.add_system(named_system.in_set(Base::A));
-        }
-
-        #[test]
-        #[should_panic]
-        fn disallow_adding_sets_to_system_with_in_base_set() {
-            let mut schedule = Schedule::new();
-            schedule.add_system(named_system.in_base_set(Normal::X));
-        }
-
-        #[test]
-        #[should_panic]
-        fn disallow_adding_base_sets_to_systems_with_in_set() {
-            let mut schedule = Schedule::new();
-            schedule.add_systems((named_system, named_system).in_set(Base::A));
-        }
-
-        #[test]
-        #[should_panic]
-        fn disallow_adding_sets_to_systems_with_in_base_set() {
-            let mut schedule = Schedule::new();
-            schedule.add_systems((named_system, named_system).in_base_set(Normal::X));
-        }
-
-        #[test]
-        #[should_panic]
-        fn disallow_adding_base_sets_to_set_with_in_set() {
-            let mut schedule = Schedule::new();
-            schedule.configure_set(Normal::Y.in_set(Base::A));
-        }
-
-        #[test]
-        #[should_panic]
-        fn disallow_adding_sets_to_set_with_in_base_set() {
-            let mut schedule = Schedule::new();
-            schedule.configure_set(Normal::Y.in_base_set(Normal::X));
-        }
-
-        #[test]
-        #[should_panic]
-        fn disallow_adding_base_sets_to_sets_with_in_set() {
-            let mut schedule = Schedule::new();
-            schedule.configure_sets((Normal::X, Normal::Y).in_set(Base::A));
-        }
-
-        #[test]
-        #[should_panic]
-        fn disallow_adding_sets_to_sets_with_in_base_set() {
-            let mut schedule = Schedule::new();
-            schedule.configure_sets((Normal::X, Normal::Y).in_base_set(Normal::Z));
         }
 
         #[test]

crates/bevy_ecs/src/schedule/set.rs

@@ -37,6 +37,16 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static {
     fn dyn_clone(&self) -> Box<dyn SystemSet>;
 }
 
+/// A system set that is never contained in another set.
+/// Systems and other system sets may only belong to one base set.
+///
+/// This should only be implemented for types that return `true` from [`SystemSet::is_base`].
+pub trait BaseSystemSet: SystemSet {}
+
+/// System sets that are *not* base sets. This should not be implemented for
+/// any types that implement [`BaseSystemSet`].
+pub trait FreeSystemSet: SystemSet {}
+
 impl PartialEq for dyn SystemSet {
     fn eq(&self, other: &Self) -> bool {
         self.dyn_eq(other.as_dyn_eq())