Skip to content

Move WindowPlugin exit systems to Last#23624

Open
doonv wants to merge 8 commits intobevyengine:mainfrom
doonv:move-window-despawn
Open

Move WindowPlugin exit systems to Last#23624
doonv wants to merge 8 commits intobevyengine:mainfrom
doonv:move-window-despawn

Conversation

@doonv
Copy link
Copy Markdown
Contributor

@doonv doonv commented Apr 2, 2026 •
edited
Loading

Objective

Fix #11874.

Solution

Old me said that he had problems fixing this, but I just changed the exit systems to Last and I have encountered no issues.

Testing

CI passes.
I ran several examples including multiple_windows and I did the same changes to 0.18.1 and tested it with my library, I found no errors while closing the window in both cases.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Windowing Platform-agnostic interface layer to run your app in M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide X-Contentious There are nontrivial implications that should be thought through S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 2, 2026
@doonv doonv mentioned this pull request Apr 2, 2026
Merged
@doonv
Change despawn_windows.before( to after
a552c6d 
`despawn_windows` despawns the winit window, not the `Window` entity, and it reads from AppExit, written by ExitSystem, so it should run after. (This was the original order before the PR)
github-merge-queue bot pushed a commit that referenced this pull request Apr 2, 2026
@doonv
Replace unnecessary &Window in exit_on_all_closed with `With<Wind…
842fa5c 
…ow>` (#23625)

Tiny optimization I came across while working on
#23624
shunkie
shunkie reviewed Apr 3, 2026
View reviewed changes
doonv and others added 3 commits April 3, 2026 11:51
@doonv @shunkie
Update crates/bevy_window/src/system.rs
5daf625 
Co-authored-by: shunkie <shunkie@qq.com>
@doonv
fix stupid
cf7fcc3 
github suggestions do not change the references of stuff
@doonv
missed one
31b8dda
@doonv doonv requested a review from shunkie April 6, 2026 17:02
@Zeophlite Zeophlite added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 7, 2026
@hymm
Copy link
Copy Markdown
Contributor

hymm commented Apr 7, 2026

What platform did you test on? Has this been tested on other os's?

@doonv
Copy link
Copy Markdown
Contributor Author

doonv commented Apr 7, 2026

What platform did you test on? Has this been tested on other os's?

I personally tested on Arch Linux. But GitHub actions also tested on Windows and MacOS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

4 more reviewers

@Zeophlite Zeophlite Zeophlite approved these changes

@xremming xremming xremming approved these changes

@hukasu hukasu hukasu approved these changes

@shunkie shunkie shunkie approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Window despawns before PostUpdate

7 participants

@doonv @hymm @Zeophlite @xremming @hukasu @shunkie @alice-i-cecile