Move GridMapEditor to bottom panel

[Nodragem]

implements the proposal: godotengine/godot-proposals#10405

new_gridmap_demo.mp4

with:

• minimum changes to the current tools and functionality of Gridmap;
• a cursor that changes color depending on the tool you are using
• I might have corrected some unexpected behaviours along the way (e.g. no tile selected when first opening)

Bugsquad edit:

• Fixes Gridmap spatial gizmo is not responding #19860
• Fixes Grid Map 3D controls overrides camera movement. #74520
• Fixes Tapping Q and E while GridMap node is selected and right-click is pressed down triggers floor change in Editor #87487
• Fixes Gridmap selection only follows world grid and not gridmap #92655
• Closes: Add a GridMap bottom panel (similar to TileMap) godot-proposals#10405

[Nodragem]

the conflict does not seem to be super important, it's about a #include, but I would need to double-check which #include are really needed.

<<<<<<< moving-gridmap-panel-one-commit
#include "editor/editor_command_palette.h"
=======
#include "editor/editor_main_screen.h"
>>>>>>> master

[ShawnHardern]

I think this would be a fantastic addition! 🥳

From an engine usability perspective, this change greatly improves the consistency between the placement of the GridMapEditor and the TileMap (for example).

From personal experience, the difference has been a bit confusing, and this update would make the UI feel much more cohesive and intuitive.

From a maintainer's point of view, was there originally a reason the GridMapEditor was placed on the right and not made consistent with something like the TileMap?

[HeadClot]

I am very happy that grid map is getting some much needed love in the UI/UX Department. Hope to see this get merged for dev 3 or 4.

[Nodragem]

I forgot to mentionned that:

• I resolved the merge conflict (I don't use the minimim width anymore),
• I cleaned the code so that it passes the automated checks,

The merge conflict made me realised that we can probably delete some of the editor settings for Gridmap which aren't relevant anymore.

[smix8]

In general in favor of moving it to the bottom panel. I think having the panel on the bottom works better in terms of gui real-estate.

I tested a little and while overall it worked decent it did not take long to encounter random visual or selection bugs. E.g. a giant version of the new selection cursor appearing at scene load out of nowhere. I also had multiple random instances when playing with the toolbar options the selection wouldn't update correctly after e.g. a fill, move or cutout.

Maybe some of those issues are old and already existed before but since there where no visuals it was hard to notice but now those turned into "in your face" bugs for users. Whatever the case this needs more testing from people to iron those things out. If those selection bugs can be fixed I would like to see this merged.

[Nodragem]

Thank you for your feedback @smix8 ! there are definitely bugs that existed before, but I was waiting to see if there is interest to merge this PR before to spend more time on it.

For instance, the fact that the selection is not well updated after a rotation was not really visible before I fixed the pasting feature in #95322.

E.g. a giant version of the new selection cursor appearing at scene load out of nowhere.

From my testing, it seems to happen when you open Godot with a 3d scene containing a gridmap already opened? Sometimes the camera position is updated quickly and you see the selection cube from wherever the camera is. Sometimes, the camera position is not updated yet, so it is somewhere closer to (0,0,0) and you see a giant cube. I will have a go at fixing it; the selection cube should not be visible until the gridmap is visible.

I also had multiple random instances when playing with the toolbar options the selection wouldn't update correctly after e.g. a fill, move or cutout.

If you can come up with a list of when the selection is not updated correcly, it would be very helpful.

On my side, I noticed these behaviours, some are expected, some can be argued to be unexpected:

• in keep selection = true mode, after a duplicate/move action, the selection does not update correctly if the cut-out/duplicate was rotated (this is a pre-existing bug!).
• in keep selection = false mode, after a duplicate action, the selection remains where it was (expected behaviour)
• in keep selection = false mode, after a move action, the selection is discarded (expected behaviour? sounds okay to me)
• after a fill action (whatever the keep selection is), the selection is discarded (we could argue that we want to keep the selection).

Note that keep selection is this option here:

It was initially called paste selects and I renamed it in an attempt to make it clearer. I am happy to revert the name change if judged unnecessary.

[smix8]

The "big selection cursor" happens for me by just opening a scene with a GridMap node or switching to it. The GridMap node does not even need to be selected or something.

I think this is one of the main issues that should be fixed for this PR because it is something that all users will notice.

I am personally fine with keeping the already existing GridMap bugs for other PRs, rome wasn't (re)build in a day as well.

[Nodragem]

Hello, I had a go with the code today.

Here are the changes/fixes:

• made sure that the cursor is not visible when entering the scene tree (the giant cube issue)
• fixed the rotation of the selection when using keep selection and rotations during move or duplicate action
• reintroduced the clear rotation action in the tool popup menu (quite useful to reset the rotation!!)
• deleted a few lines of code that where obsolete

Before we can merge this PR, I would like to fix a bug that was already there before but that is a little bit worse now:

• when using Gridmap, we can't use the move/rotate/scale tool of the 3D Editor to transform the Gridmap

This issue already existed, but you could use ESC to unselect the current tile in the Gridmap's palette. In this particular state of the Gridmap you could use the move/rotate/scale tool of the 3D editor. This walkaround won't work anymore, and it was not easy to find anyway.

What I would like to do is to add a transform mode on the left to the selection mode:

It will be obvious to find for the users and in this mode, the input will be relegated to the 3D Editor, allowing the move/rotate/scale tool to work.

[Nodragem]

Today I added the Transform Mode which can be toggled with T. This makes it easy for the user to be in a state where they can move, rotate, scale the gridmap.

When in Transform Mode, the Transform gizmo of the 3D Editor will be visible and QWER selects the 3D Editor tools: Node Selection, Move, Rotate and Scale.

When NOT in Transform Mode, the Transform gizmo of the 3D editor is hidden, and QWER selects the Gridmap tools: Selection Mode, Paint Mode, Erased Mode, etc.

Here is a demo (although we don't see which key I am typing!):

demo-transform-tool.mp4

I also fixed the following (pre-existing) bug where the selection floor was not applying the transform of the Gridmap:

[Nodragem]

Speaking with @smix8 today, we think that for this PR, I may have to drop the following feature:

• when not in Transform mode in the Gridmap bottom panel, the Transform gizmo of the 3D Editor is hidden.

This is because I edited the Node3DEditorViewport code to achieve that and it might delay the reviewing process.

EDIT: I might found a solution that is to use the Node3DEditorViewport::set_state(const Dictionary &p_state) function. Will give it a try later.

[Nodragem]

I implemented the new solution to hide/show the transform gizmo, using set_state(). Hence all my changes are now contained within the Gridmap module.

[kitbdev]

Is this the same bug that was fixed?

• Gridmap spatial gizmo is not responding #19860

If so, then related:

• Add separate button for painting in the grid map editor #97664

[Nodragem]

If I understand well, the conflict is about replacing set_icon with set_button_icon, so it should not be too complicated. I will have a look now.

[smix8]

Approving the change to bottom panel. I think it is a good change and consistent with TileMap. The side panel also limited what could be reasonably added in the editor for GridMap considering how already cramped it was around the main Viewport window.

I didn't encounter (new) bugs while testing it except issues that already exists in GridMap and need to be solved in with different PRs.

PR still needs a code review from someone.

[Repiteo]

Thanks!

[geowarin]

@Nodragem This is a really great change, thank you for your amazing work! 😄

However I fear that this reintroduced a bug related to conflicting shortcuts in the editor originally fixed in #79529.

For instance, I have bound the S key to "begin scale transformation" (blender-like shortcuts).

If I'm on the gridmap and I press S to rotate a mesh, it also triggers the scaling of the whole gridmap, making a mess.

[Nodragem]

I will have a look today! Should the fix be a new PR?

[akien-mga]

Should the fix be a new PR?

Yes, as this PR was merged so it can't be edited further.

[Nodragem]

Here is the new PR: #99170
Nice catch @geowarin !

[geowarin]

After using it a bit more, woah, what a glow up!

Here are a couple of random thoughts, for future improvements:

• I'd like the picker tool to automatically switch to paint mode after picking a mesh
• The fill tool only works when in paint mode. Whereas the intuitive thing is to go to select mode, select an area and click fill immediately, which does not work (you have to switch to select, select area, switch to paint, fill)
• I'd love the arrow keys (or maybe pgup, pgdown if there are conflicts) to move to the next and previous mesh for total keyboard control 😄

[Nodragem]

@geowarin Thanks for the feedback and the ideas!

Would you like to leave your ideas for the next steps in this meta-proposal I've made: godotengine/godot-proposals#10992 ? Like that we can discuss all related ideas in one place :) thank you!

Re: fill tool, it works in select mode on my side: I select something and then press Z (or click fill) to fill it with the currently selected mesh.