Skip to content

Commit

Permalink
Exclusive Systems Now Implement System. Flexible Exclusive System P…
Browse files Browse the repository at this point in the history
…arams (bevyengine#6083)

# Objective

The [Stageless RFC](bevyengine/rfcs#45) involves allowing exclusive systems to be referenced and ordered relative to parallel systems. We've agreed that unifying systems under `System` is the right move.

This is an alternative to bevyengine#4166 (see rationale in the comments I left there). Note that this builds on the learnings established there (and borrows some patterns).

## Solution

This unifies parallel and exclusive systems under the shared `System` trait, removing the old `ExclusiveSystem` trait / impls. This is accomplished by adding a new `ExclusiveFunctionSystem` impl similar to `FunctionSystem`. It is backed by `ExclusiveSystemParam`, which is similar to `SystemParam`. There is a new flattened out SystemContainer api (which cuts out a lot of trait and type complexity). 

This means you can remove all cases of `exclusive_system()`:

```rust
// before
commands.add_system(some_system.exclusive_system());
// after
commands.add_system(some_system);
```

I've also implemented `ExclusiveSystemParam` for `&mut QueryState` and `&mut SystemState`, which makes this possible in exclusive systems:

```rust
fn some_exclusive_system(
    world: &mut World,
    transforms: &mut QueryState<&Transform>,
    state: &mut SystemState<(Res<Time>, Query<&Player>)>,
) {
    for transform in transforms.iter(world) {
        println!("{transform:?}");
    }
    let (time, players) = state.get(world);
    for player in players.iter() {
        println!("{player:?}");
    }
}
```

Note that "exclusive function systems" assume `&mut World` is present (and the first param). I think this is a fair assumption, given that the presence of `&mut World` is what defines the need for an exclusive system.

I added some targeted SystemParam `static` constraints, which removed the need for this:
``` rust
fn some_exclusive_system(state: &mut SystemState<(Res<'static, Time>, Query<&'static Player>)>) {}
```

## Related

- bevyengine#2923
- bevyengine#3001
- bevyengine#3946

## Changelog

- `ExclusiveSystem` trait (and implementations) has been removed in favor of sharing the `System` trait.
- `ExclusiveFunctionSystem` and `ExclusiveSystemParam` were added, enabling flexible exclusive function systems
- `&mut SystemState` and `&mut QueryState` now implement `ExclusiveSystemParam`
- Exclusive and parallel System configuration is now done via a unified `SystemDescriptor`, `IntoSystemDescriptor`, and `SystemContainer` api.

## Migration Guide

Calling `.exclusive_system()` is no longer required (or supported) for converting exclusive system functions to exclusive systems:

```rust
// Old (0.8)
app.add_system(some_exclusive_system.exclusive_system());
// New (0.9)
app.add_system(some_exclusive_system);
```

Converting "normal" parallel systems to exclusive systems is done by calling the exclusive ordering apis:

```rust
// Old (0.8)
app.add_system(some_system.exclusive_system().at_end());
// New (0.9)
app.add_system(some_system.at_end());
```

Query state in exclusive systems can now be cached via ExclusiveSystemParams, which should be preferred for clarity and performance reasons:
```rust
// Old (0.8)
fn some_system(world: &mut World) {
  let mut transforms = world.query::<&Transform>();
  for transform in transforms.iter(world) {
  }
}
// New (0.9)
fn some_system(world: &mut World, transforms: &mut QueryState<&Transform>) {
  for transform in transforms.iter(world) {
  }
}
```
  • Loading branch information
cart authored and ItsDoot committed Feb 1, 2023
1 parent c406425 commit 6d3eaab
Show file tree
Hide file tree
Showing 37 changed files with 735 additions and 835 deletions.
8 changes: 1 addition & 7 deletions benches/benches/bevy_ecs/scheduling/run_criteria.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
use bevy_ecs::{
component::Component,
prelude::{ParallelSystemDescriptorCoercion, Res, Resource, RunCriteriaDescriptorCoercion},
schedule::{RunCriteriaLabel, ShouldRun, Stage, SystemStage},
system::Query,
world::World,
};
use bevy_ecs::{prelude::*, schedule::ShouldRun};
use criterion::Criterion;

fn run_stage(stage: &mut SystemStage, world: &mut World) {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_animation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use bevy_ecs::{
entity::Entity,
prelude::Component,
reflect::ReflectComponent,
schedule::ParallelSystemDescriptorCoercion,
schedule::IntoSystemDescriptor,
system::{Query, Res},
};
use bevy_hierarchy::Children;
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pub use bevy_derive::AppLabel;
use bevy_derive::{Deref, DerefMut};
use bevy_ecs::{
event::{Event, Events},
prelude::{FromWorld, IntoExclusiveSystem},
prelude::FromWorld,
schedule::{
IntoSystemDescriptor, Schedule, ShouldRun, Stage, StageLabel, State, StateData, SystemSet,
SystemStage,
Expand Down Expand Up @@ -84,7 +84,7 @@ impl Default for App {

app.add_default_stages()
.add_event::<AppExit>()
.add_system_to_stage(CoreStage::Last, World::clear_trackers.exclusive_system());
.add_system_to_stage(CoreStage::Last, World::clear_trackers);

#[cfg(feature = "bevy_ci_testing")]
{
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ type IdCursor = isize;
/// }
/// #
/// # bevy_ecs::system::assert_is_system(setup);
/// # bevy_ecs::system::IntoExclusiveSystem::exclusive_system(exclusive_system);
/// # bevy_ecs::system::assert_is_system(exclusive_system);
/// ```
///
/// It can be used to refer to a specific entity to apply [`EntityCommands`], or to call [`Query::get`] (or similar methods) to access its components.
Expand Down
11 changes: 5 additions & 6 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,13 @@ pub mod prelude {
event::{EventReader, EventWriter, Events},
query::{Added, AnyOf, ChangeTrackers, Changed, Or, QueryState, With, Without},
schedule::{
ExclusiveSystemDescriptorCoercion, ParallelSystemDescriptorCoercion, RunCriteria,
RunCriteriaDescriptorCoercion, RunCriteriaLabel, Schedule, Stage, StageLabel, State,
SystemLabel, SystemSet, SystemStage,
IntoSystemDescriptor, RunCriteria, RunCriteriaDescriptorCoercion, RunCriteriaLabel,
Schedule, Stage, StageLabel, State, SystemLabel, SystemSet, SystemStage,
},
system::{
adapter as system_adapter, Commands, In, IntoChainSystem, IntoExclusiveSystem,
IntoSystem, Local, NonSend, NonSendMut, ParallelCommands, ParamSet, Query,
RemovedComponents, Res, ResMut, Resource, System, SystemParamFunction,
adapter as system_adapter, Commands, In, IntoChainSystem, IntoSystem, Local, NonSend,
NonSendMut, ParallelCommands, ParamSet, Query, RemovedComponents, Res, ResMut,
Resource, System, SystemParamFunction,
},
world::{FromWorld, Mut, World},
};
Expand Down
27 changes: 14 additions & 13 deletions crates/bevy_ecs/src/schedule/ambiguity_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ impl SystemOrderAmbiguity {
stage: &SystemStage,
world: &World,
) -> Self {
use crate::schedule::graph_utils::GraphNode;
use SystemStageSegment::*;

// TODO: blocked on https://github.com/bevyengine/bevy/pull/4166
Expand Down Expand Up @@ -220,7 +219,7 @@ impl SystemStage {
/// Returns vector containing all pairs of indices of systems with ambiguous execution order,
/// along with specific components that have triggered the warning.
/// Systems must be topologically sorted beforehand.
fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec<ComponentId>)> {
fn find_ambiguities(systems: &[SystemContainer]) -> Vec<(usize, usize, Vec<ComponentId>)> {
let mut all_dependencies = Vec::<FixedBitSet>::with_capacity(systems.len());
let mut all_dependants = Vec::<FixedBitSet>::with_capacity(systems.len());
for (index, container) in systems.iter().enumerate() {
Expand Down Expand Up @@ -263,15 +262,17 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec<
// .take(index_a)
{
if !processed.contains(index_b) {
let a_access = systems[index_a].component_access();
let b_access = systems[index_b].component_access();
if let (Some(a), Some(b)) = (a_access, b_access) {
let conflicts = a.get_conflicts(b);
let system_a = &systems[index_a];
let system_b = &systems[index_b];
if system_a.is_exclusive() || system_b.is_exclusive() {
ambiguities.push((index_a, index_b, Vec::new()));
} else {
let a_access = systems[index_a].component_access();
let b_access = systems[index_b].component_access();
let conflicts = a_access.get_conflicts(b_access);
if !conflicts.is_empty() {
ambiguities.push((index_a, index_b, conflicts));
}
} else {
ambiguities.push((index_a, index_b, Vec::new()));
}
}
}
Expand Down Expand Up @@ -467,12 +468,12 @@ mod tests {
let mut test_stage = SystemStage::parallel();
test_stage
// All 3 of these conflict with each other
.add_system(write_world_system.exclusive_system())
.add_system(write_world_system.exclusive_system().at_end())
.add_system(res_system.exclusive_system())
.add_system(write_world_system)
.add_system(write_world_system.at_end())
.add_system(res_system.at_start())
// These do not, as they're in different segments of the stage
.add_system(write_world_system.exclusive_system().at_start())
.add_system(write_world_system.exclusive_system().before_commands());
.add_system(write_world_system.at_start())
.add_system(write_world_system.before_commands());

test_stage.run(&mut world);

Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_ecs/src/schedule/executor.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use crate::{schedule::ParallelSystemContainer, world::World};
use crate::{schedule::SystemContainer, world::World};
use downcast_rs::{impl_downcast, Downcast};

pub trait ParallelSystemExecutor: Downcast + Send + Sync {
/// Called by `SystemStage` whenever `systems` have been changed.
fn rebuild_cached_data(&mut self, systems: &[ParallelSystemContainer]);
fn rebuild_cached_data(&mut self, systems: &[SystemContainer]);

fn run_systems(&mut self, systems: &mut [ParallelSystemContainer], world: &mut World);
fn run_systems(&mut self, systems: &mut [SystemContainer], world: &mut World);
}

impl_downcast!(ParallelSystemExecutor);
Expand All @@ -14,9 +14,9 @@ impl_downcast!(ParallelSystemExecutor);
pub struct SingleThreadedExecutor;

impl ParallelSystemExecutor for SingleThreadedExecutor {
fn rebuild_cached_data(&mut self, _: &[ParallelSystemContainer]) {}
fn rebuild_cached_data(&mut self, _: &[SystemContainer]) {}

fn run_systems(&mut self, systems: &mut [ParallelSystemContainer], world: &mut World) {
fn run_systems(&mut self, systems: &mut [SystemContainer], world: &mut World) {
for system in systems {
if system.should_run() {
#[cfg(feature = "trace")]
Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_ecs/src/schedule/executor_parallel.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
archetype::ArchetypeComponentId,
query::Access,
schedule::{ParallelSystemContainer, ParallelSystemExecutor},
schedule::{ParallelSystemExecutor, SystemContainer},
world::World,
};
use async_channel::{Receiver, Sender};
Expand Down Expand Up @@ -77,7 +77,7 @@ impl Default for ParallelExecutor {
}

impl ParallelSystemExecutor for ParallelExecutor {
fn rebuild_cached_data(&mut self, systems: &[ParallelSystemContainer]) {
fn rebuild_cached_data(&mut self, systems: &[SystemContainer]) {
self.system_metadata.clear();
self.queued.grow(systems.len());
self.running.grow(systems.len());
Expand All @@ -104,7 +104,7 @@ impl ParallelSystemExecutor for ParallelExecutor {
}
}

fn run_systems(&mut self, systems: &mut [ParallelSystemContainer], world: &mut World) {
fn run_systems(&mut self, systems: &mut [SystemContainer], world: &mut World) {
#[cfg(test)]
if self.events_sender.is_none() {
let (sender, receiver) = async_channel::unbounded::<SchedulingEvent>();
Expand Down Expand Up @@ -167,7 +167,7 @@ impl ParallelExecutor {
fn prepare_systems<'scope>(
&mut self,
scope: &mut Scope<'scope, ()>,
systems: &'scope mut [ParallelSystemContainer],
systems: &'scope mut [SystemContainer],
world: &'scope World,
) {
// These are used as a part of a unit test.
Expand Down
Loading

0 comments on commit 6d3eaab

Please sign in to comment.