Skip to content

Commit ab242e5

Browse files
cartcart
committed
Resolve system ambiguities and remove some internal stages. Print ambiguity conflicts
1 parent e9a501e commit ab242e5

File tree

21 files changed

+173
-130
lines changed

21 files changed

+173
-130
lines changed

‎crates/bevy_app/src/app_builder.rs‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -211,29 +211,27 @@ impl AppBuilder {
211211
}
212212

213213
pub fn add_default_stages(&mut self) -> &mut Self {
214-
self.add_stage(
215-
CoreStage::Startup,
216-
Schedule::default()
217-
.with_run_criteria(RunOnce::default())
218-
.with_stage(StartupStage::PreStartup, SystemStage::parallel())
219-
.with_stage(StartupStage::Startup, SystemStage::parallel())
220-
.with_stage(StartupStage::PostStartup, SystemStage::parallel()),
221-
)
222-
.add_stage(CoreStage::First, SystemStage::parallel())
223-
.add_stage(CoreStage::PreEvent, SystemStage::parallel())
224-
.add_stage(CoreStage::Event, SystemStage::parallel())
225-
.add_stage(CoreStage::PreUpdate, SystemStage::parallel())
226-
.add_stage(CoreStage::Update, SystemStage::parallel())
227-
.add_stage(CoreStage::PostUpdate, SystemStage::parallel())
228-
.add_stage(CoreStage::Last, SystemStage::parallel())
214+
self.add_stage(CoreStage::First, SystemStage::parallel())
215+
.add_stage(
216+
CoreStage::Startup,
217+
Schedule::default()
218+
.with_run_criteria(RunOnce::default())
219+
.with_stage(StartupStage::PreStartup, SystemStage::parallel())
220+
.with_stage(StartupStage::Startup, SystemStage::parallel())
221+
.with_stage(StartupStage::PostStartup, SystemStage::parallel()),
222+
)
223+
.add_stage(CoreStage::PreUpdate, SystemStage::parallel())
224+
.add_stage(CoreStage::Update, SystemStage::parallel())
225+
.add_stage(CoreStage::PostUpdate, SystemStage::parallel())
226+
.add_stage(CoreStage::Last, SystemStage::parallel())
229227
}
230228

231229
pub fn add_event<T>(&mut self) -> &mut Self
232230
where
233231
T: Component,
234232
{
235233
self.insert_resource(Events::<T>::default())
236-
.add_system_to_stage(CoreStage::Event, Events::<T>::update_system.system())
234+
.add_system_to_stage(CoreStage::First, Events::<T>::update_system.system())
237235
}
238236

239237
/// Inserts a resource to the current [App] and overwrites any resource previously added of the same type.

‎crates/bevy_app/src/event.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ use bevy_ecs::{
33
system::{Local, Res, ResMut, SystemParam},
44
};
55
use bevy_utils::tracing::trace;
6-
use std::{fmt, marker::PhantomData};
6+
use std::{
7+
fmt::{self},
8+
hash::Hash,
9+
marker::PhantomData,
10+
};
711

812
/// An `EventId` uniquely identifies an event.
913
///

‎crates/bevy_app/src/lib.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@ pub enum CoreStage {
3131
Startup,
3232
/// Name of app stage that runs before all other app stages
3333
First,
34-
/// Name of app stage that runs before EVENT
35-
PreEvent,
36-
/// Name of app stage that updates events. Runs before UPDATE
37-
Event,
3834
/// Name of app stage responsible for performing setup before an update. Runs before UPDATE.
3935
PreUpdate,
4036
/// Name of app stage responsible for doing most app logic. Systems should be registered here by default.

‎crates/bevy_core/src/lib.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub mod prelude {
1717
}
1818

1919
use bevy_app::prelude::*;
20-
use bevy_ecs::{entity::Entity, system::IntoSystem};
20+
use bevy_ecs::{entity::Entity, prelude::IntoExclusiveSystem, system::IntoSystem};
2121
use bevy_utils::HashSet;
2222
use std::ops::Range;
2323

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

‎crates/bevy_ecs/src/schedule/stage.rs‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::{
2+
component::ComponentId,
23
schedule::{
34
BoxedSystemLabel, ExclusiveSystemContainer, InsertionPoint, ParallelExecutor,
45
ParallelSystemContainer, ParallelSystemExecutor, RunCriteria, ShouldRun,
@@ -263,22 +264,30 @@ impl SystemStage {
263264
}
264265

265266
/// Logs execution order ambiguities between systems. System orders must be fresh.
266-
fn report_ambiguities(&self) {
267+
fn report_ambiguities(&self, world: &World) {
267268
debug_assert!(!self.systems_modified);
268269
use std::fmt::Write;
269270
fn write_display_names_of_pairs(
270271
string: &mut String,
271272
systems: &[impl SystemContainer],
272-
mut ambiguities: Vec<(usize, usize)>,
273+
mut ambiguities: Vec<(usize, usize, Vec<ComponentId>)>,
274+
world: &World,
273275
) {
274-
for (index_a, index_b) in ambiguities.drain(..) {
276+
for (index_a, index_b, conflicts) in ambiguities.drain(..) {
275277
writeln!(
276278
string,
277279
" -- {:?} and {:?}",
278280
systems[index_a].name(),
279281
systems[index_b].name()
280282
)
281283
.unwrap();
284+
if !conflicts.is_empty() {
285+
let names = conflicts
286+
.iter()
287+
.map(|id| world.components().get_info(*id).unwrap().name())
288+
.collect::<Vec<_>>();
289+
writeln!(string, " conflicts: {:?}", names).unwrap();
290+
}
282291
}
283292
}
284293
let parallel = find_ambiguities(&self.parallel);
@@ -295,23 +304,29 @@ impl SystemStage {
295304
.to_owned();
296305
if !parallel.is_empty() {
297306
writeln!(string, " * Parallel systems:").unwrap();
298-
write_display_names_of_pairs(&mut string, &self.parallel, parallel);
307+
write_display_names_of_pairs(&mut string, &self.parallel, parallel, world);
299308
}
300309
if !at_start.is_empty() {
301310
writeln!(string, " * Exclusive systems at start of stage:").unwrap();
302-
write_display_names_of_pairs(&mut string, &self.exclusive_at_start, at_start);
311+
write_display_names_of_pairs(
312+
&mut string,
313+
&self.exclusive_at_start,
314+
at_start,
315+
world,
316+
);
303317
}
304318
if !before_commands.is_empty() {
305319
writeln!(string, " * Exclusive systems before commands of stage:").unwrap();
306320
write_display_names_of_pairs(
307321
&mut string,
308322
&self.exclusive_before_commands,
309323
before_commands,
324+
world,
310325
);
311326
}
312327
if !at_end.is_empty() {
313328
writeln!(string, " * Exclusive systems at end of stage:").unwrap();
314-
write_display_names_of_pairs(&mut string, &self.exclusive_at_end, at_end);
329+
write_display_names_of_pairs(&mut string, &self.exclusive_at_end, at_end, world);
315330
}
316331
info!("{}", string);
317332
}
@@ -456,7 +471,7 @@ fn topological_order(
456471

457472
/// Returns vector containing all pairs of indices of systems with ambiguous execution order.
458473
/// Systems must be topologically sorted beforehand.
459-
fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize)> {
474+
fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec<ComponentId>)> {
460475
let mut ambiguity_set_labels = HashMap::default();
461476
for set in systems.iter().flat_map(|c| c.ambiguity_sets()) {
462477
let len = ambiguity_set_labels.len();
@@ -511,9 +526,17 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize)> {
511526
{
512527
if !processed.contains(index_b)
513528
&& all_ambiguity_sets[index_a].is_disjoint(&all_ambiguity_sets[index_b])
514-
&& !systems[index_a].is_compatible(&systems[index_b])
515529
{
516-
ambiguities.push((index_a, index_b));
530+
let a_access = systems[index_a].component_access();
531+
let b_access = systems[index_b].component_access();
532+
if let (Some(a), Some(b)) = (a_access, b_access) {
533+
let conflicts = a.get_conflicts(b);
534+
if !conflicts.is_empty() {
535+
ambiguities.push((index_a, index_b, conflicts))
536+
}
537+
} else {
538+
ambiguities.push((index_a, index_b, Vec::new()));
539+
}
517540
}
518541
}
519542
processed.insert(index_a);
@@ -549,7 +572,7 @@ impl Stage for SystemStage {
549572
self.executor.rebuild_cached_data(&self.parallel);
550573
self.executor_modified = false;
551574
if world.contains_resource::<ReportExecutionOrderAmbiguities>() {
552-
self.report_ambiguities();
575+
self.report_ambiguities(world);
553576
}
554577
} else if self.executor_modified {
555578
self.executor.rebuild_cached_data(&self.parallel);
@@ -1184,7 +1207,7 @@ mod tests {
11841207
) -> Vec<(BoxedSystemLabel, BoxedSystemLabel)> {
11851208
find_ambiguities(systems)
11861209
.drain(..)
1187-
.map(|(index_a, index_b)| {
1210+
.map(|(index_a, index_b, _conflicts)| {
11881211
(
11891212
systems[index_a].labels()[0].clone(),
11901213
systems[index_b].labels()[0].clone(),

‎crates/bevy_ecs/src/schedule/system_container.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
use crate::{
2+
component::ComponentId,
3+
query::Access,
24
schedule::{
35
BoxedAmbiguitySetLabel, BoxedSystemLabel, ExclusiveSystemDescriptor,
46
ParallelSystemDescriptor,
@@ -16,7 +18,7 @@ pub(super) trait SystemContainer {
1618
fn before(&self) -> &[BoxedSystemLabel];
1719
fn after(&self) -> &[BoxedSystemLabel];
1820
fn ambiguity_sets(&self) -> &[BoxedAmbiguitySetLabel];
19-
fn is_compatible(&self, other: &Self) -> bool;
21+
fn component_access(&self) -> Option<&Access<ComponentId>>;
2022
}
2123

2224
pub(super) struct ExclusiveSystemContainer {
@@ -81,8 +83,8 @@ impl SystemContainer for ExclusiveSystemContainer {
8183
&self.ambiguity_sets
8284
}
8385

84-
fn is_compatible(&self, _: &Self) -> bool {
85-
false
86+
fn component_access(&self) -> Option<&Access<ComponentId>> {
87+
None
8688
}
8789
}
8890

@@ -178,9 +180,7 @@ impl SystemContainer for ParallelSystemContainer {
178180
&self.ambiguity_sets
179181
}
180182

181-
fn is_compatible(&self, other: &Self) -> bool {
182-
self.system()
183-
.component_access()
184-
.is_compatible(other.system().component_access())
183+
fn component_access(&self) -> Option<&Access<ComponentId>> {
184+
Some(self.system().component_access())
185185
}
186186
}

‎crates/bevy_ecs/src/system/system_param.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ impl<'a, T: Component> SystemParam for Option<Res<'a, T>> {
248248

249249
unsafe impl<T: Component> SystemParamState for OptionResState<T> {
250250
type Config = ();
251+
251252
fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self {
252253
Self(ResState::init(world, system_state, ()))
253254
}
@@ -383,6 +384,7 @@ impl<'a, T: Component> SystemParam for Option<ResMut<'a, T>> {
383384

384385
unsafe impl<T: Component> SystemParamState for OptionResMutState<T> {
385386
type Config = ();
387+
386388
fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self {
387389
Self(ResMutState::init(world, system_state, ()))
388390
}

‎crates/bevy_gilrs/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ impl Plugin for GilrsPlugin {
2424
gilrs_event_startup_system.exclusive_system(),
2525
)
2626
.add_system_to_stage(
27-
CoreStage::PreEvent,
27+
CoreStage::PreUpdate,
2828
gilrs_event_system.exclusive_system(),
2929
);
3030
}

‎crates/bevy_input/src/lib.rs‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ pub mod system;
77
pub mod touch;
88

99
pub use axis::*;
10-
use bevy_ecs::system::IntoSystem;
10+
use bevy_ecs::{
11+
schedule::{ParallelSystemDescriptorCoercion, SystemLabel},
12+
system::IntoSystem,
13+
};
1114
pub use input::*;
1215

1316
pub mod prelude {
@@ -37,27 +40,46 @@ use gamepad::{
3740
#[derive(Default)]
3841
pub struct InputPlugin;
3942

43+
#[derive(Debug, PartialEq, Eq, Clone, Hash, SystemLabel)]
44+
pub struct InputSystem;
45+
4046
impl Plugin for InputPlugin {
4147
fn build(&self, app: &mut AppBuilder) {
42-
app.add_event::<KeyboardInput>()
48+
app
49+
// keyboard
50+
.add_event::<KeyboardInput>()
51+
.init_resource::<Input<KeyCode>>()
52+
.add_system_to_stage(
53+
CoreStage::PreUpdate,
54+
keyboard_input_system.system().label(InputSystem),
55+
)
56+
// mouse
4357
.add_event::<MouseButtonInput>()
4458
.add_event::<MouseMotion>()
4559
.add_event::<MouseWheel>()
46-
.init_resource::<Input<KeyCode>>()
47-
.add_system_to_stage(CoreStage::Event, keyboard_input_system.system())
4860
.init_resource::<Input<MouseButton>>()
49-
.add_system_to_stage(CoreStage::Event, mouse_button_input_system.system())
61+
.add_system_to_stage(
62+
CoreStage::PreUpdate,
63+
mouse_button_input_system.system().label(InputSystem),
64+
)
65+
// gamepad
5066
.add_event::<GamepadEvent>()
5167
.add_event::<GamepadEventRaw>()
5268
.init_resource::<GamepadSettings>()
5369
.init_resource::<Input<GamepadButton>>()
5470
.init_resource::<Axis<GamepadAxis>>()
5571
.init_resource::<Axis<GamepadButton>>()
56-
.add_system_to_stage(CoreStage::Event, gamepad_event_system.system())
57-
.add_startup_system_to_stage(StartupStage::Startup, gamepad_event_system.system())
72+
.add_system_to_stage(
73+
CoreStage::PreUpdate,
74+
gamepad_event_system.system().label(InputSystem),
75+
)
76+
// touch
5877
.add_event::<TouchInput>()
5978
.init_resource::<Touches>()
60-
.add_system_to_stage(CoreStage::Event, touch_screen_input_system.system());
79+
.add_system_to_stage(
80+
CoreStage::PreUpdate,
81+
touch_screen_input_system.system().label(InputSystem),
82+
);
6183
}
6284
}
6385

‎crates/bevy_render/src/lib.rs‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ pub mod texture;
1313
pub mod wireframe;
1414

1515
use bevy_ecs::{
16-
schedule::SystemStage,
16+
schedule::{ParallelSystemDescriptorCoercion, SystemStage},
1717
system::{IntoExclusiveSystem, IntoSystem},
1818
};
19+
use bevy_transform::TransformSystem;
1920
use draw::Visible;
2021
pub use once_cell;
2122

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

61+
#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemLabel)]
62+
pub enum RenderSystem {
63+
VisibleEntities,
64+
}
65+
6066
/// The names of "render" App stages
6167
#[derive(Debug, Hash, PartialEq, Eq, Clone, StageLabel)]
6268
pub enum RenderStage {
@@ -157,16 +163,22 @@ impl Plugin for RenderPlugin {
157163
)
158164
.add_system_to_stage(
159165
CoreStage::PostUpdate,
160-
camera::camera_system::<OrthographicProjection>.system(),
166+
camera::camera_system::<OrthographicProjection>
167+
.system()
168+
.before(RenderSystem::VisibleEntities),
161169
)
162170
.add_system_to_stage(
163171
CoreStage::PostUpdate,
164-
camera::camera_system::<PerspectiveProjection>.system(),
172+
camera::camera_system::<PerspectiveProjection>
173+
.system()
174+
.before(RenderSystem::VisibleEntities),
165175
)
166-
// registration order matters here. this must come after all camera_system::<T> systems
167176
.add_system_to_stage(
168177
CoreStage::PostUpdate,
169-
camera::visible_entities_system.system(),
178+
camera::visible_entities_system
179+
.system()
180+
.label(RenderSystem::VisibleEntities)
181+
.after(TransformSystem::TransformPropagate),
170182
)
171183
.add_system_to_stage(
172184
RenderStage::RenderResource,

0 commit comments

Comments
 (0)