Skip to content

Commit b1c3e98

Browse files
committed
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!
1 parent d51b54a commit b1c3e98

File tree

6 files changed

+120
-51
lines changed

6 files changed

+120
-51
lines changed

crates/bevy_ecs/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ pub mod prelude {
3535
system::{
3636
Commands, In, IntoChainSystem, IntoExclusiveSystem, IntoSystem, Local, NonSend,
3737
NonSendMut, Query, QuerySet, RemovedComponents, Res, ResMut, System,
38+
SystemParamFunction,
3839
},
3940
world::{FromWorld, Mut, World},
4041
};

crates/bevy_ecs/src/schedule/stage.rs

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1578,15 +1578,25 @@ mod tests {
15781578
fn ambiguity_detection() {
15791579
use super::{find_ambiguities, SystemContainer};
15801580

1581-
fn find_ambiguities_first_labels(
1581+
fn find_ambiguities_first_str_labels(
15821582
systems: &[impl SystemContainer],
15831583
) -> Vec<(BoxedSystemLabel, BoxedSystemLabel)> {
15841584
find_ambiguities(systems)
15851585
.drain(..)
15861586
.map(|(index_a, index_b, _conflicts)| {
15871587
(
1588-
systems[index_a].labels()[0].clone(),
1589-
systems[index_b].labels()[0].clone(),
1588+
systems[index_a]
1589+
.labels()
1590+
.iter()
1591+
.find(|a| (&***a).type_id() == std::any::TypeId::of::<&str>())
1592+
.unwrap()
1593+
.clone(),
1594+
systems[index_b]
1595+
.labels()
1596+
.iter()
1597+
.find(|a| (&***a).type_id() == std::any::TypeId::of::<&str>())
1598+
.unwrap()
1599+
.clone(),
15901600
)
15911601
})
15921602
.collect()
@@ -1616,7 +1626,7 @@ mod tests {
16161626
.with_system(component.label("4"));
16171627
stage.initialize_systems(&mut world);
16181628
stage.rebuild_orders_and_dependencies();
1619-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1629+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
16201630
assert!(
16211631
ambiguities.contains(&(Box::new("1"), Box::new("4")))
16221632
|| ambiguities.contains(&(Box::new("4"), Box::new("1")))
@@ -1631,7 +1641,7 @@ mod tests {
16311641
.with_system(resource.label("4"));
16321642
stage.initialize_systems(&mut world);
16331643
stage.rebuild_orders_and_dependencies();
1634-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1644+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
16351645
assert!(
16361646
ambiguities.contains(&(Box::new("1"), Box::new("4")))
16371647
|| ambiguities.contains(&(Box::new("4"), Box::new("1")))
@@ -1656,7 +1666,7 @@ mod tests {
16561666
.with_system(resource.label("4"));
16571667
stage.initialize_systems(&mut world);
16581668
stage.rebuild_orders_and_dependencies();
1659-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1669+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
16601670
assert!(
16611671
ambiguities.contains(&(Box::new("0"), Box::new("3")))
16621672
|| ambiguities.contains(&(Box::new("3"), Box::new("0")))
@@ -1675,7 +1685,7 @@ mod tests {
16751685
.with_system(resource.label("4").in_ambiguity_set("a"));
16761686
stage.initialize_systems(&mut world);
16771687
stage.rebuild_orders_and_dependencies();
1678-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1688+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
16791689
assert!(
16801690
ambiguities.contains(&(Box::new("0"), Box::new("3")))
16811691
|| ambiguities.contains(&(Box::new("3"), Box::new("0")))
@@ -1688,7 +1698,7 @@ mod tests {
16881698
.with_system(component.label("2"));
16891699
stage.initialize_systems(&mut world);
16901700
stage.rebuild_orders_and_dependencies();
1691-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1701+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
16921702
assert!(
16931703
ambiguities.contains(&(Box::new("0"), Box::new("1")))
16941704
|| ambiguities.contains(&(Box::new("1"), Box::new("0")))
@@ -1701,7 +1711,7 @@ mod tests {
17011711
.with_system(component.label("2").after("0"));
17021712
stage.initialize_systems(&mut world);
17031713
stage.rebuild_orders_and_dependencies();
1704-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1714+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
17051715
assert!(
17061716
ambiguities.contains(&(Box::new("1"), Box::new("2")))
17071717
|| ambiguities.contains(&(Box::new("2"), Box::new("1")))
@@ -1715,7 +1725,7 @@ mod tests {
17151725
.with_system(component.label("3").after("1").after("2"));
17161726
stage.initialize_systems(&mut world);
17171727
stage.rebuild_orders_and_dependencies();
1718-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1728+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
17191729
assert!(
17201730
ambiguities.contains(&(Box::new("1"), Box::new("2")))
17211731
|| ambiguities.contains(&(Box::new("2"), Box::new("1")))
@@ -1729,7 +1739,7 @@ mod tests {
17291739
.with_system(component.label("3").after("1").after("2"));
17301740
stage.initialize_systems(&mut world);
17311741
stage.rebuild_orders_and_dependencies();
1732-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1742+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
17331743
assert_eq!(ambiguities.len(), 0);
17341744

17351745
let mut stage = SystemStage::parallel()
@@ -1739,7 +1749,7 @@ mod tests {
17391749
.with_system(component.label("3").after("1").after("2"));
17401750
stage.initialize_systems(&mut world);
17411751
stage.rebuild_orders_and_dependencies();
1742-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1752+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
17431753
assert!(
17441754
ambiguities.contains(&(Box::new("1"), Box::new("2")))
17451755
|| ambiguities.contains(&(Box::new("2"), Box::new("1")))
@@ -1769,7 +1779,7 @@ mod tests {
17691779
);
17701780
stage.initialize_systems(&mut world);
17711781
stage.rebuild_orders_and_dependencies();
1772-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1782+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
17731783
assert!(
17741784
ambiguities.contains(&(Box::new("1"), Box::new("2")))
17751785
|| ambiguities.contains(&(Box::new("2"), Box::new("1")))
@@ -1819,7 +1829,7 @@ mod tests {
18191829
);
18201830
stage.initialize_systems(&mut world);
18211831
stage.rebuild_orders_and_dependencies();
1822-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1832+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
18231833
assert_eq!(ambiguities.len(), 0);
18241834

18251835
let mut stage = SystemStage::parallel()
@@ -1850,7 +1860,7 @@ mod tests {
18501860
);
18511861
stage.initialize_systems(&mut world);
18521862
stage.rebuild_orders_and_dependencies();
1853-
let ambiguities = find_ambiguities_first_labels(&stage.parallel);
1863+
let ambiguities = find_ambiguities_first_str_labels(&stage.parallel);
18541864
assert!(
18551865
ambiguities.contains(&(Box::new("1"), Box::new("4")))
18561866
|| ambiguities.contains(&(Box::new("4"), Box::new("1")))
@@ -1884,7 +1894,7 @@ mod tests {
18841894
.with_system(empty.exclusive_system().label("6").after("2").after("5"));
18851895
stage.initialize_systems(&mut world);
18861896
stage.rebuild_orders_and_dependencies();
1887-
let ambiguities = find_ambiguities_first_labels(&stage.exclusive_at_start);
1897+
let ambiguities = find_ambiguities_first_str_labels(&stage.exclusive_at_start);
18881898
assert!(
18891899
ambiguities.contains(&(Box::new("1"), Box::new("3")))
18901900
|| ambiguities.contains(&(Box::new("3"), Box::new("1")))
@@ -1921,7 +1931,7 @@ mod tests {
19211931
.with_system(empty.exclusive_system().label("6").after("2").after("5"));
19221932
stage.initialize_systems(&mut world);
19231933
stage.rebuild_orders_and_dependencies();
1924-
let ambiguities = find_ambiguities_first_labels(&stage.exclusive_at_start);
1934+
let ambiguities = find_ambiguities_first_str_labels(&stage.exclusive_at_start);
19251935
assert!(
19261936
ambiguities.contains(&(Box::new("2"), Box::new("3")))
19271937
|| ambiguities.contains(&(Box::new("3"), Box::new("2")))
@@ -1947,7 +1957,7 @@ mod tests {
19471957
.with_system(empty.exclusive_system().label("3").in_ambiguity_set("a"));
19481958
stage.initialize_systems(&mut world);
19491959
stage.rebuild_orders_and_dependencies();
1950-
let ambiguities = find_ambiguities_first_labels(&stage.exclusive_at_start);
1960+
let ambiguities = find_ambiguities_first_str_labels(&stage.exclusive_at_start);
19511961
assert_eq!(ambiguities.len(), 0);
19521962
}
19531963

crates/bevy_ecs/src/schedule/system_descriptor.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ use crate::{
33
AmbiguitySetLabel, BoxedAmbiguitySetLabel, BoxedSystemLabel, IntoRunCriteria,
44
RunCriteriaDescriptorOrLabel, SystemLabel,
55
},
6-
system::{BoxedSystem, ExclusiveSystem, ExclusiveSystemCoerced, ExclusiveSystemFn, IntoSystem},
6+
system::{
7+
AsSystemLabel, BoxedSystem, ExclusiveSystem, ExclusiveSystemCoerced, ExclusiveSystemFn,
8+
IntoSystem,
9+
},
710
};
811

912
/// Encapsulates a system and information on when it run in a `SystemStage`.
@@ -105,9 +108,9 @@ pub struct ParallelSystemDescriptor {
105108

106109
fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescriptor {
107110
ParallelSystemDescriptor {
111+
labels: system.default_labels(),
108112
system,
109113
run_criteria: None,
110-
labels: Vec::new(),
111114
before: Vec::new(),
112115
after: Vec::new(),
113116
ambiguity_sets: Vec::new(),
@@ -126,10 +129,10 @@ pub trait ParallelSystemDescriptorCoercion<Params> {
126129
fn label(self, label: impl SystemLabel) -> ParallelSystemDescriptor;
127130

128131
/// Specifies that the system should run before systems with the given label.
129-
fn before(self, label: impl SystemLabel) -> ParallelSystemDescriptor;
132+
fn before<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor;
130133

131134
/// Specifies that the system should run after systems with the given label.
132-
fn after(self, label: impl SystemLabel) -> ParallelSystemDescriptor;
135+
fn after<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor;
133136

134137
/// Specifies that the system is exempt from execution order ambiguity detection
135138
/// with other systems in this set.
@@ -150,13 +153,13 @@ impl ParallelSystemDescriptorCoercion<()> for ParallelSystemDescriptor {
150153
self
151154
}
152155

153-
fn before(mut self, label: impl SystemLabel) -> ParallelSystemDescriptor {
154-
self.before.push(Box::new(label));
156+
fn before<Marker>(mut self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
157+
self.before.push(Box::new(label.as_system_label()));
155158
self
156159
}
157160

158-
fn after(mut self, label: impl SystemLabel) -> ParallelSystemDescriptor {
159-
self.after.push(Box::new(label));
161+
fn after<Marker>(mut self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
162+
self.after.push(Box::new(label.as_system_label()));
160163
self
161164
}
162165

@@ -182,11 +185,11 @@ where
182185
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).label(label)
183186
}
184187

185-
fn before(self, label: impl SystemLabel) -> ParallelSystemDescriptor {
188+
fn before<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
186189
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).before(label)
187190
}
188191

189-
fn after(self, label: impl SystemLabel) -> ParallelSystemDescriptor {
192+
fn after<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
190193
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).after(label)
191194
}
192195

@@ -207,11 +210,11 @@ impl ParallelSystemDescriptorCoercion<()> for BoxedSystem<(), ()> {
207210
new_parallel_descriptor(self).label(label)
208211
}
209212

210-
fn before(self, label: impl SystemLabel) -> ParallelSystemDescriptor {
213+
fn before<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
211214
new_parallel_descriptor(self).before(label)
212215
}
213216

214-
fn after(self, label: impl SystemLabel) -> ParallelSystemDescriptor {
217+
fn after<Marker>(self, label: impl AsSystemLabel<Marker>) -> ParallelSystemDescriptor {
215218
new_parallel_descriptor(self).after(label)
216219
}
217220

crates/bevy_ecs/src/system/function_system.rs

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ use crate::{
22
archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId},
33
component::ComponentId,
44
query::{Access, FilteredAccessSet},
5+
schedule::SystemLabel,
56
system::{
67
check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch,
78
SystemParamState,
89
},
910
world::{World, WorldId},
1011
};
1112
use bevy_ecs_macros::all_tuples;
12-
use std::{borrow::Cow, marker::PhantomData};
13+
use std::{borrow::Cow, fmt::Debug, hash::Hash, marker::PhantomData};
1314

1415
/// The metadata of a [`System`].
1516
pub struct SystemMeta {
@@ -422,6 +423,47 @@ where
422423
self.system_meta.name.as_ref(),
423424
);
424425
}
426+
fn default_labels(&self) -> Vec<Box<dyn SystemLabel>> {
427+
vec![Box::new(self.func.as_system_label())]
428+
}
429+
}
430+
431+
/// A [`SystemLabel`] that was automatically generated for a system on the basis of its `TypeId`.
432+
pub struct SystemTypeIdLabel<T: 'static>(PhantomData<fn() -> T>);
433+
434+
impl<T> Debug for SystemTypeIdLabel<T> {
435+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
436+
f.debug_tuple("SystemTypeIdLabel")
437+
.field(&std::any::type_name::<T>())
438+
.finish()
439+
}
440+
}
441+
impl<T> Hash for SystemTypeIdLabel<T> {
442+
fn hash<H: std::hash::Hasher>(&self, _state: &mut H) {
443+
// All SystemTypeIds of a given type are the same.
444+
}
445+
}
446+
impl<T> Clone for SystemTypeIdLabel<T> {
447+
fn clone(&self) -> Self {
448+
Self(PhantomData)
449+
}
450+
}
451+
452+
impl<T> Copy for SystemTypeIdLabel<T> {}
453+
454+
impl<T> PartialEq for SystemTypeIdLabel<T> {
455+
#[inline]
456+
fn eq(&self, _other: &Self) -> bool {
457+
// All labels of a given type are equal, as they will all have the same type id
458+
true
459+
}
460+
}
461+
impl<T> Eq for SystemTypeIdLabel<T> {}
462+
463+
impl<T> SystemLabel for SystemTypeIdLabel<T> {
464+
fn dyn_clone(&self) -> Box<dyn SystemLabel> {
465+
Box::new(*self)
466+
}
425467
}
426468

427469
/// A trait implemented for all functions that can be used as [`System`]s.
@@ -490,3 +532,28 @@ macro_rules! impl_system_function {
490532
}
491533

492534
all_tuples!(impl_system_function, 0, 16, F);
535+
536+
/// Used to implicitly convert systems to their default labels. For example, it will convert
537+
/// "system functions" to their [`SystemTypeIdLabel`].
538+
pub trait AsSystemLabel<Marker> {
539+
type SystemLabel: SystemLabel;
540+
fn as_system_label(&self) -> Self::SystemLabel;
541+
}
542+
543+
impl<In, Out, Param: SystemParam, Marker, T: SystemParamFunction<In, Out, Param, Marker>>
544+
AsSystemLabel<(In, Out, Param, Marker)> for T
545+
{
546+
type SystemLabel = SystemTypeIdLabel<Self>;
547+
548+
fn as_system_label(&self) -> Self::SystemLabel {
549+
SystemTypeIdLabel(PhantomData::<fn() -> Self>)
550+
}
551+
}
552+
553+
impl<T: SystemLabel + Clone> AsSystemLabel<()> for T {
554+
type SystemLabel = T;
555+
556+
fn as_system_label(&self) -> Self::SystemLabel {
557+
self.clone()
558+
}
559+
}

crates/bevy_ecs/src/system/system.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::{
44
archetype::{Archetype, ArchetypeComponentId},
55
component::ComponentId,
66
query::Access,
7+
schedule::SystemLabel,
78
world::World,
89
};
910
use std::borrow::Cow;
@@ -56,6 +57,10 @@ pub trait System: Send + Sync + 'static {
5657
/// Initialize the system.
5758
fn initialize(&mut self, _world: &mut World);
5859
fn check_change_tick(&mut self, change_tick: u32);
60+
/// The default labels for the system
61+
fn default_labels(&self) -> Vec<Box<dyn SystemLabel>> {
62+
Vec::new()
63+
}
5964
}
6065

6166
/// A convenience type alias for a boxed [`System`] trait object.

0 commit comments

Comments
 (0)