Skip to content

Commit

Permalink
Convert to fallible system in IntoSystemConfigs (bevyengine#17051)
Browse files Browse the repository at this point in the history
# Objective

- bevyengine#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`.
  • Loading branch information
hymm authored and ecoskey committed Jan 6, 2025
1 parent 9417adf commit 0017ab0
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 140 deletions.
18 changes: 7 additions & 11 deletions crates/bevy_ecs/src/schedule/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -519,6 +519,7 @@ impl IntoSystemConfigs<()> for SystemConfigs {
}
}

/// Marker component to allow for conflicting implementations of [`IntoSystemConfigs`]
#[doc(hidden)]
pub struct Infallible;

Expand All @@ -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;

Expand All @@ -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)
}
}

Expand Down
7 changes: 5 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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) {}
Expand Down Expand Up @@ -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},
};

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};

Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_ecs/src/schedule/executor/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use crate::{
schedule::{
executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule,
},
system::System,
world::World,
};

Expand Down Expand Up @@ -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(),
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_ecs/src/schedule/executor/single_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use tracing::info_span;

use crate::{
schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
system::System,
world::World,
};

Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_ecs/src/schedule/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
prelude::Component,
result::Result,
schedule::*,
system::{IntoSystem, Resource, ScheduleSystem, System},
system::{IntoSystem, Resource, ScheduleSystem},
world::World,
};

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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());

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/schedule/stepping.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down
Loading

0 comments on commit 0017ab0

Please sign in to comment.