diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 566de2686c9ca..f3c6d6ecb370a 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -854,6 +854,83 @@ impl App { .configure_schedules(schedule_build_settings); self } + + /// When doing [ambiguity checking](bevy_ecs::schedule::ScheduleBuildSettings) this + /// ignores systems that are ambiguious on [`Component`] T. + /// + /// This settings only applies to the main world. To apply this to other worlds call the + /// [corresponding method](World::allow_ambiguous_component) on World + /// + /// ## Example + /// + /// ```rust + /// # use bevy_app::prelude::*; + /// # use bevy_ecs::prelude::*; + /// # use bevy_ecs::schedule::{LogLevel, ScheduleBuildSettings}; + /// # use bevy_utils::default; + /// + /// #[derive(Component)] + /// struct A; + /// + /// // these systems are ambiguous on A + /// fn system_1(_: Query<&mut A>) {} + /// fn system_2(_: Query<&A>) {} + /// + /// let mut app = App::new(); + /// app.configure_schedules(ScheduleBuildSettings { + /// ambiguity_detection: LogLevel::Error, + /// ..default() + /// }); + /// + /// app.add_systems(Update, ( system_1, system_2 )); + /// app.allow_ambiguous_component::(); + /// + /// // running the app does not error. + /// app.update(); + /// ``` + pub fn allow_ambiguous_component(&mut self) -> &mut Self { + self.world.allow_ambiguous_component::(); + self + } + + /// When doing [ambiguity checking](bevy_ecs::schedule::ScheduleBuildSettings) this + /// ignores systems that are ambiguious on [`Resource`] T. + /// + /// This settings only applies to the main world. To apply this to other worlds call the + /// [corresponding method](World::allow_ambiguous_resource) on World + /// + /// ## Example + /// + /// ```rust + /// # use bevy_app::prelude::*; + /// # use bevy_ecs::prelude::*; + /// # use bevy_ecs::schedule::{LogLevel, ScheduleBuildSettings}; + /// # use bevy_utils::default; + /// + /// #[derive(Resource)] + /// struct R; + /// + /// // these systems are ambiguous on R + /// fn system_1(_: ResMut) {} + /// fn system_2(_: Res) {} + /// + /// let mut app = App::new(); + /// app.configure_schedules(ScheduleBuildSettings { + /// ambiguity_detection: LogLevel::Error, + /// ..default() + /// }); + /// app.insert_resource(R); + /// + /// app.add_systems(Update, ( system_1, system_2 )); + /// app.allow_ambiguous_resource::(); + /// + /// // running the app does not error. + /// app.update(); + /// ``` + pub fn allow_ambiguous_resource(&mut self) -> &mut Self { + self.world.allow_ambiguous_resource::(); + self + } } fn run_once(mut app: App) { diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index fcf4b09f19ba3..751b56d8b3260 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -261,6 +261,9 @@ pub trait AssetApp { /// * Registering the [`Asset`] in the [`AssetServer`] /// * Initializing the [`AssetEvent`] resource for the [`Asset`] /// * Adding other relevant systems and resources for the [`Asset`] + /// * Ignoring schedule ambiguities in [`Assets`] resource. Any time a system takes + /// mutable access to this resource this causes a conflict, but they rarely actually + /// modify the same underlying asset. fn init_asset(&mut self) -> &mut Self; /// Registers the asset type `T` using `[App::register]`, /// and adds [`ReflectAsset`] type data to `T` and [`ReflectHandle`] type data to [`Handle`] in the type registry. @@ -301,6 +304,7 @@ impl AssetApp for App { )); } self.insert_resource(assets) + .allow_ambiguous_resource::>() .add_event::>() .register_type::>() .register_type::>() @@ -427,8 +431,11 @@ mod tests { }; use bevy_app::{App, Update}; use bevy_core::TaskPoolPlugin; - use bevy_ecs::event::ManualEventReader; use bevy_ecs::prelude::*; + use bevy_ecs::{ + event::ManualEventReader, + schedule::{LogLevel, ScheduleBuildSettings}, + }; use bevy_log::LogPlugin; use bevy_reflect::TypePath; use bevy_utils::BoxedFuture; @@ -1166,4 +1173,23 @@ mod tests { None }); } + + #[test] + fn ignore_system_ambiguities_on_assets() { + let mut app = App::new(); + app.add_plugins(AssetPlugin::default()) + .init_asset::(); + + fn uses_assets(_asset: ResMut>) {} + app.add_systems(Update, (uses_assets, uses_assets)); + app.edit_schedule(Update, |s| { + s.set_build_settings(ScheduleBuildSettings { + ambiguity_detection: LogLevel::Error, + ..Default::default() + }); + }); + + // running schedule does not error on ambiguity between the 2 uses_assets systems + app.world.run_schedule(Update); + } } diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 1d58a7d59fe72..4ed863944acfb 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -718,6 +718,9 @@ mod tests { } mod system_ambiguity { + use std::collections::BTreeSet; + + use super::*; // Required to make the derive macro behave use crate as bevy_ecs; use crate::event::Events; @@ -981,14 +984,12 @@ mod tests { assert_eq!(schedule.graph().conflicting_systems().len(), 0); } + #[derive(ScheduleLabel, Hash, PartialEq, Eq, Debug, Clone)] + struct TestSchedule; + // Tests that the correct ambiguities were reported in the correct order. #[test] fn correct_ambiguities() { - use super::*; - - #[derive(ScheduleLabel, Hash, PartialEq, Eq, Debug, Clone)] - struct TestSchedule; - fn system_a(_res: ResMut) {} fn system_b(_res: ResMut) {} fn system_c(_res: ResMut) {} @@ -1008,9 +1009,11 @@ mod tests { )); schedule.graph_mut().initialize(&mut world); - let _ = schedule - .graph_mut() - .build_schedule(world.components(), &TestSchedule.dyn_clone()); + let _ = schedule.graph_mut().build_schedule( + world.components(), + &TestSchedule.dyn_clone(), + &BTreeSet::new(), + ); let ambiguities: Vec<_> = schedule .graph() @@ -1050,19 +1053,16 @@ mod tests { // Related issue https://github.com/bevyengine/bevy/issues/9641 #[test] fn anonymous_set_name() { - use super::*; - - #[derive(ScheduleLabel, Hash, PartialEq, Eq, Debug, Clone)] - struct TestSchedule; - let mut schedule = Schedule::new(TestSchedule); schedule.add_systems((resmut_system, resmut_system).run_if(|| true)); let mut world = World::new(); schedule.graph_mut().initialize(&mut world); - let _ = schedule - .graph_mut() - .build_schedule(world.components(), &TestSchedule.dyn_clone()); + let _ = schedule.graph_mut().build_schedule( + world.components(), + &TestSchedule.dyn_clone(), + &BTreeSet::new(), + ); let ambiguities: Vec<_> = schedule .graph() @@ -1078,5 +1078,24 @@ mod tests { ) ); } + + #[test] + fn ignore_component_resource_ambiguities() { + let mut world = World::new(); + world.insert_resource(R); + world.allow_ambiguous_resource::(); + let mut schedule = Schedule::new(TestSchedule); + + //check resource + schedule.add_systems((resmut_system, res_system)); + schedule.initialize(&mut world).unwrap(); + assert!(schedule.graph().conflicting_systems().is_empty()); + + // check components + world.allow_ambiguous_component::(); + schedule.add_systems((write_component_system, read_component_system)); + schedule.initialize(&mut world).unwrap(); + assert!(schedule.graph().conflicting_systems().is_empty()); + } } } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 79e4e99916893..434454e83482c 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -1,11 +1,12 @@ use std::{ + collections::BTreeSet, fmt::{Debug, Write}, result::Result, }; -use bevy_utils::default; #[cfg(feature = "trace")] use bevy_utils::tracing::info_span; +use bevy_utils::{default, tracing::info}; use bevy_utils::{ petgraph::{algo::TarjanScc, prelude::*}, thiserror::Error, @@ -18,6 +19,7 @@ use fixedbitset::FixedBitSet; use crate::{ self as bevy_ecs, component::{ComponentId, Components, Tick}, + prelude::Component, schedule::*, system::{BoxedSystem, Resource, System}, world::World, @@ -27,6 +29,8 @@ use crate::{ #[derive(Default, Resource)] pub struct Schedules { inner: HashMap, + /// List of [`ComponentId`]s to ignore when reporting system order ambiguity conflicts + pub ignored_scheduling_ambiguities: BTreeSet, } impl Schedules { @@ -34,6 +38,7 @@ impl Schedules { pub fn new() -> Self { Self { inner: HashMap::new(), + ignored_scheduling_ambiguities: BTreeSet::new(), } } @@ -110,6 +115,38 @@ impl Schedules { schedule.set_build_settings(schedule_build_settings.clone()); } } + + /// Ignore system order ambiguities caused by conflicts on [`Component`]s of type `T`. + pub fn allow_ambiguous_component(&mut self, world: &mut World) { + self.ignored_scheduling_ambiguities + .insert(world.init_component::()); + } + + /// Ignore system order ambiguities caused by conflicts on [`Resource`]s of type `T`. + pub fn allow_ambiguous_resource(&mut self, world: &mut World) { + self.ignored_scheduling_ambiguities + .insert(world.components.init_resource::()); + } + + /// Iterate through the [`ComponentId`]'s that will be ignored. + pub fn iter_ignored_ambiguities(&self) -> impl Iterator + '_ { + self.ignored_scheduling_ambiguities.iter() + } + + /// Prints the names of the components and resources with [`info`] + /// + /// May panic or retrieve incorrect names if [`Components`] is not from the same + /// world + pub fn print_ignored_ambiguities(&self, components: &Components) { + let mut message = + "System order ambiguities caused by conflicts on the following types are ignored:\n" + .to_string(); + for id in self.iter_ignored_ambiguities() { + writeln!(message, "{}", components.get_name(*id).unwrap()).unwrap(); + } + + info!("{}", message); + } } fn make_executor(kind: ExecutorKind) -> Box { @@ -262,8 +299,16 @@ impl Schedule { pub fn initialize(&mut self, world: &mut World) -> Result<(), ScheduleBuildError> { if self.graph.changed { self.graph.initialize(world); - self.graph - .update_schedule(&mut self.executable, world.components(), &self.name)?; + let ignored_ambiguities = world + .get_resource_or_insert_with::(Schedules::default) + .ignored_scheduling_ambiguities + .clone(); + self.graph.update_schedule( + &mut self.executable, + world.components(), + &ignored_ambiguities, + &self.name, + )?; self.graph.changed = false; self.executor_initialized = false; } @@ -871,6 +916,7 @@ impl ScheduleGraph { &mut self, components: &Components, schedule_label: &BoxedScheduleLabel, + ignored_ambiguities: &BTreeSet, ) -> Result { // check hierarchy for cycles self.hierarchy.topsort = @@ -919,8 +965,11 @@ impl ScheduleGraph { let ambiguous_with_flattened = self.get_ambiguous_with_flattened(&set_systems); // check for conflicts - let conflicting_systems = - self.get_conflicting_systems(&flat_results.disconnected, &ambiguous_with_flattened); + let conflicting_systems = self.get_conflicting_systems( + &flat_results.disconnected, + &ambiguous_with_flattened, + ignored_ambiguities, + ); self.optionally_check_conflicts(&conflicting_systems, components, schedule_label)?; self.conflicting_systems = conflicting_systems; @@ -1040,6 +1089,7 @@ impl ScheduleGraph { &self, flat_results_disconnected: &Vec<(NodeId, NodeId)>, ambiguous_with_flattened: &GraphMap, + ignored_ambiguities: &BTreeSet, ) -> Vec<(NodeId, NodeId, Vec)> { let mut conflicting_systems = Vec::new(); for &(a, b) in flat_results_disconnected { @@ -1058,8 +1108,15 @@ impl ScheduleGraph { let access_a = system_a.component_access(); let access_b = system_b.component_access(); if !access_a.is_compatible(access_b) { - let conflicts = access_a.get_conflicts(access_b); - conflicting_systems.push((a, b, conflicts)); + let conflicts: Vec<_> = access_a + .get_conflicts(access_b) + .into_iter() + .filter(|id| !ignored_ambiguities.contains(id)) + .collect(); + + if !conflicts.is_empty() { + conflicting_systems.push((a, b, conflicts)); + } } } } @@ -1171,6 +1228,7 @@ impl ScheduleGraph { &mut self, schedule: &mut SystemSchedule, components: &Components, + ignored_ambiguities: &BTreeSet, schedule_label: &BoxedScheduleLabel, ) -> Result<(), ScheduleBuildError> { if !self.uninit.is_empty() { @@ -1196,7 +1254,7 @@ impl ScheduleGraph { self.system_set_conditions[id.index()] = conditions; } - *schedule = self.build_schedule(components, schedule_label)?; + *schedule = self.build_schedule(components, schedule_label, ignored_ambiguities)?; // move systems into new schedule for &id in &schedule.system_ids { diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index be43fcce3d770..f4ca74b7185af 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2087,6 +2087,20 @@ impl World { pub fn run_schedule(&mut self, label: impl AsRef) { self.schedule_scope(label, |world, sched| sched.run(world)); } + + /// Ignore system order ambiguities caused by conflicts on [`Component`]s of type `T`. + pub fn allow_ambiguous_component(&mut self) { + let mut schedules = self.remove_resource::().unwrap_or_default(); + schedules.allow_ambiguous_component::(self); + self.insert_resource(schedules); + } + + /// Ignore system order ambiguities caused by conflicts on [`Resource`]s of type `T`. + pub fn allow_ambiguous_resource(&mut self) { + let mut schedules = self.remove_resource::().unwrap_or_default(); + schedules.allow_ambiguous_resource::(self); + self.insert_resource(schedules); + } } impl fmt::Debug for World {