Skip to content
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

Add Key::CTRL_OR_CMD and use it to fix shortcut for tilemap painting tools on macOS #64021

Merged
merged 2 commits into from
Jan 16, 2023
Merged

Add Key::CTRL_OR_CMD and use it to fix shortcut for tilemap painting tools on macOS #64021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4309dff
add CMD_OR_CTRL as a Key and not just a key modifier
ajreckof Aug 6, 2022
fa528b8
replace the use of CTRL by CMD_OR_CTRL in three places where using CT…
ajreckof Jan 15, 2023
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
3 changes: 3 additions & 0 deletions core/os/keyboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,15 @@ static const _KeyCodeText _keycodes[] = {
{Key::CTRL ,"Ctrl"},
#if defined(MACOS_ENABLED)
{Key::META ,"Command"},
{Key::CMD_OR_CTRL ,"Command"},
{Key::ALT ,"Option"},
#elif defined(WINDOWS_ENABLED)
{Key::META ,"Windows"},
{Key::CMD_OR_CTRL ,"Ctrl"},
{Key::ALT ,"Alt"},
#else
{Key::META ,"Meta"},
{Key::CMD_OR_CTRL ,"Ctrl"},
{Key::ALT ,"Alt"},
#endif
{Key::CAPSLOCK ,"CapsLock"},
Expand Down
5 changes: 5 additions & 0 deletions core/os/keyboard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ enum class Key {
SHIFT = SPECIAL | 0x15,
CTRL = SPECIAL | 0x16,
META = SPECIAL | 0x17,
#if defined(MACOS_ENABLED)
CMD_OR_CTRL = META,
#else
CMD_OR_CTRL = CTRL,
#endif
ALT = SPECIAL | 0x18,
CAPSLOCK = SPECIAL | 0x19,
NUMLOCK = SPECIAL | 0x1A,
Expand Down
8 changes: 4 additions & 4 deletions editor/plugins/canvas_item_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1850,7 +1850,7 @@ bool CanvasItemEditor::_gui_input_scale(const Ref<InputEvent> &p_event) {

// Drag resize handles
if (drag_type == DRAG_NONE) {
if (b.is_valid() && b->get_button_index() == MouseButton::LEFT && b->is_pressed() && ((b->is_alt_pressed() && b->is_ctrl_pressed()) || tool == TOOL_SCALE)) {
if (b.is_valid() && b->get_button_index() == MouseButton::LEFT && b->is_pressed() && ((b->is_alt_pressed() && b->is_command_or_control_pressed()) || tool == TOOL_SCALE)) {
List<CanvasItem *> selection = _get_edited_canvas_items();
if (selection.size() == 1) {
CanvasItem *ci = selection[0];
Expand Down Expand Up @@ -1897,7 +1897,7 @@ bool CanvasItemEditor::_gui_input_scale(const Ref<InputEvent> &p_event) {
Transform2D simple_xform = (viewport->get_transform() * unscaled_transform).affine_inverse() * transform;

bool uniform = m->is_shift_pressed();
bool is_ctrl = Input::get_singleton()->is_key_pressed(Key::CTRL);
bool is_ctrl = m->is_ctrl_pressed();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be CTRL or CMD_OR_CTRL here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctrl works fine here as you are already dragging when pushing the control button. I just changed it to the event key modifier as it was more in sync with the other declaration just above and there was no reason to call the Input Singleton.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works, but is it what makes sense semantically? Usually Ctrl being used as modifier on Windows/Linux is intended to be Command on macOS, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the time tes but not always more over something have been Cyril for a long time on Godot so people might have already learned it is Godot so I think for now if it is not causing a bug we should not address it right and open an issue to discuss we want to get rid and of all of them / most of them but keep some edge case.
I think also we might want to discuss wether we would like to add a warning when is_ctrl_pressed is used for people not on a mac to understand it is most likely not what they want to use (if there is a way to disable warning for a line)

I think I will be opening an issue that details everything that I found when I'm done looking through all the is_ctrl_pressed. There are around 50 of them so it might take me a week or so. As I can see it most use cases can be summarised in a few groups. I think those groups should be moved out from ctrl to command but some edge case might better to keep on ctrl.


Point2 drag_from_local = simple_xform.xform(drag_from);
Point2 drag_to_local = simple_xform.xform(drag_to);
Expand Down Expand Up @@ -1989,7 +1989,7 @@ bool CanvasItemEditor::_gui_input_move(const Ref<InputEvent> &p_event) {
if (drag_type == DRAG_NONE) {
//Start moving the nodes
if (b.is_valid() && b->get_button_index() == MouseButton::LEFT && b->is_pressed()) {
if ((b->is_alt_pressed() && !b->is_ctrl_pressed()) || tool == TOOL_MOVE) {
if ((b->is_alt_pressed() && !b->is_command_or_control_pressed()) || tool == TOOL_MOVE) {
List<CanvasItem *> selection = _get_edited_canvas_items();

drag_selection.clear();
Expand Down Expand Up @@ -3418,7 +3418,7 @@ void CanvasItemEditor::_draw_selection() {
}

// Draw the move handles
bool is_ctrl = Input::get_singleton()->is_key_pressed(Key::CTRL);
bool is_ctrl = Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL);
bool is_alt = Input::get_singleton()->is_key_pressed(Key::ALT);
if (tool == TOOL_MOVE && show_transformation_gizmos) {
if (_is_node_movable(ci)) {
Expand Down
2 changes: 1 addition & 1 deletion editor/plugins/texture_region_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ void TextureRegionEditor::_region_input(const Ref<InputEvent> &p_input) {
for (const Rect2 &E : autoslice_cache) {
if (E.has_point(point)) {
rect = E;
if (Input::get_singleton()->is_key_pressed(Key::CTRL) && !(Input::get_singleton()->is_key_pressed(Key(Key::SHIFT | Key::ALT)))) {
if (Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL) && !(Input::get_singleton()->is_key_pressed(Key(Key::SHIFT | Key::ALT)))) {
Rect2 r;
if (atlas_tex.is_valid()) {
r = atlas_tex->get_region();
Expand Down
24 changes: 12 additions & 12 deletions editor/plugins/tiles/tile_map_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,12 +646,12 @@ bool TileMapEditorTilesPlugin::forward_canvas_gui_input(const Ref<InputEvent> &p
}
} else {
// Check if we are picking a tile.
if (picker_button->is_pressed() || (Input::get_singleton()->is_key_pressed(Key::CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT))) {
if (picker_button->is_pressed() || (Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT))) {
drag_type = DRAG_TYPE_PICK;
drag_start_mouse_pos = mpos;
} else {
// Paint otherwise.
if (tool_buttons_group->get_pressed_button() == paint_tool_button && !Input::get_singleton()->is_key_pressed(Key::CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT)) {
if (tool_buttons_group->get_pressed_button() == paint_tool_button && !Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT)) {
drag_type = DRAG_TYPE_PAINT;
drag_start_mouse_pos = mpos;
drag_modified.clear();
Expand All @@ -667,11 +667,11 @@ bool TileMapEditorTilesPlugin::forward_canvas_gui_input(const Ref<InputEvent> &p
tile_map->set_cell(tile_map_layer, coords, E.value.source_id, E.value.get_atlas_coords(), E.value.alternative_tile);
}
_fix_invalid_tiles_in_tile_map_selection();
} else if (tool_buttons_group->get_pressed_button() == line_tool_button || (tool_buttons_group->get_pressed_button() == paint_tool_button && Input::get_singleton()->is_key_pressed(Key::SHIFT) && !Input::get_singleton()->is_key_pressed(Key::CTRL))) {
} else if (tool_buttons_group->get_pressed_button() == line_tool_button || (tool_buttons_group->get_pressed_button() == paint_tool_button && Input::get_singleton()->is_key_pressed(Key::SHIFT) && !Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL))) {
drag_type = DRAG_TYPE_LINE;
drag_start_mouse_pos = mpos;
drag_modified.clear();
} else if (tool_buttons_group->get_pressed_button() == rect_tool_button || (tool_buttons_group->get_pressed_button() == paint_tool_button && Input::get_singleton()->is_key_pressed(Key::SHIFT) && Input::get_singleton()->is_key_pressed(Key::CTRL))) {
} else if (tool_buttons_group->get_pressed_button() == rect_tool_button || (tool_buttons_group->get_pressed_button() == paint_tool_button && Input::get_singleton()->is_key_pressed(Key::SHIFT) && Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL))) {
drag_type = DRAG_TYPE_RECT;
drag_start_mouse_pos = mpos;
drag_modified.clear();
Expand Down Expand Up @@ -742,7 +742,7 @@ void TileMapEditorTilesPlugin::forward_canvas_draw_over_viewport(Control *p_over
// Draw the selection.
if ((tiles_bottom_panel->is_visible_in_tree() || patterns_bottom_panel->is_visible_in_tree()) && tool_buttons_group->get_pressed_button() == select_tool_button) {
// In select mode, we only draw the current selection if we are modifying it (pressing control or shift).
if (drag_type == DRAG_TYPE_MOVE || (drag_type == DRAG_TYPE_SELECT && !Input::get_singleton()->is_key_pressed(Key::CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT))) {
if (drag_type == DRAG_TYPE_MOVE || (drag_type == DRAG_TYPE_SELECT && !Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT))) {
// Do nothing
} else {
Color grid_color = EDITOR_GET("editors/tiles_editor/grid_color");
Expand Down Expand Up @@ -812,7 +812,7 @@ void TileMapEditorTilesPlugin::forward_canvas_draw_over_viewport(Control *p_over
Vector2i coords = tile_map->map_pattern(tile_map->local_to_map(drag_last_mouse_pos - mouse_offset), clipboard_used_cells[i], tile_map_clipboard);
preview[coords] = TileMapCell(tile_map_clipboard->get_cell_source_id(clipboard_used_cells[i]), tile_map_clipboard->get_cell_atlas_coords(clipboard_used_cells[i]), tile_map_clipboard->get_cell_alternative_tile(clipboard_used_cells[i]));
}
} else if (!picker_button->is_pressed() && !(drag_type == DRAG_TYPE_NONE && Input::get_singleton()->is_key_pressed(Key::CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT))) {
} else if (!picker_button->is_pressed() && !(drag_type == DRAG_TYPE_NONE && Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT))) {
bool expand_grid = false;
if (tool_buttons_group->get_pressed_button() == paint_tool_button && drag_type == DRAG_TYPE_NONE) {
// Preview for a single pattern.
Expand Down Expand Up @@ -1246,14 +1246,14 @@ void TileMapEditorTilesPlugin::_stop_dragging() {
undo_redo->create_action(TTR("Change selection"));
undo_redo->add_undo_method(this, "_set_tile_map_selection", _get_tile_map_selection());

if (!Input::get_singleton()->is_key_pressed(Key::SHIFT) && !Input::get_singleton()->is_key_pressed(Key::CTRL)) {
if (!Input::get_singleton()->is_key_pressed(Key::SHIFT) && !Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL)) {
tile_map_selection.clear();
}
Rect2i rect = Rect2i(tile_map->local_to_map(drag_start_mouse_pos), tile_map->local_to_map(mpos) - tile_map->local_to_map(drag_start_mouse_pos)).abs();
for (int x = rect.position.x; x <= rect.get_end().x; x++) {
for (int y = rect.position.y; y <= rect.get_end().y; y++) {
Vector2i coords = Vector2i(x, y);
if (Input::get_singleton()->is_key_pressed(Key::CTRL)) {
if (Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL)) {
if (tile_map_selection.has(coords)) {
tile_map_selection.erase(coords);
}
Expand Down Expand Up @@ -2875,7 +2875,7 @@ bool TileMapEditorTerrainsPlugin::forward_canvas_gui_input(const Ref<InputEvent>
drag_type = DRAG_TYPE_PICK;
} else {
// Paint otherwise.
if (tool_buttons_group->get_pressed_button() == paint_tool_button && !Input::get_singleton()->is_key_pressed(Key::CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT)) {
if (tool_buttons_group->get_pressed_button() == paint_tool_button && !Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT)) {
if (selected_terrain_set < 0 || selected_terrain < 0 || (selected_type == SELECTED_TYPE_PATTERN && !selected_terrains_pattern.is_valid())) {
return true;
}
Expand All @@ -2890,14 +2890,14 @@ bool TileMapEditorTerrainsPlugin::forward_canvas_gui_input(const Ref<InputEvent>
drag_modified[E.key] = tile_map->get_cell(tile_map_layer, E.key);
tile_map->set_cell(tile_map_layer, E.key, E.value.source_id, E.value.get_atlas_coords(), E.value.alternative_tile);
}
} else if (tool_buttons_group->get_pressed_button() == line_tool_button || (tool_buttons_group->get_pressed_button() == paint_tool_button && Input::get_singleton()->is_key_pressed(Key::SHIFT) && !Input::get_singleton()->is_key_pressed(Key::CTRL))) {
} else if (tool_buttons_group->get_pressed_button() == line_tool_button || (tool_buttons_group->get_pressed_button() == paint_tool_button && Input::get_singleton()->is_key_pressed(Key::SHIFT) && !Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL))) {
if (selected_terrain_set < 0 || selected_terrain < 0 || (selected_type == SELECTED_TYPE_PATTERN && !selected_terrains_pattern.is_valid())) {
return true;
}
drag_type = DRAG_TYPE_LINE;
drag_start_mouse_pos = mpos;
drag_modified.clear();
} else if (tool_buttons_group->get_pressed_button() == rect_tool_button || (tool_buttons_group->get_pressed_button() == paint_tool_button && Input::get_singleton()->is_key_pressed(Key::SHIFT) && Input::get_singleton()->is_key_pressed(Key::CTRL))) {
} else if (tool_buttons_group->get_pressed_button() == rect_tool_button || (tool_buttons_group->get_pressed_button() == paint_tool_button && Input::get_singleton()->is_key_pressed(Key::SHIFT) && Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL))) {
if (selected_terrain_set < 0 || selected_terrain < 0 || (selected_type == SELECTED_TYPE_PATTERN && !selected_terrains_pattern.is_valid())) {
return true;
}
Expand Down Expand Up @@ -2982,7 +2982,7 @@ void TileMapEditorTerrainsPlugin::forward_canvas_draw_over_viewport(Control *p_o
tile_xform.set_scale(tile_shape_size);
tile_set->draw_tile_shape(p_overlay, xform * tile_xform, Color(1.0, 1.0, 1.0), false);
}
} else if (!picker_button->is_pressed() && !(drag_type == DRAG_TYPE_NONE && Input::get_singleton()->is_key_pressed(Key::CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT))) {
} else if (!picker_button->is_pressed() && !(drag_type == DRAG_TYPE_NONE && Input::get_singleton()->is_key_pressed(Key::CMD_OR_CTRL) && !Input::get_singleton()->is_key_pressed(Key::SHIFT))) {
bool expand_grid = false;
if (tool_buttons_group->get_pressed_button() == paint_tool_button && drag_type == DRAG_TYPE_NONE) {
// Preview for a single tile.
Expand Down