Skip to content

GridMap: Add hexagonal cell support - #9

Open
dsarno wants to merge 2 commits into
masterfrom
claude/gridmap-hex-cells
Open

GridMap: Add hexagonal cell support#9
dsarno wants to merge 2 commits into
masterfrom
claude/gridmap-hex-cells

Conversation

@dsarno

@dsarno dsarno commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

Adds hexagonal cells to GridMap alongside the existing square ones: axial coordinate mapping, hex-aware cell/world conversion, selection and paste in local space, hexagonal grid and selection drawing, and editor support for the six edit planes. modules/gridmap only, plus its tests and class reference.

Provenance

The first commit is third-party work, squashed and rebased onto current master with original authorship preserved (Casper Beyer, co-authored by David M. Lary and Chad Stewart). It was 6,875 commits behind; 5 files conflicted and the editor plugin alone had 22 conflicting hunks. The second commit is review fixes.

This branch was produced by an AI agent. Everything below was verified by building and running it, but it warrants a human read.

Conflict resolution worth knowing about

Upstream had moved on substantially in the same files, so several conflicts were semantic rather than textual:

  • Edit axis. Upstream replaced the plain enum with edit_axis_select + viewport_axis + a _get_edit_axis() viewport override. Adopted upstream's machinery retyped to the 6-value axis; the viewport-derived Z maps to R for hex.
  • Cursor rotation. Upstream collapsed six menu cases into one block with undo/redo. Adopted it and parameterized the angle by cell shape (60° about Y, 180° about X/Z for hex). This also drops a double-applied rotation and an uninitialized-basis bug that were in the original.
  • Cut → Move. Kept upstream's clipboard_is_move / _setup_paste_mode / preview-restore undo, re-expressed against the local-space clipboard by adding ClipboardItem::source_cell.
  • Drag painting. Upstream's Bresenham interpolation is kept for square cells; hex paints the single pointer cell, since axial coordinates are not a 2D lattice.
  • Shortcuts. The original bound plain Z/C to plane rotation, but upstream now uses those for Fill and Duplicate. The two new shortcuts are registered with no default key — see open questions.

Defects found and fixed

High — axial_round() used integer abs(). The fractional remainders were truncated to 0 before comparison, so any point that wasn't dead-centre in a hexagon could resolve to the wrong cell. One character (Math::abs), but it silently broke every local_to_map() call for hex.

High — make_baked_meshes() kept rectangular placement. Baking a hex map displaced every mesh, because the bake path computed positions directly instead of going through map_to_local().

Medium — cell_orientations was a TypedArray<Basis>. Every read heap-allocated a Basis inside a Variant, once per cell in _octant_update(), get_meshes() and make_baked_meshes(). Replaced with static tables exposed as Span<const Basis>.

Medium — RID leaks. cursor_mesh, cursor_instance and selection_multimesh_instance were never freed; confirmed via editor-exit leak reports, and gone after the fix. _update_selection() also left a dangling RID that would double-free on the next call.

Public API bugGridMapEditorPlugin.set_selection() passed cell coordinates into a local-space setter.

Also fixed while merging: multimesh instances allocated for plane-filtered-out cells; _get_selected_cells() / _get_selection() iterating local-space floats as cell indices; a null Ref<Mesh> dereference for meshless clipboard items; if (int index = ... != -1) in _update_options_menu(); a duplicate "Settings…" menu entry; a #define leaking from a header; node == NULL; and -Wshadow / deprecated-free() warnings.

Testing

*GridMap*: 9 cases / 250 assertions, all passing. Full suite 1423/1424 — the single failure is an IPv6 test that is environmental and fails on master here too.

Revert-proof: reverting the two High fixes makes 2 cases / 17 assertions fail — 16 in local_to_map() for hex cells (e.g. (2,0,-1) == (1,0,0)) and 1 in the new baked-mesh placement case. Restoring returns 250/250.

Also verified: clang-format clean, --doctool regenerates the class reference with zero diff, no compiler warnings in modules/gridmap, and an editor smoke test on a hex-GridMap scene starts and exits cleanly with no leaks.

Open questions for review

  • Default keys for the two hex plane-rotation shortcuts — left unbound to avoid colliding with Fill and Duplicate. Needs a call.
  • get_octants_in_bounds() / get_used_octants_in_bounds() and octant debug visualisation are wrong for hex. They divide local bounds by cell_size, but an octant of axial cells is a rhombus in local space. This needs a semantic decision rather than a mechanical fix, so it was left alone.
  • Hex drag-paint interpolation is not implemented — one cell per event.
  • The editor cursor is still a box for hex cells, so it doesn't line up with the hexagon outline the selection draws.
  • Compatibility: the _editor_floor_ meta changed from Vector3 to a 6-plane PackedInt32Array. Existing scenes lose their remembered editor floor once.

Generated by Claude Code

caspervonb and others added 2 commits July 29, 2026 04:00
Add a `cell_shape` property to `GridMap` that selects between square and
hexagonal cells, along with the editor support needed to author hex maps.

`GridMap`:
- Hexagonal cells are regular (all sides equal). `cell_size.x` holds the
  cell radius (center to vertex) and `cell_size.y` the cell height;
  `cell_size.z` is kept in sync with `cell_size.x`.
- Hex cells are addressed with axial coordinates, stored as (q, level, r)
  in the x/y/z fields of the cell `Vector3i`.
- Hex cells use their own orientation set: six 60 degree steps about the
  Y axis, and the same six again after flipping the tile over.
- Add `local_region_to_map()` returning every cell index inside an
  axis-aligned box given in local space.
- Add `get_cell_neighbors()` returning the indices adjacent to a cell.
- Add a `cell_shape_changed` signal.

`GridMapEditor`:
- Rotate hex tiles in 60 degree steps about the Y axis, and flip them
  over about the X/Z axis.
- Add the Q/R/S editing axes used by hex maps, and cycle through them
  with the existing axis shortcuts.
- Draw the selection as a per-cell multimesh rather than a bounding box,
  so the selected cells are unambiguous for hex shapes.
- Update fill, clear, duplicate and paste for hex cells.
- Fix the floor grid being drawn offset from the level being edited.

The editor previously reimplemented `GridMap`'s cell addressing in
several places, which does not survive a second cell shape. It now asks
`GridMap` to map between local space and cell indices.

Co-authored-by: David M. Lary <dmlary@gmail.com>
Co-authored-by: Chad Stewart <chad@binarysolo.com>
`GridMap`:
- `axial_round()` compared the coordinate remainders with the integer
  `abs()`, which truncated both to zero and always rounded towards the
  Q axis. Points off the center of a hexagon were mapped to the wrong
  cell; use `Math::abs()` and `Math::round()`.
- `make_baked_meshes()` still placed each cell with the rectangular
  layout, so baking a hexagonal map moved every mesh.
- Keep the cell orientations in static tables instead of a
  `TypedArray<Basis>`. Reading one boxed a `Basis` into a `Variant`, and
  that happens once per cell in `_octant_update()`, `get_meshes()` and
  `make_baked_meshes()`.
- Reject out-of-range values in `set_cell_shape()`, and skip the octant
  rebuild when the shape does not actually change.
- Take `Vector3i`/`Vector3` arguments by const reference, and build the
  neighbor offsets as `Vector3i` rather than round-tripping them
  through floating point.

`GridMapEditor`:
- Free the cursor mesh, the cursor instance and the selection multimesh
  instance when the editor is destroyed; they leaked their RIDs.
- `GridMapEditorPlugin.set_selection()` takes cell coordinates, but the
  selection is kept in local space, so convert before applying it.
- Do not shift the selection tiles by half a cell on X and Z for
  hexagonal cells, whose position never includes the centering offset.
- Use `free_rid()` rather than the deprecated `RenderingServer::free()`,
  and stop shadowing a member with a local named `shortcut`.

Tests:
- Cover the position of baked meshes for both cell shapes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants