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.

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

Replace all flags with one value when holding Ctrl/Cmd in the layers editor #39364

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Jun 7, 2020

This behavior is inspired by Blender (except it's the other way around to preserve the current default behavior).

Trying to enable a single enabled value with Cmd held will invert the current flags, which makes enabling all flags but one faster.

Preview

On the GIF below, at first, I'm checking boxes as usual, then I hold down Ctrl (Cmd on macOS) to replace existing values.

file

June 2021 patch
From 5155ff04b1615d809051565cbd9b2a04cdccf8b1 Mon Sep 17 00:00:00 2001
From: Hugo Locurcio <[email protected]>
Date: Sun, 7 Jun 2020 15:14:18 +0200
Subject: [PATCH] Replace all flags with one value when holding Cmd in the
 layers editor

This behavior is inspired by Blender (except it's the other way
around to preserve the current default behavior).

Trying to enable a single enabled value with Cmd held will invert the
current flags, which makes enabling all flags but one faster.
---
 editor/editor_properties.cpp | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/editor/editor_properties.cpp b/editor/editor_properties.cpp
index 84105f0cb733..f7b123219e09 100644
--- a/editor/editor_properties.cpp
+++ b/editor/editor_properties.cpp
@@ -590,7 +590,8 @@ class EditorPropertyLayersGrid : public Control {
 	Vector<Rect2> flag_rects;
 	Vector<String> names;
 	Vector<String> tooltips;
-	int hovered_index;
+	// Set to `INT32_MAX` when nothing is hovered.
+	uint32_t hovered_index;
 
 	virtual Size2 get_minimum_size() const override {
 		Ref<Font> font = get_theme_font("font", "Label");
@@ -625,10 +626,22 @@ class EditorPropertyLayersGrid : public Control {
 		if (mb.is_valid() && mb->get_button_index() == MOUSE_BUTTON_LEFT && mb->is_pressed() && hovered_index >= 0) {
 			// Toggle the flag.
 			// We base our choice on the hovered flag, so that it always matches the hovered flag.
-			if (value & (1 << hovered_index)) {
-				value &= ~(1 << hovered_index);
+			if (mb->is_command_pressed()) {
+				// Holding Command will replace all flags with the hovered flag ("solo mode"),
+				// instead of toggling the hovered flags while preserving other flags's state.
+				if (value == 1 << hovered_index) {
+					// If the flag is already enbled, enable all other items and disable the current flag.
+					// This allows for quicker toggling.
+					value = INT32_MAX - (1 << hovered_index);
+				} else {
+					value = 1 << hovered_index;
+				}
 			} else {
-				value |= (1 << hovered_index);
+				if (value & (1 << hovered_index)) {
+					value &= ~(1 << hovered_index);
+				} else {
+					value |= (1 << hovered_index);
+				}
 			}
 
 			emit_signal("flag_changed", value);
@@ -661,7 +674,7 @@ class EditorPropertyLayersGrid : public Control {
 							o.x += 1;
 						}
 
-						const int idx = i * 10 + j;
+						const uint32_t idx = i * 10 + j;
 						const bool on = value & (1 << idx);
 						Rect2 rect2 = Rect2(o, Size2(bsize, bsize));
 
@@ -677,7 +690,7 @@ class EditorPropertyLayersGrid : public Control {
 				}
 			} break;
 			case NOTIFICATION_MOUSE_EXIT: {
-				hovered_index = -1;
+				hovered_index = INT32_MAX; // Nothing is hovered.
 				update();
 			} break;
 			default:
@@ -697,7 +710,7 @@ class EditorPropertyLayersGrid : public Control {
 
 	EditorPropertyLayersGrid() {
 		value = 0;
-		hovered_index = -1; // Nothing is hovered.
+		hovered_index = INT32_MAX; // Nothing is hovered.
 	}
 };
 void EditorPropertyLayers::_grid_changed(uint32_t p_grid) {

@Calinou Calinou added cherrypick:3.x Considered for cherry-picking into a future 3.x release enhancement topic:editor usability labels Jun 7, 2020
@Calinou Calinou added this to the 4.0 milestone Jun 7, 2020
@akien-mga
Copy link
Member

This behavior is inspired by Blender (except it's the other way around to preserve the current default behavior).

Wouldn't that be confusing? The "Hold Shift to select multiple items" behavior is quite common on most OSes, and also used in Godot.

Maybe using a different hotkey would reduce confusion, e.g. Ctrl+Click?

@Calinou Calinou changed the title Replace all flags with one value when holding Shift in the layers editor Replace all flags with one value when holding Cmd in the layers editor Jun 14, 2020
@Calinou
Copy link
Member Author

Calinou commented Jun 14, 2020

I changed the hotkey to from Shift to Ctrl (Cmd on macOS).

@akien-mga
Copy link
Member

I think it might be the same issue with Cmd/Ctrl though, as Ctrl+Click is typically used to add new items to an existing selection (so the opposite to what this feature does).

Eventually it probably doesn't matter so much either way (Shift, Ctrl or another key) as long as we can make this discoverable, which isn't really the case as is. Can we add a tooltip?

@Calinou
Copy link
Member Author

Calinou commented Jun 21, 2021

Rebased and tested again, it works successfully.

I added a new behavior for added convenience:

Trying to enable a single enabled value with Cmd held will invert the current flags, which makes enabling all flags but one faster.

simplescreenrecorder-2021-06-21_18.29.28.mp4

@akien-mga akien-mga requested a review from a team June 21, 2021 19:01
@akien-mga
Copy link
Member

It fails building:

editor/editor_properties.cpp: In member function 'void EditorPropertyLayersGrid::_gui_input(const Ref<InputEvent>&)':
editor/editor_properties.cpp:630:15: error: comparison of integer expressions of different signedness: 'uint32_t' {aka 'unsigned int'} and 'int' [-Werror=sign-compare]
  630 |     if (value == 1 << hovered_index) {
      |         ~~~~~~^~~~~~~~~~~~~~~~~~~~~

@Calinou Calinou force-pushed the editor-flags-solo-mode branch 2 times, most recently from 2c6c3c9 to e9311b9 Compare June 19, 2022 04:19
@clayjohn
Copy link
Member

clayjohn commented Aug 9, 2022

Reviewed in PR review, looks good, just needs to be updated to fix CI errors

@Calinou
Copy link
Member Author

Calinou commented Jul 17, 2023

Rebased and tested again (with CI fixed), it works as expected.

This behavior is inspired by Blender (except it's the other way
around to preserve the current default behavior).

Trying to enable a single enabled value with Cmd held will invert the
current flags, which makes enabling all flags but one faster.
@akien-mga akien-mga merged commit 16b5cd9 into godotengine:master Aug 17, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga changed the title Replace all flags with one value when holding Cmd in the layers editor Replace all flags with one value when holding Ctrl/Cmd in the layers editor Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:3.x Considered for cherry-picking into a future 3.x release enhancement topic:editor usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants