Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formatting of system schedule cycle detected error is very hard to follow when many systems form a cycle #7367

Closed
alice-i-cecile opened this issue Jan 26, 2023 · 3 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jan 26, 2023

Bevy version

#7267, using the new schedule_v3 module.

Problem

When cycles are detected, the error message is very hard to read:

schedule contains at least 1 cycle(s) -- cycle(s) found within:
 ---- 0: ["bevy_asset::assets::Assets<bevy_animation::AnimationClip>::asset_event_system", "bevy_asset::assets::Assets<bevy_audio::audio_output::AudioSink>::asset_event_system", "bevy_asset::assets::Assets<bevy_audio::audio_source::AudioSource>::asset_event_system", "bevy_asset::assets::Assets<bevy_gltf::GltfMesh>::asset_event_system", "bevy_asset::assets::Assets<bevy_gltf::GltfPrimitive>::asset_event_system", "bevy_asset::assets::Assets<bevy_gltf::GltfNode>::asset_event_system", "bevy_asset::assets::Assets<bevy_gltf::Gltf>::asset_event_system", "bevy_asset::assets::Assets<bevy_pbr::pbr_material::StandardMaterial>::asset_event_system", "bevy_asset::assets::Assets<bevy_text::font_atlas_set::FontAtlasSet>::asset_event_system", "bevy_asset::assets::Assets<bevy_text::font::Font>::asset_event_system", "bevy_asset::assets::Assets<bevy_sprite::mesh2d::color_material::ColorMaterial>::asset_event_system", "bevy_asset::assets::Assets<bevy_sprite::texture_atlas::TextureAtlas>::asset_event_system", "bevy_asset::assets::Assets<bevy_render::texture::image::Image>::asset_event_system", "bevy_asset::assets::Assets<bevy_render::mesh::mesh::skinning::SkinnedMeshInverseBindposes>::asset_event_system", "bevy_asset::assets::Assets<bevy_render::mesh::mesh::Mesh>::asset_event_system", "bevy_asset::assets::Assets<bevy_render::render_resource::shader::Shader>::asset_event_system", "bevy_asset::assets::Assets<bevy_scene::scene::Scene>::asset_event_system", "bevy_asset::assets::Assets<bevy_scene::dynamic_scene::DynamicScene>::asset_event_system", "bevy_pbr::light::check_light_mesh_visibility", "bevy_pbr::light::update_point_light_frusta", "bevy_pbr::light::update_spot_light_frusta", "bevy_pbr::light::assign_lights_to_clusters", "bevy_pbr::light::update_directional_light_frusta", "bevy_render::view::visibility::check_visibility", "bevy_render::view::visibility::update_frusta<bevy_render::camera::projection::OrthographicProjection>", "bevy_render::view::visibility::update_frusta<bevy_render::camera::projection::PerspectiveProjection>", "bevy_render::view::visibility::update_frusta<bevy_render::camera::projection::Projection>", "bevy_ui::update::update_clipping_system", "bevy_transform::systems::propagate_transforms", "bevy_transform::systems::sync_simple_transforms", "bevy_animation::animation_player", "bevy_audio::audio_output::play_queued_audio_system<bevy_audio::audio_source::AudioSource>", "bevy_pbr::light::add_clusters", "bevy_ui::stack::ui_stack_system", "bevy_ui::flex::flex_node_system", "bevy_ui::widget::image::update_image_calculated_size_system", "bevy_ui::widget::text::text_system", "bevy_text::text2d::update_text2d_layout", "bevy_render::view::visibility::visibility_propagate_system", "bevy_render::camera::camera::camera_system<bevy_render::camera::projection::PerspectiveProjection>", "bevy_render::camera::camera::camera_system<bevy_render::camera::projection::OrthographicProjection>", "bevy_render::camera::camera::camera_system<bevy_render::camera::projection::Projection>", "bevy_winit::system::despawn_window", "bevy_winit::system::changed_window", "bevy_window::system::exit_on_all_closed", "bevy_ecs::scheduling::executor::apply_system_buffers"]

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: DependencyCycle', crates/bevy_ecs/src/scheduling/schedule.rs:181:32
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

These don't have the appropriate line breaks in the terminal.

image

Proposed fixes

  1. Use double line breaks for each cycle, and single line breaks for each system.
  2. Use short-names by default, with an option to see the full name.
  3. List the total number of disconnected cycles at the top of the error message.
  4. List the sets that each system is in.

Ambiguity detection recently had similar changes made; check that code for breadcrumbs.
If some of these fixes end up being more involved, submit the easy ones in their own PR.

@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jan 26, 2023
@alice-i-cecile alice-i-cecile changed the title Formatting of cycle detected error is very hard to follow Formatting of system schedule cycle detected error is very hard to follow when many systems form a cycle Jan 26, 2023
@JoJoJet
Copy link
Member

JoJoJet commented Jan 27, 2023

I wonder how hard it would be to draw the cycle in ASCII...

Something basic like

(1.)--->(2.)--->(3.)
 ^               |
 |               V
 +--------------(4.)

1. `my_path::my_system_1`
2. `my_path::my_system_4`
3. `your_path::some_system`
4. `our_path::revolution`

@alice-i-cecile
Copy link
Member Author

Yeah, the other thing I really want is to list exactly what sets and constraints between sets are responsible. And then, as very much a stretch goal, link to the line numbers.

@maniwani
Copy link
Contributor

maniwani commented Feb 1, 2023

It should be possible to enumerate the specific cycles in each strongly-connected component, but I don't know the exact algorithm.

We can use #[track_caller] to know where before, after, and in_set are called.

@bors bors bot closed this as completed in 3ec87e4 Feb 21, 2023
Shfty pushed a commit to shfty-rust/bevy that referenced this issue Mar 19, 2023
Graph theory make head hurty. Closes bevyengine#7367.

Basically, when we topologically sort the dependency graph, we already find its strongly-connected components (a really [neat algorithm][1]). This PR adds an algorithm that can dissect those into simple cycles, giving us more useful error reports.

test: `system_build_errors::dependency_cycle`
```
schedule has 1 before/after cycle(s):
cycle 1: system set 'A' must run before itself
system set 'A'
 ... which must run before system set 'B'
 ... which must run before system set 'A'
```
```
schedule has 1 before/after cycle(s):
cycle 1: system 'foo' must run before itself
system 'foo'
 ... which must run before system 'bar'
 ... which must run before system 'foo'
```

test: `system_build_errors::hierarchy_cycle`
```
schedule has 1 in_set cycle(s):
cycle 1: system set 'A' contains itself
system set 'A'
 ... which contains system set 'B'
 ... which contains system set 'A'
 ```

[1]: https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants