make the design less confusing

Author: joseph-gio

benches/benches/bevy_ecs/scheduling/schedule.rs

@@ -73,9 +73,8 @@ pub fn build_schedule(criterion: &mut Criterion) {
         fn data(&self) -> u64 {
             self.0
         }
-        #[inline]
-        fn dbg(&self) -> fn(u64, &mut std::fmt::Formatter) -> std::fmt::Result {
-            |x, f| write!(f, "NumLabel({x})")
+        fn fmt(data: u64, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+            f.debug_tuple("NumLabel").field(&data).finish()
         }
     }
 

crates/bevy_ecs/src/schedule/mod.rs

@@ -211,9 +211,10 @@ impl Schedule {
             )
         }
 
+        let stage_label = stage_label.as_label();
         let stage = self
-            .get_stage_mut::<SystemStage>(&stage_label)
-            .unwrap_or_else(move || stage_not_found(&stage_label.as_label()));
+            .get_stage_mut::<SystemStage>(stage_label)
+            .unwrap_or_else(move || stage_not_found(&stage_label));
         stage.add_system(system);
         self
     }
@@ -281,11 +282,9 @@ impl Schedule {
         label: impl StageLabel,
         func: F,
     ) -> &mut Self {
-        let stage = self.get_stage_mut::<T>(&label).unwrap_or_else(move || {
-            panic!(
-                "stage '{:?}' does not exist or is the wrong type",
-                label.as_label()
-            )
+        let label = label.as_label();
+        let stage = self.get_stage_mut::<T>(label).unwrap_or_else(move || {
+            panic!("stage '{:?}' does not exist or is the wrong type", label)
         });
         func(stage);
         self
@@ -306,7 +305,7 @@ impl Schedule {
     /// #
     /// let stage = schedule.get_stage::<SystemStage>(&"my_stage").unwrap();
     /// ```
-    pub fn get_stage<T: Stage>(&self, label: &dyn StageLabel) -> Option<&T> {
+    pub fn get_stage<T: Stage>(&self, label: StageLabelId) -> Option<&T> {
         self.stages
             .get(&label.as_label())
             .and_then(|stage| stage.downcast_ref::<T>())
@@ -327,7 +326,7 @@ impl Schedule {
     /// #
     /// let stage = schedule.get_stage_mut::<SystemStage>(&"my_stage").unwrap();
     /// ```
-    pub fn get_stage_mut<T: Stage>(&mut self, label: &dyn StageLabel) -> Option<&mut T> {
+    pub fn get_stage_mut<T: Stage>(&mut self, label: StageLabelId) -> Option<&mut T> {
         self.stages
             .get_mut(&label.as_label())
             .and_then(|stage| stage.downcast_mut::<T>())

crates/bevy_ecs/src/schedule/state.rs

@@ -6,9 +6,9 @@ use crate::{
     system::{In, IntoChainSystem, Local, Res, ResMut},
 };
 use std::{
-    any::TypeId,
     fmt::{self, Debug},
     hash::Hash,
+    marker::PhantomData,
 };
 
 pub trait StateData: Send + Sync + Clone + Eq + Debug + Hash + 'static {}
@@ -52,28 +52,23 @@ enum ScheduledOperation<T: StateData> {
     Push(T),
 }
 
-struct DriverLabel(
-    TypeId,
-    fn(u64, &mut std::fmt::Formatter) -> std::fmt::Result,
-);
-impl RunCriteriaLabel for DriverLabel {
-    fn type_id(&self) -> core::any::TypeId {
-        self.0
+struct DriverLabel<T: StateData>(PhantomData<fn() -> T>);
+impl<T: StateData> RunCriteriaLabel for DriverLabel<T> {
+    #[inline]
+    fn type_id(&self) -> std::any::TypeId {
+        std::any::TypeId::of::<T>()
     }
+    #[inline]
     fn data(&self) -> u64 {
         0
     }
-    fn dbg(&self) -> fn(u64, &mut std::fmt::Formatter) -> std::fmt::Result {
-        self.1
+    fn fmt(data: u64, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        write!(f, "{}", std::any::type_name::<T>())
     }
 }
 
-impl DriverLabel {
-    fn of<T: 'static>() -> Self {
-        Self(TypeId::of::<T>(), |_, f| {
-            write!(f, "{}", std::any::type_name::<T>())
-        })
-    }
+fn driver_label<T: StateData>() -> DriverLabel<T> {
+    DriverLabel(PhantomData)
 }
 
 impl<T> State<T>
@@ -85,7 +80,7 @@ where
             state.stack.last().unwrap() == &pred && state.transition.is_none()
         })
         .chain(should_run_adapter::<T>)
-        .after(DriverLabel::of::<T>())
+        .after(driver_label::<T>())
     }
 
     pub fn on_inactive_update(pred: T) -> RunCriteriaDescriptor {
@@ -101,7 +96,7 @@ where
             None => *is_inactive,
         })
         .chain(should_run_adapter::<T>)
-        .after(DriverLabel::of::<T>())
+        .after(driver_label::<T>())
     }
 
     pub fn on_in_stack_update(pred: T) -> RunCriteriaDescriptor {
@@ -124,7 +119,7 @@ where
             None => *is_in_stack,
         })
         .chain(should_run_adapter::<T>)
-        .after(DriverLabel::of::<T>())
+        .after(driver_label::<T>())
     }
 
     pub fn on_enter(pred: T) -> RunCriteriaDescriptor {
@@ -139,7 +134,7 @@ where
                 })
         })
         .chain(should_run_adapter::<T>)
-        .after(DriverLabel::of::<T>())
+        .after(driver_label::<T>())
     }
 
     pub fn on_exit(pred: T) -> RunCriteriaDescriptor {
@@ -154,7 +149,7 @@ where
                 })
         })
         .chain(should_run_adapter::<T>)
-        .after(DriverLabel::of::<T>())
+        .after(driver_label::<T>())
     }
 
     pub fn on_pause(pred: T) -> RunCriteriaDescriptor {
@@ -168,7 +163,7 @@ where
                 })
         })
         .chain(should_run_adapter::<T>)
-        .after(DriverLabel::of::<T>())
+        .after(driver_label::<T>())
     }
 
     pub fn on_resume(pred: T) -> RunCriteriaDescriptor {
@@ -182,7 +177,7 @@ where
                 })
         })
         .chain(should_run_adapter::<T>)
-        .after(DriverLabel::of::<T>())
+        .after(driver_label::<T>())
     }
 
     pub fn on_update_set(s: T) -> SystemSet {
@@ -214,7 +209,7 @@ where
     /// Important note: this set must be inserted **before** all other state-dependant sets to work
     /// properly!
     pub fn get_driver() -> SystemSet {
-        SystemSet::default().with_run_criteria(state_cleaner::<T>.label(DriverLabel::of::<T>()))
+        SystemSet::default().with_run_criteria(state_cleaner::<T>.label(driver_label::<T>()))
     }
 
     pub fn new(initial: T) -> Self {

crates/bevy_ecs/src/system/function_system.rs

@@ -461,9 +461,8 @@ impl<T: 'static> SystemLabel for SystemTypeIdLabel<T> {
     fn data(&self) -> u64 {
         0
     }
-    #[inline]
-    fn dbg(&self) -> fn(u64, &mut fmt::Formatter) -> fmt::Result {
-        |_, f| write!(f, "{}", std::any::type_name::<T>())
+    fn fmt(_: u64, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        write!(f, "{}", std::any::type_name::<T>())
     }
 }
 

crates/bevy_macro_utils/src/lib.rs

@@ -141,7 +141,7 @@ pub fn derive_label(
         .predicates
         .push(syn::parse2(quote! { Self: 'static }).unwrap());
 
-    let (data, as_str) = match input.data {
+    let (data, fmt) = match input.data {
         syn::Data::Struct(d) => {
             // see if the user tried to ignore fields incorrectly
             if let Some(attr) = d
@@ -161,7 +161,7 @@ pub fn derive_label(
             if matches!(d.fields, syn::Fields::Unit) || ignore_fields {
                 let lit = ident.to_string();
                 let data = quote! { 0 };
-                let as_str = quote! { |_, f| write!(f, #lit) };
+                let as_str = quote! { write!(f, #lit) };
                 (data, as_str)
             } else {
                 let err_msg = format!("Labels cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`");
@@ -182,7 +182,7 @@ pub fn derive_label(
             }
 
             let mut data_arms = Vec::with_capacity(d.variants.len());
-            let mut str_arms = Vec::with_capacity(d.variants.len());
+            let mut fmt_arms = Vec::with_capacity(d.variants.len());
 
             for (i, v) in d.variants.iter().enumerate() {
                 // Variants must either be fieldless, or explicitly ignore the fields.
@@ -195,7 +195,7 @@ pub fn derive_label(
                     data_arms.push(quote! { #path { .. } => #i });
 
                     let lit = format!("{ident}::{}", v.ident.clone());
-                    str_arms.push(quote! { #path { .. } => { |_, f| write!(f, #lit) } });
+                    fmt_arms.push(quote! { #i => { write!(f, #lit) } });
                 } else {
                     let err_msg = format!("Label variants cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`");
                     return quote_spanned! {
@@ -210,12 +210,13 @@ pub fn derive_label(
                     #(#data_arms),*
                 }
             };
-            let as_str = quote! {
-                match self {
-                    #(#str_arms),*
+            let fmt = quote! {
+                match data {
+                    #(#fmt_arms),*
+                    _ => ::std::unreachable!(),
                 }
             };
-            (data, as_str)
+            (data, fmt)
         }
         syn::Data::Union(_) => {
             return quote_spanned! {
@@ -231,8 +232,8 @@ pub fn derive_label(
             fn data(&self) -> u64 {
                 #data
             }
-            fn dbg(&self) -> fn(u64, &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
-                #as_str
+            fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
+                #fmt
             }
         }
     })

crates/bevy_utils/src/label.rs

@@ -76,18 +76,18 @@ macro_rules! define_label {
         impl ::std::fmt::Debug for $id_name {
             fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
                 let data = self.data();
-                self.dbg()(data, f)
+                self.2(data, f)
             }
         }
 
         $(#[$label_attr])*
         pub trait $label_name: 'static {
             /// Converts this type into an opaque, strongly-typed label.
+            #[inline]
             fn as_label(&self) -> $id_name {
                 let ty = self.type_id();
                 let data = self.data();
-                let dbg = self.dbg();
-                $id_name(ty, data, dbg)
+                $id_name(ty, data, Self::fmt)
             }
             /// Returns the [`TypeId`] used to differentiate labels.
             #[inline]
@@ -96,7 +96,12 @@ macro_rules! define_label {
             }
             /// Returns a number used to distinguish different labels of the same type.
             fn data(&self) -> u64;
-            fn dbg(&self) -> fn(u64, &mut ::std::fmt::Formatter) -> ::std::fmt::Result;
+            /// Writes debug info for a label of the current type.
+            /// * `data`: the result of calling [`data()`](#method.data) on an instance of this type.
+            ///
+            /// You should not call this method directly, as it may panic for some types;
+            /// use [`as_label`](#method.as_label) instead.
+            fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result;
         }
 
         impl $label_name for $id_name {
@@ -112,9 +117,9 @@ macro_rules! define_label {
             fn data(&self) -> u64 {
                 self.1
             }
-            #[inline]
-            fn dbg(&self) -> fn(u64, &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
-                self.2
+            #[track_caller]
+            fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> std::fmt::Result {
+                ::std::unimplemented!("do not call `Label::fmt` directly -- use the result of `as_label()` for formatting instead")
             }
         }