Skip to content

Commit

Permalink
Resolve system ambiguities and remove some internal stages. Print amb…
Browse files Browse the repository at this point in the history
…iguity conflicts
  • Loading branch information
cart committed Mar 10, 2021
1 parent e9a501e commit 330ad00
Show file tree
Hide file tree
Showing 21 changed files with 176 additions and 133 deletions.
30 changes: 14 additions & 16 deletions crates/bevy_app/src/app_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,29 +211,27 @@ impl AppBuilder {
}

pub fn add_default_stages(&mut self) -> &mut Self {
self.add_stage(
CoreStage::Startup,
Schedule::default()
.with_run_criteria(RunOnce::default())
.with_stage(StartupStage::PreStartup, SystemStage::parallel())
.with_stage(StartupStage::Startup, SystemStage::parallel())
.with_stage(StartupStage::PostStartup, SystemStage::parallel()),
)
.add_stage(CoreStage::First, SystemStage::parallel())
.add_stage(CoreStage::PreEvent, SystemStage::parallel())
.add_stage(CoreStage::Event, SystemStage::parallel())
.add_stage(CoreStage::PreUpdate, SystemStage::parallel())
.add_stage(CoreStage::Update, SystemStage::parallel())
.add_stage(CoreStage::PostUpdate, SystemStage::parallel())
.add_stage(CoreStage::Last, SystemStage::parallel())
self.add_stage(CoreStage::First, SystemStage::parallel())
.add_stage(
CoreStage::Startup,
Schedule::default()
.with_run_criteria(RunOnce::default())
.with_stage(StartupStage::PreStartup, SystemStage::parallel())
.with_stage(StartupStage::Startup, SystemStage::parallel())
.with_stage(StartupStage::PostStartup, SystemStage::parallel()),
)
.add_stage(CoreStage::PreUpdate, SystemStage::parallel())
.add_stage(CoreStage::Update, SystemStage::parallel())
.add_stage(CoreStage::PostUpdate, SystemStage::parallel())
.add_stage(CoreStage::Last, SystemStage::parallel())
}

pub fn add_event<T>(&mut self) -> &mut Self
where
T: Component,
{
self.insert_resource(Events::<T>::default())
.add_system_to_stage(CoreStage::Event, Events::<T>::update_system.system())
.add_system_to_stage(CoreStage::First, Events::<T>::update_system.system())
}

/// Inserts a resource to the current [App] and overwrites any resource previously added of the same type.
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_app/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ use bevy_ecs::{
system::{Local, Res, ResMut, SystemParam},
};
use bevy_utils::tracing::trace;
use std::{fmt, marker::PhantomData};
use std::{
fmt::{self},
hash::Hash,
marker::PhantomData,
};

/// An `EventId` uniquely identifies an event.
///
Expand Down
4 changes: 0 additions & 4 deletions crates/bevy_app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ pub enum CoreStage {
Startup,
/// Name of app stage that runs before all other app stages
First,
/// Name of app stage that runs before EVENT
PreEvent,
/// Name of app stage that updates events. Runs before UPDATE
Event,
/// Name of app stage responsible for performing setup before an update. Runs before UPDATE.
PreUpdate,
/// Name of app stage responsible for doing most app logic. Systems should be registered here by default.
Expand Down
6 changes: 4 additions & 2 deletions crates/bevy_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub mod prelude {
}

use bevy_app::prelude::*;
use bevy_ecs::{entity::Entity, system::IntoSystem};
use bevy_ecs::{entity::Entity, prelude::IntoExclusiveSystem, system::IntoSystem};
use bevy_utils::HashSet;
use std::ops::Range;

Expand All @@ -44,7 +44,9 @@ impl Plugin for CorePlugin {
.register_type::<Labels>()
.register_type::<Range<f32>>()
.register_type::<Timer>()
.add_system_to_stage(CoreStage::First, time_system.system())
// time system is added as an "exclusive system" to ensure it runs before other systems in CoreStage::First
// this also ensures that it runs before other exclusive systems added after CorePlugin
.add_system_to_stage(CoreStage::First, time_system.exclusive_system())
.add_startup_system_to_stage(StartupStage::PostStartup, entity_labels_system.system())
.add_system_to_stage(CoreStage::PostUpdate, entity_labels_system.system());

Expand Down
45 changes: 34 additions & 11 deletions crates/bevy_ecs/src/schedule/stage.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{
component::ComponentId,
schedule::{
BoxedSystemLabel, ExclusiveSystemContainer, InsertionPoint, ParallelExecutor,
ParallelSystemContainer, ParallelSystemExecutor, RunCriteria, ShouldRun,
Expand Down Expand Up @@ -263,22 +264,30 @@ impl SystemStage {
}

/// Logs execution order ambiguities between systems. System orders must be fresh.
fn report_ambiguities(&self) {
fn report_ambiguities(&self, world: &World) {
debug_assert!(!self.systems_modified);
use std::fmt::Write;
fn write_display_names_of_pairs(
string: &mut String,
systems: &[impl SystemContainer],
mut ambiguities: Vec<(usize, usize)>,
mut ambiguities: Vec<(usize, usize, Vec<ComponentId>)>,
world: &World,
) {
for (index_a, index_b) in ambiguities.drain(..) {
for (index_a, index_b, conflicts) in ambiguities.drain(..) {
writeln!(
string,
" -- {:?} and {:?}",
systems[index_a].name(),
systems[index_b].name()
)
.unwrap();
if !conflicts.is_empty() {
let names = conflicts
.iter()
.map(|id| world.components().get_info(*id).unwrap().name())
.collect::<Vec<_>>();
writeln!(string, " conflicts: {:?}", names).unwrap();
}
}
}
let parallel = find_ambiguities(&self.parallel);
Expand All @@ -295,23 +304,29 @@ impl SystemStage {
.to_owned();
if !parallel.is_empty() {
writeln!(string, " * Parallel systems:").unwrap();
write_display_names_of_pairs(&mut string, &self.parallel, parallel);
write_display_names_of_pairs(&mut string, &self.parallel, parallel, world);
}
if !at_start.is_empty() {
writeln!(string, " * Exclusive systems at start of stage:").unwrap();
write_display_names_of_pairs(&mut string, &self.exclusive_at_start, at_start);
write_display_names_of_pairs(
&mut string,
&self.exclusive_at_start,
at_start,
world,
);
}
if !before_commands.is_empty() {
writeln!(string, " * Exclusive systems before commands of stage:").unwrap();
write_display_names_of_pairs(
&mut string,
&self.exclusive_before_commands,
before_commands,
world,
);
}
if !at_end.is_empty() {
writeln!(string, " * Exclusive systems at end of stage:").unwrap();
write_display_names_of_pairs(&mut string, &self.exclusive_at_end, at_end);
write_display_names_of_pairs(&mut string, &self.exclusive_at_end, at_end, world);
}
info!("{}", string);
}
Expand Down Expand Up @@ -456,7 +471,7 @@ fn topological_order(

/// Returns vector containing all pairs of indices of systems with ambiguous execution order.
/// Systems must be topologically sorted beforehand.
fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize)> {
fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec<ComponentId>)> {
let mut ambiguity_set_labels = HashMap::default();
for set in systems.iter().flat_map(|c| c.ambiguity_sets()) {
let len = ambiguity_set_labels.len();
Expand Down Expand Up @@ -511,9 +526,17 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize)> {
{
if !processed.contains(index_b)
&& all_ambiguity_sets[index_a].is_disjoint(&all_ambiguity_sets[index_b])
&& !systems[index_a].is_compatible(&systems[index_b])
{
ambiguities.push((index_a, 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);
if !conflicts.is_empty() {
ambiguities.push((index_a, index_b, conflicts))
}
} else {
ambiguities.push((index_a, index_b, Vec::new()));
}
}
}
processed.insert(index_a);
Expand Down Expand Up @@ -549,7 +572,7 @@ impl Stage for SystemStage {
self.executor.rebuild_cached_data(&self.parallel);
self.executor_modified = false;
if world.contains_resource::<ReportExecutionOrderAmbiguities>() {
self.report_ambiguities();
self.report_ambiguities(world);
}
} else if self.executor_modified {
self.executor.rebuild_cached_data(&self.parallel);
Expand Down Expand Up @@ -1184,7 +1207,7 @@ mod tests {
) -> Vec<(BoxedSystemLabel, BoxedSystemLabel)> {
find_ambiguities(systems)
.drain(..)
.map(|(index_a, index_b)| {
.map(|(index_a, index_b, _conflicts)| {
(
systems[index_a].labels()[0].clone(),
systems[index_b].labels()[0].clone(),
Expand Down
14 changes: 7 additions & 7 deletions crates/bevy_ecs/src/schedule/system_container.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::{
component::ComponentId,
query::Access,
schedule::{
BoxedAmbiguitySetLabel, BoxedSystemLabel, ExclusiveSystemDescriptor,
ParallelSystemDescriptor,
Expand All @@ -16,7 +18,7 @@ pub(super) trait SystemContainer {
fn before(&self) -> &[BoxedSystemLabel];
fn after(&self) -> &[BoxedSystemLabel];
fn ambiguity_sets(&self) -> &[BoxedAmbiguitySetLabel];
fn is_compatible(&self, other: &Self) -> bool;
fn component_access(&self) -> Option<&Access<ComponentId>>;
}

pub(super) struct ExclusiveSystemContainer {
Expand Down Expand Up @@ -81,8 +83,8 @@ impl SystemContainer for ExclusiveSystemContainer {
&self.ambiguity_sets
}

fn is_compatible(&self, _: &Self) -> bool {
false
fn component_access(&self) -> Option<&Access<ComponentId>> {
None
}
}

Expand Down Expand Up @@ -178,9 +180,7 @@ impl SystemContainer for ParallelSystemContainer {
&self.ambiguity_sets
}

fn is_compatible(&self, other: &Self) -> bool {
self.system()
.component_access()
.is_compatible(other.system().component_access())
fn component_access(&self) -> Option<&Access<ComponentId>> {
Some(self.system().component_access())
}
}
2 changes: 2 additions & 0 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ impl<'a, T: Component> SystemParam for Option<Res<'a, T>> {

unsafe impl<T: Component> SystemParamState for OptionResState<T> {
type Config = ();

fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self {
Self(ResState::init(world, system_state, ()))
}
Expand Down Expand Up @@ -383,6 +384,7 @@ impl<'a, T: Component> SystemParam for Option<ResMut<'a, T>> {

unsafe impl<T: Component> SystemParamState for OptionResMutState<T> {
type Config = ();

fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self {
Self(ResMutState::init(world, system_state, ()))
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_gilrs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl Plugin for GilrsPlugin {
gilrs_event_startup_system.exclusive_system(),
)
.add_system_to_stage(
CoreStage::PreEvent,
CoreStage::PreUpdate,
gilrs_event_system.exclusive_system(),
);
}
Expand Down
38 changes: 30 additions & 8 deletions crates/bevy_input/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ pub mod system;
pub mod touch;

pub use axis::*;
use bevy_ecs::system::IntoSystem;
use bevy_ecs::{
schedule::{ParallelSystemDescriptorCoercion, SystemLabel},
system::IntoSystem,
};
pub use input::*;

pub mod prelude {
Expand Down Expand Up @@ -37,27 +40,46 @@ use gamepad::{
#[derive(Default)]
pub struct InputPlugin;

#[derive(Debug, PartialEq, Eq, Clone, Hash, SystemLabel)]
pub struct InputSystem;

impl Plugin for InputPlugin {
fn build(&self, app: &mut AppBuilder) {
app.add_event::<KeyboardInput>()
app
// keyboard
.add_event::<KeyboardInput>()
.init_resource::<Input<KeyCode>>()
.add_system_to_stage(
CoreStage::PreUpdate,
keyboard_input_system.system().label(InputSystem),
)
// mouse
.add_event::<MouseButtonInput>()
.add_event::<MouseMotion>()
.add_event::<MouseWheel>()
.init_resource::<Input<KeyCode>>()
.add_system_to_stage(CoreStage::Event, keyboard_input_system.system())
.init_resource::<Input<MouseButton>>()
.add_system_to_stage(CoreStage::Event, mouse_button_input_system.system())
.add_system_to_stage(
CoreStage::PreUpdate,
mouse_button_input_system.system().label(InputSystem),
)
// gamepad
.add_event::<GamepadEvent>()
.add_event::<GamepadEventRaw>()
.init_resource::<GamepadSettings>()
.init_resource::<Input<GamepadButton>>()
.init_resource::<Axis<GamepadAxis>>()
.init_resource::<Axis<GamepadButton>>()
.add_system_to_stage(CoreStage::Event, gamepad_event_system.system())
.add_startup_system_to_stage(StartupStage::Startup, gamepad_event_system.system())
.add_system_to_stage(
CoreStage::PreUpdate,
gamepad_event_system.system().label(InputSystem),
)
// touch
.add_event::<TouchInput>()
.init_resource::<Touches>()
.add_system_to_stage(CoreStage::Event, touch_screen_input_system.system());
.add_system_to_stage(
CoreStage::PreUpdate,
touch_screen_input_system.system().label(InputSystem),
);
}
}

Expand Down
24 changes: 18 additions & 6 deletions crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ pub mod texture;
pub mod wireframe;

use bevy_ecs::{
schedule::SystemStage,
schedule::{ParallelSystemDescriptorCoercion, SystemStage},
system::{IntoExclusiveSystem, IntoSystem},
};
use bevy_transform::TransformSystem;
use draw::Visible;
pub use once_cell;

Expand All @@ -37,7 +38,7 @@ use crate::prelude::*;
use base::Msaa;
use bevy_app::prelude::*;
use bevy_asset::{AddAsset, AssetStage};
use bevy_ecs::schedule::StageLabel;
use bevy_ecs::schedule::{StageLabel, SystemLabel};
use camera::{
ActiveCameras, Camera, DepthCalculation, OrthographicProjection, PerspectiveProjection,
RenderLayers, ScalingMode, VisibleEntities, WindowOrigin,
Expand All @@ -57,6 +58,11 @@ use texture::HdrTextureLoader;
#[cfg(feature = "png")]
use texture::ImageTextureLoader;

#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemLabel)]
pub enum RenderSystem {
VisibleEntities,
}

/// The names of "render" App stages
#[derive(Debug, Hash, PartialEq, Eq, Clone, StageLabel)]
pub enum RenderStage {
Expand Down Expand Up @@ -157,16 +163,22 @@ impl Plugin for RenderPlugin {
)
.add_system_to_stage(
CoreStage::PostUpdate,
camera::camera_system::<OrthographicProjection>.system(),
camera::camera_system::<OrthographicProjection>
.system()
.before(RenderSystem::VisibleEntities),
)
.add_system_to_stage(
CoreStage::PostUpdate,
camera::camera_system::<PerspectiveProjection>.system(),
camera::camera_system::<PerspectiveProjection>
.system()
.before(RenderSystem::VisibleEntities),
)
// registration order matters here. this must come after all camera_system::<T> systems
.add_system_to_stage(
CoreStage::PostUpdate,
camera::visible_entities_system.system(),
camera::visible_entities_system
.system()
.label(RenderSystem::VisibleEntities)
.after(TransformSystem::TransformPropagate),
)
.add_system_to_stage(
RenderStage::RenderResource,
Expand Down
Loading

0 comments on commit 330ad00

Please sign in to comment.