From acff2210c05b2861e685c8bd3a483b0fdfaa82e1 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Mon, 20 Feb 2023 15:31:10 +0000 Subject: [PATCH] Add `report_sets` option to `ScheduleBuildSettings` (#7756) # Objective - Fixes #7442. ## Solution - Added `report_sets` option to `ScheduleBuildSettings` like described in the linked issue. The output of the `3d_scene` example when reporting ambiguities with `report_sets` and `use_shortnames` set to `true` (and with #7755 applied) now looks like this: ``` 82 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these: -- filesystem_watcher_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- update_asset_storage_system (LoadAssets) and apply_system_buffers (FirstFlush) conflict on: bevy_ecs::world::World -- scene_spawner_system (Update) and close_when_requested (Update) conflict on: bevy_ecs::world::World -- exit_on_all_closed (PostUpdate) and apply_system_buffers (CalculateBoundsFlush, PostUpdate) conflict on: bevy_ecs::world::World -- exit_on_all_closed (PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- camera_system (CameraUpdateSystem, PostUpdate) and apply_system_buffers (CalculateBoundsFlush, PostUpdate) conflict on: bevy_ecs::world::World -- camera_system (CameraUpdateSystem, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- camera_system (CameraUpdateSystem, PostUpdate) and apply_system_buffers (CalculateBoundsFlush, PostUpdate) conflict on: bevy_ecs::world::World -- camera_system (CameraUpdateSystem, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- camera_system (CameraUpdateSystem, PostUpdate) and apply_system_buffers (CalculateBoundsFlush, PostUpdate) conflict on: bevy_ecs::world::World -- camera_system (CameraUpdateSystem, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- calculate_bounds (CalculateBounds, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and visibility_propagate_system (PostUpdate, VisibilityPropagate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_text2d_layout (PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and ui_stack_system (PostUpdate, Stack) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and text_system (PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_image_calculated_size_system (PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and flex_node_system (Flex, PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and add_clusters (AddClusters, PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and play_queued_audio_system (PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and animation_player (PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and propagate_transforms (PostUpdate, TransformPropagate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and sync_simple_transforms (PostUpdate, TransformPropagate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_directional_light_cascades (PostUpdate, UpdateDirectionalLightCascades) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_clipping_system (PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_frusta (PostUpdate, UpdateProjectionFrusta) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_frusta (PostUpdate, UpdatePerspectiveFrusta) conflict on: bevy_ecs::world::World -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_frusta (PostUpdate, UpdateOrthographicFrusta) conflict on: bevy_ecs::world::World -- visibility_propagate_system (PostUpdate, VisibilityPropagate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- update_text2d_layout (PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- ui_stack_system (PostUpdate, Stack) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- text_system (PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- update_image_calculated_size_system (PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- flex_node_system (Flex, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate) conflict on: bevy_ecs::world::World -- flex_node_system (Flex, PostUpdate) and animation_player (PostUpdate) conflict on: ["bevy_transform::components::transform::Transform"] -- apply_system_buffers (AddClustersFlush, PostUpdate) and play_queued_audio_system (PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and animation_player (PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and propagate_transforms (PostUpdate, TransformPropagate) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and sync_simple_transforms (PostUpdate, TransformPropagate) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_directional_light_cascades (PostUpdate, UpdateDirectionalLightCascades) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_clipping_system (PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_frusta (PostUpdate, UpdateProjectionFrusta) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_frusta (PostUpdate, UpdatePerspectiveFrusta) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_frusta (PostUpdate, UpdateOrthographicFrusta) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and check_visibility (CheckVisibility, PostUpdate) conflict on: bevy_ecs::world::World -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_directional_light_frusta (PostUpdate, UpdateLightFrusta) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World -- Assets::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush) conflict on: bevy_ecs::world::World ``` Co-authored-by: Edgar Geier --- crates/bevy_ecs/src/schedule/schedule.rs | 50 +++++++++++++++++++++++- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 4475cbed11..5a9dfbf1aa 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -1322,7 +1322,21 @@ impl ScheduleGraph { impl ScheduleGraph { fn get_node_name(&self, id: &NodeId) -> String { let mut name = match id { - NodeId::System(_) => self.systems[id.index()].get().unwrap().name().to_string(), + NodeId::System(_) => { + let name = self.systems[id.index()].get().unwrap().name().to_string(); + if self.settings.report_sets { + let sets = self.names_of_sets_containing_node(id); + if sets.is_empty() { + name + } else if sets.len() == 1 { + format!("{name} (in set {})", sets[0]) + } else { + format!("{name} (in sets {})", sets.join(", ")) + } + } else { + name + } + } NodeId::Set(_) => self.system_sets[id.index()].name(), }; if self.settings.use_shortnames { @@ -1453,6 +1467,27 @@ impl ScheduleGraph { warn!("{}", string); } + + fn traverse_sets_containing_node(&self, id: NodeId, f: &mut impl FnMut(NodeId) -> bool) { + for (set_id, _, _) in self.hierarchy.graph.edges_directed(id, Direction::Incoming) { + if f(set_id) { + self.traverse_sets_containing_node(set_id, f); + } + } + } + + fn names_of_sets_containing_node(&self, id: &NodeId) -> Vec { + let mut sets = HashSet::new(); + self.traverse_sets_containing_node(*id, &mut |set_id| { + !self.system_sets[set_id.index()].is_system_type() && sets.insert(set_id) + }); + let mut sets: Vec<_> = sets + .into_iter() + .map(|set_id| self.get_node_name(&set_id)) + .collect(); + sets.sort(); + sets + } } /// Category of errors encountered during schedule construction. @@ -1525,13 +1560,23 @@ pub enum LogLevel { pub struct ScheduleBuildSettings { /// Determines whether the presence of ambiguities (systems with conflicting access but indeterminate order) /// is only logged or also results in an [`Ambiguity`](ScheduleBuildError::Ambiguity) error. + /// + /// Defaults to [`LogLevel::Ignore`]. pub ambiguity_detection: LogLevel, /// Determines whether the presence of redundant edges in the hierarchy of system sets is only /// logged or also results in a [`HierarchyRedundancy`](ScheduleBuildError::HierarchyRedundancy) /// error. + /// + /// Defaults to [`LogLevel::Warn`]. pub hierarchy_detection: LogLevel, /// If set to true, node names will be shortened instead of the fully qualified type path. + /// + /// Defaults to `true`. pub use_shortnames: bool, + /// If set to true, report all system sets the conflicting systems are part of. + /// + /// Defaults to `true`. + pub report_sets: bool, } impl Default for ScheduleBuildSettings { @@ -1545,7 +1590,8 @@ impl ScheduleBuildSettings { Self { ambiguity_detection: LogLevel::Ignore, hierarchy_detection: LogLevel::Warn, - use_shortnames: false, + use_shortnames: true, + report_sets: true, } } }