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.

Sign up for GitHub

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

Jump to bottom

Bevy contains internal system-order ambiguities #9511

Closed
alice-i-cecile opened this issue Aug 20, 2023 · 2 comments · Fixed by #10411
Closed

Bevy contains internal system-order ambiguities #9511

alice-i-cecile opened this issue Aug 20, 2023 · 2 comments · Fixed by #10411
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior

Comments

@alice-i-cecile
Copy link
Member

Bevy version

0.11.2

What you did

Enable system order ambiguities warnings on an App with only DefaultPlugins.

What went wrong

Noisy and bug-prone warnings caused by Bevy-internal systems.

In this case:

 -- scene_spawner_system and set_texture_to_copy_src
    conflict on: bevy_ecs::world::World
 -- scene_spawner_system and close_when_requested
    conflict on: bevy_ecs::world::World

Additional context

We should enforce this in CI. In addition to reducing noise for users, quite a few of these have turned out to be real bugs in the past.
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events A-Build-System Related to build systems or continuous integration labels Aug 20, 2023
@alice-i-cecile alice-i-cecile added this to the 0.12 milestone Aug 20, 2023
@alice-i-cecile
Copy link
Member Author

PostUpdate is much worse:

2023-08-20T17:46:16.627077Z  WARN bevy_ecs::schedule::schedule: 108 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:

@alice-i-cecile alice-i-cecile mentioned this issue Aug 28, 2023
Open
@bushrat011899 bushrat011899 mentioned this issue Sep 17, 2023
Closed
@alice-i-cecile alice-i-cecile removed this from the 0.12 milestone Oct 26, 2023
@alice-i-cecile
Copy link
Member Author

This is fixed (for now) by #10411.

@alice-i-cecile alice-i-cecile mentioned this issue Nov 6, 2023
Merged
@alice-i-cecile alice-i-cecile removed the A-Build-System Related to build systems or continuous integration label Nov 6, 2023
@alice-i-cecile alice-i-cecile mentioned this issue Nov 6, 2023
Closed
github-merge-queue bot pushed a commit that referenced this issue Jan 9, 2024
@jakobhellermann @alice-i-cecile @mockersf
resolve all internal ambiguities (#10411)
a657478 
- ignore all ambiguities that are not a problem
- remove `.before(Assets::<Image>::track_assets),` that points into a
different schedule (-> should this be caught?)
- add some explicit orderings:
- run `poll_receivers` and `update_accessibility_nodes` after
`window_closed` in `bevy_winit::accessibility`
  - run `bevy_ui::accessibility::calc_bounds` after `CameraUpdateSystem`
- run ` bevy_text::update_text2d_layout` and `bevy_ui::text_system`
after `font_atlas_set::remove_dropped_font_atlas_sets`
- add `app.ignore_ambiguity(a, b)` function for cases where you want to
ignore an ambiguity between two independent plugins `A` and `B`
- add `IgnoreAmbiguitiesPlugin` in `DefaultPlugins` that allows
cross-crate ambiguities like `bevy_animation`/`bevy_ui`
- Fixes #9511

## Before
**Render**
![render_schedule_Render
dot](https://github.com/bevyengine/bevy/assets/22177966/1c677968-7873-40cc-848c-91fca4c8e383)

**PostUpdate**
![schedule_PostUpdate
dot](https://github.com/bevyengine/bevy/assets/22177966/8fc61304-08d4-4533-8110-c04113a7367a)

## After
**Render**
![render_schedule_Render
dot](https://github.com/bevyengine/bevy/assets/22177966/462f3b28-cef7-4833-8619-1f5175983485)
**PostUpdate**
![schedule_PostUpdate
dot](https://github.com/bevyengine/bevy/assets/22177966/8cfb3d83-7842-4a84-9082-46177e1a6c70)

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
Subserial pushed a commit to Subserial/bevy_winit_hook that referenced this issue Feb 20, 2024
@jakobhellermann @alice-i-cecile @mockersf
resolve all internal ambiguities (#10411)
af99759 
- ignore all ambiguities that are not a problem
- remove `.before(Assets::<Image>::track_assets),` that points into a
different schedule (-> should this be caught?)
- add some explicit orderings:
- run `poll_receivers` and `update_accessibility_nodes` after
`window_closed` in `bevy_winit::accessibility`
  - run `bevy_ui::accessibility::calc_bounds` after `CameraUpdateSystem`
- run ` bevy_text::update_text2d_layout` and `bevy_ui::text_system`
after `font_atlas_set::remove_dropped_font_atlas_sets`
- add `app.ignore_ambiguity(a, b)` function for cases where you want to
ignore an ambiguity between two independent plugins `A` and `B`
- add `IgnoreAmbiguitiesPlugin` in `DefaultPlugins` that allows
cross-crate ambiguities like `bevy_animation`/`bevy_ui`
- Fixes bevyengine/bevy#9511

## Before
**Render**
![render_schedule_Render
dot](https://github.com/bevyengine/bevy/assets/22177966/1c677968-7873-40cc-848c-91fca4c8e383)

**PostUpdate**
![schedule_PostUpdate
dot](https://github.com/bevyengine/bevy/assets/22177966/8fc61304-08d4-4533-8110-c04113a7367a)

## After
**Render**
![render_schedule_Render
dot](https://github.com/bevyengine/bevy/assets/22177966/462f3b28-cef7-4833-8619-1f5175983485)
**PostUpdate**
![schedule_PostUpdate
dot](https://github.com/bevyengine/bevy/assets/22177966/8cfb3d83-7842-4a84-9082-46177e1a6c70)

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior
Projects
None yet
Milestone
No milestone
1 participant
@alice-i-cecile