Merge pull request #101895 from Hilderin/fix-close-request-notification-embedded-game

Fix NOTIFICATION_WM_CLOSE_REQUEST in Embedded Floating Window

Author: Repiteo

editor/plugins/game_view_plugin.cpp

@@ -652,14 +652,20 @@ void GameView::_update_arguments_for_instance(int p_idx, List<String> &r_argumen
 	r_arguments.insert_after(N, itos(rect.size.x) + "x" + itos(rect.size.y));
 }
 
-void GameView::_window_before_closing() {
+void GameView::_window_close_request() {
 	// Before the parent window closed, we close the embedded game. That prevents
 	// the embedded game to be seen without a parent window for a fraction of second.
 	if (EditorRunBar::get_singleton()->is_playing() && (embedded_process->is_embedding_completed() || embedded_process->is_embedding_in_progress())) {
+		// Try to gracefully close the window. That way, the NOTIFICATION_WM_CLOSE_REQUEST
+		// notification should be propagated in the game process.
 		embedded_process->reset();
-		// Call deferred to prevent the _stop_pressed callback to be executed before the wrapper window
-		// actually closes.
-		callable_mp(EditorRunBar::get_singleton(), &EditorRunBar::stop_playing).call_deferred();
+
+		// When the embedding is not complete, we need to kill the process.
+		if (embedded_process->is_embedding_in_progress()) {
+			// Call deferred to prevent the _stop_pressed callback to be executed before the wrapper window
+			// actually closes.
+			callable_mp(EditorRunBar::get_singleton(), &EditorRunBar::stop_playing).call_deferred();
+		}
 	}
 }
 
@@ -829,7 +835,8 @@ GameView::GameView(Ref<GameViewDebugger> p_debugger, WindowWrapper *p_wrapper) {
 	p_debugger->connect("session_started", callable_mp(this, &GameView::_sessions_changed));
 	p_debugger->connect("session_stopped", callable_mp(this, &GameView::_sessions_changed));
 
-	p_wrapper->connect("window_before_closing", callable_mp(this, &GameView::_window_before_closing));
+	p_wrapper->set_override_close_request(true);
+	p_wrapper->connect("window_close_requested", callable_mp(this, &GameView::_window_close_request));
 	p_wrapper->connect("window_size_changed", callable_mp(this, &GameView::_update_floating_window_settings));
 }
 

editor/plugins/game_view_plugin.h

@@ -162,7 +162,7 @@ class GameView : public VBoxContainer {
 	void _camera_override_button_toggled(bool p_pressed);
 	void _camera_override_menu_id_pressed(int p_id);
 
-	void _window_before_closing();
+	void _window_close_request();
 	void _update_floating_window_settings();
 	void _attach_script_debugger();
 	void _detach_script_debugger();

editor/window_wrapper.cpp

@@ -101,12 +101,6 @@ void WindowWrapper::_set_window_enabled_with_rect(bool p_visible, const Rect2 p_
 
 	Node *parent = _get_wrapped_control_parent();
 
-	// In the GameView plugin, we need to the the signal before the window is actually closed
-	// to prevent the embedded game to be seen the parent window for a fraction of a second.
-	if (!p_visible) {
-		emit_signal("window_before_closing");
-	}
-
 	if (wrapped_control->get_parent() != parent) {
 		// Move the control to the window.
 		wrapped_control->reparent(parent, false);
@@ -120,7 +114,7 @@ void WindowWrapper::_set_window_enabled_with_rect(bool p_visible, const Rect2 p_
 	}
 
 	window->set_visible(p_visible);
-	if (!p_visible) {
+	if (!p_visible && !override_close_request) {
 		emit_signal("window_close_requested");
 	}
 	emit_signal("window_visibility_changed", p_visible);
@@ -141,10 +135,17 @@ void WindowWrapper::_window_size_changed() {
 	emit_signal(SNAME("window_size_changed"));
 }
 
+void WindowWrapper::_window_close_request() {
+	if (override_close_request) {
+		emit_signal("window_close_requested");
+	} else {
+		set_window_enabled(false);
+	}
+}
+
 void WindowWrapper::_bind_methods() {
 	ADD_SIGNAL(MethodInfo("window_visibility_changed", PropertyInfo(Variant::BOOL, "visible")));
 	ADD_SIGNAL(MethodInfo("window_close_requested"));
-	ADD_SIGNAL(MethodInfo("window_before_closing"));
 	ADD_SIGNAL(MethodInfo("window_size_changed"));
 }
 
@@ -330,6 +331,10 @@ void WindowWrapper::grab_window_focus() {
 	}
 }
 
+void WindowWrapper::set_override_close_request(bool p_enabled) {
+	override_close_request = p_enabled;
+}
+
 WindowWrapper::WindowWrapper() {
 	if (!EditorNode::get_singleton()->is_multi_window_enabled()) {
 		return;
@@ -342,7 +347,7 @@ WindowWrapper::WindowWrapper() {
 	add_child(window);
 	window->hide();
 
-	window->connect("close_requested", callable_mp(this, &WindowWrapper::set_window_enabled).bind(false));
+	window->connect("close_requested", callable_mp(this, &WindowWrapper::_window_close_request));
 	window->connect("size_changed", callable_mp(this, &WindowWrapper::_window_size_changed));
 
 	ShortcutBin *capturer = memnew(ShortcutBin);

editor/window_wrapper.h

@@ -50,12 +50,15 @@ class WindowWrapper : public MarginContainer {
 
 	Ref<Shortcut> enable_shortcut;
 
+	bool override_close_request = false;
+
 	Rect2 _get_default_window_rect() const;
 	Node *_get_wrapped_control_parent() const;
 
 	void _set_window_enabled_with_rect(bool p_visible, const Rect2 p_rect);
 	void _set_window_rect(const Rect2 p_rect);
 	void _window_size_changed();
+	void _window_close_request();
 
 protected:
 	static void _bind_methods();
@@ -84,6 +87,8 @@ class WindowWrapper : public MarginContainer {
 	void set_margins_enabled(bool p_enabled);
 	void grab_window_focus();
 
+	void set_override_close_request(bool p_enabled);
+
 	WindowWrapper();
 	~WindowWrapper();
 };

platform/linuxbsd/x11/display_server_x11.cpp

@@ -5849,6 +5849,24 @@ Error DisplayServerX11::remove_embedded_process(OS::ProcessID p_pid) {
 	}
 
 	EmbeddedProcessData *ep = embedded_processes.get(p_pid);
+
+	// Handle bad window errors silently because just in case the embedded window was closed.
+	int (*oldHandler)(Display *, XErrorEvent *) = XSetErrorHandler(&bad_window_error_handler);
+
+	// Send the message to gracefully close the window.
+	XEvent ev;
+	memset(&ev, 0, sizeof(ev));
+	ev.xclient.type = ClientMessage;
+	ev.xclient.window = ep->process_window;
+	ev.xclient.message_type = XInternAtom(x11_display, "WM_PROTOCOLS", True);
+	ev.xclient.format = 32;
+	ev.xclient.data.l[0] = XInternAtom(x11_display, "WM_DELETE_WINDOW", False);
+	ev.xclient.data.l[1] = CurrentTime;
+	XSendEvent(x11_display, ep->process_window, False, NoEventMask, &ev);
+
+	// Restore default error handler.
+	XSetErrorHandler(oldHandler);
+
 	embedded_processes.erase(p_pid);
 	memdelete(ep);
 

platform/windows/display_server_windows.cpp

@@ -2975,6 +2975,9 @@ Error DisplayServerWindows::remove_embedded_process(OS::ProcessID p_pid) {
 
 	EmbeddedProcessData *ep = embedded_processes.get(p_pid);
 
+	// Send a close message to gracefully close the process.
+	PostMessage(ep->window_handle, WM_CLOSE, 0, 0);
+
 	// This is a workaround to ensure the parent window correctly regains focus after the
 	// embedded window is closed. When the embedded window is closed while it has focus,
 	// the parent window (the editor) does not become active. It appears focused but is not truly activated.