From ff52c7c8538f425a3d667bb755856a1a83e04349 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Thu, 5 May 2022 13:35:43 +0000 Subject: [PATCH] Allow closing windows at runtime (#3575) # Objective Fixes #3180, builds from https://github.com/bevyengine/bevy/pull/2898 ## Solution Support requesting a window to be closed and closing a window in `bevy_window`, and handle this in `bevy_winit`. This is a stopgap until we move to windows as entites, which I'm sure I'll get around to eventually. ## Changelog ### Added - `Window::close` to allow closing windows. - `WindowClosed` to allow reacting to windows being closed. ### Changed Replaced `bevy::system::exit_on_esc_system` with `bevy::window::close_on_esc`. ## Fixed The app no longer exits when any window is closed. This difference is only observable when there are multiple windows. ## Migration Guide `bevy::input::system::exit_on_esc_system` has been removed. Use `bevy::window::close_on_esc` instead. `CloseWindow` has been removed. Use `Window::close` instead. The `Close` variant has been added to `WindowCommand`. Handle this by closing the relevant window. --- crates/bevy_input/src/lib.rs | 1 - crates/bevy_input/src/system.rs | 22 ---------- crates/bevy_render/src/view/window.rs | 13 ++++-- crates/bevy_window/Cargo.toml | 2 + crates/bevy_window/src/event.rs | 29 ++++++++++--- crates/bevy_window/src/lib.rs | 32 +++++++++++--- crates/bevy_window/src/system.rs | 59 +++++++++++++++++++++++--- crates/bevy_window/src/window.rs | 16 +++++++ crates/bevy_window/src/windows.rs | 4 ++ crates/bevy_winit/src/lib.rs | 48 ++++++++++++++------- crates/bevy_winit/src/winit_windows.rs | 6 +++ examples/2d/rotation.rs | 2 +- examples/games/alien_cake_addict.rs | 2 +- examples/games/breakout.rs | 2 +- examples/window/multiple_windows.rs | 1 + tests/window/resizing.rs | 4 +- 16 files changed, 178 insertions(+), 65 deletions(-) delete mode 100644 crates/bevy_input/src/system.rs diff --git a/crates/bevy_input/src/lib.rs b/crates/bevy_input/src/lib.rs index 142f1883550cc..625b8ed746a61 100644 --- a/crates/bevy_input/src/lib.rs +++ b/crates/bevy_input/src/lib.rs @@ -3,7 +3,6 @@ pub mod gamepad; mod input; pub mod keyboard; pub mod mouse; -pub mod system; pub mod touch; pub use axis::*; diff --git a/crates/bevy_input/src/system.rs b/crates/bevy_input/src/system.rs deleted file mode 100644 index d21fd70ee390f..0000000000000 --- a/crates/bevy_input/src/system.rs +++ /dev/null @@ -1,22 +0,0 @@ -use crate::{ButtonState, KeyCode, KeyboardInput}; -use bevy_app::AppExit; -use bevy_ecs::prelude::{EventReader, EventWriter}; - -/// Sends an [`AppExit`] event whenever the `ESC` key is pressed. -/// -/// ## Note -/// -/// This system is not added as part of the `DefaultPlugins`. You can add the [`exit_on_esc_system`] -/// yourself if desired. -pub fn exit_on_esc_system( - mut keyboard_input_events: EventReader, - mut app_exit_events: EventWriter, -) { - for event in keyboard_input_events.iter() { - if let Some(key_code) = event.key_code { - if event.state == ButtonState::Pressed && key_code == KeyCode::Escape { - app_exit_events.send_default(); - } - } - } -} diff --git a/crates/bevy_render/src/view/window.rs b/crates/bevy_render/src/view/window.rs index c06776c54a878..305a56ddd115b 100644 --- a/crates/bevy_render/src/view/window.rs +++ b/crates/bevy_render/src/view/window.rs @@ -7,7 +7,7 @@ use crate::{ use bevy_app::{App, Plugin}; use bevy_ecs::prelude::*; use bevy_utils::{tracing::debug, HashMap, HashSet}; -use bevy_window::{PresentMode, RawWindowHandleWrapper, WindowId, Windows}; +use bevy_window::{PresentMode, RawWindowHandleWrapper, WindowClosed, WindowId, Windows}; use std::ops::{Deref, DerefMut}; use wgpu::TextureFormat; @@ -67,8 +67,12 @@ impl DerefMut for ExtractedWindows { } } -fn extract_windows(mut render_world: ResMut, windows: Res) { - let mut extracted_windows = render_world.resource_mut::(); +fn extract_windows( + mut render_world: ResMut, + mut closed: EventReader, + windows: Res, +) { + let mut extracted_windows = render_world.get_resource_mut::().unwrap(); for window in windows.iter() { let (new_width, new_height) = ( window.physical_width().max(1), @@ -105,6 +109,9 @@ fn extract_windows(mut render_world: ResMut, windows: Res) extracted_window.physical_height = new_height; } } + for closed_window in closed.iter() { + extracted_windows.remove(&closed_window.id); + } } #[derive(Default)] diff --git a/crates/bevy_window/Cargo.toml b/crates/bevy_window/Cargo.toml index d8bdd76e5bffb..f77605cc76c80 100644 --- a/crates/bevy_window/Cargo.toml +++ b/crates/bevy_window/Cargo.toml @@ -14,6 +14,8 @@ bevy_app = { path = "../bevy_app", version = "0.8.0-dev" } bevy_ecs = { path = "../bevy_ecs", version = "0.8.0-dev" } bevy_math = { path = "../bevy_math", version = "0.8.0-dev" } bevy_utils = { path = "../bevy_utils", version = "0.8.0-dev" } +# Used for close_on_esc +bevy_input = { path = "../bevy_input", version = "0.8.0-dev" } raw-window-handle = "0.4.2" # other diff --git a/crates/bevy_window/src/event.rs b/crates/bevy_window/src/event.rs index 8673edf550c89..8452c482035ca 100644 --- a/crates/bevy_window/src/event.rs +++ b/crates/bevy_window/src/event.rs @@ -25,22 +25,37 @@ pub struct CreateWindow { #[derive(Debug, Clone)] pub struct RequestRedraw; -/// An event that indicates a window should be closed. +/// An event that is sent whenever a new window is created. +/// +/// To create a new window, send a [`CreateWindow`] event - this +/// event will be sent in the handler for that event. #[derive(Debug, Clone)] -pub struct CloseWindow { +pub struct WindowCreated { pub id: WindowId, } -/// An event that is sent whenever a new window is created. +/// An event that is sent whenever the operating systems requests that a window +/// be closed. This will be sent when the close button of the window is pressed. +/// +/// If the default [`WindowPlugin`] is used, these events are handled +/// by [closing] the corresponding [`Window`]. +/// To disable this behaviour, set `close_when_requested` on the [`WindowPlugin`] +/// to `false`. +/// +/// [`WindowPlugin`]: crate::WindowPlugin +/// [`Window`]: crate::Window +/// [closing]: crate::Window::close #[derive(Debug, Clone)] -pub struct WindowCreated { +pub struct WindowCloseRequested { pub id: WindowId, } -/// An event that is sent whenever a close was requested for a window. For example: when the "close" -/// button is pressed on a window. +/// An event that is sent whenever a window is closed. This will be sent by the +/// handler for [`Window::close`]. +/// +/// [`Window::close`]: crate::Window::close #[derive(Debug, Clone)] -pub struct WindowCloseRequested { +pub struct WindowClosed { pub id: WindowId, } diff --git a/crates/bevy_window/src/lib.rs b/crates/bevy_window/src/lib.rs index 9e191889aafa8..6e5b2abd87680 100644 --- a/crates/bevy_window/src/lib.rs +++ b/crates/bevy_window/src/lib.rs @@ -24,15 +24,34 @@ use bevy_app::prelude::*; use bevy_ecs::{event::Events, schedule::SystemLabel}; pub struct WindowPlugin { + /// Whether to create a window when added. + /// + /// Note that if there are no windows, by default the App will exit, + /// due to [`exit_on_all_closed`]. pub add_primary_window: bool, - pub exit_on_close: bool, + /// Whether to exit the app when there are no open windows. + /// If disabling this, ensure that you send the [`bevy_app::AppExit`] + /// event when the app should exit. If this does not occur, you will + /// create 'headless' processes (processes without windows), which may + /// surprise your users. It is recommended to leave this setting as `true`. + /// + /// If true, this plugin will add [`exit_on_all_closed`] to [`CoreStage::Update`]. + pub exit_on_all_closed: bool, + /// Whether to close windows when they are requested to be closed (i.e. + /// when the close button is pressed) + /// + /// If true, this plugin will add [`close_when_requested`] to [`CoreStage::Update`]. + /// If this system (or a replacement) is not running, the close button will have no effect. + /// This may surprise your users. It is recommended to leave this setting as `true`. + pub close_when_requested: bool, } impl Default for WindowPlugin { fn default() -> Self { WindowPlugin { add_primary_window: true, - exit_on_close: true, + exit_on_all_closed: true, + close_when_requested: true, } } } @@ -42,9 +61,9 @@ impl Plugin for WindowPlugin { app.add_event::() .add_event::() .add_event::() + .add_event::() .add_event::() .add_event::() - .add_event::() .add_event::() .add_event::() .add_event::() @@ -69,8 +88,11 @@ impl Plugin for WindowPlugin { }); } - if self.exit_on_close { - app.add_system(exit_on_window_close_system); + if self.exit_on_all_closed { + app.add_system(exit_on_all_closed); + } + if self.close_when_requested { + app.add_system(close_when_requested); } } } diff --git a/crates/bevy_window/src/system.rs b/crates/bevy_window/src/system.rs index 3dc8f6a45cb9b..45a374fe2e09d 100644 --- a/crates/bevy_window/src/system.rs +++ b/crates/bevy_window/src/system.rs @@ -1,12 +1,57 @@ -use crate::WindowCloseRequested; +use crate::{Window, WindowCloseRequested, WindowFocused, WindowId, Windows}; + use bevy_app::AppExit; -use bevy_ecs::event::{EventReader, EventWriter}; +use bevy_ecs::prelude::*; +use bevy_input::{keyboard::KeyCode, Input}; -pub fn exit_on_window_close_system( - mut app_exit_events: EventWriter, - mut window_close_requested_events: EventReader, -) { - if window_close_requested_events.iter().next().is_some() { +/// Exit the application when there are no open windows. +/// +/// This system is added by the [`WindowPlugin`] in the default configuration. +/// To disable this behaviour, set `close_when_requested` (on the [`WindowPlugin`]) to `false`. +/// Ensure that you read the caveats documented on that field if doing so. +/// +/// [`WindowPlugin`]: crate::WindowPlugin +pub fn exit_on_all_closed(mut app_exit_events: EventWriter, windows: Res) { + if windows.iter().count() == 0 { app_exit_events.send(AppExit); } } + +/// Close windows in response to [`WindowCloseRequested`] (e.g. when the close button is pressed). +/// +/// This system is added by the [`WindowPlugin`] in the default configuration. +/// To disable this behaviour, set `close_when_requested` (on the [`WindowPlugin`]) to `false`. +/// Ensure that you read the caveats documented on that field if doing so. +/// +/// [`WindowPlugin`]: crate::WindowPlugin +pub fn close_when_requested( + mut windows: ResMut, + mut closed: EventReader, +) { + for event in closed.iter() { + windows.get_mut(event.id).map(Window::close); + } +} + +/// Close the focused window whenever the escape key (Esc) is pressed +/// +/// This is useful for examples or prototyping. +pub fn close_on_esc( + mut focused: Local>, + mut focused_events: EventReader, + mut windows: ResMut, + input: Res>, +) { + // TODO: Track this in e.g. a resource to ensure consistent behaviour across similar systems + for event in focused_events.iter() { + *focused = event.focused.then(|| event.id); + } + + if let Some(focused) = &*focused { + if input.just_pressed(KeyCode::Escape) { + if let Some(window) = windows.get_mut(*focused) { + window.close(); + } + } + } +} diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index 47c03d085ce7e..dc4225db9b384 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -219,6 +219,7 @@ pub enum WindowCommand { SetResizeConstraints { resize_constraints: WindowResizeConstraints, }, + Close, } /// Defines the way a window is displayed @@ -571,6 +572,21 @@ impl Window { }); } + /// Close the operating system window corresponding to this [`Window`]. + /// This will also lead to this [`Window`] being removed from the + /// [`Windows`] resource. + /// + /// If the default [`WindowPlugin`] is used, when no windows are + /// open, the [app will exit](bevy_app::AppExit). + /// To disable this behaviour, set `exit_on_all_closed` on the [`WindowPlugin`] + /// to `false` + /// + /// [`Windows`]: crate::Windows + /// [`WindowPlugin`]: crate::WindowPlugin + pub fn close(&mut self) { + self.command_queue.push(WindowCommand::Close); + } + #[inline] pub fn drain_commands(&mut self) -> impl Iterator + '_ { self.command_queue.drain(..) diff --git a/crates/bevy_window/src/windows.rs b/crates/bevy_window/src/windows.rs index acbe92adf6ba5..df20688018133 100644 --- a/crates/bevy_window/src/windows.rs +++ b/crates/bevy_window/src/windows.rs @@ -70,4 +70,8 @@ impl Windows { pub fn iter_mut(&mut self) -> impl Iterator { self.windows.values_mut() } + + pub fn remove(&mut self, id: WindowId) -> Option { + self.windows.remove(&id) + } } diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 438b722cba698..fb77c5fc22b17 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -2,39 +2,38 @@ mod converters; mod winit_config; mod winit_windows; -use bevy_input::{ - keyboard::KeyboardInput, - mouse::{MouseButtonInput, MouseMotion, MouseScrollUnit, MouseWheel}, - touch::TouchInput, -}; pub use winit_config::*; pub use winit_windows::*; use bevy_app::{App, AppExit, CoreStage, Plugin}; +use bevy_ecs::prelude::*; use bevy_ecs::{ - event::{EventWriter, Events, ManualEventReader}, - schedule::ParallelSystemDescriptorCoercion, - system::{NonSend, ResMut}, + event::{Events, ManualEventReader}, world::World, }; +use bevy_input::{ + keyboard::KeyboardInput, + mouse::{MouseButtonInput, MouseMotion, MouseScrollUnit, MouseWheel}, + touch::TouchInput, +}; use bevy_math::{ivec2, DVec2, Vec2}; use bevy_utils::{ - tracing::{error, trace, warn}, + tracing::{error, info, trace, warn}, Instant, }; use bevy_window::{ CreateWindow, CursorEntered, CursorLeft, CursorMoved, FileDragAndDrop, ModifiesWindows, ReceivedCharacter, RequestRedraw, WindowBackendScaleFactorChanged, WindowCloseRequested, - WindowCreated, WindowFocused, WindowMoved, WindowResized, WindowScaleFactorChanged, Windows, + WindowClosed, WindowCreated, WindowFocused, WindowMoved, WindowResized, + WindowScaleFactorChanged, Windows, }; + use winit::{ - dpi::PhysicalPosition, + dpi::{LogicalSize, PhysicalPosition}, event::{self, DeviceEvent, Event, StartCause, WindowEvent}, event_loop::{ControlFlow, EventLoop, EventLoopWindowTarget}, }; -use winit::dpi::LogicalSize; - #[derive(Default)] pub struct WinitPlugin; @@ -51,10 +50,12 @@ impl Plugin for WinitPlugin { } fn change_window( - winit_windows: NonSend, + mut winit_windows: NonSendMut, mut windows: ResMut, mut window_dpi_changed_events: EventWriter, + mut window_close_events: EventWriter, ) { + let mut removed_windows = vec![]; for bevy_window in windows.iter_mut() { let id = bevy_window.id(); for command in bevy_window.drain_commands() { @@ -166,9 +167,25 @@ fn change_window( window.set_max_inner_size(Some(max_inner_size)); } } + bevy_window::WindowCommand::Close => { + // Since we have borrowed `windows` to iterate through them, we can't remove the window from it. + // Add the removal requests to a queue to solve this + removed_windows.push(id); + // No need to run any further commands - this drops the rest of the commands, although the `bevy_window::Window` will be dropped later anyway + break; + } } } } + if !removed_windows.is_empty() { + for id in removed_windows { + // Close the OS window. (The `Drop` impl actually closes the window) + let _ = winit_windows.remove_window(id); + // Clean up our own data structures + windows.remove(id); + window_close_events.send(WindowClosed { id }); + } + } } fn run(event_loop: EventLoop<()>, event_handler: F) -> ! @@ -316,7 +333,8 @@ pub fn winit_runner_with(mut app: App) { let window = if let Some(window) = windows.get_mut(window_id) { window } else { - warn!("Skipped event for unknown Window Id {:?}", winit_window_id); + // If we're here, this window was previously opened + info!("Skipped event for closed window: {:?}", window_id); return; }; winit_state.low_power_event = true; diff --git a/crates/bevy_winit/src/winit_windows.rs b/crates/bevy_winit/src/winit_windows.rs index 65354e80882f8..020b474650924 100644 --- a/crates/bevy_winit/src/winit_windows.rs +++ b/crates/bevy_winit/src/winit_windows.rs @@ -181,6 +181,12 @@ impl WinitWindows { pub fn get_window_id(&self, id: winit::window::WindowId) -> Option { self.winit_to_window_id.get(&id).cloned() } + + pub fn remove_window(&mut self, id: WindowId) -> Option { + let winit_id = self.window_id_to_winit.remove(&id)?; + // Don't remove from winit_to_window_id, to track that we used to know about this winit window + self.windows.remove(&winit_id) + } } pub fn get_fitting_videomode( diff --git a/examples/2d/rotation.rs b/examples/2d/rotation.rs index 8ab5b7fd32050..1eeab613cb034 100644 --- a/examples/2d/rotation.rs +++ b/examples/2d/rotation.rs @@ -18,7 +18,7 @@ fn main() { .with_system(snap_to_player_system) .with_system(rotate_to_player_system), ) - .add_system(bevy::input::system::exit_on_esc_system) + .add_system(bevy::window::close_on_esc) .run(); } diff --git a/examples/games/alien_cake_addict.rs b/examples/games/alien_cake_addict.rs index 7a57886622bcf..4e3c626575616 100644 --- a/examples/games/alien_cake_addict.rs +++ b/examples/games/alien_cake_addict.rs @@ -31,7 +31,7 @@ fn main() { .with_run_criteria(FixedTimestep::step(5.0)) .with_system(spawn_bonus), ) - .add_system(bevy::input::system::exit_on_esc_system) + .add_system(bevy::window::close_on_esc) .run(); } diff --git a/examples/games/breakout.rs b/examples/games/breakout.rs index fbc1bdcd8c94f..06d091f76d93c 100644 --- a/examples/games/breakout.rs +++ b/examples/games/breakout.rs @@ -68,7 +68,7 @@ fn main() { .with_system(play_collision_sound.after(check_for_collisions)), ) .add_system(update_scoreboard) - .add_system(bevy::input::system::exit_on_esc_system) + .add_system(bevy::window::close_on_esc) .run(); } diff --git a/examples/window/multiple_windows.rs b/examples/window/multiple_windows.rs index caa62c36135d3..a0d168c570881 100644 --- a/examples/window/multiple_windows.rs +++ b/examples/window/multiple_windows.rs @@ -18,6 +18,7 @@ fn main() { .add_plugin(SecondWindowCameraPlugin) .add_startup_system(setup) .add_startup_system(create_new_window) + .add_system(bevy::window::close_on_esc) .run(); } diff --git a/tests/window/resizing.rs b/tests/window/resizing.rs index 9c4c422a6a21b..32b192128cdf7 100644 --- a/tests/window/resizing.rs +++ b/tests/window/resizing.rs @@ -1,7 +1,7 @@ //! A test to confirm that `bevy` allows setting the window to arbitrary small sizes //! This is run in CI to ensure that this doesn't regress again. -use bevy::{input::system::exit_on_esc_system, prelude::*}; +use bevy::prelude::*; // The smallest size reached is 1x1, as X11 doesn't support windows with a 0 dimension // TODO: Add a check for platforms other than X11 for 0xk and kx0, despite those currently unsupported on CI. @@ -33,7 +33,7 @@ fn main() { .insert_resource(Phase::ContractingY) .add_system(change_window_size) .add_system(sync_dimensions) - .add_system(exit_on_esc_system) + .add_system(bevy::window::close_on_esc) .add_startup_system(setup_3d) .add_startup_system(setup_2d) .run();