Merge pull request #71328 from reduz/button-shortcuts-no-longer-press

Button shortcuts no longer "press" the Button.

4 files changed, 65 insertions, 28 deletions

diff --git a/doc/classes/BaseButton.xml b/doc/classes/BaseButton.xml
index aedb8f4420..68f62d1c00 100644
--- a/doc/classes/BaseButton.xml
+++ b/doc/classes/BaseButton.xml
@@ -70,7 +70,7 @@
 			[Shortcut] associated to the button.
 		</member>
 		<member name="shortcut_feedback" type="bool" setter="set_shortcut_feedback" getter="is_shortcut_feedback" default="true">
-			If [code]true[/code], the button will appear pressed when its shortcut is activated. If [code]false[/code] and [member toggle_mode] is [code]false[/code], the shortcut will activate the button without appearing to press the button.
+			If [code]true[/code], the button will highlight for a short amount of time when its shortcut is activated. If [code]false[/code] and [member toggle_mode] is [code]false[/code], the shortcut will activate without any visual feedback.
 		</member>
 		<member name="shortcut_in_tooltip" type="bool" setter="set_shortcut_in_tooltip" getter="is_shortcut_in_tooltip_enabled" default="true">
 			If [code]true[/code], the button will add information about its shortcut in the tooltip.
diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml
index f954e8bca8..9e1c751662 100644
--- a/doc/classes/ProjectSettings.xml
+++ b/doc/classes/ProjectSettings.xml
@@ -760,6 +760,9 @@
 		<member name="gui/theme/lcd_subpixel_layout" type="int" setter="" getter="" default="1">
 			LCD subpixel layout used for font anti-aliasing. See [enum TextServer.FontLCDSubpixelLayout].
 		</member>
+		<member name="gui/timers/button_shortcut_feedback_highlight_time" type="float" setter="" getter="" default="0.2">
+			When [member BaseButton.shortcut_feedback] is enabled, this is the time the [BaseButton] will remain highlighted after a shortcut.
+		</member>
 		<member name="gui/timers/incremental_search_max_interval_msec" type="int" setter="" getter="" default="2000">
 			Timer setting for incremental search in [Tree], [ItemList], etc. controls (in milliseconds).
 		</member>
diff --git a/scene/gui/base_button.cpp b/scene/gui/base_button.cpp
index 9cc25bf743..d0326290ac 100644
--- a/scene/gui/base_button.cpp
+++ b/scene/gui/base_button.cpp
@@ -30,6 +30,7 @@
 
 #include "base_button.h"
 
+#include "core/config/project_settings.h"
 #include "core/os/keyboard.h"
 #include "scene/main/window.h"
 #include "scene/scene_string_names.h"
@@ -127,7 +128,6 @@ void BaseButton::_notification(int p_what) {
 			status.hovering = false;
 			status.press_attempt = false;
 			status.pressing_inside = false;
-			status.shortcut_press = false;
 		} break;
 	}
 }
@@ -154,14 +154,10 @@ void BaseButton::on_action_event(Ref<InputEvent> p_event) {
 	if (status.press_attempt && status.pressing_inside) {
 		if (toggle_mode) {
 			bool is_pressed = p_event->is_pressed();
-			if (Object::cast_to<InputEventShortcut>(*p_event)) {
-				is_pressed = false;
-			}
 			if ((is_pressed && action_mode == ACTION_MODE_BUTTON_PRESS) || (!is_pressed && action_mode == ACTION_MODE_BUTTON_RELEASE)) {
 				if (action_mode == ACTION_MODE_BUTTON_PRESS) {
 					status.press_attempt = false;
 					status.pressing_inside = false;
-					status.shortcut_press = false;
 				}
 				status.pressed = !status.pressed;
 				_unpress_group();
@@ -187,7 +183,6 @@ void BaseButton::on_action_event(Ref<InputEvent> p_event) {
 		}
 		status.press_attempt = false;
 		status.pressing_inside = false;
-		status.shortcut_press = false;
 		emit_signal(SNAME("button_up"));
 	}
 
@@ -212,7 +207,6 @@ void BaseButton::set_disabled(bool p_disabled) {
 		}
 		status.press_attempt = false;
 		status.pressing_inside = false;
-		status.shortcut_press = false;
 	}
 	queue_redraw();
 }
@@ -267,6 +261,10 @@ BaseButton::DrawMode BaseButton::get_draw_mode() const {
 		return DRAW_DISABLED;
 	}
 
+	if (in_shortcut_feedback) {
+		return DRAW_HOVER_PRESSED;
+	}
+
 	if (!status.press_attempt && status.hovering) {
 		if (status.pressed) {
 			return DRAW_HOVER_PRESSED;
@@ -285,7 +283,7 @@ BaseButton::DrawMode BaseButton::get_draw_mode() const {
 			pressing = status.pressed;
 		}
 
-		if ((shortcut_feedback || !status.shortcut_press) && pressing) {
+		if (pressing) {
 			return DRAW_PRESSED;
 		} else {
 			return DRAW_NORMAL;
@@ -339,6 +337,14 @@ bool BaseButton::is_keep_pressed_outside() const {
 	return keep_pressed_outside;
 }
 
+void BaseButton::set_shortcut_feedback(bool p_enable) {
+	shortcut_feedback = p_enable;
+}
+
+bool BaseButton::is_shortcut_feedback() const {
+	return shortcut_feedback;
+}
+
 void BaseButton::set_shortcut(const Ref<Shortcut> &p_shortcut) {
 	shortcut = p_shortcut;
 	set_process_shortcut_input(shortcut.is_valid());
@@ -348,13 +354,45 @@ Ref<Shortcut> BaseButton::get_shortcut() const {
 	return shortcut;
 }
 
+void BaseButton::_shortcut_feedback_timeout() {
+	in_shortcut_feedback = false;
+	queue_redraw();
+}
+
 void BaseButton::shortcut_input(const Ref<InputEvent> &p_event) {
 	ERR_FAIL_COND(p_event.is_null());
 
-	if (!is_disabled() && is_visible_in_tree() && !p_event->is_echo() && shortcut.is_valid() && shortcut->matches_event(p_event)) {
-		status.shortcut_press = true;
-		on_action_event(p_event);
+	if (!is_disabled() && p_event->is_pressed() && is_visible_in_tree() && !p_event->is_echo() && shortcut.is_valid() && shortcut->matches_event(p_event)) {
+		if (toggle_mode) {
+			status.pressed = !status.pressed;
+
+			if (status.pressed) {
+				_unpress_group();
+				if (button_group.is_valid()) {
+					button_group->emit_signal(SNAME("pressed"), this);
+				}
+			}
+
+			_toggled(status.pressed);
+			_pressed();
+
+		} else {
+			_pressed();
+		}
+		queue_redraw();
 		accept_event();
+
+		if (shortcut_feedback) {
+			if (shortcut_feedback_timer == nullptr) {
+				shortcut_feedback_timer = memnew(Timer);
+				add_child(shortcut_feedback_timer);
+				shortcut_feedback_timer->set_wait_time(GLOBAL_GET("gui/timers/button_shortcut_feedback_highlight_time"));
+				shortcut_feedback_timer->connect("timeout", callable_mp(this, &BaseButton::_shortcut_feedback_timeout));
+			}
+
+			in_shortcut_feedback = true;
+			shortcut_feedback_timer->start();
+		}
 	}
 }
 
@@ -393,14 +431,6 @@ bool BaseButton::_was_pressed_by_mouse() const {
 	return was_mouse_pressed;
 }
 
-void BaseButton::set_shortcut_feedback(bool p_feedback) {
-	shortcut_feedback = p_feedback;
-}
-
-bool BaseButton::is_shortcut_feedback() const {
-	return shortcut_feedback;
-}
-
 PackedStringArray BaseButton::get_configuration_warnings() const {
 	PackedStringArray warnings = Control::get_configuration_warnings();
 
@@ -429,6 +459,8 @@ void BaseButton::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_draw_mode"), &BaseButton::get_draw_mode);
 	ClassDB::bind_method(D_METHOD("set_keep_pressed_outside", "enabled"), &BaseButton::set_keep_pressed_outside);
 	ClassDB::bind_method(D_METHOD("is_keep_pressed_outside"), &BaseButton::is_keep_pressed_outside);
+	ClassDB::bind_method(D_METHOD("set_shortcut_feedback", "enabled"), &BaseButton::set_shortcut_feedback);
+	ClassDB::bind_method(D_METHOD("is_shortcut_feedback"), &BaseButton::is_shortcut_feedback);
 
 	ClassDB::bind_method(D_METHOD("set_shortcut", "shortcut"), &BaseButton::set_shortcut);
 	ClassDB::bind_method(D_METHOD("get_shortcut"), &BaseButton::get_shortcut);
@@ -436,9 +468,6 @@ void BaseButton::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("set_button_group", "button_group"), &BaseButton::set_button_group);
 	ClassDB::bind_method(D_METHOD("get_button_group"), &BaseButton::get_button_group);
 
-	ClassDB::bind_method(D_METHOD("set_shortcut_feedback", "enabled"), &BaseButton::set_shortcut_feedback);
-	ClassDB::bind_method(D_METHOD("is_shortcut_feedback"), &BaseButton::is_shortcut_feedback);
-
 	GDVIRTUAL_BIND(_pressed);
 	GDVIRTUAL_BIND(_toggled, "button_pressed");
 
@@ -466,6 +495,8 @@ void BaseButton::_bind_methods() {
 
 	BIND_ENUM_CONSTANT(ACTION_MODE_BUTTON_PRESS);
 	BIND_ENUM_CONSTANT(ACTION_MODE_BUTTON_RELEASE);
+
+	GLOBAL_DEF(PropertyInfo(Variant::FLOAT, "gui/timers/button_shortcut_feedback_highlight_time", PROPERTY_HINT_RANGE, "0.01,10,0.01,suffix:s"), 0.2);
 }
 
 BaseButton::BaseButton() {
diff --git a/scene/gui/base_button.h b/scene/gui/base_button.h
index f7c864c5fb..962a16c453 100644
--- a/scene/gui/base_button.h
+++ b/scene/gui/base_button.h
@@ -51,9 +51,9 @@ private:
 	bool shortcut_in_tooltip = true;
 	bool was_mouse_pressed = false;
 	bool keep_pressed_outside = false;
+	bool shortcut_feedback = true;
 	Ref<Shortcut> shortcut;
 	ObjectID shortcut_context;
-	bool shortcut_feedback = true;
 
 	ActionMode action_mode = ACTION_MODE_BUTTON_RELEASE;
 	struct Status {
@@ -61,7 +61,6 @@ private:
 		bool hovering = false;
 		bool press_attempt = false;
 		bool pressing_inside = false;
-		bool shortcut_press = false;
 
 		bool disabled = false;
 
@@ -75,6 +74,10 @@ private:
 
 	void on_action_event(Ref<InputEvent> p_event);
 
+	Timer *shortcut_feedback_timer = nullptr;
+	bool in_shortcut_feedback = false;
+	void _shortcut_feedback_timeout();
+
 protected:
 	virtual void pressed();
 	virtual void toggled(bool p_pressed);
@@ -122,6 +125,9 @@ public:
 	void set_keep_pressed_outside(bool p_on);
 	bool is_keep_pressed_outside() const;
 
+	void set_shortcut_feedback(bool p_enable);
+	bool is_shortcut_feedback() const;
+
 	void set_button_mask(BitField<MouseButtonMask> p_mask);
 	BitField<MouseButtonMask> get_button_mask() const;
 
@@ -133,9 +139,6 @@ public:
 	void set_button_group(const Ref<ButtonGroup> &p_group);
 	Ref<ButtonGroup> get_button_group() const;
 
-	void set_shortcut_feedback(bool p_feedback);
-	bool is_shortcut_feedback() const;
-
 	PackedStringArray get_configuration_warnings() const override;
 
 	BaseButton();