Merge pull request godotengine#108260 from Silver1063/master

Fix modifier order in keycode string generation

Author: Repiteo

core/input/input_event.cpp

@@ -260,12 +260,12 @@ String InputEventWithModifiers::as_text() const {
 	if (is_ctrl_pressed()) {
 		mod_names.push_back(find_keycode_name(Key::CTRL));
 	}
-	if (is_shift_pressed()) {
-		mod_names.push_back(find_keycode_name(Key::SHIFT));
-	}
 	if (is_alt_pressed()) {
 		mod_names.push_back(find_keycode_name(Key::ALT));
 	}
+	if (is_shift_pressed()) {
+		mod_names.push_back(find_keycode_name(Key::SHIFT));
+	}
 	if (is_meta_pressed()) {
 		mod_names.push_back(find_keycode_name(Key::META));
 	}

core/os/keyboard.cpp

@@ -360,51 +360,45 @@ bool keycode_has_unicode(Key p_keycode) {
 }
 
 String keycode_get_string(Key p_code) {
-	String codestr;
-	if ((p_code & KeyModifierMask::SHIFT) != Key::NONE) {
-		codestr += find_keycode_name(Key::SHIFT);
-		codestr += "+";
+	Vector<String> keycode_string;
+	if ((p_code & KeyModifierMask::CMD_OR_CTRL) != Key::NONE && !(OS::get_singleton()->has_feature("macos") || OS::get_singleton()->has_feature("web_macos") || OS::get_singleton()->has_feature("web_ios"))) {
+		keycode_string.push_back(find_keycode_name(Key::CTRL));
+	}
+	if ((p_code & KeyModifierMask::CTRL) != Key::NONE) {
+		keycode_string.push_back(find_keycode_name(Key::CTRL));
 	}
 	if ((p_code & KeyModifierMask::ALT) != Key::NONE) {
-		codestr += find_keycode_name(Key::ALT);
-		codestr += "+";
+		keycode_string.push_back(find_keycode_name(Key::ALT));
 	}
-	if ((p_code & KeyModifierMask::CMD_OR_CTRL) != Key::NONE) {
-		if (OS::get_singleton()->has_feature("macos") || OS::get_singleton()->has_feature("web_macos") || OS::get_singleton()->has_feature("web_ios")) {
-			codestr += find_keycode_name(Key::META);
-		} else {
-			codestr += find_keycode_name(Key::CTRL);
-		}
-		codestr += "+";
+	if ((p_code & KeyModifierMask::SHIFT) != Key::NONE) {
+		keycode_string.push_back(find_keycode_name(Key::SHIFT));
 	}
-	if ((p_code & KeyModifierMask::CTRL) != Key::NONE) {
-		codestr += find_keycode_name(Key::CTRL);
-		codestr += "+";
+	if ((p_code & KeyModifierMask::CMD_OR_CTRL) != Key::NONE && (OS::get_singleton()->has_feature("macos") || OS::get_singleton()->has_feature("web_macos") || OS::get_singleton()->has_feature("web_ios"))) {
+		keycode_string.push_back(find_keycode_name(Key::META));
 	}
 	if ((p_code & KeyModifierMask::META) != Key::NONE) {
-		codestr += find_keycode_name(Key::META);
-		codestr += "+";
+		keycode_string.push_back(find_keycode_name(Key::META));
 	}
 
 	p_code &= KeyModifierMask::CODE_MASK;
 	if ((char32_t)p_code == 0) {
 		// The key was just a modifier without any code.
-		return codestr;
+		return String("+").join(keycode_string);
 	}
 
+	// The key is a named keycode.
 	const _KeyCodeText *kct = &_keycodes[0];
-
 	while (kct->text) {
 		if (kct->code == p_code) {
-			codestr += kct->text;
-			return codestr;
+			keycode_string.push_back(kct->text);
+			return String("+").join(keycode_string);
 		}
 		kct++;
 	}
 
-	codestr += String::chr((char32_t)p_code);
-
-	return codestr;
+	// The key is a single character.
+	keycode_string.push_back(String::chr((char32_t)p_code));
+	return String("+").join(keycode_string);
 }
 
 Key find_keycode(const String &p_codestr) {

tests/core/input/test_input_event_key.h

@@ -125,6 +125,17 @@ TEST_CASE("[InputEventKey] Key correctly converts itself to text") {
 	none_key.set_physical_keycode(Key::ENTER);
 	CHECK(none_key.as_text() == "Ctrl+Enter (Physical)");
 
+	// Key is None WITH a physical key AND multiple modifiers, checks for correct ordering.
+	none_key.set_alt_pressed(true);
+	none_key.set_shift_pressed(true);
+#ifdef MACOS_ENABLED
+	CHECK(none_key.as_text() != "Ctrl+Shift+Option+Enter (Physical)");
+	CHECK(none_key.as_text() == "Ctrl+Option+Shift+Enter (Physical)");
+#else
+	CHECK(none_key.as_text() != "Ctrl+Shift+Alt+Enter (Physical)");
+	CHECK(none_key.as_text() == "Ctrl+Alt+Shift+Enter (Physical)");
+#endif
+
 	InputEventKey none_key2;
 
 	// Key is None without modifiers with a physical key.
@@ -145,6 +156,17 @@ TEST_CASE("[InputEventKey] Key correctly converts itself to text") {
 	CHECK(key.as_text() != "Space");
 	CHECK(key.as_text() == "Ctrl+Space");
 
+	// Key has keycode and multiple modifiers, checks for correct ordering.
+	key.set_alt_pressed(true);
+	key.set_shift_pressed(true);
+#ifdef MACOS_ENABLED
+	CHECK(key.as_text() != "Ctrl+Shift+Option+Space");
+	CHECK(key.as_text() == "Ctrl+Option+Shift+Space");
+#else
+	CHECK(key.as_text() != "Ctrl+Shift+Alt+Space");
+	CHECK(key.as_text() == "Ctrl+Alt+Shift+Space");
+#endif
+
 	// Since the keycode is set to Key::NONE upon initialization of the
 	// InputEventKey and you can only update it with another Key, the keycode
 	// cannot be empty, so the kc.is_empty() case cannot be tested.