Skip to content

Make SystemSet marker traits mutually exclusive #8032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 10 additions & 16 deletions crates/bevy_ecs/macros/src/set.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use proc_macro::TokenStream;
use quote::{format_ident, quote, ToTokens};
use quote::{quote, ToTokens};
use syn::parse::{Parse, ParseStream};

use crate::bevy_ecs_path;

pub static SYSTEM_SET_ATTRIBUTE_NAME: &str = "system_set";
pub static BASE_ATTRIBUTE_NAME: &str = "base";

Expand All @@ -12,13 +14,7 @@ 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 path = bevy_ecs_path();

let ident = input.ident;

Expand Down Expand Up @@ -73,14 +69,10 @@ 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 {}
}
let kind = if is_base {
quote! { #path::schedule::BaseSystemSetMarker }
} else {
quote! {
impl #impl_generics #free_trait_path for #ident #ty_generics #where_clause {}
}
quote! { #path::schedule::FreeSystemSetMarker }
};

(quote! {
Expand All @@ -94,7 +86,9 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea
}
}

#marker_impl
impl #impl_generics #path::schedule::KindedSystemSet for #ident #ty_generics #where_clause {
type Kind = #kind;
}
})
.into()
}
43 changes: 33 additions & 10 deletions crates/bevy_ecs/src/schedule/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,42 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static {
fn dyn_clone(&self) -> Box<dyn SystemSet>;
}

/// A marker trait for `SystemSet` types where [`is_base`] returns `true`.
/// This should only be implemented for types that satisfy this requirement.
/// It is automatically implemented for base set types by `#[derive(SystemSet)]`.
/// Supertrait for [`BaseSystemSet`] and [`FreeSystemSet`].
pub trait KindedSystemSet: SystemSet {
/// Whether this is a [`BaseSystemSet`] or [`FreeSystemSet`].
///
/// If [`SystemSet::is_base`] returns true, this should be `BaseSystemSetMarker`.
/// If it returns `false`, this should be `FreeSystemSetMarker`.
type Kind: sealed::SystemSetKind;
}

mod sealed {
pub trait SystemSetKind {}
}

/// Marker for [`BaseSystemSet`] when using [`KindedSystemSet`].
pub struct BaseSystemSetMarker;

/// Marker for [`FreeSystemSet`] when using [`KindedSystemSet`].
pub struct FreeSystemSetMarker;

impl sealed::SystemSetKind for BaseSystemSetMarker {}
impl sealed::SystemSetKind for FreeSystemSetMarker {}

/// A system set that is never contained in another set.
/// Systems and other system sets may only belong to one base set.
///
/// [`is_base`]: SystemSet::is_base
pub trait BaseSystemSet: SystemSet {}
/// [`SystemSet::is_base`] always returns `true` for these types.
pub trait BaseSystemSet: KindedSystemSet<Kind = BaseSystemSetMarker> {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These traits should also be sealed to avoid users manually implementing them.


/// A marker trait for `SystemSet` types where [`is_base`] returns `false`.
/// This should only be implemented for types that satisfy this requirement.
/// It is automatically implemented for non-base set types by `#[derive(SystemSet)]`.
/// System sets that are not base sets.
/// They have no extra restrictions on ordering in the schedule.
///
/// [`is_base`]: SystemSet::is_base
pub trait FreeSystemSet: SystemSet {}
/// [`SystemSet::is_base`] always returns `false` for these types.
pub trait FreeSystemSet: KindedSystemSet<Kind = FreeSystemSetMarker> {}

impl<T: KindedSystemSet<Kind = BaseSystemSetMarker>> BaseSystemSet for T {}
impl<T: KindedSystemSet<Kind = FreeSystemSetMarker>> FreeSystemSet for T {}

impl PartialEq for dyn SystemSet {
fn eq(&self, other: &Self) -> bool {
Expand Down