Skip to content

Commit 2931761

Browse files
Reduce internal system order ambiguities, and add an example explaining them (#7383)
# Objective - Bevy should not have any "internal" execution order ambiguities. These clutter the output of user-facing error reporting, and can result in nasty, nondetermistic, very difficult to solve bugs. - Verifying this currently involves repeated non-trivial manual work. ## Solution - [x] add an example to quickly check this - ~~[ ] ensure that this example panics if there are any unresolved ambiguities~~ - ~~[ ] run the example in CI 😈~~ There's one tricky ambiguity left, between UI and animation. I don't have the tools to fix this without system set configuration, so the remaining work is going to be left to #7267 or another PR after that. ``` 2023-01-27T18:38:42.989405Z INFO bevy_ecs::schedule::ambiguity_detection: Execution order ambiguities detected, you might want to add an explicit dependency relation between some of these systems: * Parallel systems: -- "bevy_animation::animation_player" and "bevy_ui::flex::flex_node_system" conflicts: ["bevy_transform::components::transform::Transform"] ``` ## Changelog Resolved internal execution order ambiguities for: 1. Transform propagation (ignored, we need smarter filter checking). 2. Gamepad processing (fixed). 3. bevy_winit's window handling (fixed). 4. Cascaded shadow maps and perspectives (fixed). Also fixed a desynchronized state bug that could occur when the `Window` component is removed and then added to the same entity in a single frame.
1 parent d2975ca commit 2931761

File tree

2 files changed

+14
-7
lines changed

2 files changed

+14
-7
lines changed

src/lib.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ use bevy_utils::{
2525
Instant,
2626
};
2727
use bevy_window::{
28-
CursorEntered, CursorLeft, CursorMoved, FileDragAndDrop, Ime, ModifiesWindows,
29-
ReceivedCharacter, RequestRedraw, Window, WindowBackendScaleFactorChanged,
28+
exit_on_all_closed, CursorEntered, CursorLeft, CursorMoved, FileDragAndDrop, Ime,
29+
ModifiesWindows, ReceivedCharacter, RequestRedraw, Window, WindowBackendScaleFactorChanged,
3030
WindowCloseRequested, WindowCreated, WindowFocused, WindowMoved, WindowResized,
3131
WindowScaleFactorChanged,
3232
};
@@ -55,8 +55,11 @@ impl Plugin for WinitPlugin {
5555
CoreStage::PostUpdate,
5656
SystemSet::new()
5757
.label(ModifiesWindows)
58-
.with_system(changed_window)
59-
.with_system(despawn_window),
58+
// exit_on_all_closed only uses the query to determine if the query is empty,
59+
// and so doesn't care about ordering relative to changed_window
60+
.with_system(changed_window.ambiguous_with(exit_on_all_closed))
61+
// Update the state of the window before attempting to despawn to ensure consistent event ordering
62+
.with_system(despawn_window.after(changed_window)),
6063
);
6164

6265
#[cfg(target_arch = "wasm32")]

src/system.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,18 @@ pub struct WindowTitleCache(HashMap<Entity, String>);
8787

8888
pub(crate) fn despawn_window(
8989
closed: RemovedComponents<Window>,
90+
window_entities: Query<&Window>,
9091
mut close_events: EventWriter<WindowClosed>,
9192
mut winit_windows: NonSendMut<WinitWindows>,
9293
) {
9394
for window in closed.iter() {
9495
info!("Closing window {:?}", window);
95-
96-
winit_windows.remove_window(window);
97-
close_events.send(WindowClosed { window });
96+
// Guard to verify that the window is in fact actually gone,
97+
// rather than having the component added and removed in the same frame.
98+
if !window_entities.contains(window) {
99+
winit_windows.remove_window(window);
100+
close_events.send(WindowClosed { window });
101+
}
98102
}
99103
}
100104

0 commit comments

Comments
 (0)