New Input Action Editor and previously hard-coded shortcuts can now be customised.

[EricEzaM]

1. InputEvent changed as_text() to return a display-friendly and human readable string, to be used for UI display. to_string() can be used to get a non-display friendly version which has more information for debugging. Before, some as_text() methods returned a nice readable string, and some displayed more debug-style information. Calinou comment (Can't assign mouse buttons or mouse wheel to editor shortcuts #38326 (comment))
2. Renamed load_from_globals() to load_from_project_settings(). Closes 1 item from [TRACKER] Methods, properties and signals to consider for renaming in next planned compatibility breakage #16863
3. Added ability to optionally store Command or Control (on Windows) or Meta (on Mac). This fixes some cross-platform shortcut issues. For example, previously if you set a keyboard shortcut on Windows to Control+E, this would serialize to command=true, control=true, meaning that on Mac, you would need to press Command AND Control to execute the shortcut, which is not the intention. For this example, now you can choose whether to serialise Command or Control. Discussed at length with Reduz on IRC about this. Closes On Windows, it is impossible to set shortcuts for Mac which use Control/Command #42351
4. Added static methods ::create_reference on InputEventKey and JoypadButton which is a one-line way to do:

Ref<InputEventJoypadButton> ie;
ie.instance();
ie->set_button_index(p_btn_index);

The above code is used in many places with Buttons and Keys, so moving it to a utility one-liner saves A LOT of lines of code, at least a couple hundred.

5. Moved builtin input actions away from being defined twice in ProjectSettings and EditorSettings to only being defined once in InputMap. Project/Editor Settings instead just get the builtin actions and loop through them to add them to their settings list. Reduces code duplication.

6. The main part…

• Made shortcuts which were previously hardcoded customisable.
• Shortcuts that fall into this category are things like cut, copy paste, etc on TextEdit. In the code, these were defined in gui_input with things like
  if (k->is_pressed() && k->get_keycode() == KEY_C && k->get_command() == true){ /* copy */}
• Cleaned up logic... it is much more readable now. The below code example handles 1) moving the cursor left 1 char, 2) moving the cursor left by 1 word 3) moving the cursor to the start of the line (on Apple only) and finally 4) doing the previous 3 with selection (shift) pressed.

godot/scene/gui/text_edit.cpp

Lines 2876 to 2945 in 07112b9

	case KEY_LEFT: {
	if (k->get_shift()) {
	_pre_shift_selection();
	#ifdef APPLE_STYLE_KEYS
	} else {
	#else
	} else if (!k->get_alt()) {
	#endif
	deselect();
	}
	#ifdef APPLE_STYLE_KEYS
	if (k->get_command()) {
	// Start at first column (it's slightly faster that way) and look for the first non-whitespace character.
	int new_cursor_pos = 0;
	for (int i = 0; i < text[cursor.line].length(); ++i) {
	if (!_is_whitespace(text[cursor.line][i])) {
	new_cursor_pos = i;
	break;
	}
	}
	if (new_cursor_pos == cursor.column) {
	// We're already at the first text character, so move to the very beginning of the line.
	cursor_set_column(0);
	} else {
	// We're somewhere to the right of the first text character; move to the first one.
	cursor_set_column(new_cursor_pos);
	}
	} else if (k->get_alt()) {
	#else
	if (k->get_alt()) {
	keycode_handled = false;
	break;
	} else if (k->get_command()) {
	#endif
	int cc = cursor.column;
	if (cc == 0 && cursor.line > 0) {
	cursor_set_line(cursor.line - 1);
	cursor_set_column(text[cursor.line].length());
	} else {
	bool prev_char = false;
	while (cc > 0) {
	bool ischar = _is_text_char(text[cursor.line][cc - 1]);
	if (prev_char && !ischar) {
	break;
	}
	prev_char = ischar;
	cc--;
	}
	cursor_set_column(cc);
	}
	} else if (cursor.column == 0) {
	if (cursor.line > 0) {
	cursor_set_line(cursor.line - num_lines_from(CLAMP(cursor.line - 1, 0, text.size() - 1), -1));
	cursor_set_column(text[cursor.line].length());
	}
	} else {
	cursor_set_column(cursor_get_column() - 1);
	}
	if (k->get_shift()) {
	_post_shift_selection();
	}
	} break;

Quite hard to follow in my opinion. This logic now looks like this, except the following logic can use any key combination, not just the left arrow key.

// In gui_input:
if (k->is_action("ui_text_caret_left")) {
	_move_cursor_left(shift_pressed, false);
	event_accepted = true;
}
if (k->is_action("ui_text_caret_word_left")) {
	_move_cursor_left(shift_pressed, true);
	event_accepted = true;
}
// …
if (k->is_action("ui_text_caret_line_start")) {
	_move_cursor_to_line_start(shift_pressed);
	event_accepted = true;
}
// Other methods in the class:
void TextEdit::_move_cursor_left(bool p_select, bool p_move_by_word);
void TextEdit::_move_cursor_to_line_start(bool p_select);

The keys used to perform these actions are defined in InputMap:

inputs = List<Ref<InputEvent>>();
inputs.push_back(InputEventKey::create_reference(KEY_HOME));
map.insert("ui_text_caret_line_start", inputs);

inputs = List<Ref<InputEvent>>();
inputs.push_back(InputEventKey::create_reference(KEY_A, KEY_MASK_CTRL));
inputs.push_back(InputEventKey::create_reference(KEY_LEFT, KEY_MASK_CMD));
map.insert("ui_text_caret_line_start.OSX", inputs);

Closes #17927
Closes #29490
Closes #12164
Closes #42139
Closes #26854
Closes godotengine/godot-proposals#1532

7. The other main part… rework of the action map editor dialog.

• Completely independent of ProjectSettings, so it can be used anywhere (unlike previous InputMapEditor, which was coupled). Keeps code segregated and more in line with SRP.
• Works on signals, so the parent decides how action changes are handled.
• Added back ability to drag and drop to rearrange actions, and added ability to use D&D for reordering events within actions. (Closes Input map action reordering #16833)
• Built-in actions can now be hidden. (Closes Add ability to remove default input map godot-proposals#303)
• Added filtering of actions (Closes Search and filter input map actions in project settings godot-proposals#203, Closes Cannot search for an action in the project setting input_map #26595)
  
  
  

[EricEzaM]

Marking as draft until all CI passes and more testing is done... but I think it's pretty much there.

[groud]

Regarding the interface:

1. In the Event Configuration popup, the filter and the key display are redundant. Pressing a key should fill in the filter LineEdit, select the correct event and modifiers. For a better UI, I would make the "record button" a single icon at the right of the filter LineEdit. The LineEdit should also be grayed out when the record is active.
2. I don't think the user will understand the difference between Shortcuts and Built-in actions in the editor settings. These should be merged together somehow.

[EricEzaM]

@groud thanks,

1. I have updated the UI as below. The tree will now jump to the correct item depending on the key, joybutton or joyaxis you press. I have kept the label as it is a quick way to see what event you have selected. Your eyes don't need to find the item in the tree, and then also look at the modifier checkboxes to see what mods you have enabled. It is written out clearly at the top so you always know where to look for that information.
   
2. Yeah I wasn't convinced by adding it as another tab but I'm not sure how to do it. The shortcuts editor and the actions editor are different in the way they work (and need to be). I could merge them into one tab, but there would still be some toggle to show the "built-in" options instead of the editor shortcuts.

[groud]

I have kept the label as it is a quick way to see what event you have selected. Your eyes don't need to find the item in the tree, and then also look at the modifier checkboxes to see what mods you have enabled. It is written out clearly at the top so you always know where to look for that information.

What I suggested is to use the filter field as a label. As you only need to use the filter when you are not recording, the filter label should be used to show the entered keys combination when recording. The LineEdit should also be disabled (grayed out) when you are recording. So to sum up:

• Recording state: the LineEdit is disabled and show the combination of keys
• Not recording state: the LineEdit is editable. It might also be selected automatically on quitting recording mode, and the text fully selected so that it can be replaced fast (but not removed, we want to keep the selected key in the recorded mode).

Yeah I wasn't convinced by adding it as another tab but I'm not sure how to do it. The shortcuts editor and the actions editor are different in the way they work (and need to be). I could merge them into one tab, but there would still be some toggle to show the "built-in" options instead of the editor shortcuts.

I think a global category at the top of the shortcut list should work. No need to hide them there, it wont bother users there, unlike in the project input mapping.

[Leleat]

#41998 is another case of hardcoded shortcuts.

[Paulb23]

Overall nice work! Spotted some issues I've noted inline, will have to do some local testing.

One thing to note this will probably clash with #41100

[EricEzaM]

One thing to note this will probably clash with #41100

Holy... that is a monster PR.

Thanks for the feedback - I'll take a look and action it.

[EricEzaM]

I do plan to finish this at some point...

I'm just a bit stuck on how to unify the "Shortcuts" and "Built-in Actions" for the editor... Got stuck on that, never thought of a good way of doing it, got distracted by other stuff, never finished this PR so as a result haven't been very active in contributing other new stuff.

Any ideas welcomed 😃

[Paulb23]

Would something like this work?

[EricEzaM]

@Paulb23 is that 2 separate scroll containers or one big one which contains both general and editor?

[Paulb23]

One scroll container, though if it's easier two shouldn't be an issue.

[EricEzaM]

I decided to go for a Hsplit container. When I had it in a vsplit the labels for Editor/Built-in shortcuts got lost visually.

Thoughts?

[akien-mga]

There's a misunderstanding here, built-in actions are not editor settings. They're part of the core runtime and used by various Controls for their internal logic (e.g. dialogs, buttons, tabs, etc.).

So it doesn't make sense to allow configuring them in the Editor Settings dialog when they're project-related, and any change to their value would and should influence the behavior of Controls used in the game.

Similarly, different games could use different bindings for the built-in actions, so configuring them editor-wide is not an option.

[EricEzaM]

@akien-mga I understand that. You can edit them in both project and editor settings. Project settings affects the actual game you are making, editor settings changes the editor.

For example, if a user wanted to change the editor shortcut for copying, they would change the ui_copy builtin action in the editor settings. If they changed that in the project settings, it would change the shortcuts for their game, but the editor would still use the default (ctrl+c)

[Paulb23]

To me that looks a little crowded, though I'm not totally against it. Not quite sure which direction to take it from here.

[EricEzaM]

@Paulb23 I agree honestly... it is a little bit crowded.

But I am not sure how else to do it. Would it be the worst thing to only have one visible at a time? Not in a completely separate tab like I originally had it, but instead just hiding/showing dependent on a button status... kind of like this, except with "Editor & Builtin" rather than "Game & Engine":

I guess the confusing part is that they are all "Shortcuts" but some of them apply to specific editor areas, the "Builtin/Editor" ones apply to all controls like textedit/lineedit/graphedit, etc. So, should they be separated or not?

[Paulb23]

Yeah the thought did cross my mind to remove the editor shortcuts altogether and move it all to the editor input map now it's available, this way we'd only have one system to maintain. Though that's probably a separate proposal.

I would definitely keep them in the editor settings as it's not changed per project. Personally not a big fan of subtabbing as there's not that much difference between that, and having them as separate main tabs. Not sure if @groud has any thoughts/preference on the UI?

[EricEzaM]

How's this? I added functionality to the existing Editor Shortcuts editor, rather than trying to re-use the action map editor.

[akien-mga]

Hm.

[Zireael07]

looks like bad merge?

[EricEzaM]

Whoops lol.

I haven't pushed the code from my latest changes btw (like the functionality shown in the shortcuts editor Gif)

[akien-mga]

Some feedback from IRC:

• I'm not convinced by the UX of having the removal of bindings done via a "Cross" icon popup menu, as this is usually used to remove the whole line and not one specific binding in it. That's how the current InputMapEditor works: the "Bin" icon on an action name will remove the action, on a binding line it will remove the binding:
  
  That editor has its own bad UX issues though, but I think we should aim to solve this. It could be done in a follow up PR though after this is merged.

• On that topic, I think this PR is difficult to review as it mixes a lot of unrelated changes, all meaningful for the final output (being able to change more shortcuts in the editor), but without giving us good options to review the intermediate changes which are as if not more important than this final outcome. Therefore I suggest to split those intermediate changes in their own PRs so that they can be reviewed for their own merit, tested and merged independently of this new editor plugin:

  • Commit 1 (as_text() / to_string() is only used by the final commit and for cosmetic purposes (to_string() itself is not used at all in this PR). It conflicts with Update joy button and stick names, enums and documentation #43591 so depending on which one is merged first, there will be rebase work needed for this specifically, independently of the rest. As such I guess it could be in a standalone PR as it doesn't seem necessary for this one, even if it impacts what strings are shown to the user in the end.

  • Commit 2 ("Rename load_from_globals") is a no-brainer and is actually unrelated to the rest of this PR, as there's no code using it. It should be in its own PR and could be merged right away.

  • Commit 3 ("Optional serialization of Command") should be in its own PR, it's an important change to the Input code and needs to be reviewed and tested for its own merit. It might be worth cherry-picking for 3.2 if it doesn't break compat, but also it needs proper design review to make sure that this is the correct approach to solve this problem. Its optional feature is important to complement the work done here on configuration of Control shortcuts, and to fix cross-platform discrepancies, but it's not directly required by the later commits in this PR AFAICT. It is used though and can be kept in this PR too while it's being reviewed in a separate PR, to avoid having to undo/redo work that depends on it. This PR can then be rebased once its merged, or modified if further changes are requested for this specifically.

  • Commit 4 ("Static create_reference helper") adds a helper method used in commit 5, 6 and 7. It can stay here.

  • Commit 5 ("Dehardcode built-in input actions in Controls") is IMO the most important part of this proposal, and I'd prefer it to be reviewed as standalone. It was done for editor configuration purposes, but it also makes all these actions configurable for projects, which doesn't require extra work on the existing InputMapEditor to make use of them. All this is pretty core and needs to be reviewed thoroughly (which @Paulb23 already did in part, but others should have a close look too). That commit depends on commit 4 so both should go in a PR together. Commits 6 and 7 depend on the changes in commit 5 so it should also stay in this PR, which can be rebased once the separate PR is merged.

  • Commit 6 and 7 are related and are the directly visible change from this PR (New Input Action Editor with customizable hard-coded editor shortcuts) and should of course stay here.

TL;DR:

• Make a PR with commit 1 - I would suggest removing it from this PR so it can be treated independently. I checked and there's no merge conflicts when removing it.
• Make a PR with commit 2, can be merged right away. A tiny rebase will be needed once merge but it's trivial.
• Make a PR with commit 3. set_store_command is used in ActionMapEditor so you can keep the commit here too, and rebase once its own PR is merged.
• Make a PR with commits 4 and 5. Keep them here too as commits 6 and 7 depend on them.

[EricEzaM]

@akien-mga Done, as you can see from the mentions above! :)

I think for the UI stuff I will wait for the above PRs to to be merged so that there is less to review in the UI PR, since it relies on a lot of it.

[EricEzaM]

Closed as this has been split into smaller PRs.