Skip to content

Clean up WinitWindows::remove_window#12749

Merged
james7132 merged 1 commit intobevyengine:mainfrom
UkoeHB:window_cleanup
Mar 29, 2024
Merged

Clean up WinitWindows::remove_window#12749
james7132 merged 1 commit intobevyengine:mainfrom
UkoeHB:window_cleanup

Conversation

@UkoeHB
Copy link
Copy Markdown
Contributor

@UkoeHB UkoeHB commented Mar 27, 2024

Objective

  • Avoid unbounded HashMap growth for opening/closing windows.

Solution

  • Remove map entry in WinitWindows::remove_window.

Migration Guide

  • WinitWindows::get_window_entity now returns None after a window is closed, instead of a dead entity.

Comments

The comment this PR replaces was added in #3575. Since get_window_entity now returns an Entity instead of a WindowId, this no longer seems useful. Note that get_window_entity is only used here, immediately followed by a warning if the entity returned doesn't exist.

@matiqo15 matiqo15 added A-Windowing Platform-agnostic interface layer to run your app in C-Code-Quality A section of code that is hard to understand or change M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Mar 28, 2024
@github-actions
Copy link
Copy Markdown
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@mockersf
Copy link
Copy Markdown
Member

can't think of a scenario where the info of "that window used to exist" is needed and that couldn't be handled by the user themselves

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 28, 2024
@james7132 james7132 added this pull request to the merge queue Mar 29, 2024
Merged via the queue into bevyengine:main with commit c971b45 Mar 29, 2024
@UkoeHB UkoeHB deleted the window_cleanup branch March 29, 2024 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Windowing Platform-agnostic interface layer to run your app in C-Code-Quality A section of code that is hard to understand or change 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants