Skip to content
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

Remove restrictions on Label types #5715

Closed
wants to merge 21 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
25 changes: 12 additions & 13 deletions benches/benches/bevy_ecs/scheduling/schedule.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bevy_app::App;
use bevy_ecs::prelude::*;
use bevy_ecs::{prelude::*, schedule::IntoSystemLabel};
use criterion::Criterion;

pub fn schedule(c: &mut Criterion) {
Expand Down Expand Up @@ -64,14 +64,17 @@ 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)]
struct NumLabel(usize);
struct NumLabel(u64);
#[derive(Debug, Clone, Copy, SystemLabel)]
struct DummyLabel;

impl SystemLabel for NumLabel {
fn as_str(&self) -> &'static str {
let s = self.0.to_string();
Box::leak(s.into_boxed_str())
impl IntoSystemLabel for NumLabel {
#[inline]
fn data(&self) -> u64 {
self.0
}
fn fmt(data: u64, f: &mut std::fmt::Formatter) -> std::fmt::Result {
f.debug_tuple("NumLabel").field(&data).finish()
}
}

Expand All @@ -82,10 +85,6 @@ pub fn build_schedule(criterion: &mut Criterion) {
// Method: generate a set of `graph_size` systems which have a One True Ordering.
// Add system to the stage with full constraints. Hopefully this should be maximimally
// difficult for bevy to figure out.
// Also, we are performing the `as_label` operation outside of the loop since that
// requires an allocation and a leak. This is not something that would be necessary in a
// real scenario, just a contrivance for the benchmark.
let labels: Vec<_> = (0..1000).map(|i| NumLabel(i).as_label()).collect();

// Benchmark graphs of different sizes.
for graph_size in [100, 500, 1000] {
Expand All @@ -109,12 +108,12 @@ pub fn build_schedule(criterion: &mut Criterion) {
// Build a fully-connected dependency graph describing the One True Ordering.
// Not particularly realistic but this can be refined later.
for i in 0..graph_size {
let mut sys = empty_system.label(labels[i]).before(DummyLabel);
let mut sys = empty_system.label(NumLabel(i)).before(DummyLabel);
for a in 0..i {
sys = sys.after(labels[a]);
sys = sys.after(NumLabel(a));
}
for b in i + 1..graph_size {
sys = sys.before(labels[b]);
sys = sys.before(NumLabel(b));
}
app.add_system(sys);
}
Expand Down
18 changes: 11 additions & 7 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ use bevy_utils::tracing::info_span;
bevy_utils::define_label!(
/// A strongly-typed class of labels used to identify an [`App`].
AppLabel,
/// Types that can be converted into [`AppLabelId`], except for `AppLabelId` itself.
///
/// Implementing this trait automatically implements [`AppLabel`] due to a blanket implementation.
IntoAppLabel,
/// A strongly-typed identifier for an [`AppLabel`].
AppLabelId,
);
Expand Down Expand Up @@ -391,9 +395,9 @@ impl App {
stage_label: impl StageLabel,
system: impl IntoSystemDescriptor<Params>,
) -> &mut Self {
use std::any::TypeId;
let stage_label = stage_label.as_label();
assert!(
stage_label.type_id() != TypeId::of::<StartupStage>(),
!stage_label.is::<StartupStage>(),
"use `add_startup_system_to_stage` instead of `add_system_to_stage` to add a system to a StartupStage"
);
self.schedule.add_system_to_stage(stage_label, system);
Expand Down Expand Up @@ -426,9 +430,9 @@ impl App {
stage_label: impl StageLabel,
system_set: SystemSet,
) -> &mut Self {
use std::any::TypeId;
let stage_label = stage_label.as_label();
assert!(
stage_label.type_id() != TypeId::of::<StartupStage>(),
!stage_label.is::<StartupStage>(),
"use `add_startup_system_set_to_stage` instead of `add_system_set_to_stage` to add system sets to a StartupStage"
);
self.schedule
Expand Down Expand Up @@ -964,7 +968,7 @@ impl App {
pub fn sub_app_mut(&mut self, label: impl AppLabel) -> &mut App {
match self.get_sub_app_mut(label) {
Ok(app) => app,
Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_str()),
Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_label()),
}
}

Expand All @@ -986,13 +990,13 @@ impl App {
pub fn sub_app(&self, label: impl AppLabel) -> &App {
match self.get_sub_app(label) {
Ok(app) => app,
Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_str()),
Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_label()),
}
}

/// Retrieves a `SubApp` inside this [`App`] with the given label, if it exists. Otherwise returns
/// an [`Err`] containing the given label.
pub fn get_sub_app(&self, label: impl AppLabel) -> Result<&App, impl AppLabel> {
pub fn get_sub_app<L: AppLabel>(&self, label: L) -> Result<&App, L> {
self.sub_apps
.get(&label.as_label())
.map(|sub_app| &sub_app.app)
Expand Down
14 changes: 11 additions & 3 deletions crates/bevy_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,20 @@ pub fn derive_enum_variant_meta(input: TokenStream) -> TokenStream {

/// Generates an impl of the `AppLabel` trait.
///
/// This works only for unit structs, or enums with only unit variants.
/// You may force a struct or variant to behave as if it were fieldless with `#[app_label(ignore_fields)]`.
/// For unit structs and enums with only unit variants, a cheap implementation can easily be created.
///
/// More complex types must be boxed and interned
/// - opt in to this by annotating the entire item with `#[app_label(intern)]`.
///
/// Alternatively, you may force a struct or variant to behave as if
/// it were fieldless with `#[app_label(ignore_fields)]`.
/// This is especially useful for [`PhantomData`](core::marker::PhantomData) fields.
#[proc_macro_derive(AppLabel, attributes(app_label))]
pub fn derive_app_label(input: TokenStream) -> TokenStream {
let input = syn::parse_macro_input!(input as syn::DeriveInput);
let mut trait_path = BevyManifest::default().get_path("bevy_app");
trait_path.segments.push(format_ident!("AppLabel").into());
trait_path
.segments
.push(format_ident!("IntoAppLabel").into());
derive_label(input, &trait_path, "app_label")
}
49 changes: 48 additions & 1 deletion crates/bevy_ecs/examples/derive_label.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::marker::PhantomData;
use std::{fmt::Debug, hash::Hash, marker::PhantomData};

use bevy_ecs::prelude::*;

Expand All @@ -18,6 +18,40 @@ fn main() {
GenericLabel::<f64>::One.as_label(),
GenericLabel::<char>::One.as_label(),
);

assert_eq!(format!("{:?}", UnitLabel.as_label()), "UnitLabel");
assert_eq!(format!("{:?}", WeirdLabel(1).as_label()), "WeirdLabel");
assert_eq!(format!("{:?}", WeirdLabel(2).as_label()), "WeirdLabel");
assert_eq!(
format!("{:?}", GenericLabel::<f64>::One.as_label()),
"GenericLabel::One::<f64>"
);
assert_eq!(
format!("{:?}", ConstGenericLabel::<21>.as_label()),
"ConstGenericLabel::<21>"
);

// Working with labels that need to be heap allocated.
let label = ComplexLabel {
people: vec!["John", "William", "Sharon"],
};
// Convert it to a LabelId. Its type gets erased.
let id = label.as_label();
assert_eq!(
format!("{id:?}"),
r#"ComplexLabel { people: ["John", "William", "Sharon"] }"#
);

// Generic heap-allocated labels.
let id = WrapLabel(1_i128).as_label();
assert_eq!(format!("{id:?}"), "WrapLabel(1)");

// Different types with the same type constructor.
let id2 = WrapLabel(1_u32).as_label();
// The debug representations are the same...
assert_eq!(format!("{id:?}"), format!("{id2:?}"));
// ...but they do not compare equal.
assert_ne!(id, id2);
}

#[derive(SystemLabel)]
Expand All @@ -40,6 +74,9 @@ pub enum GenericLabel<T> {
Two(PhantomData<T>),
}

#[derive(SystemLabel)]
pub struct ConstGenericLabel<const N: usize>;

// FIXME: this should be a compile_fail test
/*#[derive(SystemLabel)]
pub union Foo {
Expand All @@ -60,3 +97,13 @@ pub struct BadLabel2 {
#[system_label(ignore_fields)]
x: (),
}*/

#[derive(Debug, Clone, PartialEq, Eq, Hash, SystemLabel)]
#[system_label(intern)]
pub struct ComplexLabel {
people: Vec<&'static str>,
}

#[derive(Debug, Clone, PartialEq, Eq, Hash, SystemLabel)]
#[system_label(intern)]
pub struct WrapLabel<T>(T);
38 changes: 29 additions & 9 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,44 +436,64 @@ pub fn derive_world_query(input: TokenStream) -> TokenStream {

/// Generates an impl of the `SystemLabel` trait.
///
/// This works only for unit structs, or enums with only unit variants.
/// You may force a struct or variant to behave as if it were fieldless with `#[system_label(ignore_fields)]`.
/// For unit structs and enums with only unit variants, a cheap implementation can easily be created.
///
/// More complex types must be boxed and interned
/// - opt in to this by annotating the entire item with `#[system_label(intern)]`.
///
/// Alternatively, you may force a struct or variant to behave as if
/// it were fieldless with `#[system_label(ignore_fields)]`.
/// This is especially useful for [`PhantomData`](core::marker::PhantomData) fields.
#[proc_macro_derive(SystemLabel, attributes(system_label))]
pub fn derive_system_label(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let mut trait_path = bevy_ecs_path();
trait_path.segments.push(format_ident!("schedule").into());
trait_path
.segments
.push(format_ident!("SystemLabel").into());
.push(format_ident!("IntoSystemLabel").into());
derive_label(input, &trait_path, "system_label")
}

/// Generates an impl of the `StageLabel` trait.
///
/// This works only for unit structs, or enums with only unit variants.
/// You may force a struct or variant to behave as if it were fieldless with `#[stage_label(ignore_fields)]`.
/// For unit structs and enums with only unit variants, a cheap implementation can easily be created.
///
/// More complex types must be boxed and interned
/// - opt in to this by annotating the entire item with `#[stage_label(intern)]`.
///
/// Alternatively, you may force a struct or variant to behave as if
/// it were fieldless with `#[stage_label(ignore_fields)]`.
/// This is especially useful for [`PhantomData`](core::marker::PhantomData) fields.
#[proc_macro_derive(StageLabel, attributes(stage_label))]
pub fn derive_stage_label(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let mut trait_path = bevy_ecs_path();
trait_path.segments.push(format_ident!("schedule").into());
trait_path.segments.push(format_ident!("StageLabel").into());
trait_path
.segments
.push(format_ident!("IntoStageLabel").into());
derive_label(input, &trait_path, "stage_label")
}

/// Generates an impl of the `RunCriteriaLabel` trait.
///
/// This works only for unit structs, or enums with only unit variants.
/// You may force a struct or variant to behave as if it were fieldless with `#[run_criteria_label(ignore_fields)]`.
/// For unit structs and enums with only unit variants, a cheap implementation can easily be created.
///
/// More complex types must be boxed and interned
/// - opt in to this by annotating the entire item with `#[run_criteria_label(intern)]`.
///
/// Alternatively, you may force a struct or variant to behave as if
/// it were fieldless with `#[run_criteria_label(ignore_fields)]`.
/// This is especially useful for [`PhantomData`](core::marker::PhantomData) fields.
#[proc_macro_derive(RunCriteriaLabel, attributes(run_criteria_label))]
pub fn derive_run_criteria_label(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let mut trait_path = bevy_ecs_path();
trait_path.segments.push(format_ident!("schedule").into());
trait_path
.segments
.push(format_ident!("RunCriteriaLabel").into());
.push(format_ident!("IntoRunCriteriaLabel").into());
derive_label(input, &trait_path, "run_criteria_label")
}

Expand Down
12 changes: 12 additions & 0 deletions crates/bevy_ecs/src/schedule/label.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,30 @@ use bevy_utils::define_label;
define_label!(
/// A strongly-typed class of labels used to identify [`Stage`](crate::schedule::Stage)s.
JoJoJet marked this conversation as resolved.
Show resolved Hide resolved
StageLabel,
/// Types that can be converted into [`StageLabelId`], except for `StageLabelId` itself.
///
/// Implementing this trait automatically implements [`StageLabel`] due to a blanket implementation.
IntoStageLabel,
/// Strongly-typed identifier for a [`StageLabel`].
StageLabelId,
);
define_label!(
/// A strongly-typed class of labels used to identify [`System`](crate::system::System)s.
SystemLabel,
/// Types that can be converted into [`SystemLabelId`], except for `SystemLabelId` itself.
///
/// Implementing this trait automatically implements [`SystemLabel`] due to a blanket implementation.
IntoSystemLabel,
/// Strongly-typed identifier for a [`SystemLabel`].
SystemLabelId,
);
define_label!(
/// A strongly-typed class of labels used to identify [run criteria](crate::schedule::RunCriteria).
RunCriteriaLabel,
/// Types that can be converted into [`RunCriteriaLabelId`], except for `RunCriteriaLabelId` itself.
///
/// Implementing this trait automatically implements [`RunCriteriaLabel`] due to a blanket implementation.
IntoRunCriteriaLabel,
/// Strongly-typed identifier for a [`RunCriteriaLabel`].
RunCriteriaLabelId,
);
23 changes: 9 additions & 14 deletions crates/bevy_ecs/src/schedule/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::{
system::{In, IntoChainSystem, Local, Res, ResMut, Resource},
};
use std::{
any::TypeId,
fmt::{self, Debug},
hash::Hash,
};
Expand Down Expand Up @@ -54,20 +53,16 @@ enum ScheduledOperation<T: StateData> {
Push(T),
}

#[derive(Debug, PartialEq, Eq, Clone, Hash)]
struct DriverLabel(TypeId, &'static str);
impl RunCriteriaLabel for DriverLabel {
fn type_id(&self) -> core::any::TypeId {
self.0
}
fn as_str(&self) -> &'static str {
self.1
}
}

struct DriverLabel;
impl DriverLabel {
fn of<T: 'static>() -> Self {
Self(TypeId::of::<T>(), std::any::type_name::<T>())
fn of<T: 'static>() -> impl RunCriteriaLabel {
use std::marker::PhantomData;

#[derive(RunCriteriaLabel)]
#[run_criteria_label(ignore_fields)]
struct DriverLabel<T>(PhantomData<fn() -> T>);

DriverLabel::<T>(PhantomData)
}
}

Expand Down
11 changes: 7 additions & 4 deletions crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
component::ComponentId,
prelude::FromWorld,
query::{Access, FilteredAccessSet},
schedule::{SystemLabel, SystemLabelId},
schedule::{IntoSystemLabel, SystemLabel, SystemLabelId},
system::{
check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch,
SystemParamItem, SystemParamState,
Expand Down Expand Up @@ -461,10 +461,13 @@ where
/// A [`SystemLabel`] that was automatically generated for a system on the basis of its `TypeId`.
pub struct SystemTypeIdLabel<T: 'static>(PhantomData<fn() -> T>);

impl<T: 'static> SystemLabel for SystemTypeIdLabel<T> {
impl<T: 'static> IntoSystemLabel for SystemTypeIdLabel<T> {
#[inline]
fn as_str(&self) -> &'static str {
std::any::type_name::<T>()
fn data(&self) -> u64 {
0
}
fn fmt(_: u64, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "{}", std::any::type_name::<T>())
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/bevy_macro_utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ keywords = ["bevy"]
toml = "0.5.8"
syn = "1.0"
quote = "1.0"
proc-macro2 = "1.0"
Loading