Auto-label function systems with SystemTypeIdLabel (#4224)

This adds the concept of "default labels" for systems (currently scoped to "parallel systems", but this could just as easily be implemented for "exclusive systems"). Function systems now include their function's `SystemTypeIdLabel` by default.

This enables the following patterns:

```rust
// ordering two systems without manually defining labels
app
  .add_system(update_velocity)
  .add_system(movement.after(update_velocity))

// ordering sets of systems without manually defining labels
app
  .add_system(foo)
  .add_system_set(
    SystemSet::new()
      .after(foo)
      .with_system(bar)
      .with_system(baz)
  )
```

Fixes: #4219
Related to: #4220 

Credit to @aevyrie @alice-i-cecile @DJMcNab (and probably others) for proposing (and supporting) this idea about a year ago. I was a big dummy that both shut down this (very good) idea and then forgot I did that. Sorry. You all were right!
This commit is contained in:
Carter Anderson 2022-03-23 22:53:56 +00:00
parent d51b54a658
commit b1c3e9862d
6 changed files with 120 additions and 51 deletions

View file

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

View file

@ -1578,15 +1578,25 @@ mod tests {
fn ambiguity_detection() { fn ambiguity_detection() {
use super::{find_ambiguities, SystemContainer}; use super::{find_ambiguities, SystemContainer};
fn find_ambiguities_first_labels( fn find_ambiguities_first_str_labels(
systems: &[impl SystemContainer], systems: &[impl SystemContainer],
) -> Vec<(BoxedSystemLabel, BoxedSystemLabel)> { ) -> Vec<(BoxedSystemLabel, BoxedSystemLabel)> {
find_ambiguities(systems) find_ambiguities(systems)
.drain(..) .drain(..)
.map(|(index_a, index_b, _conflicts)| { .map(|(index_a, index_b, _conflicts)| {
( (
systems[index_a].labels()[0].clone(), systems[index_a]
systems[index_b].labels()[0].clone(), .labels()
.iter()
.find(|a| (&***a).type_id() == std::any::TypeId::of::<&str>())
.unwrap()
.clone(),
systems[index_b]
.labels()
.iter()
.find(|a| (&***a).type_id() == std::any::TypeId::of::<&str>())
.unwrap()
.clone(),
) )
}) })
.collect() .collect()
@ -1616,7 +1626,7 @@ mod tests {
.with_system(component.label("4")); .with_system(component.label("4"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("1"), Box::new("4"))) ambiguities.contains(&(Box::new("1"), Box::new("4")))
|| ambiguities.contains(&(Box::new("4"), Box::new("1"))) || ambiguities.contains(&(Box::new("4"), Box::new("1")))
@ -1631,7 +1641,7 @@ mod tests {
.with_system(resource.label("4")); .with_system(resource.label("4"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("1"), Box::new("4"))) ambiguities.contains(&(Box::new("1"), Box::new("4")))
|| ambiguities.contains(&(Box::new("4"), Box::new("1"))) || ambiguities.contains(&(Box::new("4"), Box::new("1")))
@ -1656,7 +1666,7 @@ mod tests {
.with_system(resource.label("4")); .with_system(resource.label("4"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("0"), Box::new("3"))) ambiguities.contains(&(Box::new("0"), Box::new("3")))
|| ambiguities.contains(&(Box::new("3"), Box::new("0"))) || ambiguities.contains(&(Box::new("3"), Box::new("0")))
@ -1675,7 +1685,7 @@ mod tests {
.with_system(resource.label("4").in_ambiguity_set("a")); .with_system(resource.label("4").in_ambiguity_set("a"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("0"), Box::new("3"))) ambiguities.contains(&(Box::new("0"), Box::new("3")))
|| ambiguities.contains(&(Box::new("3"), Box::new("0"))) || ambiguities.contains(&(Box::new("3"), Box::new("0")))
@ -1688,7 +1698,7 @@ mod tests {
.with_system(component.label("2")); .with_system(component.label("2"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("0"), Box::new("1"))) ambiguities.contains(&(Box::new("0"), Box::new("1")))
|| ambiguities.contains(&(Box::new("1"), Box::new("0"))) || ambiguities.contains(&(Box::new("1"), Box::new("0")))
@ -1701,7 +1711,7 @@ mod tests {
.with_system(component.label("2").after("0")); .with_system(component.label("2").after("0"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("1"), Box::new("2"))) ambiguities.contains(&(Box::new("1"), Box::new("2")))
|| ambiguities.contains(&(Box::new("2"), Box::new("1"))) || ambiguities.contains(&(Box::new("2"), Box::new("1")))
@ -1715,7 +1725,7 @@ mod tests {
.with_system(component.label("3").after("1").after("2")); .with_system(component.label("3").after("1").after("2"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("1"), Box::new("2"))) ambiguities.contains(&(Box::new("1"), Box::new("2")))
|| ambiguities.contains(&(Box::new("2"), Box::new("1"))) || ambiguities.contains(&(Box::new("2"), Box::new("1")))
@ -1729,7 +1739,7 @@ mod tests {
.with_system(component.label("3").after("1").after("2")); .with_system(component.label("3").after("1").after("2"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert_eq!(ambiguities.len(), 0); assert_eq!(ambiguities.len(), 0);
let mut stage = SystemStage::parallel() let mut stage = SystemStage::parallel()
@ -1739,7 +1749,7 @@ mod tests {
.with_system(component.label("3").after("1").after("2")); .with_system(component.label("3").after("1").after("2"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("1"), Box::new("2"))) ambiguities.contains(&(Box::new("1"), Box::new("2")))
|| ambiguities.contains(&(Box::new("2"), Box::new("1"))) || ambiguities.contains(&(Box::new("2"), Box::new("1")))
@ -1769,7 +1779,7 @@ mod tests {
); );
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("1"), Box::new("2"))) ambiguities.contains(&(Box::new("1"), Box::new("2")))
|| ambiguities.contains(&(Box::new("2"), Box::new("1"))) || ambiguities.contains(&(Box::new("2"), Box::new("1")))
@ -1819,7 +1829,7 @@ mod tests {
); );
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert_eq!(ambiguities.len(), 0); assert_eq!(ambiguities.len(), 0);
let mut stage = SystemStage::parallel() let mut stage = SystemStage::parallel()
@ -1850,7 +1860,7 @@ mod tests {
); );
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.parallel); let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
assert!( assert!(
ambiguities.contains(&(Box::new("1"), Box::new("4"))) ambiguities.contains(&(Box::new("1"), Box::new("4")))
|| ambiguities.contains(&(Box::new("4"), Box::new("1"))) || ambiguities.contains(&(Box::new("4"), Box::new("1")))
@ -1884,7 +1894,7 @@ mod tests {
.with_system(empty.exclusive_system().label("6").after("2").after("5")); .with_system(empty.exclusive_system().label("6").after("2").after("5"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.exclusive_at_start); let ambiguities = find_ambiguities_first_str_labels(&stage.exclusive_at_start);
assert!( assert!(
ambiguities.contains(&(Box::new("1"), Box::new("3"))) ambiguities.contains(&(Box::new("1"), Box::new("3")))
|| ambiguities.contains(&(Box::new("3"), Box::new("1"))) || ambiguities.contains(&(Box::new("3"), Box::new("1")))
@ -1921,7 +1931,7 @@ mod tests {
.with_system(empty.exclusive_system().label("6").after("2").after("5")); .with_system(empty.exclusive_system().label("6").after("2").after("5"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.exclusive_at_start); let ambiguities = find_ambiguities_first_str_labels(&stage.exclusive_at_start);
assert!( assert!(
ambiguities.contains(&(Box::new("2"), Box::new("3"))) ambiguities.contains(&(Box::new("2"), Box::new("3")))
|| ambiguities.contains(&(Box::new("3"), Box::new("2"))) || ambiguities.contains(&(Box::new("3"), Box::new("2")))
@ -1947,7 +1957,7 @@ mod tests {
.with_system(empty.exclusive_system().label("3").in_ambiguity_set("a")); .with_system(empty.exclusive_system().label("3").in_ambiguity_set("a"));
stage.initialize_systems(&mut world); stage.initialize_systems(&mut world);
stage.rebuild_orders_and_dependencies(); stage.rebuild_orders_and_dependencies();
let ambiguities = find_ambiguities_first_labels(&stage.exclusive_at_start); let ambiguities = find_ambiguities_first_str_labels(&stage.exclusive_at_start);
assert_eq!(ambiguities.len(), 0); assert_eq!(ambiguities.len(), 0);
} }

View file

@ -3,7 +3,10 @@ use crate::{
AmbiguitySetLabel, BoxedAmbiguitySetLabel, BoxedSystemLabel, IntoRunCriteria, AmbiguitySetLabel, BoxedAmbiguitySetLabel, BoxedSystemLabel, IntoRunCriteria,
RunCriteriaDescriptorOrLabel, SystemLabel, RunCriteriaDescriptorOrLabel, SystemLabel,
}, },
system::{BoxedSystem, ExclusiveSystem, ExclusiveSystemCoerced, ExclusiveSystemFn, IntoSystem}, system::{
AsSystemLabel, BoxedSystem, ExclusiveSystem, ExclusiveSystemCoerced, ExclusiveSystemFn,
IntoSystem,
},
}; };
/// Encapsulates a system and information on when it run in a `SystemStage`. /// Encapsulates a system and information on when it run in a `SystemStage`.
@ -105,9 +108,9 @@ pub struct ParallelSystemDescriptor {
fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescriptor { fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescriptor {
ParallelSystemDescriptor { ParallelSystemDescriptor {
labels: system.default_labels(),
system, system,
run_criteria: None, run_criteria: None,
labels: Vec::new(),
before: Vec::new(), before: Vec::new(),
after: Vec::new(), after: Vec::new(),
ambiguity_sets: Vec::new(), ambiguity_sets: Vec::new(),
@ -126,10 +129,10 @@ pub trait ParallelSystemDescriptorCoercion<Params> {
fn label(self, label: impl SystemLabel) -> ParallelSystemDescriptor; fn label(self, label: impl SystemLabel) -> ParallelSystemDescriptor;
/// Specifies that the system should run before systems with the given label. /// Specifies that the system should run before systems with the given label.
fn before(self, label: impl SystemLabel) -> ParallelSystemDescriptor; fn before<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor;
/// Specifies that the system should run after systems with the given label. /// Specifies that the system should run after systems with the given label.
fn after(self, label: impl SystemLabel) -> ParallelSystemDescriptor; fn after<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor;
/// Specifies that the system is exempt from execution order ambiguity detection /// Specifies that the system is exempt from execution order ambiguity detection
/// with other systems in this set. /// with other systems in this set.
@ -150,13 +153,13 @@ impl ParallelSystemDescriptorCoercion<()> for ParallelSystemDescriptor {
self self
} }
fn before(mut self, label: impl SystemLabel) -> ParallelSystemDescriptor { fn before<Marker>(mut self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
self.before.push(Box::new(label)); self.before.push(Box::new(label.as_system_label()));
self self
} }
fn after(mut self, label: impl SystemLabel) -> ParallelSystemDescriptor { fn after<Marker>(mut self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
self.after.push(Box::new(label)); self.after.push(Box::new(label.as_system_label()));
self self
} }
@ -182,11 +185,11 @@ where
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).label(label) new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).label(label)
} }
fn before(self, label: impl SystemLabel) -> ParallelSystemDescriptor { fn before<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).before(label) new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).before(label)
} }
fn after(self, label: impl SystemLabel) -> ParallelSystemDescriptor { fn after<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).after(label) new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).after(label)
} }
@ -207,11 +210,11 @@ impl ParallelSystemDescriptorCoercion<()> for BoxedSystem<(), ()> {
new_parallel_descriptor(self).label(label) new_parallel_descriptor(self).label(label)
} }
fn before(self, label: impl SystemLabel) -> ParallelSystemDescriptor { fn before<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
new_parallel_descriptor(self).before(label) new_parallel_descriptor(self).before(label)
} }
fn after(self, label: impl SystemLabel) -> ParallelSystemDescriptor { fn after<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
new_parallel_descriptor(self).after(label) new_parallel_descriptor(self).after(label)
} }

View file

@ -2,6 +2,7 @@ use crate::{
archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId}, archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId},
component::ComponentId, component::ComponentId,
query::{Access, FilteredAccessSet}, query::{Access, FilteredAccessSet},
schedule::SystemLabel,
system::{ system::{
check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch, check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch,
SystemParamState, SystemParamState,
@ -9,7 +10,7 @@ use crate::{
world::{World, WorldId}, world::{World, WorldId},
}; };
use bevy_ecs_macros::all_tuples; use bevy_ecs_macros::all_tuples;
use std::{borrow::Cow, marker::PhantomData}; use std::{borrow::Cow, fmt::Debug, hash::Hash, marker::PhantomData};
/// The metadata of a [`System`]. /// The metadata of a [`System`].
pub struct SystemMeta { pub struct SystemMeta {
@ -422,6 +423,47 @@ where
self.system_meta.name.as_ref(), self.system_meta.name.as_ref(),
); );
} }
fn default_labels(&self) -> Vec<Box<dyn SystemLabel>> {
vec![Box::new(self.func.as_system_label())]
}
}
/// 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> Debug for SystemTypeIdLabel<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_tuple("SystemTypeIdLabel")
.field(&std::any::type_name::<T>())
.finish()
}
}
impl<T> Hash for SystemTypeIdLabel<T> {
fn hash<H: std::hash::Hasher>(&self, _state: &mut H) {
// All SystemTypeIds of a given type are the same.
}
}
impl<T> Clone for SystemTypeIdLabel<T> {
fn clone(&self) -> Self {
Self(PhantomData)
}
}
impl<T> Copy for SystemTypeIdLabel<T> {}
impl<T> PartialEq for SystemTypeIdLabel<T> {
#[inline]
fn eq(&self, _other: &Self) -> bool {
// All labels of a given type are equal, as they will all have the same type id
true
}
}
impl<T> Eq for SystemTypeIdLabel<T> {}
impl<T> SystemLabel for SystemTypeIdLabel<T> {
fn dyn_clone(&self) -> Box<dyn SystemLabel> {
Box::new(*self)
}
} }
/// A trait implemented for all functions that can be used as [`System`]s. /// A trait implemented for all functions that can be used as [`System`]s.
@ -490,3 +532,28 @@ macro_rules! impl_system_function {
} }
all_tuples!(impl_system_function, 0, 16, F); all_tuples!(impl_system_function, 0, 16, F);
/// Used to implicitly convert systems to their default labels. For example, it will convert
/// "system functions" to their [`SystemTypeIdLabel`].
pub trait AsSystemLabel<Marker> {
type SystemLabel: SystemLabel;
fn as_system_label(&self) -> Self::SystemLabel;
}
impl<In, Out, Param: SystemParam, Marker, T: SystemParamFunction<In, Out, Param, Marker>>
AsSystemLabel<(In, Out, Param, Marker)> for T
{
type SystemLabel = SystemTypeIdLabel<Self>;
fn as_system_label(&self) -> Self::SystemLabel {
SystemTypeIdLabel(PhantomData::<fn() -> Self>)
}
}
impl<T: SystemLabel + Clone> AsSystemLabel<()> for T {
type SystemLabel = T;
fn as_system_label(&self) -> Self::SystemLabel {
self.clone()
}
}

View file

@ -4,6 +4,7 @@ use crate::{
archetype::{Archetype, ArchetypeComponentId}, archetype::{Archetype, ArchetypeComponentId},
component::ComponentId, component::ComponentId,
query::Access, query::Access,
schedule::SystemLabel,
world::World, world::World,
}; };
use std::borrow::Cow; use std::borrow::Cow;
@ -56,6 +57,10 @@ pub trait System: Send + Sync + 'static {
/// Initialize the system. /// Initialize the system.
fn initialize(&mut self, _world: &mut World); fn initialize(&mut self, _world: &mut World);
fn check_change_tick(&mut self, change_tick: u32); fn check_change_tick(&mut self, change_tick: u32);
/// The default labels for the system
fn default_labels(&self) -> Vec<Box<dyn SystemLabel>> {
Vec::new()
}
} }
/// A convenience type alias for a boxed [`System`] trait object. /// A convenience type alias for a boxed [`System`] trait object.

View file

@ -16,14 +16,6 @@ struct PostPhysics;
#[derive(Default)] #[derive(Default)]
struct Done(bool); struct Done(bool);
/// This is used to show that within a [`SystemSet`], individual systems can also
/// be labelled, allowing further fine tuning of run ordering.
#[derive(Clone, Hash, Debug, PartialEq, Eq, SystemLabel)]
pub enum PhysicsSystem {
UpdateVelocity,
Movement,
}
/// This example realizes the following scheme: /// This example realizes the following scheme:
/// ///
/// ```none /// ```none
@ -63,18 +55,9 @@ fn main() {
// This criteria ensures this whole system set only runs when this system's // This criteria ensures this whole system set only runs when this system's
// output says so (ShouldRun::Yes) // output says so (ShouldRun::Yes)
.with_run_criteria(run_for_a_second) .with_run_criteria(run_for_a_second)
.with_system( .with_system(update_velocity)
update_velocity // Make movement run after update_velocity
// Only applied to the `update_velocity` system .with_system(movement.after(update_velocity)),
.label(PhysicsSystem::UpdateVelocity),
)
.with_system(
movement
// Only applied to the `movement` system
.label(PhysicsSystem::Movement)
// Enforce order within this system by specifying this
.after(PhysicsSystem::UpdateVelocity),
),
) )
.add_system_set( .add_system_set(
SystemSet::new() SystemSet::new()