Closing multi windows

[MrGVSV]

Objective

Currently, if any window is closed, the app quits entirely. This goal of this PR is to create a configurable resource to control how the app reacts to a window being closed.

Fixes #3180.

Solution

Removed the exit_on_window_close_system from bevy_window and replaced its functionality with a function in bevy_winit. The reason for this was to allow the reading and writing to world resources without having to add the bevy_ecs dependency to bevy_window, and to drop the Window from the WinitWindows resource.

Using the WindowsConfig resource, we can specify an exit_method. I added the following methods to start:

pub enum WindowExitMethod {
    /// Exits the app when any window is closed
    AnyClosed,
    /// Exits the app when the primary window is closed
    PrimaryClosed,
    /// Exits the app when the last window is closed
    LastClosed,
    /// Exits the app when a specified window is closed
    WindowClosed(WindowId),
    /// Does not exit the app when windows are closed
    KeepOpen,
}

The WindowsConfig resource defaults with the WindowExitMethod::PrimaryClosed method.

I'm not sure if this is the best way to solve this or not, but it at least provides some functionality.

Issues

Removing the Window

One thing I was trying to do was also remove the window from the Windows resource, but I wasn't sure how to solve the following error though:

thread 'main' panicked at 'Window texture node refers to a non-existent window.', crates/bevy_render/src/render_graph/nodes/window_texture_node.rs:54:14

I imagine this is a system ordering or caching issue of some kind. Is there anyone who has an idea of what might be causing the problem?

Resetting the Primary Window

Currently, the primary window is defined as

pub fn primary() -> Self {
    WindowId(Uuid::from_u128(0))
}

But one question, I wanted to ask is, "Should we update the primary window if it's closed?" Personally, I think it's best to keep this as is, but I wanted to hear other thoughts as well.

[MrGVSV]

Is bevy_window meant to work apart from bevy_winit? If so, I think this should be reverted. Since closing by WindowExitMethod is controlled by bevy_winit, it might make sense to leave this option in for the pure bevy_window (and instead maybe just default the value to false).

[DJMcNab]

The best way to fix the rendering issue is probably to wait until the old renderer is removed; the new renderer only requires small changes to make it not panic.

That being said, I think the solution in #2898 is much cleaner; it's a bit more code, but it composes much more nicely. Is there a reason you disregarded that approach?

Fwiw, I was intending to re-open #2898 (or re-create it into a new PR) once the old renderer was removed, although obviously I don't mind someone else driving it forwards.

[MrGVSV]

That being said, I think the solution in #2898 is much cleaner; it's a bit more code, but it composes much more nicely. Is there a reason you disregarded that approach?

This was a huge misstep on my part. For some reason, I thought that #2898 was merged and that all the window-related code was from that PR as well.

I do agree that it's much cleaner, especially using standard systems to handle whether the app should be exited or not. I originally tried to do that, but the version of main I was based on had removed the bevy_ecs dependency in bevy_window.

Although, I think the configuration should be made into a resource as described in #3180 so it's easier to access/modify. And an enum like WindowExitMethod might be a good way to provide some default options.

But we can close this for now and wait for the old renderer to be removed, then open up #2898 again. Sorry for the mess! 😅