Eliminate walkable/transparent duplication between GridData and TCODMap #333

New issue

Closed

opened 2026-07-02 12:29:40 +00:00 by john · 1 comment

john commented

2026-07-02 12:29:40 +00:00

Owner

From the 2026-07-02 memory-model review. Depends on the GridData SoA refactor (#332).

Current behavior

walkable/transparent live in both the UIGridPoint cells and libtcod's TCOD_map_t (new TCODMap in GridData::initStorage, src/GridData.cpp:76-77). Sync is one-directional and eager:

every single-cell Python write triggers syncTCODMapCell (src/GridData.cpp:133-140 via src/UIGridPoint.cpp:90)
bulk ops do a full-grid syncTCODMap loop (src/GridData.cpp:120-131)
pathfinding collision labels temporarily mutate the TCOD map and restore it (src/UIGridPathfinding.cpp:575-611)

Proposal

After SoA planes exist, make the TCOD map a derived, lazily-rebuilt cache: rebuild (or incrementally patch) only when FOV/Dijkstra/A* is requested and transparency_generation (src/GridData.h:93) is newer than the map's generation. Per-write sync disappears; bulk writes (including future numpy ingest) cost one rebuild at next query instead of N syncs.

Alternative considered: making TCOD the primary store -- rejected; TCOD_map packs its own struct and would put libtcod's ABI in the middle of every cell read.

Longer term this is also where the libtcod-path work (integer-native, N-dim, header-only) could replace TCODMap entirely, taking the planes as direct input.

From the 2026-07-02 memory-model review. Depends on the GridData SoA refactor (#332). ## Current behavior walkable/transparent live in **both** the `UIGridPoint` cells and libtcod's `TCOD_map_t` (`new TCODMap` in `GridData::initStorage`, `src/GridData.cpp:76-77`). Sync is one-directional and eager: - every single-cell Python write triggers `syncTCODMapCell` (`src/GridData.cpp:133-140` via `src/UIGridPoint.cpp:90`) - bulk ops do a full-grid `syncTCODMap` loop (`src/GridData.cpp:120-131`) - pathfinding collision labels *temporarily mutate* the TCOD map and restore it (`src/UIGridPathfinding.cpp:575-611`) ## Proposal After SoA planes exist, make the TCOD map a **derived, lazily-rebuilt cache**: rebuild (or incrementally patch) only when FOV/Dijkstra/A* is requested and `transparency_generation` (`src/GridData.h:93`) is newer than the map's generation. Per-write sync disappears; bulk writes (including future numpy ingest) cost one rebuild at next query instead of N syncs. Alternative considered: making TCOD the primary store -- rejected; TCOD_map packs its own struct and would put libtcod's ABI in the middle of every cell read. Longer term this is also where the libtcod-path work (integer-native, N-dim, header-only) could replace TCODMap entirely, taking the planes as direct input.

john added the

system:grid

priority:tier3-future

system:performance

Refactoring & Cleanup

labels

2026-07-02 12:29:40 +00:00

john referenced this issue

2026-07-02 12:30:42 +00:00

GridData SoA refactor: dense uint8 planes for walkable/transparent #332

john added a new dependency

2026-07-02 12:30:44 +00:00

#332 GridData SoA refactor: dense uint8 planes for walkable/transparent

john commented

2026-07-11 05:52:03 +00:00

Author

Owner

Unblocked now that #332 (SoA planes) has landed, but deferring after a closer read of the current cost.

The per-write sync this issue targets is already O(1) per cell — syncTCODMapCell does a single tcod_map->setProperties(...), not a full-grid rebuild — and the bulk writers (apply_threshold/apply_ranges heightmap paths) already sync once after their loop, not per cell. So there is no current hot bottleneck to remove; the win is architectural (decouple the TCOD map from the write path so a future numpy bulk-ingest of walkable/transparent writes planes directly and rebuilds once).

Against that modest, forward-looking benefit, the change carries real FOV + pathfinding correctness risk: the lazy rebuild must interleave correctly with the pathfinding collision-label temporary mutate/restore (UIGridPathfinding.cpp markCollisionLabel/restoreCollisionLabel), and every FOV/Dijkstra/A* entry point must gain an ensureTCODMapCurrent() guard. Worth doing deliberately with a dedicated FOV/path regression pass, not bundled into the current sprint.

Note: transparency_generation (bumped on every walkable/transparent change) is already the right dirty signal for the lazy rebuild when someone picks this up — add a tcod_map_generation and rebuild when it lags.

Unblocked now that #332 (SoA planes) has landed, but **deferring** after a closer read of the current cost. The per-write sync this issue targets is already **O(1) per cell** — `syncTCODMapCell` does a single `tcod_map->setProperties(...)`, not a full-grid rebuild — and the bulk writers (`apply_threshold`/`apply_ranges` heightmap paths) already sync **once** after their loop, not per cell. So there is no current hot bottleneck to remove; the win is architectural (decouple the TCOD map from the write path so a future numpy bulk-ingest of `walkable`/`transparent` writes planes directly and rebuilds once). Against that modest, forward-looking benefit, the change carries real FOV + pathfinding correctness risk: the lazy rebuild must interleave correctly with the pathfinding collision-label temporary mutate/restore (`UIGridPathfinding.cpp` `markCollisionLabel`/`restoreCollisionLabel`), and every FOV/Dijkstra/A* entry point must gain an `ensureTCODMapCurrent()` guard. Worth doing deliberately with a dedicated FOV/path regression pass, not bundled into the current sprint. Note: `transparency_generation` (bumped on every walkable/transparent change) is already the right dirty signal for the lazy rebuild when someone picks this up — add a `tcod_map_generation` and rebuild when it lags.

john referenced this issue from a commit

2026-07-11 14:42:07 +00:00

refactor(grid): GridData SoA -- dense uint8 planes for walkable/transparent; closes #332

john closed this issue

2026-07-11 14:52:35 +00:00

john referenced this issue from a commit

2026-07-16 03:56:24 +00:00

docs(roadmap): sync open-issue state; 0.2.8 published