Replace Ctrl in editor shortcuts with Cmd or Ctrl depending on platform #71905

ajreckof · 2023-01-23T09:33:36Z

following #64021
related issue #64456 (Fix the issue but the issue is for 3.5 so not really a fix to this issue)

After a thorough check of all instances where CTRL is used(is_ctrl_pressed(),Key::CTRL and KeyModifierMask::CTRL) i have sorted the uses in multiple categories here in this PR are three categories where i think modifications should be done :

the first commit consist on modification that aims for better readibility and a clearer code. This is mostly replacing places where CMD_OR_CTRL was reimplemented on the fly and remove him from places where he was checked in conjonction with both CTRL and META
the second commit consist on modification that are needed because the use of CTRL is either preventing MacOS user from using the functionality or makes it inconsistent with either what is said or what is already done else where.
the last commit is about modified actions for dropping when CTRL is pressed. This functionality is used consistently with ctrl but on mac. it seems CMD would be more appropriate especially since you might start pressing the modifier button when starting to drag.

There are a few other places where CTRL is used and i think it should stay like this but i'm not entirely sure see here for all places:

snapping : it is pretty clear when you press control it activates a snap. I think it should stay as CTRL but this is the one i'm the least sure of.
zoom with CTRL+ SCROLL: when scrolling pressing CTRL will enables you to zoom. This is better as CTRL as this is fundamentally not a MacOS way to do it. Therefore even on mac if i think of doing it i would use CTRL as if i was on a windows.
one case where pressing control would switch between scroll and pan movements. I was not sure about this one but it seemed similar to zooming as i would look for CTRL instead of CMD

The problem here is that i currently have only my experience and what feels right to me. So i would really like the opinion of other MacOS users on the subject.

Left to do (maybe another PR):

check all documentation linked to CMD_OR_CTRL to add it is CMD if it is not already done (edit : did everything in the editor part might do the documentation part later)
rename is_command_or_control_pressed() to is_cmd_or_ctrl_pressed()to be more in line with other similar functions (is_ctrl_pressed())

bruvzg · 2023-01-24T06:47:09Z

editor/plugins/abstract_polygon_2d_editor.cpp

@@ -282,7 +282,7 @@ bool AbstractPolygon2DEditor::forward_gui_input(const Ref<InputEvent> &p_event)
 		if (mode == MODE_EDIT || (_is_line() && mode == MODE_CREATE)) {
 			if (mb->get_button_index() == MouseButton::LEFT) {
 				if (mb->is_pressed()) {
-					if (mb->is_ctrl_pressed() || mb->is_shift_pressed() || mb->is_alt_pressed()) {
+					if (mb->is_command_or_control_pressed() || mb->is_shift_pressed() || mb->is_alt_pressed()) {


In this case, the check seems t be no modifiers, so it's probably should be ctrl || meta || shift || alt.

you are totally right went a bit too quick on this one because you can't access it without click left mouse button. Gonna modify it.

ajreckof · 2023-01-26T20:55:12Z

editor/editor_spin_slider.cpp

@@ -215,7 +210,7 @@ void EditorSpinSlider::_value_input_gui_input(const Ref<InputEvent> &p_event) {
 			}
 		}

-		if (k->is_ctrl_pressed()) {
+		if (k->is_command_or_control_pressed()) {


While checking all of the modifications. I realized that this case is totally a case of snaping. I had put it on CMD_OR_CTRL as tooltip already said you should use command even though you had to press ctrl in fact. I'm wondering if i should remove it and move it back to CTRL or if it is a sign cmd should be used for snapping.

ajreckof · 2023-06-08T23:14:48Z

Finally took the time to update this PR so it is up to date. This fix a loot of places where you had to press ctrl instead of command in MAcOS which was inintuitive and in more than half the case just impossible. Most of those bug are not reported or reported for 3.x as most of them were already present in 3.x

Calinou

Tested locally on Linux (rebased on top of master 6758a7f), it works as expected. This needs to be rebased though.

The list of keys changed seems logical to me.

Also, please amend the commit message to follow existing style:

Replace Ctrl in editor shortcuts with Cmd or Ctrl depending on platform

akien-mga · 2023-09-08T06:45:57Z

CC @bruvzg

akien-mga

Looks fine to me overall.

akien-mga · 2023-09-20T11:14:26Z

Thanks!

ajreckof requested review from a team as code owners January 23, 2023 09:33

akien-mga added bug enhancement platform:macos topic:editor usability labels Jan 23, 2023

akien-mga added this to the 4.0 milestone Jan 23, 2023

akien-mga requested a review from bruvzg January 23, 2023 09:56

bruvzg reviewed Jan 26, 2023

View reviewed changes

ajreckof commented Jan 26, 2023

View reviewed changes

ajreckof force-pushed the change_CTRL_for_command_or_control branch from 139a02c to 3b8a29e Compare January 27, 2023 04:13

akien-mga modified the milestones: 4.0, 4.x Feb 10, 2023

ajreckof force-pushed the change_CTRL_for_command_or_control branch 4 times, most recently from 81596e5 to bf31c21 Compare February 15, 2023 15:26

ajreckof requested a review from a team as a code owner February 15, 2023 15:26

ajreckof force-pushed the change_CTRL_for_command_or_control branch from bf31c21 to d05cbb7 Compare February 15, 2023 15:42

ajreckof force-pushed the change_CTRL_for_command_or_control branch 3 times, most recently from 0022592 to 2c84ebb Compare March 3, 2023 19:29

ajreckof force-pushed the change_CTRL_for_command_or_control branch 2 times, most recently from 116eb5d to 805a914 Compare March 4, 2023 03:27

KoBeWi added the breaks compat label Apr 15, 2023

mhilbrunner mentioned this pull request May 5, 2023

Update 01.game_setup.rst key binding on Mac godotengine/godot-docs#6482

Merged

ajreckof force-pushed the change_CTRL_for_command_or_control branch 2 times, most recently from 07e70d6 to da3d1dc Compare June 8, 2023 22:54

akien-mga requested a review from bruvzg June 9, 2023 06:58

Calinou approved these changes Aug 23, 2023

View reviewed changes

ajreckof force-pushed the change_CTRL_for_command_or_control branch 2 times, most recently from 23945c2 to c1013cc Compare September 7, 2023 12:30

akien-mga changed the title ~~Change ctrl for command or control~~ Replace Ctrl in editor shortcuts with Cmd or Ctrl depending on platform Sep 7, 2023

akien-mga modified the milestones: 4.x, 4.2 Sep 7, 2023

Replace Ctrl in editor shortcuts with Cmd or Ctrl depending on platform

6afadba

ajreckof force-pushed the change_CTRL_for_command_or_control branch from c1013cc to 6afadba Compare September 19, 2023 08:29

akien-mga approved these changes Sep 19, 2023

View reviewed changes

akien-mga merged commit bfcfa10 into godotengine:master Sep 20, 2023
16 checks passed

YuriSizov mentioned this pull request Nov 23, 2023

Ctrl-drag on Mac doesn't add @onready variable anymore #85276

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Ctrl in editor shortcuts with Cmd or Ctrl depending on platform #71905

Replace Ctrl in editor shortcuts with Cmd or Ctrl depending on platform #71905

ajreckof commented Jan 23, 2023 •

edited

Loading

bruvzg Jan 24, 2023

ajreckof Jan 26, 2023

ajreckof Jan 26, 2023

ajreckof commented Jun 8, 2023 •

edited

Loading

Calinou left a comment •

edited

Loading

akien-mga commented Sep 8, 2023

akien-mga left a comment

akien-mga commented Sep 20, 2023

Replace Ctrl in editor shortcuts with Cmd or Ctrl depending on platform #71905

Replace Ctrl in editor shortcuts with Cmd or Ctrl depending on platform #71905

Conversation

ajreckof commented Jan 23, 2023 • edited Loading

bruvzg Jan 24, 2023

Choose a reason for hiding this comment

ajreckof Jan 26, 2023

Choose a reason for hiding this comment

ajreckof Jan 26, 2023

Choose a reason for hiding this comment

ajreckof commented Jun 8, 2023 • edited Loading

Calinou left a comment • edited Loading

Choose a reason for hiding this comment

akien-mga commented Sep 8, 2023

akien-mga left a comment

Choose a reason for hiding this comment

akien-mga commented Sep 20, 2023

ajreckof commented Jan 23, 2023 •

edited

Loading

ajreckof commented Jun 8, 2023 •

edited

Loading

Calinou left a comment •

edited

Loading