Optional .system (bevyengine#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.

Author: DJMcNab

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,18 +180,18 @@ impl AppBuilder {
     /// App::build()
     ///     .add_system(my_system.system());
     /// ```
-    pub fn add_system(&mut self, system: impl Into<SystemDescriptor>) -> &mut Self {
+    pub fn add_system<Params>(&mut self, system: impl IntoSystemDescriptor<Params>) -> &mut Self {
         self.add_system_to_stage(CoreStage::Update, system)
     }
 
     pub fn add_system_set(&mut self, system_set: SystemSet) -> &mut Self {
         self.add_system_set_to_stage(CoreStage::Update, system_set)
     }
 
-    pub fn add_system_to_stage(
+    pub fn add_system_to_stage<Params>(
         &mut self,
         stage_label: impl StageLabel,
-        system: impl Into<SystemDescriptor>,
+        system: impl IntoSystemDescriptor<Params>,
     ) -> &mut Self {
         self.app.schedule.add_system_to_stage(stage_label, system);
         self
@@ -228,18 +228,21 @@ impl AppBuilder {
     /// App::build()
     ///     .add_startup_system(my_startup_system.system());
     /// ```
-    pub fn add_startup_system(&mut self, system: impl Into<SystemDescriptor>) -> &mut Self {
+    pub fn add_startup_system<Params>(
+        &mut self,
+        system: impl IntoSystemDescriptor<Params>,
+    ) -> &mut Self {
         self.add_startup_system_to_stage(StartupStage::Startup, system)
     }
 
     pub fn add_startup_system_set(&mut self, system_set: SystemSet) -> &mut Self {
         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<Params>(
         &mut self,
         stage_label: impl StageLabel,
-        system: impl Into<SystemDescriptor>,
+        system: impl IntoSystemDescriptor<Params>,
     ) -> &mut Self {
         self.app
             .schedule

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<Params>(
         mut self,
         stage_label: impl StageLabel,
-        system: impl Into<SystemDescriptor>,
+        system: impl IntoSystemDescriptor<Params>,
     ) -> 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<Params>(
         &mut self,
         stage_label: impl StageLabel,
-        system: impl Into<SystemDescriptor>,
+        system: impl IntoSystemDescriptor<Params>,
     ) -> &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.

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<SystemDescriptor>) -> Self {
+    pub fn single<Params>(system: impl IntoSystemDescriptor<Params>) -> 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<SystemDescriptor>) -> Self {
+    pub fn with_system<Params>(mut self, system: impl IntoSystemDescriptor<Params>) -> Self {
         self.add_system(system);
         self
     }
 
-    pub fn add_system(&mut self, system: impl Into<SystemDescriptor>) -> &mut Self {
-        self.add_system_inner(system.into(), None);
+    pub fn add_system<Params>(&mut self, system: impl IntoSystemDescriptor<Params>) -> &mut Self {
+        self.add_system_inner(system.into_descriptor(), None);
         self
     }
 

crates/bevy_ecs/src/schedule/system_descriptor.rs

@@ -39,44 +39,48 @@ pub enum SystemDescriptor {
     Exclusive(ExclusiveSystemDescriptor),
 }
 
+pub trait IntoSystemDescriptor<Params> {
+    fn into_descriptor(self) -> SystemDescriptor;
+}
+
 pub struct SystemLabelMarker;
 
-impl From<ParallelSystemDescriptor> for SystemDescriptor {
-    fn from(descriptor: ParallelSystemDescriptor) -> Self {
-        SystemDescriptor::Parallel(descriptor)
+impl IntoSystemDescriptor<()> for ParallelSystemDescriptor {
+    fn into_descriptor(self) -> SystemDescriptor {
+        SystemDescriptor::Parallel(self)
     }
 }
 
-impl<S> From<S> for SystemDescriptor
+impl<Params, S> IntoSystemDescriptor<Params> for S
 where
-    S: System<In = (), Out = ()>,
+    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<BoxedSystem<(), ()>> 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<ExclusiveSystemDescriptor> for SystemDescriptor {
-    fn from(descriptor: ExclusiveSystemDescriptor) -> Self {
-        SystemDescriptor::Exclusive(descriptor)
+impl IntoSystemDescriptor<()> for ExclusiveSystemDescriptor {
+    fn into_descriptor(self) -> SystemDescriptor {
+        SystemDescriptor::Exclusive(self)
     }
 }
 
-impl From<ExclusiveSystemFn> 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<ExclusiveSystemCoerced> 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()
     }
 }
 

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<SystemDescriptor>,
@@ -89,8 +91,8 @@ impl SystemSet {
         self
     }
 
-    pub fn with_system(mut self, system: impl Into<SystemDescriptor>) -> Self {
-        self.systems.push(system.into());
+    pub fn with_system<Params>(mut self, system: impl IntoSystemDescriptor<Params>) -> Self {
+        self.systems.push(system.into_descriptor());
         self
     }
 

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<S, Params, SystemType> IntoExclusiveSystem<(Params, SystemType), ExclusiveSystemCoerced> for S
+impl<S, Params> IntoExclusiveSystem<Params, ExclusiveSystemCoerced> for S
 where
-    S: IntoSystem<Params, SystemType>,
-    SystemType: System<In = (), Out = ()>,
+    S: IntoSystem<(), (), Params>,
 {
     fn exclusive_system(self) -> ExclusiveSystemCoerced {
         ExclusiveSystemCoerced {

crates/bevy_ecs/src/system/function_system.rs

@@ -173,13 +173,20 @@ impl<Param: SystemParam> SystemState<Param> {
 ///
 /// let system = my_system_function.system();
 /// ```
-pub trait IntoSystem<Params, SystemType: System> {
+// 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<In, Out, Params> {
+    type System: System<In = In, Out = Out>;
     /// Turns this value into its corresponding [`System`].
-    fn system(self) -> SystemType;
+    fn system(self) -> Self::System;
 }
 
+pub struct AlreadyWasSystem;
+
 // Systems implicitly implement IntoSystem
-impl<Sys: System> IntoSystem<(), Sys> for Sys {
+impl<In, Out, Sys: System<In = In, Out = Out>> IntoSystem<In, Out, AlreadyWasSystem> for Sys {
+    type System = Sys;
     fn system(self) -> Sys {
         self
     }
@@ -256,16 +263,18 @@ impl<In, Out, Param: SystemParam, Marker, F> FunctionSystem<In, Out, Param, Mark
         self
     }
 }
+pub struct IsFunctionSystem;
 
-impl<In, Out, Param, Marker, F> IntoSystem<Param, FunctionSystem<In, Out, Param, Marker, F>> for F
+impl<In, Out, Param, Marker, F> IntoSystem<In, Out, (IsFunctionSystem, Param, Marker)> for F
 where
     In: 'static,
     Out: 'static,
     Param: SystemParam + 'static,
     Marker: 'static,
     F: SystemParamFunction<In, Out, Param, Marker> + Send + Sync + 'static,
 {
-    fn system(self) -> FunctionSystem<In, Out, Param, Marker, F> {
+    type System = FunctionSystem<In, Out, Param, Marker, F>;
+    fn system(self) -> Self::System {
         FunctionSystem {
             func: self,
             param_state: None,
@@ -376,30 +385,47 @@ pub trait SystemParamFunction<In, Out, Param: SystemParam, Marker>: Send + Sync
 macro_rules! impl_system_function {
     ($($param: ident),*) => {
         #[allow(non_snake_case)]
-        impl<Out, Func, $($param: SystemParam),*> SystemParamFunction<(), Out, ($($param,)*), ()> for Func
+        impl<Out, Func: Send + Sync + 'static, $($param: SystemParam),*> 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<Out, $($param,)*>(
+                    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<Input, Out, Func, $($param: SystemParam),*> SystemParamFunction<Input, Out, ($($param,)*), InputMarker> for Func
+        impl<Input, Out, Func: Send + Sync + 'static, $($param: SystemParam),*> SystemParamFunction<Input, Out, ($($param,)*), InputMarker> for Func
         where
-            Func:
+        for <'a> &'a mut Func:
                 FnMut(In<Input>, $($param),*) -> Out +
-                FnMut(In<Input>, $(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out + Send + Sync + 'static, Out: 'static
+                FnMut(In<Input>, $(<<$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<Input, Out, $($param,)*>(
+                    mut f: impl FnMut(In<Input>, $($param,)*)->Out,
+                    input: In<Input>,
+                    $($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),*)
             }
         }
     };

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();
 }
 

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()

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();
 }
 

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();
 }
 

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();
 }
 

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();
 }