Skip to content

Fix modifier order in keycode string generation #108260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+44 −28
Open

Fix modifier order in keycode string generation #108260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/input/input_event.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,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));
}
Expand Down
44 changes: 19 additions & 25 deletions core/os/keyboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 22 additions & 0 deletions tests/core/input/test_input_event_key.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down