Convert to fallible system in IntoSystemConfigs (#17051)

# Objective

- #16589 added an enum to switch between fallible and infallible system.
This branching should be unnecessary if we wrap infallible systems in a
function to return `Ok(())`.

## Solution

- Create a wrapper system for `System<(), ()>`s that returns `Ok` on the
call to `run` and `run_unsafe`. The wrapper should compile out, but I
haven't checked.
- I removed the `impl IntoSystemConfigs for BoxedSystem<(), ()>` as I
couldn't figure out a way to keep the impl without double boxing.

## Testing

- ran `many_foxes` example to check if it still runs.

## Migration Guide

- `IntoSystemConfigs` has been removed for `BoxedSystem<(), ()>`. Either
use `InfallibleSystemWrapper` before boxing or make your system return
`bevy::ecs::prelude::Result`.

crates/bevy_ecs/src/schedule/config.rs

@@ -9,7 +9,7 @@ use crate::{
         set::{InternedSystemSet, IntoSystemSet, SystemSet},
         Chain,
     },
-    system::{BoxedSystem, IntoSystem, ScheduleSystem, System},
+    system::{BoxedSystem, InfallibleSystemWrapper, IntoSystem, ScheduleSystem, System},
 };
 
 fn new_condition<M>(condition: impl Condition<M>) -> BoxedCondition {
@@ -519,6 +519,7 @@ impl IntoSystemConfigs<()> for SystemConfigs {
     }
 }
 
+/// Marker component to allow for conflicting implementations of [`IntoSystemConfigs`]
 #[doc(hidden)]
 pub struct Infallible;
 
@@ -527,17 +528,12 @@ where
     F: IntoSystem<(), (), Marker>,
 {
     fn into_configs(self) -> SystemConfigs {
-        let boxed_system = Box::new(IntoSystem::into_system(self));
-        SystemConfigs::new_system(ScheduleSystem::Infallible(boxed_system))
-    }
-}
-
-impl IntoSystemConfigs<()> for BoxedSystem<(), ()> {
-    fn into_configs(self) -> SystemConfigs {
-        SystemConfigs::new_system(ScheduleSystem::Infallible(self))
+        let wrapper = InfallibleSystemWrapper::new(IntoSystem::into_system(self));
+        SystemConfigs::new_system(Box::new(wrapper))
     }
 }
 
+/// Marker component to allow for conflicting implementations of [`IntoSystemConfigs`]
 #[doc(hidden)]
 pub struct Fallible;
 
@@ -547,13 +543,13 @@ where
 {
     fn into_configs(self) -> SystemConfigs {
         let boxed_system = Box::new(IntoSystem::into_system(self));
-        SystemConfigs::new_system(ScheduleSystem::Fallible(boxed_system))
+        SystemConfigs::new_system(boxed_system)
     }
 }
 
 impl IntoSystemConfigs<()> for BoxedSystem<(), Result> {
     fn into_configs(self) -> SystemConfigs {
-        SystemConfigs::new_system(ScheduleSystem::Fallible(self))
+        SystemConfigs::new_system(self)
     }
 }
 

crates/bevy_ecs/src/schedule/executor/mod.rs

@@ -18,6 +18,7 @@ use crate::{
     component::{ComponentId, Tick},
     prelude::{IntoSystemSet, SystemSet},
     query::Access,
+    result::Result,
     schedule::{BoxedCondition, InternedSystemSet, NodeId, SystemTypeSet},
     system::{ScheduleSystem, System, SystemIn},
     world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
@@ -158,7 +159,7 @@ pub(super) fn is_apply_deferred(system: &ScheduleSystem) -> bool {
 
 impl System for ApplyDeferred {
     type In = ();
-    type Out = ();
+    type Out = Result<()>;
 
     fn name(&self) -> Cow<'static, str> {
         Cow::Borrowed("bevy_ecs::apply_deferred")
@@ -203,11 +204,13 @@ impl System for ApplyDeferred {
     ) -> Self::Out {
         // This system does nothing on its own. The executor will apply deferred
         // commands from other systems instead of running this system.
+        Ok(())
     }
 
     fn run(&mut self, _input: SystemIn<'_, Self>, _world: &mut World) -> Self::Out {
         // This system does nothing on its own. The executor will apply deferred
         // commands from other systems instead of running this system.
+        Ok(())
     }
 
     fn apply_deferred(&mut self, _world: &mut World) {}
@@ -259,7 +262,7 @@ mod __rust_begin_short_backtrace {
 
     use crate::{
         result::Result,
-        system::{ReadOnlySystem, ScheduleSystem, System},
+        system::{ReadOnlySystem, ScheduleSystem},
         world::{unsafe_world_cell::UnsafeWorldCell, World},
     };
 

crates/bevy_ecs/src/schedule/executor/multi_threaded.rs

@@ -20,7 +20,7 @@ use crate::{
     prelude::Resource,
     query::Access,
     schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
-    system::{ScheduleSystem, System},
+    system::ScheduleSystem,
     world::{unsafe_world_cell::UnsafeWorldCell, World},
 };
 

crates/bevy_ecs/src/schedule/executor/simple.rs

@@ -7,7 +7,6 @@ use crate::{
     schedule::{
         executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule,
     },
-    system::System,
     world::World,
 };
 
@@ -136,7 +135,7 @@ impl SystemExecutor for SimpleExecutor {
 
 impl SimpleExecutor {
     /// Creates a new simple executor for use in a [`Schedule`](crate::schedule::Schedule).
-    /// This calls each system in order and immediately calls [`System::apply_deferred`].
+    /// This calls each system in order and immediately calls [`System::apply_deferred`](crate::system::System).
     pub const fn new() -> Self {
         Self {
             evaluated_sets: FixedBitSet::new(),

crates/bevy_ecs/src/schedule/executor/single_threaded.rs

@@ -5,7 +5,6 @@ use tracing::info_span;
 
 use crate::{
     schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
-    system::System,
     world::World,
 };
 

crates/bevy_ecs/src/schedule/schedule.rs

@@ -21,7 +21,7 @@ use crate::{
     prelude::Component,
     result::Result,
     schedule::*,
-    system::{IntoSystem, Resource, ScheduleSystem, System},
+    system::{IntoSystem, Resource, ScheduleSystem},
     world::World,
 };
 
@@ -1053,7 +1053,7 @@ impl ScheduleGraph {
         Ok(())
     }
 
-    /// Initializes any newly-added systems and conditions by calling [`System::initialize`]
+    /// Initializes any newly-added systems and conditions by calling [`System::initialize`](crate::system::System)
     pub fn initialize(&mut self, world: &mut World) {
         for (id, i) in self.uninit.drain(..) {
             match id {
@@ -1200,8 +1200,8 @@ impl ScheduleGraph {
         let id = NodeId::System(self.systems.len());
 
         self.systems
-            .push(SystemNode::new(ScheduleSystem::Infallible(Box::new(
-                IntoSystem::into_system(ApplyDeferred),
+            .push(SystemNode::new(Box::new(IntoSystem::into_system(
+                ApplyDeferred,
             ))));
         self.system_conditions.push(Vec::new());
 

crates/bevy_ecs/src/schedule/stepping.rs

@@ -1,6 +1,6 @@
 use crate::{
     schedule::{InternedScheduleLabel, NodeId, Schedule, ScheduleLabel},
-    system::{IntoSystem, ResMut, Resource, System},
+    system::{IntoSystem, ResMut, Resource},
 };
 use alloc::vec::Vec;
 use bevy_utils::{HashMap, TypeIdMap};

crates/bevy_ecs/src/system/schedule_system.rs

@@ -1,183 +1,120 @@
 use alloc::{borrow::Cow, vec::Vec};
-use core::any::TypeId;
 
 use crate::{
     archetype::ArchetypeComponentId,
     component::{ComponentId, Tick},
     query::Access,
     result::Result,
-    schedule::InternedSystemSet,
     system::{input::SystemIn, BoxedSystem, System},
     world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
 };
 
-/// A type which wraps and unifies the different sorts of systems that can be added to a schedule.
-pub enum ScheduleSystem {
-    /// A system that does not return a result.
-    Infallible(BoxedSystem<(), ()>),
-    /// A system that does return a result.
-    Fallible(BoxedSystem<(), Result>),
+use super::IntoSystem;
+
+/// A wrapper system to change a system that returns `()` to return `Ok(())` to make it into a [`ScheduleSystem`]
+pub struct InfallibleSystemWrapper<S: System<In = (), Out = ()>>(S);
+
+impl<S: System<In = (), Out = ()>> InfallibleSystemWrapper<S> {
+    /// Create a new `OkWrapperSystem`
+    pub fn new(system: S) -> Self {
+        Self(IntoSystem::into_system(system))
+    }
 }
 
-impl System for ScheduleSystem {
+impl<S: System<In = (), Out = ()>> System for InfallibleSystemWrapper<S> {
     type In = ();
     type Out = Result;
 
-    #[inline(always)]
+    #[inline]
     fn name(&self) -> Cow<'static, str> {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.name(),
-            ScheduleSystem::Fallible(inner_system) => inner_system.name(),
-        }
+        self.0.name()
     }
 
-    #[inline(always)]
-    fn type_id(&self) -> TypeId {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.type_id(),
-            ScheduleSystem::Fallible(inner_system) => inner_system.type_id(),
-        }
-    }
-
-    #[inline(always)]
+    #[inline]
     fn component_access(&self) -> &Access<ComponentId> {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.component_access(),
-            ScheduleSystem::Fallible(inner_system) => inner_system.component_access(),
-        }
+        self.0.component_access()
     }
 
-    #[inline(always)]
+    #[inline]
     fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.archetype_component_access(),
-            ScheduleSystem::Fallible(inner_system) => inner_system.archetype_component_access(),
-        }
+        self.0.archetype_component_access()
     }
 
-    #[inline(always)]
+    #[inline]
+    fn is_send(&self) -> bool {
+        self.0.is_send()
+    }
+
+    #[inline]
     fn is_exclusive(&self) -> bool {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.is_exclusive(),
-            ScheduleSystem::Fallible(inner_system) => inner_system.is_exclusive(),
-        }
+        self.0.is_exclusive()
     }
 
-    #[inline(always)]
+    #[inline]
     fn has_deferred(&self) -> bool {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.has_deferred(),
-            ScheduleSystem::Fallible(inner_system) => inner_system.has_deferred(),
-        }
+        self.0.has_deferred()
     }
 
-    #[inline(always)]
+    #[inline]
     unsafe fn run_unsafe(
         &mut self,
         input: SystemIn<'_, Self>,
         world: UnsafeWorldCell,
     ) -> Self::Out {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => {
-                inner_system.run_unsafe(input, world);
-                Ok(())
-            }
-            ScheduleSystem::Fallible(inner_system) => inner_system.run_unsafe(input, world),
-        }
+        self.0.run_unsafe(input, world);
+        Ok(())
     }
 
-    #[inline(always)]
+    #[inline]
     fn run(&mut self, input: SystemIn<'_, Self>, world: &mut World) -> Self::Out {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => {
-                inner_system.run(input, world);
-                Ok(())
-            }
-            ScheduleSystem::Fallible(inner_system) => inner_system.run(input, world),
-        }
+        self.0.run(input, world);
+        Ok(())
     }
 
-    #[inline(always)]
+    #[inline]
     fn apply_deferred(&mut self, world: &mut World) {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.apply_deferred(world),
-            ScheduleSystem::Fallible(inner_system) => inner_system.apply_deferred(world),
-        }
+        self.0.apply_deferred(world);
     }
 
-    #[inline(always)]
+    #[inline]
     fn queue_deferred(&mut self, world: DeferredWorld) {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.queue_deferred(world),
-            ScheduleSystem::Fallible(inner_system) => inner_system.queue_deferred(world),
-        }
+        self.0.queue_deferred(world);
     }
 
-    #[inline(always)]
-    fn is_send(&self) -> bool {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.is_send(),
-            ScheduleSystem::Fallible(inner_system) => inner_system.is_send(),
-        }
-    }
-
-    #[inline(always)]
+    #[inline]
     unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.validate_param_unsafe(world),
-            ScheduleSystem::Fallible(inner_system) => inner_system.validate_param_unsafe(world),
-        }
+        self.0.validate_param_unsafe(world)
     }
 
-    #[inline(always)]
+    #[inline]
     fn initialize(&mut self, world: &mut World) {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.initialize(world),
-            ScheduleSystem::Fallible(inner_system) => inner_system.initialize(world),
-        }
+        self.0.initialize(world);
     }
 
-    #[inline(always)]
+    #[inline]
     fn update_archetype_component_access(&mut self, world: UnsafeWorldCell) {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => {
-                inner_system.update_archetype_component_access(world);
-            }
-            ScheduleSystem::Fallible(inner_system) => {
-                inner_system.update_archetype_component_access(world);
-            }
-        }
+        self.0.update_archetype_component_access(world);
     }
 
-    #[inline(always)]
+    #[inline]
     fn check_change_tick(&mut self, change_tick: Tick) {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.check_change_tick(change_tick),
-            ScheduleSystem::Fallible(inner_system) => inner_system.check_change_tick(change_tick),
-        }
+        self.0.check_change_tick(change_tick);
     }
 
-    #[inline(always)]
-    fn default_system_sets(&self) -> Vec<InternedSystemSet> {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.default_system_sets(),
-            ScheduleSystem::Fallible(inner_system) => inner_system.default_system_sets(),
-        }
-    }
-
-    #[inline(always)]
+    #[inline]
     fn get_last_run(&self) -> Tick {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.get_last_run(),
-            ScheduleSystem::Fallible(inner_system) => inner_system.get_last_run(),
-        }
+        self.0.get_last_run()
     }
 
-    #[inline(always)]
+    #[inline]
     fn set_last_run(&mut self, last_run: Tick) {
-        match self {
-            ScheduleSystem::Infallible(inner_system) => inner_system.set_last_run(last_run),
-            ScheduleSystem::Fallible(inner_system) => inner_system.set_last_run(last_run),
-        }
+        self.0.set_last_run(last_run);
+    }
+
+    fn default_system_sets(&self) -> Vec<crate::schedule::InternedSystemSet> {
+        self.0.default_system_sets()
     }
 }
+
+/// Type alias for a `BoxedSystem` that a `Schedule` can store.
+pub type ScheduleSystem = BoxedSystem<(), Result>;