Gridmap selection hotkeys no longer working since 4.2

[TiagoKuribara]

Tested versions

Reproducible in 4.2 onwards. it was working in Godot 3, and it was working up until 4.1.3

System information

Godot v4.2.stable - Windows 10.0.22621 - Vulkan (Forward+) - integrated AMD Radeon(TM) Graphics (Advanced Micro Devices, Inc.; 31.0.12044.56000) - AMD Ryzen 7 5700U with Radeon Graphics (16 Threads)

Issue description

You can hold shift and hold the left mouse button to select cells in a gridmap. While holding shift, you used to be able to be able to select multiple floors by also pressing the "e" or "q" keys, as seem in the video below:

https://www.youtube.com/watch?v=xond8D3mf2w&feature=youtu.be

But since 4.2, this stopped working, and now nothing happens, as seem in the video below:

https://www.youtube.com/watch?v=S2RFTIe3XoQ&feature=youtu.be

I am pressing "q" and "e" in the video, but nothing happens

Steps to reproduce

• Create a Scene with a GridMap
• Create a MeshLibrary and put it in the GridMap
• Try to select multiple floors by holding shift, holding the left mouse button and pressing either "q" or "e"

Minimal reproduction project (MRP)

N/A

[akien-mga]

We keep getting conflicting reports for this in #67497, where for some it worked before and no longer works in 4.2, and for others it didn't work in 4.1 and it works now. Needs investigating.

Which keyboard layout are you using?

[Calinou]

I can confirm this in 4.2.stable (Linux, fr-oss AZERTY keyboard layout).

In comparison, you could do this in 4.1.2.stable:

simplescreenrecorder-2024-01-12_15.12.30.mp4

The issue appeared between 4.2.dev3 and 4.2.dev4. I'll look into bisecting this.

Edit: I bisected the regression to f80f4eb. cc @geowarin

[KoBeWi]

Might be caused by #79529

[TiagoKuribara]

We keep getting conflicting reports for this in #67497, where for some it worked before and no longer works in 4.2, and for others it didn't work in 4.1 and it works now. Needs investigating.

Which keyboard layout are you using?

Im using a brazilian qwerty keyboard

[geowarin]

Edit: I bisected the regression to f80f4eb. cc @geowarin

Yeah, I never knew that was a feature! My bad.

Seems the code responsible for this feature is here:

godot/modules/gridmap/editor/grid_map_editor_plugin.cpp

Lines 759 to 770 in 1a1c542

	if (k->is_shift_pressed() && selection.active && input_action != INPUT_PASTE) {
	if (k->get_keycode() == (Key)options->get_popup()->get_item_accelerator(options->get_popup()->get_item_index(MENU_OPTION_PREV_LEVEL))) {
	selection.click[edit_axis]--;
	_validate_selection();
	return EditorPlugin::AFTER_GUI_INPUT_STOP;
	}
	if (k->get_keycode() == (Key)options->get_popup()->get_item_accelerator(options->get_popup()->get_item_index(MENU_OPTION_NEXT_LEVEL))) {
	selection.click[edit_axis]++;
	_validate_selection();
	return EditorPlugin::AFTER_GUI_INPUT_STOP;
	}
	}

But it can't be executed because of the block that I added a few lines above:

godot/modules/gridmap/editor/grid_map_editor_plugin.cpp

Lines 747 to 757 in 1a1c542

	// Consume input to avoid conflicts with other plugins.
	if (k.is_valid() && k->is_pressed() && !k->is_echo()) {
	for (int i = 0; i < options->get_popup()->get_item_count(); ++i) {
	const Ref<Shortcut> &shortcut = options->get_popup()->get_item_shortcut(i);
	if (shortcut.is_valid() && shortcut->matches_event(p_event)) {
	accept_event();
	_menu_option(options->get_popup()->get_item_id(i));
	return EditorPlugin::AFTER_GUI_INPUT_STOP;
	}
	}
	}

(this will return if "q" or "e" are pressed)

The two blocks of code probably have to be merged now (or swapped?), I'll take a stab at it tomorrow

[geowarin]

Mmm.... turns out that adding ED_SHORTCUT to the options menu makes this dead code.

(Key)options->get_popup()->get_item_accelerator(options->get_popup()->get_item_index(MENU_OPTION_PREV_LEVEL))

return KEY::NONE

My first idea was to use get_item_shortcut() and then match it with shortcut->matches_event(p_event) but they don't match.

My guess here is since, the shift key also is pressed when the event is triggered, it is not a match for the shortcut (that does not include the shift key).

full code that I tried for clarity:

if (k->is_shift_pressed() && selection.active && input_action != INPUT_PASTE) {
    const Ref<Shortcut> &shortcut1 = options->get_popup()->get_item_shortcut(options->get_popup()->get_item_index(MENU_OPTION_PREV_LEVEL));
    if (shortcut1.is_valid() && shortcut1->matches_event(p_event)) {
        // NOPE
    }
}

Any idea? @Calinou @KoBeWi

[KoBeWi]

I investigated and you don't need to hold Shift all the time, only to start selecting. While selection is active, you can release Shift and use the shortcuts normally.
Although it doesn't work correctly, the code quoted above needs to be moved somewhere else.