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

GridMap editor fixes and improvements #25406

Merged
merged 1 commit into from
May 1, 2019

Conversation

aqnuep
Copy link
Contributor

@aqnuep aqnuep commented Jan 28, 2019

This change fixes a few outstanding issues and greatly improves the usability
of the GridMap editor through the following changes:

  • Copied mesh now gets displayed during pasting (also renamed the related
    identifiers accordingly)
  • Duplication/paste indicator now gets rotated around the correct pivot point
    (duplication worked properly before, but the indicator was shown misplaced
    when rotated)
  • Selected mesh library item cursor is no longer shown during selection and
    duplication/pasting
  • Back rotate X/Y/Z is now working during duplication/pasting
  • Added true cut operation thanks to now having a proper clipboard (clear
    operation got remapped to the DEL key)
  • Got rid of some weird workarounds in the duplication code
  • Fill and clear operations now correctly make the selection marker inactive
    as this was broken partly due to the workarounds mentioned above
    (duplication continues to keep the selection marker active to allow
    subsequent duplications)
  • Clear current selection on RMB, but treat selection as an action so previous
    selection can be restored on undo
  • Separated selection and paste indicator data as it's prone to error and
    confusion and it's anyway needed now that selection is treated as an action
  • Added support for cancelling paste, selection, and even unselect the
    currently selected mesh library item with the ESC key (previously there
    wasn't a way to unselect)
  • Changed the key binding of fill/clear/duplicate operations to use Ctrl as a
    modifier
  • Changed erase to use RMB instead of Shift+RMB (free look is available
    through Shift+F anyway, so no need to occupy RMB for it during gridmap
    editing)
  • Removed unused area, external connector, and configure menu items (there's
    also the non-functional clip mode menu items, but I'm not sure whether there
    are any plans with that, I suppose it's meant to be an editor aid)
  • Renamed INPUT_COPY to INPUT_PICK to better reflect its purpose
  • Added support for using Shift+Q and Shift+E to select multiple floors/planes
    without actually changing the current floor/plane as it happens when using
    e.g. the mouse wheel

Fixes #25373 and fixes #15883

@akien-mga
Copy link
Member

Great work, thanks!

There seems to be an unused variable:

modules/gridmap/grid_map_editor_plugin.cpp: In member function 'void GridMapEditor::_set_clipboard_data()':
modules/gridmap/grid_map_editor_plugin.cpp:556:9: error: unused variable 'orientation' [-Werror=unused-variable]
     int orientation = node->get_cell_item_orientation(i, j, k);
         ^

@aqnuep
Copy link
Contributor Author

aqnuep commented Jan 28, 2019

Thanks, I just noticed the builds failed due to that. Apparently the generated VS solution doesn't generate an error on that, probably that's worth fixing too.

@akien-mga
Copy link
Member

Apparently the generated VS solution doesn't generate an error on that, probably that's worth fixing too.

It might if you build with werror=yes, but it's off by default.

modules/gridmap/grid_map_editor_plugin.cpp Outdated Show resolved Hide resolved
Ref<InputEventKey> k = p_event;

if (k.is_valid()) {
if (k->is_pressed()) {
Copy link
Member

Choose a reason for hiding this comment

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

These can be combined into if (k.is_valid() && k->is_pressed()) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, you can, but if we'd ever add an else branch that'd be only needed for (k->is_pressed()) only (to handle key release), so I'd better keep this as is.

@akien-mga
Copy link
Member

Changes look pretty good to me. A rebase is needed, and ideally a review by someone more familiar with GridMap than I am.

This change fixes a few outstanding issues and greatly improves the usability
of the GridMap editor through the following changes:

- Copied mesh now gets displayed during pasting (also renamed the related
  identifiers accordingly)
- Duplication/paste indicator now gets rotated around the correct pivot point
  (duplication worked properly before, but the indicator was shown misplaced
  when rotated)
- Selected mesh library item cursor is no longer shown during selection and
  duplication/pasting
- Back rotate X/Y/Z is now working during duplication/pasting
- Added true cut operation thanks to now having a proper clipboard (clear
  operation got remapped to the DEL key)
- Got rid of some weird workarounds in the duplication code
- Fill and clear operations now correctly make the selection marker inactive
  as this was broken partly due to the workarounds mentioned above
  (duplication continues to keep the selection marker active to allow
  subsequent duplications)
- Clear current selection on RMB, but treat selection as an action so previous
  selection can be restored on undo
- Separated selection and paste indicator data as it's prone to error and
  confusion and it's anyway needed now that selection is treated as an action
- Added support for cancelling paste, selection, and even unselect the
  currently selected mesh library item with the ESC key (previously there
  wasn't a way to unselect)
- Changed the key binding of fill/clear/duplicate operations to use Ctrl as a
  modifier
- Changed erase to use RMB instead of Shift+RMB (free look is available
  through Shift+F anyway, so no need to occupy RMB for it during gridmap
  editing)
- Removed unused area, external connector, and configure menu items (there's
  also the non-functional clip mode menu items, but I'm not sure whether there
  are any plans with that, I suppose it's meant to be an editor aid)
- Renamed INPUT_COPY to INPUT_PICK to better reflect its purpose
- Added support for using Shift+Q and Shift+E to select multiple floors/planes
  without actually changing the current floor/plane as it happens when using
  e.g. the mouse wheel

Fixes godotengine#25373 and godotengine#15883
@aqnuep
Copy link
Contributor Author

aqnuep commented Apr 30, 2019

Changes look pretty good to me. A rebase is needed, and ideally a review by someone more familiar with GridMap than I am.

Rebased.

Copy link
Contributor

@JFonS JFonS left a comment

Choose a reason for hiding this comment

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

I'm not a Grimap expert but the overall code looks good and I haven't found any weird issues after testing it for a while. So LGTM.

@akien-mga akien-mga merged commit 3cfab06 into godotengine:master May 1, 2019
@akien-mga
Copy link
Member

Thanks a lot!

@JFonS
Copy link
Contributor

JFonS commented May 1, 2019

@aqnuep Such a great work btw, Gridmap definitely needed some love :)

dmlary added a commit to dmlary/godot that referenced this pull request May 19, 2024
Editing along the Z-axis with hex cells results in unexpected behavior
for users as cells will be placed in a zig-zag pattern.  This is because
with pointy orientation, the cell centers are offset every other row as
you move along the Z-axis.

In this commit, I added support for editing cells along the Q-axis
(northwest to southeast), and S-axis (southwest to northeast).  The
terms come from the axial coordinate system used in GridMap, and
derrived from:
https://www.redblobgames.com/grids/hexagons/#coordinates-axial

The Z and C hotkeys were repurposed to rotate around the various non-Y
axis options when using hex-shaped cells, using X, Q, Z, then R axis
when rotating clockwise (hotkey C), and the reverse for
counter-clockwise (hotkey Z). For square-shaped cells, these hotkeys
still map to the X-axis and Z-axis.

This commit does not include required changes to the `GridMap` editor
menu to show/hide the hex-specific axis.

Note, also this commit in particular removes code for functionality that
has been broken for some time.  PR godotengine#25406 Added support for using
Shift-Q and Shift-E to expand selection regions to other planes/floors
without changing the actual floor.  This does not function in 4.2.2.  It
looks like at some point the keybindings for change level were changed
from an accelerator to a shortcut, and the code for checking for the key
combination was not updated.

As the functionality does not currently work, and it's non-trivial to
reimplement with hex cells, I'm leaving it out for the time being.
A dedicated PR should be opened to expand selection capabilities within
the GridMapEditor later to re-introduce the functonality.
Adriankhl pushed a commit to Adriankhl/godot that referenced this pull request Jun 6, 2024
`GridMap` for square tiles needed to support 24 different orientations.
When hexagonal tile support was added, the hex tiles were being rotated
as they were cubes, 90 degrees on whichever axis.  This does not work
for hexagonal tiles.

Hex cell rotation is 60 degrees around the y-axis for 6 possible
orientations, then 180 degrees around the x-axis or z-axis (flipping the
tile over).  Any other rotation around x/z would cause a hex tile to
clip any neighboring cells. This results in 12 possible orientations for
hex cells in a GridMap.

In this commit, I added the hex-specific orientations to the GridMap,
along with the 60/180 degree rotations in the GridMap editor plugin.

In addition, I added `Math_SQRT3` as it is used within the hex
orientation.

GridMap: use axial coordinate system for hex cells
The offset-based indexing approach offered many options for mapping
points into map indexes, but the current code had problems correctly
mapping various regions of a cell consistently
(godotengine#85890 (comment)).
Looking at the layout code, there were a lot of options with various
mathematical transforms for each one; verifying where the issue was, and
if it affected the other layouts was challenging.

There is also a comment at the top of the PR (godotengine#85890), and in the
discussion for the previous PR (godotengine#59842) about reducing the complexity of
this PR by removing the mixture of cell layout and orientation options.
That seemed like the best approach.

In this commit, I remove the existing layout & orientation options and
replace them with a single coordinate system (axial coordinates
described in
https://www.redblobgames.com/grids/hexagons/#coordinates-axial), and a
single cell orientation (pointy, previously the vertical orientation).

In addition I added tests for `GridMap::local_to_map()` and
`GridMap::map_to_local()` to verify we can correctly identify the edges
of the cell.

With this change we loose the ability to have irregularly shaped hexagon
cells where not all sides are the same length.  This is a side effect of
the current implementation of the algorithm to map between axial and 3d
coordinates.  I don't think this will be a big problem as most hex-based
3d games are using regular hexagons.

GridMap: support Center-y for hex cells

GridMapEditor: fix floorgrid offset issue

Currently if you exit editing a GridMap on floor != 0, when you return
to the GridMap the floor grid will be redrawn offset from the cusor.  So
the cursor is at floor 0, but the grid is drawn for floor -1.

The cause of this is `_draw_floor_grid()` takes into account the current
floor level when creating the floor grid.  The problem is that the grid
should be created at floor = 0, as it is offset by the floor number when
displayed.

This commit I removed the floor number argument from
`_draw_floor_grid()`.  As a result the floor grid is generated with
coordinates for floor == 0, and is correctly offset now.

HexGridEditor: fix for x-plane editing hex cells

With the conversion to axial coordinates for hex cells, editing cells
along the x-plane caused cells to be drawn in the wrong place.  It
turned out this was because `GridMapEditor::do_input_action()` assumed
it could modify the floor value directly after calling
`GridMap::local_to_map()`.  This looks like it was a workaround for
floating point errors that occur when raycasting against the picking
plane.

This causes a problem with hex cells as the axial coordinate system
means more math is required to set the x value.

In this commit I remove the code that modifies the floor/plane value for
the cell, and instead move the raypicking plane into the middle of the
cell plane. This will eliminate the majority of raypicking errors that
result from being directly on the edge of the cell.

GridMapEditor: cell selection for hex shaped cells

* GridMap: Add `local_region_to_map()` to get cell indices within a
  bounding box
* GridMap: Added `cell_shape_changed` signal to GridMap
* GridMapEditor: Update grid floor when cell shape changes
* GridMapEditor: Add cell shape mesh for hex tiles
* GridMapEditor: Update tile selection mesh when cell shape changes
* GridMapEditor: Fill selection for hex shaped cells
* GridMapEditor: Clear selection for hex shaped cells
* GridMapEditor: Paste for hex shaped cells

GridMapEditor: Hex cell Q/R/S Axis editing support

Editing along the Z-axis with hex cells results in unexpected behavior
for users as cells will be placed in a zig-zag pattern.  This is because
with pointy orientation, the cell centers are offset every other row as
you move along the Z-axis.

In this commit, I added support for editing cells along the Q-axis
(northwest to southeast), and S-axis (southwest to northeast).  The
terms come from the axial coordinate system used in GridMap, and
derrived from:
https://www.redblobgames.com/grids/hexagons/#coordinates-axial

The Z and C hotkeys were repurposed to rotate around the various non-Y
axis options when using hex-shaped cells, using X, Q, Z, then R axis
when rotating clockwise (hotkey C), and the reverse for
counter-clockwise (hotkey Z). For square-shaped cells, these hotkeys
still map to the X-axis and Z-axis.

This commit does not include required changes to the `GridMap` editor
menu to show/hide the hex-specific axis.

Note, also this commit in particular removes code for functionality that
has been broken for some time.  PR godotengine#25406 Added support for using
Shift-Q and Shift-E to expand selection regions to other planes/floors
without changing the actual floor.  This does not function in 4.2.2.  It
looks like at some point the keybindings for change level were changed
from an accelerator to a shortcut, and the code for checking for the key
combination was not updated.

As the functionality does not currently work, and it's non-trivial to
reimplement with hex cells, I'm leaving it out for the time being.
A dedicated PR should be opened to expand selection capabilities within
the GridMapEditor later to re-introduce the functonality.

GridMapEditor: Menu updates for Q/R/S axis editing

GridMap: Fix get_meshes() for NavMesh generation

Needed to update GridMap::get_meshes() to use map_to_local() instead of
manually calculating transforms.

GridMap: Add get_cell_neighbors()

With the transition to axial coordinates for hex shaped cells in
GridMap, it became more difficult to navigate grid map cells to their
neighbors.  This is because the Q/R axial coordinates do not directly
map to x/y equivalents.

In this commit I add GridMap::get_cell_neighbors() which is the minimum
implementation required to map from a GridMap to an AStar3d without
duplicating the axial coordinate math.

This commit should be expanded on to add direction-based neighbor
functions, but I'd rather do that work in a dedicated PR to abstract
navigating cell indexes into a dedicated class for each cell type.

GridMapEditor: Use multimesh for cell highlighting

Minor perf improvement to switch selection to use a multimesh for
highlighting selected cells.

GridMapEditor: Add lines around selection tiles

HexGridEditor: Fix editing grid for Q/R/S axis

GridMapEditor: selection on Q/S planes fixed

Selection along the Q & S planes was selecting a cube volume due to
`GridMap::local_region_to_map()` using the two points as the corners of
an AABB.  This isn't a problem when editing on X/Y/Z planes as the begin
and end points share one common value for at least one of their indices.
The Q & S planes on the other hand, run diagonal across the X/Z
coordinates, so every selection is a cube traversing X/Z.

In this commit I add support for selecting along the Q/S plane.  To do
this, GridMapEditor will now prune the selection region returned from
`GridMap::local_region_to_map()` based on which axis is being edited.

In addition, I trimmed out a lot of extranous values & logic that
weren't being used in the selection code.

GridMapEditor: Update X-Axis grid for hex cells
dmlary added a commit to dmlary/godot that referenced this pull request Jun 30, 2024
* `GridMap`
    * Added hex cell orientations (they differ from square orientations)
        * 60 degree steps about Y axis
        * 180 degree step about X axis to flip tile over
    * Use axial coordinate system for hex cells
        * https://www.redblobgames.com/grids/hexagons/#coordinates-axial
        * Q/R coordinates stored in X/Z fields of Vector3i
            * Preserve Y for level
            * See note below about internal coordinate systems leaking
        * Simplifies many hex-based algorithms (pathfinding, distance,
	  etc)
        * comments on PRs godotengine#85890 & godotengine#59842 suggested simplifying layout &
          orientation options
    * Support only normal hex cells
        * All side lengths are equal
        * Size of hexagon is defined by the radius; distance from center
	  to corners of cell
        * Supporting only normal hexes simplifies all calculations
        * Hex cell radius stored in cell_size.x
    * Add `GridMap::local_region_to_map()` to get cell indices within an
      axis-aligned bounding box
    * Add `GridMap::get_cell_neighbors()` to get cell neighbor indices
    * Add tests for `GridMap::map_to_local()` & `GridMap::local_to_map()`
        * necessary to confirm they were correctly detecting cell bounds
    * Add `cell_shape_changed` signal
* `GridMapEditor`
    * Rotate hex tiles
        * Hex tile rotation is significantly different from square tile
          rotation
        * 60 degree rotation about Y axis
        * 180 degree rotation about X/Z axis (flip tile over)
    * Support additional axes when editing hex cells
        * Q/S axis run northwest and southwest with evenly spaced cells
        * Update X-axis grid to show different cell sizes
        * Z/C shortcuts rotate through non-Y axes when using hex cells
    * Cell selection indicator changed from a bounding box to per-cell
      mesh in a multimesh
        * This was needed to make selection bounds clear when working
          with hex-shaped cells
    * Update fill/clear/duplicate for hex cells
    * Simplified cursor positioning code
    * Simplified selection & paste indicator code
    * Fixed issue where floor grid was drawn offset from the editing level
    * Removed dead code for expanding selection regions to additional
      planes/floors
        * Added in PR godotengine#25406 to expand using Shift-Q & Shift-E
        * Does not function in 4.2.2
        * Broken when keybindings for change level were changed from an
          accelesator to a shortcut
        * Non-trivial to reimplement
* Added `Math_SQRT3` as it is used extensively for hex cells

The internal coordinate system used within `GridMap` should not be
considered to be public or stable.  This has been the assumption up to
this point, and the result has been `GridMapEditor` deeply dependent on
the internal coordinate system.  This commit is so large because much of
`GridMapEditor` assumed it knew how `GridMap` was addressing individual
cells.  Many of those assumptions broke with the addition of hex cells.

Another side effect of leaking the internal coordinate system is the
extensive code duplication that is happening in users' code bases as
they all have to implement common functions for navigating the coordinate
system.  Functions such as calculating area of affect, distance, etc.

In this commit large steps have been made to isolate the coordinate system
to `GridMap`, and helpers added to navigate the coordinate system.
`GridMapEditor` has been updated to rely on `GridMap` to provide the
appropriate mapping from local space to `GridMap` coordinates.

There is more work to be done in this area, but is being held back to
limit the scope of this PR.
BSChad pushed a commit to BSChad/godot that referenced this pull request Jul 23, 2024
* `GridMap`
    * Added hex cell orientations (they differ from square orientations)
        * 60 degree steps about Y axis
        * 180 degree step about X axis to flip tile over
    * Use axial coordinate system for hex cells
        * https://www.redblobgames.com/grids/hexagons/#coordinates-axial
        * Q/R coordinates stored in X/Z fields of Vector3i
            * Preserve Y for level
            * See note below about internal coordinate systems leaking
        * Simplifies many hex-based algorithms (pathfinding, distance,
	  etc)
        * comments on PRs godotengine#85890 & godotengine#59842 suggested simplifying layout &
          orientation options
    * Support only normal hex cells
        * All side lengths are equal
        * Size of hexagon is defined by the radius; distance from center
	  to corners of cell
        * Supporting only normal hexes simplifies all calculations
        * Hex cell radius stored in cell_size.x
    * Add `GridMap::local_region_to_map()` to get cell indices within an
      axis-aligned bounding box
    * Add `GridMap::get_cell_neighbors()` to get cell neighbor indices
    * Add tests for `GridMap::map_to_local()` & `GridMap::local_to_map()`
        * necessary to confirm they were correctly detecting cell bounds
    * Add `cell_shape_changed` signal
* `GridMapEditor`
    * Rotate hex tiles
        * Hex tile rotation is significantly different from square tile
          rotation
        * 60 degree rotation about Y axis
        * 180 degree rotation about X/Z axis (flip tile over)
    * Support additional axes when editing hex cells
        * Q/S axis run northwest and southwest with evenly spaced cells
        * Update X-axis grid to show different cell sizes
        * Z/C shortcuts rotate through non-Y axes when using hex cells
    * Cell selection indicator changed from a bounding box to per-cell
      mesh in a multimesh
        * This was needed to make selection bounds clear when working
          with hex-shaped cells
    * Update fill/clear/duplicate for hex cells
    * Simplified cursor positioning code
    * Simplified selection & paste indicator code
    * Fixed issue where floor grid was drawn offset from the editing level
    * Removed dead code for expanding selection regions to additional
      planes/floors
        * Added in PR godotengine#25406 to expand using Shift-Q & Shift-E
        * Does not function in 4.2.2
        * Broken when keybindings for change level were changed from an
          accelesator to a shortcut
        * Non-trivial to reimplement
* Added `Math_SQRT3` as it is used extensively for hex cells

The internal coordinate system used within `GridMap` should not be
considered to be public or stable.  This has been the assumption up to
this point, and the result has been `GridMapEditor` deeply dependent on
the internal coordinate system.  This commit is so large because much of
`GridMapEditor` assumed it knew how `GridMap` was addressing individual
cells.  Many of those assumptions broke with the addition of hex cells.

Another side effect of leaking the internal coordinate system is the
extensive code duplication that is happening in users' code bases as
they all have to implement common functions for navigating the coordinate
system.  Functions such as calculating area of affect, distance, etc.

In this commit large steps have been made to isolate the coordinate system
to `GridMap`, and helpers added to navigate the coordinate system.
`GridMapEditor` has been updated to rely on `GridMap` to provide the
appropriate mapping from local space to `GridMap` coordinates.

There is more work to be done in this area, but is being held back to
limit the scope of this PR.
BSChad pushed a commit to BSChad/godot that referenced this pull request Jul 24, 2024
* `GridMap`
    * Added hex cell orientations (they differ from square orientations)
        * 60 degree steps about Y axis
        * 180 degree step about X axis to flip tile over
    * Use axial coordinate system for hex cells
        * https://www.redblobgames.com/grids/hexagons/#coordinates-axial
        * Q/R coordinates stored in X/Z fields of Vector3i
            * Preserve Y for level
            * See note below about internal coordinate systems leaking
        * Simplifies many hex-based algorithms (pathfinding, distance,
	  etc)
        * comments on PRs godotengine#85890 & godotengine#59842 suggested simplifying layout &
          orientation options
    * Support only normal hex cells
        * All side lengths are equal
        * Size of hexagon is defined by the radius; distance from center
	  to corners of cell
        * Supporting only normal hexes simplifies all calculations
        * Hex cell radius stored in cell_size.x
    * Add `GridMap::local_region_to_map()` to get cell indices within an
      axis-aligned bounding box
    * Add `GridMap::get_cell_neighbors()` to get cell neighbor indices
    * Add tests for `GridMap::map_to_local()` & `GridMap::local_to_map()`
        * necessary to confirm they were correctly detecting cell bounds
    * Add `cell_shape_changed` signal
* `GridMapEditor`
    * Rotate hex tiles
        * Hex tile rotation is significantly different from square tile
          rotation
        * 60 degree rotation about Y axis
        * 180 degree rotation about X/Z axis (flip tile over)
    * Support additional axes when editing hex cells
        * Q/S axis run northwest and southwest with evenly spaced cells
        * Update X-axis grid to show different cell sizes
        * Z/C shortcuts rotate through non-Y axes when using hex cells
    * Cell selection indicator changed from a bounding box to per-cell
      mesh in a multimesh
        * This was needed to make selection bounds clear when working
          with hex-shaped cells
    * Update fill/clear/duplicate for hex cells
    * Simplified cursor positioning code
    * Simplified selection & paste indicator code
    * Fixed issue where floor grid was drawn offset from the editing level
    * Removed dead code for expanding selection regions to additional
      planes/floors
        * Added in PR godotengine#25406 to expand using Shift-Q & Shift-E
        * Does not function in 4.2.2
        * Broken when keybindings for change level were changed from an
          accelesator to a shortcut
        * Non-trivial to reimplement
* Added `Math_SQRT3` as it is used extensively for hex cells

The internal coordinate system used within `GridMap` should not be
considered to be public or stable.  This has been the assumption up to
this point, and the result has been `GridMapEditor` deeply dependent on
the internal coordinate system.  This commit is so large because much of
`GridMapEditor` assumed it knew how `GridMap` was addressing individual
cells.  Many of those assumptions broke with the addition of hex cells.

Another side effect of leaking the internal coordinate system is the
extensive code duplication that is happening in users' code bases as
they all have to implement common functions for navigating the coordinate
system.  Functions such as calculating area of affect, distance, etc.

In this commit large steps have been made to isolate the coordinate system
to `GridMap`, and helpers added to navigate the coordinate system.
`GridMapEditor` has been updated to rely on `GridMap` to provide the
appropriate mapping from local space to `GridMap` coordinates.

There is more work to be done in this area, but is being held back to
limit the scope of this PR.
BSChad pushed a commit to BSChad/godot that referenced this pull request Sep 17, 2024
`GridMap` for square tiles needed to support 24 different orientations.
When hexagonal tile support was added, the hex tiles were being rotated
as they were cubes, 90 degrees on whichever axis.  This does not work
for hexagonal tiles.

Hex cell rotation is 60 degrees around the y-axis for 6 possible
orientations, then 180 degrees around the x-axis or z-axis (flipping the
tile over).  Any other rotation around x/z would cause a hex tile to
clip any neighboring cells. This results in 12 possible orientations for
hex cells in a GridMap.

In this commit, I added the hex-specific orientations to the GridMap,
along with the 60/180 degree rotations in the GridMap editor plugin.

In addition, I added `Math_SQRT3` as it is used within the hex
orientation.

GridMap: use axial coordinate system for hex cells
The offset-based indexing approach offered many options for mapping
points into map indexes, but the current code had problems correctly
mapping various regions of a cell consistently
(godotengine#85890 (comment)).
Looking at the layout code, there were a lot of options with various
mathematical transforms for each one; verifying where the issue was, and
if it affected the other layouts was challenging.

There is also a comment at the top of the PR (godotengine#85890), and in the
discussion for the previous PR (godotengine#59842) about reducing the complexity of
this PR by removing the mixture of cell layout and orientation options.
That seemed like the best approach.

In this commit, I remove the existing layout & orientation options and
replace them with a single coordinate system (axial coordinates
described in
https://www.redblobgames.com/grids/hexagons/#coordinates-axial), and a
single cell orientation (pointy, previously the vertical orientation).

In addition I added tests for `GridMap::local_to_map()` and
`GridMap::map_to_local()` to verify we can correctly identify the edges
of the cell.

With this change we loose the ability to have irregularly shaped hexagon
cells where not all sides are the same length.  This is a side effect of
the current implementation of the algorithm to map between axial and 3d
coordinates.  I don't think this will be a big problem as most hex-based
3d games are using regular hexagons.

GridMap: support Center-y for hex cells

GridMapEditor: fix floorgrid offset issue

Currently if you exit editing a GridMap on floor != 0, when you return
to the GridMap the floor grid will be redrawn offset from the cusor.  So
the cursor is at floor 0, but the grid is drawn for floor -1.

The cause of this is `_draw_floor_grid()` takes into account the current
floor level when creating the floor grid.  The problem is that the grid
should be created at floor = 0, as it is offset by the floor number when
displayed.

This commit I removed the floor number argument from
`_draw_floor_grid()`.  As a result the floor grid is generated with
coordinates for floor == 0, and is correctly offset now.

HexGridEditor: fix for x-plane editing hex cells

With the conversion to axial coordinates for hex cells, editing cells
along the x-plane caused cells to be drawn in the wrong place.  It
turned out this was because `GridMapEditor::do_input_action()` assumed
it could modify the floor value directly after calling
`GridMap::local_to_map()`.  This looks like it was a workaround for
floating point errors that occur when raycasting against the picking
plane.

This causes a problem with hex cells as the axial coordinate system
means more math is required to set the x value.

In this commit I remove the code that modifies the floor/plane value for
the cell, and instead move the raypicking plane into the middle of the
cell plane. This will eliminate the majority of raypicking errors that
result from being directly on the edge of the cell.

GridMapEditor: cell selection for hex shaped cells

* GridMap: Add `local_region_to_map()` to get cell indices within a
  bounding box
* GridMap: Added `cell_shape_changed` signal to GridMap
* GridMapEditor: Update grid floor when cell shape changes
* GridMapEditor: Add cell shape mesh for hex tiles
* GridMapEditor: Update tile selection mesh when cell shape changes
* GridMapEditor: Fill selection for hex shaped cells
* GridMapEditor: Clear selection for hex shaped cells
* GridMapEditor: Paste for hex shaped cells

GridMapEditor: Hex cell Q/R/S Axis editing support

Editing along the Z-axis with hex cells results in unexpected behavior
for users as cells will be placed in a zig-zag pattern.  This is because
with pointy orientation, the cell centers are offset every other row as
you move along the Z-axis.

In this commit, I added support for editing cells along the Q-axis
(northwest to southeast), and S-axis (southwest to northeast).  The
terms come from the axial coordinate system used in GridMap, and
derrived from:
https://www.redblobgames.com/grids/hexagons/#coordinates-axial

The Z and C hotkeys were repurposed to rotate around the various non-Y
axis options when using hex-shaped cells, using X, Q, Z, then R axis
when rotating clockwise (hotkey C), and the reverse for
counter-clockwise (hotkey Z). For square-shaped cells, these hotkeys
still map to the X-axis and Z-axis.

This commit does not include required changes to the `GridMap` editor
menu to show/hide the hex-specific axis.

Note, also this commit in particular removes code for functionality that
has been broken for some time.  PR godotengine#25406 Added support for using
Shift-Q and Shift-E to expand selection regions to other planes/floors
without changing the actual floor.  This does not function in 4.2.2.  It
looks like at some point the keybindings for change level were changed
from an accelerator to a shortcut, and the code for checking for the key
combination was not updated.

As the functionality does not currently work, and it's non-trivial to
reimplement with hex cells, I'm leaving it out for the time being.
A dedicated PR should be opened to expand selection capabilities within
the GridMapEditor later to re-introduce the functonality.

GridMapEditor: Menu updates for Q/R/S axis editing

GridMap: Fix get_meshes() for NavMesh generation

Needed to update GridMap::get_meshes() to use map_to_local() instead of
manually calculating transforms.

GridMap: Add get_cell_neighbors()

With the transition to axial coordinates for hex shaped cells in
GridMap, it became more difficult to navigate grid map cells to their
neighbors.  This is because the Q/R axial coordinates do not directly
map to x/y equivalents.

In this commit I add GridMap::get_cell_neighbors() which is the minimum
implementation required to map from a GridMap to an AStar3d without
duplicating the axial coordinate math.

This commit should be expanded on to add direction-based neighbor
functions, but I'd rather do that work in a dedicated PR to abstract
navigating cell indexes into a dedicated class for each cell type.

GridMapEditor: Use multimesh for cell highlighting

Minor perf improvement to switch selection to use a multimesh for
highlighting selected cells.

GridMapEditor: Add lines around selection tiles

HexGridEditor: Fix editing grid for Q/R/S axis

GridMapEditor: selection on Q/S planes fixed

Selection along the Q & S planes was selecting a cube volume due to
`GridMap::local_region_to_map()` using the two points as the corners of
an AABB.  This isn't a problem when editing on X/Y/Z planes as the begin
and end points share one common value for at least one of their indices.
The Q & S planes on the other hand, run diagonal across the X/Z
coordinates, so every selection is a cube traversing X/Z.

In this commit I add support for selecting along the Q/S plane.  To do
this, GridMapEditor will now prune the selection region returned from
`GridMap::local_region_to_map()` based on which axis is being edited.

In addition, I trimmed out a lot of extranous values & logic that
weren't being used in the selection code.

GridMapEditor: Update X-Axis grid for hex cells
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gridmap editor controls don't work due to conflicting key mappings Unable To Remove Gridmap
4 participants