From c893b992240fc058fd118c8247c70233931ec759 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Sun, 27 Jun 2021 00:40:09 +0000 Subject: [PATCH] Optional `.system` (#2398) This can be your 6 months post-christmas present. # Objective - Make `.system` optional - yeet - It's ugly - Alternative title: `.system` is dead; long live `.system` - **yeet** ## Solution - Use a higher ranked lifetime, and some trait magic. N.B. This PR does not actually remove any `.system`s, except in a couple of examples. Once this is merged we can do that piecemeal across crates, and decide on syntax for labels. --- crates/bevy_app/src/app_builder.rs | 17 +++--- crates/bevy_ecs/src/schedule/mod.rs | 8 +-- crates/bevy_ecs/src/schedule/stage.rs | 10 ++-- .../src/schedule/system_descriptor.rs | 42 ++++++++------- crates/bevy_ecs/src/schedule/system_set.rs | 6 ++- .../bevy_ecs/src/system/exclusive_system.rs | 7 ++- crates/bevy_ecs/src/system/function_system.rs | 52 ++++++++++++++----- examples/2d/contributors.rs | 10 ++-- examples/2d/many_sprites.rs | 2 +- examples/2d/mesh.rs | 2 +- examples/2d/sprite.rs | 2 +- examples/2d/sprite_flipping.rs | 2 +- examples/2d/sprite_sheet.rs | 4 +- examples/2d/text2d.rs | 8 +-- examples/2d/texture_atlas.rs | 6 +-- 15 files changed, 107 insertions(+), 71 deletions(-) diff --git a/crates/bevy_app/src/app_builder.rs b/crates/bevy_app/src/app_builder.rs index 90671ae48ab67..7c454cd7045bc 100644 --- a/crates/bevy_app/src/app_builder.rs +++ b/crates/bevy_app/src/app_builder.rs @@ -7,7 +7,7 @@ use bevy_ecs::{ component::{Component, ComponentDescriptor}, event::Events, schedule::{ - RunOnce, Schedule, Stage, StageLabel, State, SystemDescriptor, SystemSet, SystemStage, + IntoSystemDescriptor, RunOnce, Schedule, Stage, StageLabel, State, SystemSet, SystemStage, }, system::{IntoExclusiveSystem, IntoSystem}, world::{FromWorld, World}, @@ -180,7 +180,7 @@ impl AppBuilder { /// App::build() /// .add_system(my_system.system()); /// ``` - pub fn add_system(&mut self, system: impl Into) -> &mut Self { + pub fn add_system(&mut self, system: impl IntoSystemDescriptor) -> &mut Self { self.add_system_to_stage(CoreStage::Update, system) } @@ -188,10 +188,10 @@ impl AppBuilder { self.add_system_set_to_stage(CoreStage::Update, system_set) } - pub fn add_system_to_stage( + pub fn add_system_to_stage( &mut self, stage_label: impl StageLabel, - system: impl Into, + system: impl IntoSystemDescriptor, ) -> &mut Self { self.app.schedule.add_system_to_stage(stage_label, system); self @@ -228,7 +228,10 @@ impl AppBuilder { /// App::build() /// .add_startup_system(my_startup_system.system()); /// ``` - pub fn add_startup_system(&mut self, system: impl Into) -> &mut Self { + pub fn add_startup_system( + &mut self, + system: impl IntoSystemDescriptor, + ) -> &mut Self { self.add_startup_system_to_stage(StartupStage::Startup, system) } @@ -236,10 +239,10 @@ impl AppBuilder { self.add_startup_system_set_to_stage(StartupStage::Startup, system_set) } - pub fn add_startup_system_to_stage( + pub fn add_startup_system_to_stage( &mut self, stage_label: impl StageLabel, - system: impl Into, + system: impl IntoSystemDescriptor, ) -> &mut Self { self.app .schedule diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 75480468b0bfa..67f21025cf0bf 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -66,10 +66,10 @@ impl Schedule { self } - pub fn with_system_in_stage( + pub fn with_system_in_stage( mut self, stage_label: impl StageLabel, - system: impl Into, + system: impl IntoSystemDescriptor, ) -> Self { self.add_system_to_stage(stage_label, system); self @@ -141,10 +141,10 @@ impl Schedule { self } - pub fn add_system_to_stage( + pub fn add_system_to_stage( &mut self, stage_label: impl StageLabel, - system: impl Into, + system: impl IntoSystemDescriptor, ) -> &mut Self { // Use a function instead of a closure to ensure that it is codegend inside bevy_ecs instead // of the game. Closures inherit generic parameters from their enclosing function. diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index a92ffe7d67475..9cd49f828c61f 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -16,6 +16,8 @@ use downcast_rs::{impl_downcast, Downcast}; use fixedbitset::FixedBitSet; use std::fmt::Debug; +use super::IntoSystemDescriptor; + pub trait Stage: Downcast + Send + Sync { /// Runs the stage; this happens once per update. /// Implementors must initialize all of their state and systems before running the first time. @@ -105,7 +107,7 @@ impl SystemStage { } } - pub fn single(system: impl Into) -> Self { + pub fn single(system: impl IntoSystemDescriptor) -> Self { Self::single_threaded().with_system(system) } @@ -131,13 +133,13 @@ impl SystemStage { self.executor = executor; } - pub fn with_system(mut self, system: impl Into) -> Self { + pub fn with_system(mut self, system: impl IntoSystemDescriptor) -> Self { self.add_system(system); self } - pub fn add_system(&mut self, system: impl Into) -> &mut Self { - self.add_system_inner(system.into(), None); + pub fn add_system(&mut self, system: impl IntoSystemDescriptor) -> &mut Self { + self.add_system_inner(system.into_descriptor(), None); self } diff --git a/crates/bevy_ecs/src/schedule/system_descriptor.rs b/crates/bevy_ecs/src/schedule/system_descriptor.rs index 961826ffcf577..bc7431131002c 100644 --- a/crates/bevy_ecs/src/schedule/system_descriptor.rs +++ b/crates/bevy_ecs/src/schedule/system_descriptor.rs @@ -39,44 +39,48 @@ pub enum SystemDescriptor { Exclusive(ExclusiveSystemDescriptor), } +pub trait IntoSystemDescriptor { + fn into_descriptor(self) -> SystemDescriptor; +} + pub struct SystemLabelMarker; -impl From for SystemDescriptor { - fn from(descriptor: ParallelSystemDescriptor) -> Self { - SystemDescriptor::Parallel(descriptor) +impl IntoSystemDescriptor<()> for ParallelSystemDescriptor { + fn into_descriptor(self) -> SystemDescriptor { + SystemDescriptor::Parallel(self) } } -impl From for SystemDescriptor +impl IntoSystemDescriptor for S where - S: System, + S: crate::system::IntoSystem<(), (), Params>, { - fn from(system: S) -> Self { - new_parallel_descriptor(Box::new(system)).into() + fn into_descriptor(self) -> SystemDescriptor { + new_parallel_descriptor(Box::new(self.system())).into_descriptor() } } -impl From> for SystemDescriptor { - fn from(system: BoxedSystem<(), ()>) -> Self { - new_parallel_descriptor(system).into() +impl IntoSystemDescriptor<()> for BoxedSystem<(), ()> { + fn into_descriptor(self) -> SystemDescriptor { + new_parallel_descriptor(self).into_descriptor() } } -impl From for SystemDescriptor { - fn from(descriptor: ExclusiveSystemDescriptor) -> Self { - SystemDescriptor::Exclusive(descriptor) +impl IntoSystemDescriptor<()> for ExclusiveSystemDescriptor { + fn into_descriptor(self) -> SystemDescriptor { + SystemDescriptor::Exclusive(self) } } -impl From for SystemDescriptor { - fn from(system: ExclusiveSystemFn) -> Self { - new_exclusive_descriptor(Box::new(system)).into() +impl IntoSystemDescriptor<()> for ExclusiveSystemFn { + fn into_descriptor(self) -> SystemDescriptor { + new_exclusive_descriptor(Box::new(self)).into_descriptor() } } -impl From for SystemDescriptor { - fn from(system: ExclusiveSystemCoerced) -> Self { - new_exclusive_descriptor(Box::new(system)).into() +impl IntoSystemDescriptor<()> for ExclusiveSystemCoerced { + fn into_descriptor(self) -> SystemDescriptor { + new_exclusive_descriptor(Box::new(self)).into_descriptor() } } diff --git a/crates/bevy_ecs/src/schedule/system_set.rs b/crates/bevy_ecs/src/schedule/system_set.rs index f6e058413d8ef..2fe7fc10c786f 100644 --- a/crates/bevy_ecs/src/schedule/system_set.rs +++ b/crates/bevy_ecs/src/schedule/system_set.rs @@ -7,6 +7,8 @@ use crate::{ }; use std::{fmt::Debug, hash::Hash}; +use super::IntoSystemDescriptor; + /// A builder for describing several systems at the same time. pub struct SystemSet { pub(crate) systems: Vec, @@ -89,8 +91,8 @@ impl SystemSet { self } - pub fn with_system(mut self, system: impl Into) -> Self { - self.systems.push(system.into()); + pub fn with_system(mut self, system: impl IntoSystemDescriptor) -> Self { + self.systems.push(system.into_descriptor()); self } diff --git a/crates/bevy_ecs/src/system/exclusive_system.rs b/crates/bevy_ecs/src/system/exclusive_system.rs index 607853294c5f7..ea7c6d8c1ada9 100644 --- a/crates/bevy_ecs/src/system/exclusive_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_system.rs @@ -1,5 +1,5 @@ use crate::{ - system::{check_system_change_tick, BoxedSystem, IntoSystem, System, SystemId}, + system::{check_system_change_tick, BoxedSystem, IntoSystem, SystemId}, world::World, }; use std::borrow::Cow; @@ -99,10 +99,9 @@ impl ExclusiveSystem for ExclusiveSystemCoerced { } } -impl IntoExclusiveSystem<(Params, SystemType), ExclusiveSystemCoerced> for S +impl IntoExclusiveSystem for S where - S: IntoSystem, - SystemType: System, + S: IntoSystem<(), (), Params>, { fn exclusive_system(self) -> ExclusiveSystemCoerced { ExclusiveSystemCoerced { diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 414ba2c3fec26..566ef78dbf9c7 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -173,13 +173,20 @@ impl SystemState { /// /// let system = my_system_function.system(); /// ``` -pub trait IntoSystem { +// This trait has to be generic because we have potentially overlapping impls, in particular +// because Rust thinks a type could impl multiple different `FnMut` combinations +// even though none can currently +pub trait IntoSystem { + type System: System; /// Turns this value into its corresponding [`System`]. - fn system(self) -> SystemType; + fn system(self) -> Self::System; } +pub struct AlreadyWasSystem; + // Systems implicitly implement IntoSystem -impl IntoSystem<(), Sys> for Sys { +impl> IntoSystem for Sys { + type System = Sys; fn system(self) -> Sys { self } @@ -256,8 +263,9 @@ impl FunctionSystem IntoSystem> for F +impl IntoSystem for F where In: 'static, Out: 'static, @@ -265,7 +273,8 @@ where Marker: 'static, F: SystemParamFunction + Send + Sync + 'static, { - fn system(self) -> FunctionSystem { + type System = FunctionSystem; + fn system(self) -> Self::System { FunctionSystem { func: self, param_state: None, @@ -376,30 +385,47 @@ pub trait SystemParamFunction: Send + Sync macro_rules! impl_system_function { ($($param: ident),*) => { #[allow(non_snake_case)] - impl SystemParamFunction<(), Out, ($($param,)*), ()> for Func + impl SystemParamFunction<(), Out, ($($param,)*), ()> for Func where - Func: + for <'a> &'a mut Func: FnMut($($param),*) -> Out + - FnMut($(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out + Send + Sync + 'static, Out: 'static + FnMut($(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out, Out: 'static { #[inline] unsafe fn run(&mut self, _input: (), state: &mut <($($param,)*) as SystemParam>::Fetch, system_meta: &SystemMeta, world: &World, change_tick: u32) -> Out { + // Yes, this is strange, but rustc fails to compile this impl + // without using this function. + #[allow(clippy::too_many_arguments)] + fn call_inner( + mut f: impl FnMut($($param,)*)->Out, + $($param: $param,)* + )->Out{ + f($($param,)*) + } let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_meta, world, change_tick); - self($($param),*) + call_inner(self, $($param),*) } } #[allow(non_snake_case)] - impl SystemParamFunction for Func + impl SystemParamFunction for Func where - Func: + for <'a> &'a mut Func: FnMut(In, $($param),*) -> Out + - FnMut(In, $(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out + Send + Sync + 'static, Out: 'static + FnMut(In, $(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out, Out: 'static { #[inline] unsafe fn run(&mut self, input: Input, state: &mut <($($param,)*) as SystemParam>::Fetch, system_meta: &SystemMeta, world: &World, change_tick: u32) -> Out { + #[allow(clippy::too_many_arguments)] + fn call_inner( + mut f: impl FnMut(In, $($param,)*)->Out, + input: In, + $($param: $param,)* + )->Out{ + f(input, $($param,)*) + } let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_meta, world, change_tick); - self(In(input), $($param),*) + call_inner(self, In(input), $($param),*) } } }; diff --git a/examples/2d/contributors.rs b/examples/2d/contributors.rs index 74db49dc9bda8..7dd7db7bb9fa3 100644 --- a/examples/2d/contributors.rs +++ b/examples/2d/contributors.rs @@ -9,11 +9,11 @@ use std::{ fn main() { App::build() .add_plugins(DefaultPlugins) - .add_startup_system(setup.system()) - .add_system(velocity_system.system()) - .add_system(move_system.system()) - .add_system(collision_system.system()) - .add_system(select_system.system()) + .add_startup_system(setup) + .add_system(velocity_system) + .add_system(move_system) + .add_system(collision_system) + .add_system(select_system) .run(); } diff --git a/examples/2d/many_sprites.rs b/examples/2d/many_sprites.rs index c8dbc2a8cf47b..fcf10d83322a9 100644 --- a/examples/2d/many_sprites.rs +++ b/examples/2d/many_sprites.rs @@ -23,7 +23,7 @@ fn main() { frustum_culling_enabled: true, }) .add_plugins(DefaultPlugins) - .add_startup_system(setup.system()) + .add_startup_system(setup) .add_system(tick.system().label("Tick")) .add_system(move_camera.system().after("Tick")) .run() diff --git a/examples/2d/mesh.rs b/examples/2d/mesh.rs index 7d46abaf59b55..9c1c097867c19 100644 --- a/examples/2d/mesh.rs +++ b/examples/2d/mesh.rs @@ -9,7 +9,7 @@ use bevy::{ fn main() { App::build() .add_plugins(DefaultPlugins) - .add_startup_system(star.system()) + .add_startup_system(star) .run(); } diff --git a/examples/2d/sprite.rs b/examples/2d/sprite.rs index 4767e6b903b4d..485bbee34bc0c 100644 --- a/examples/2d/sprite.rs +++ b/examples/2d/sprite.rs @@ -3,7 +3,7 @@ use bevy::prelude::*; fn main() { App::build() .add_plugins(DefaultPlugins) - .add_startup_system(setup.system()) + .add_startup_system(setup) .run(); } diff --git a/examples/2d/sprite_flipping.rs b/examples/2d/sprite_flipping.rs index 59403cc46b3eb..8ba1a32bad46b 100644 --- a/examples/2d/sprite_flipping.rs +++ b/examples/2d/sprite_flipping.rs @@ -3,7 +3,7 @@ use bevy::prelude::*; fn main() { App::build() .add_plugins(DefaultPlugins) - .add_startup_system(setup.system()) + .add_startup_system(setup) .run(); } diff --git a/examples/2d/sprite_sheet.rs b/examples/2d/sprite_sheet.rs index a766087cc5364..b697a5fb7dc3e 100644 --- a/examples/2d/sprite_sheet.rs +++ b/examples/2d/sprite_sheet.rs @@ -3,8 +3,8 @@ use bevy::prelude::*; fn main() { App::build() .add_plugins(DefaultPlugins) - .add_startup_system(setup.system()) - .add_system(animate_sprite_system.system()) + .add_startup_system(setup) + .add_system(animate_sprite_system) .run(); } diff --git a/examples/2d/text2d.rs b/examples/2d/text2d.rs index 86232ecab58c2..0f5b07a687d7e 100644 --- a/examples/2d/text2d.rs +++ b/examples/2d/text2d.rs @@ -3,10 +3,10 @@ use bevy::prelude::*; fn main() { App::build() .add_plugins(DefaultPlugins) - .add_startup_system(setup.system()) - .add_system(animate_translation.system()) - .add_system(animate_rotation.system()) - .add_system(animate_scale.system()) + .add_startup_system(setup) + .add_system(animate_translation) + .add_system(animate_rotation) + .add_system(animate_scale) .run(); } diff --git a/examples/2d/texture_atlas.rs b/examples/2d/texture_atlas.rs index 72509d67c4b1f..9e9f28ca72174 100644 --- a/examples/2d/texture_atlas.rs +++ b/examples/2d/texture_atlas.rs @@ -7,9 +7,9 @@ fn main() { .init_resource::() .add_plugins(DefaultPlugins) .add_state(AppState::Setup) - .add_system_set(SystemSet::on_enter(AppState::Setup).with_system(load_textures.system())) - .add_system_set(SystemSet::on_update(AppState::Setup).with_system(check_textures.system())) - .add_system_set(SystemSet::on_enter(AppState::Finished).with_system(setup.system())) + .add_system_set(SystemSet::on_enter(AppState::Setup).with_system(load_textures)) + .add_system_set(SystemSet::on_update(AppState::Setup).with_system(check_textures)) + .add_system_set(SystemSet::on_enter(AppState::Finished).with_system(setup)) .run(); }