diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs index e3ba7836f7..83dbded00d 100755 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -1234,7 +1234,7 @@ impl Plugin for AnimationPlugin { ( advance_transitions, advance_animations, - animate_targets, + animate_targets.after(bevy_render::mesh::morph::inherit_weights), expire_completed_transitions, ) .chain() diff --git a/crates/bevy_app/src/terminal_ctrl_c_handler.rs b/crates/bevy_app/src/terminal_ctrl_c_handler.rs index 54e0bc5338..8bd90ffaa6 100644 --- a/crates/bevy_app/src/terminal_ctrl_c_handler.rs +++ b/crates/bevy_app/src/terminal_ctrl_c_handler.rs @@ -48,7 +48,7 @@ impl TerminalCtrlCHandlerPlugin { } /// Sends a [`AppExit`] event when the user presses `Ctrl+C` on the terminal. - fn exit_on_flag(mut events: EventWriter) { + pub fn exit_on_flag(mut events: EventWriter) { if SHOULD_EXIT.load(Ordering::Relaxed) { events.send(AppExit::from_code(130)); } diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 6c790b2b47..d49bae9e63 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -222,7 +222,11 @@ impl Plugin for AssetPlugin { .init_asset::<()>() .add_event::() .configure_sets(PreUpdate, TrackAssets.after(handle_internal_asset_events)) - .add_systems(PreUpdate, handle_internal_asset_events) + // `handle_internal_asset_events` requires the use of `&mut World`, + // and as a result has ambiguous system ordering with all other systems in `PreUpdate`. + // This is virtually never a real problem: asset loading is async and so anything that interacts directly with it + // needs to be robust to stochastic delays anyways. + .add_systems(PreUpdate, handle_internal_asset_events.ambiguous_with_all()) .register_type::(); } } diff --git a/crates/bevy_dev_tools/src/ci_testing/mod.rs b/crates/bevy_dev_tools/src/ci_testing/mod.rs index 59949fa0bf..d6d5c6d4be 100644 --- a/crates/bevy_dev_tools/src/ci_testing/mod.rs +++ b/crates/bevy_dev_tools/src/ci_testing/mod.rs @@ -6,7 +6,7 @@ mod systems; pub use self::config::*; use bevy_app::prelude::*; -use bevy_ecs::schedule::IntoSystemConfigs; +use bevy_ecs::prelude::*; use bevy_render::view::screenshot::trigger_screenshots; use bevy_time::TimeUpdateStrategy; use std::time::Duration; @@ -54,7 +54,19 @@ impl Plugin for CiTestingPlugin { Update, systems::send_events .before(trigger_screenshots) - .before(bevy_window::close_when_requested), + .before(bevy_window::close_when_requested) + .in_set(SendEvents), ); + + // The offending system does not exist in the wasm32 target. + // As a result, we must conditionally order the two systems using a system set. + #[cfg(not(target_arch = "wasm32"))] + app.configure_sets( + Update, + SendEvents.before(bevy_app::TerminalCtrlCHandlerPlugin::exit_on_flag), + ); } } + +#[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)] +struct SendEvents; diff --git a/crates/bevy_gizmos/src/aabb.rs b/crates/bevy_gizmos/src/aabb.rs index 1b62775245..6496dc9125 100644 --- a/crates/bevy_gizmos/src/aabb.rs +++ b/crates/bevy_gizmos/src/aabb.rs @@ -40,6 +40,7 @@ impl Plugin for AabbGizmoPlugin { config.config::().1.draw_all }), ) + .after(bevy_render::view::VisibilitySystems::CalculateBounds) .after(TransformSystem::TransformPropagate), ); } diff --git a/crates/bevy_gizmos/src/lib.rs b/crates/bevy_gizmos/src/lib.rs index c30d0b0931..10f7e9cdc9 100644 --- a/crates/bevy_gizmos/src/lib.rs +++ b/crates/bevy_gizmos/src/lib.rs @@ -242,6 +242,9 @@ impl AppGizmoBuilder for App { handles.list.insert(TypeId::of::(), None); handles.strip.insert(TypeId::of::(), None); + // These handles are safe to mutate in any order + self.allow_ambiguous_resource::(); + self.init_resource::>() .init_resource::>() .init_resource::>>() diff --git a/crates/bevy_internal/src/default_plugins.rs b/crates/bevy_internal/src/default_plugins.rs index a84bbdb573..7aadcb50fb 100644 --- a/crates/bevy_internal/src/default_plugins.rs +++ b/crates/bevy_internal/src/default_plugins.rs @@ -90,6 +90,11 @@ impl Plugin for IgnoreAmbiguitiesPlugin { bevy_animation::advance_animations, bevy_ui::ui_layout_system, ); + app.ignore_ambiguity( + bevy_app::PostUpdate, + bevy_animation::animate_targets, + bevy_ui::ui_layout_system, + ); } } } diff --git a/crates/bevy_pbr/src/lib.rs b/crates/bevy_pbr/src/lib.rs index ea84c49f18..5deaa88590 100644 --- a/crates/bevy_pbr/src/lib.rs +++ b/crates/bevy_pbr/src/lib.rs @@ -340,10 +340,22 @@ impl Plugin for PbrPlugin { ) .chain(), ) + .configure_sets( + PostUpdate, + SimulationLightSystems::UpdateDirectionalLightCascades + .ambiguous_with(SimulationLightSystems::UpdateDirectionalLightCascades), + ) + .configure_sets( + PostUpdate, + SimulationLightSystems::CheckLightVisibility + .ambiguous_with(SimulationLightSystems::CheckLightVisibility), + ) .add_systems( PostUpdate, ( - add_clusters.in_set(SimulationLightSystems::AddClusters), + add_clusters + .in_set(SimulationLightSystems::AddClusters) + .after(CameraUpdateSystem), assign_objects_to_clusters .in_set(SimulationLightSystems::AssignLightsToClusters) .after(TransformSystem::TransformPropagate) diff --git a/crates/bevy_pbr/src/light/mod.rs b/crates/bevy_pbr/src/light/mod.rs index d895ac7672..d0db103c93 100644 --- a/crates/bevy_pbr/src/light/mod.rs +++ b/crates/bevy_pbr/src/light/mod.rs @@ -496,12 +496,19 @@ pub enum ShadowFilteringMethod { Temporal, } +/// System sets used to run light-related systems. #[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)] pub enum SimulationLightSystems { AddClusters, AssignLightsToClusters, + /// System order ambiguities between systems in this set are ignored: + /// each [`build_directional_light_cascades`] system is independent of the others, + /// and should operate on distinct sets of entities. UpdateDirectionalLightCascades, UpdateLightFrusta, + /// System order ambiguities between systems in this set are ignored: + /// the order of systems within this set is irrelevant, as the various visibility-checking systesms + /// assumes that their operations are irreversible during the frame. CheckLightVisibility, } diff --git a/crates/bevy_picking/src/lib.rs b/crates/bevy_picking/src/lib.rs index cd75515c97..034ecfd9e8 100644 --- a/crates/bevy_picking/src/lib.rs +++ b/crates/bevy_picking/src/lib.rs @@ -225,7 +225,6 @@ impl Plugin for PickingPlugin { First, (PickSet::Input, PickSet::PostInput) .after(bevy_time::TimeSystem) - .ambiguous_with(bevy_asset::handle_internal_asset_events) .after(bevy_ecs::event::EventUpdates) .chain(), ) @@ -239,7 +238,6 @@ impl Plugin for PickingPlugin { // Eventually events will need to be dispatched here PickSet::Last, ) - .ambiguous_with(bevy_asset::handle_internal_asset_events) .chain(), ) .register_type::() diff --git a/crates/bevy_render/src/view/visibility/mod.rs b/crates/bevy_render/src/view/visibility/mod.rs index 345055ca3d..8181d3c911 100644 --- a/crates/bevy_render/src/view/visibility/mod.rs +++ b/crates/bevy_render/src/view/visibility/mod.rs @@ -278,7 +278,11 @@ pub enum VisibilitySystems { /// [`hierarchy`](bevy_hierarchy). VisibilityPropagate, /// Label for the [`check_visibility`] system updating [`ViewVisibility`] - /// of each entity and the [`VisibleEntities`] of each view. + /// of each entity and the [`VisibleEntities`] of each view.\ + /// + /// System order ambiguities between systems in this set are ignored: + /// the order of systems within this set is irrelevant, as [`check_visibility`] + /// assumes that its operations are irreversible during the frame. CheckVisibility, } @@ -294,6 +298,7 @@ impl Plugin for VisibilityPlugin { .before(CheckVisibility) .after(TransformSystem::TransformPropagate), ) + .configure_sets(PostUpdate, CheckVisibility.ambiguous_with(CheckVisibility)) .add_systems( PostUpdate, ( diff --git a/crates/bevy_ui/src/lib.rs b/crates/bevy_ui/src/lib.rs index 012917fae8..a9dcc9e270 100644 --- a/crates/bevy_ui/src/lib.rs +++ b/crates/bevy_ui/src/lib.rs @@ -164,7 +164,9 @@ impl Plugin for UiPlugin { update_target_camera_system.in_set(UiSystem::Prepare), ui_layout_system .in_set(UiSystem::Layout) - .before(TransformSystem::TransformPropagate), + .before(TransformSystem::TransformPropagate) + // Text and Text2D operate on disjoint sets of entities + .ambiguous_with(bevy_text::update_text2d_layout), ui_stack_system .in_set(UiSystem::Stack) // the systems don't care about stack index @@ -226,7 +228,8 @@ fn build_text_interop(app: &mut App) { .in_set(UiSystem::PostLayout) .after(bevy_text::remove_dropped_font_atlas_sets) // Text2d and bevy_ui text are entirely on separate entities - .ambiguous_with(bevy_text::update_text2d_layout), + .ambiguous_with(bevy_text::update_text2d_layout) + .ambiguous_with(bevy_text::calculate_bounds_text2d), ), ); diff --git a/tests/ecs/ambiguity_detection.rs b/tests/ecs/ambiguity_detection.rs index a8fcf44fc0..39cb05a5cf 100644 --- a/tests/ecs/ambiguity_detection.rs +++ b/tests/ecs/ambiguity_detection.rs @@ -9,18 +9,22 @@ use bevy::{ prelude::*, utils::HashMap, }; -use bevy_render::{pipelined_rendering::RenderExtractApp, RenderApp}; +use bevy_render::pipelined_rendering::RenderExtractApp; fn main() { let mut app = App::new(); app.add_plugins(DefaultPlugins); - let sub_app = app.main_mut(); - configure_ambiguity_detection(sub_app); - let sub_app = app.sub_app_mut(RenderApp); - configure_ambiguity_detection(sub_app); - let sub_app = app.sub_app_mut(RenderExtractApp); - configure_ambiguity_detection(sub_app); + let main_app = app.main_mut(); + configure_ambiguity_detection(main_app); + let render_extract_app = app.sub_app_mut(RenderExtractApp); + configure_ambiguity_detection(render_extract_app); + + // Ambiguities in the RenderApp are currently allowed. + // Eventually, we should forbid these: see https://github.com/bevyengine/bevy/issues/7386 + // Uncomment the lines below to show the current ambiguities in the RenderApp. + // let sub_app = app.sub_app_mut(bevy_render::RenderApp); + // configure_ambiguity_detection(sub_app); app.finish(); app.cleanup(); @@ -29,11 +33,7 @@ fn main() { let main_app_ambiguities = count_ambiguities(app.main()); assert_eq!( main_app_ambiguities.total(), - // This number *should* be zero. - // Over time, we are working to reduce the number: your PR should not increase it. - // If you decrease this by fixing an ambiguity, reduce the number to prevent regressions. - // See https://github.com/bevyengine/bevy/issues/7386 for progress. - 44, + 0, "Main app has unexpected ambiguities among the following schedules: \n{:#?}.", main_app_ambiguities, );