Entity.gridstate as DiscreteMap reference #294

Closed
opened 2026-03-08 20:19:11 +00:00 by john · 2 comments
Owner

Summary

Replace the current entity gridstate implementation (flat arrays with ensureGridstate() resize logic) with a DiscreteMap reference. Entity perspective knowledge becomes a first-class, swappable, serializable object rather than an implicit internal array.

Current problems

  1. ensureGridstate() silently destroys exploration data — moving between grids of different sizes wipes all discovery with no callback or save mechanism
  2. Same-size grids retain stale data — silently reuses the previous grid's perspective data
  3. entity.gridstate returns detached copies (SimpleNamespace snapshots), forcing O(w×h) Python iteration for save/restore — violates the principle that grid-shaped iteration stays on the C++ side
  4. No identity — the gridstate has no association with which grid it belongs to

Proposed API

# Entity's perspective is a DiscreteMap (3 states: unknown=0, discovered=1, visible=2)
perspective = entity.perspective_map  # returns DiscreteMap reference

# Save before leaving a grid
saved = perspective.to_bytes()  # from DiscreteMap serialization issue

# Assign a grid's perspective entity with existing memory
grid.perspective = (entity, saved_discretemap)
# Or with None to create a fresh (all-unknown) DiscreteMap:
grid.perspective = (entity, None)

# Assigning with wrong-sized DiscreteMap raises ValueError
grid.perspective = (entity, wrong_size_map)  # ValueError

# Simple assignment still works — creates fresh DiscreteMap
grid.perspective = entity  # equivalent to (entity, None)

Engine behavior on grid transitions

When an entity is removed from a grid (or the grid's perspective entity changes):

  • The engine disables perspective on affected GridLayers
  • The DiscreteMap remains valid and accessible — game code can save it
  • The engine does NOT automatically cache per-grid states (game devs manage this if needed)

Game-side multi-grid memory pattern

# Game code manages the grid↔memory mapping
player_memory = {}  # grid_id -> bytes

def enter_grid(player, new_grid):
    # Save current
    if player.perspective_map:
        player_memory[current_grid.id] = player.perspective_map.to_bytes()
    # Restore or create fresh
    saved = player_memory.get(new_grid.id)
    if saved:
        dmap = mcrfpy.DiscreteMap(new_grid.width, new_grid.height, 3, data=saved)
        new_grid.perspective = (player, dmap)
    else:
        new_grid.perspective = (player, None)

The engine provides correct primitives; game code composes them for its specific needs.

Dependencies

  • DiscreteMap serialization issue (to_bytes / from_bytes)
  • #252 Grid/GridView overhaul (broader architectural context)

Scope

Could be implemented incrementally on the current Entity/Grid, or as part of #252. The DiscreteMap serialization issue is a prerequisite.

## Summary Replace the current entity gridstate implementation (flat arrays with `ensureGridstate()` resize logic) with a `DiscreteMap` reference. Entity perspective knowledge becomes a first-class, swappable, serializable object rather than an implicit internal array. ## Current problems 1. **`ensureGridstate()` silently destroys exploration data** — moving between grids of different sizes wipes all discovery with no callback or save mechanism 2. **Same-size grids retain stale data** — silently reuses the previous grid's perspective data 3. **`entity.gridstate` returns detached copies** (SimpleNamespace snapshots), forcing O(w×h) Python iteration for save/restore — violates the principle that grid-shaped iteration stays on the C++ side 4. **No identity** — the gridstate has no association with which grid it belongs to ## Proposed API ```python # Entity's perspective is a DiscreteMap (3 states: unknown=0, discovered=1, visible=2) perspective = entity.perspective_map # returns DiscreteMap reference # Save before leaving a grid saved = perspective.to_bytes() # from DiscreteMap serialization issue # Assign a grid's perspective entity with existing memory grid.perspective = (entity, saved_discretemap) # Or with None to create a fresh (all-unknown) DiscreteMap: grid.perspective = (entity, None) # Assigning with wrong-sized DiscreteMap raises ValueError grid.perspective = (entity, wrong_size_map) # ValueError # Simple assignment still works — creates fresh DiscreteMap grid.perspective = entity # equivalent to (entity, None) ``` ### Engine behavior on grid transitions When an entity is removed from a grid (or the grid's perspective entity changes): - The engine disables perspective on affected GridLayers - The DiscreteMap remains valid and accessible — game code can save it - The engine does NOT automatically cache per-grid states (game devs manage this if needed) ### Game-side multi-grid memory pattern ```python # Game code manages the grid↔memory mapping player_memory = {} # grid_id -> bytes def enter_grid(player, new_grid): # Save current if player.perspective_map: player_memory[current_grid.id] = player.perspective_map.to_bytes() # Restore or create fresh saved = player_memory.get(new_grid.id) if saved: dmap = mcrfpy.DiscreteMap(new_grid.width, new_grid.height, 3, data=saved) new_grid.perspective = (player, dmap) else: new_grid.perspective = (player, None) ``` The engine provides correct primitives; game code composes them for its specific needs. ## Dependencies - DiscreteMap serialization issue (to_bytes / from_bytes) - #252 Grid/GridView overhaul (broader architectural context) ## Scope Could be implemented incrementally on the current Entity/Grid, or as part of #252. The DiscreteMap serialization issue is a prerequisite.
Author
Owner

Roadmap context

This issue is part of the Grid & Entity Overhaul Roadmap (docs/GRID_ENTITY_OVERHAUL_ROADMAP.md), placed in Phase 4 (after #252 Grid/GridView split).

The behavior system's seek/flee behaviors (#300) will initially use the current DijkstraMap type, then migrate to DiscreteMap when this issue lands. The DiscreteMap will also serve as the entity's perspective map for FOV-based TARGET triggers in the behavior system.

## Roadmap context This issue is part of the Grid & Entity Overhaul Roadmap (`docs/GRID_ENTITY_OVERHAUL_ROADMAP.md`), placed in **Phase 4** (after #252 Grid/GridView split). The behavior system's seek/flee behaviors (#300) will initially use the current `DijkstraMap` type, then migrate to `DiscreteMap` when this issue lands. The `DiscreteMap` will also serve as the entity's perspective map for FOV-based TARGET triggers in the behavior system.
Author
Owner

Implementation plan — design decisions settled 2026-04-17

Following a design review, this issue is narrowed to perspective storage only. Pathfinding/behavior integration (SEEK/FLEE referencing DiscreteMaps) has been moved to #315 and is explicitly out of scope here.

Settled design decisions

1. Data model: one DiscreteMap, three states

0 = unknown     (never seen)
1 = discovered  (seen before, not currently visible)
2 = visible     (in current FOV)

Invariant: visible ⊆ discovered — every cell that is currently visible must also be discovered. updateVisibility() enforces this by:

  • Demoting all 2 → 1 at the start of each FOV pass (discovered stays, visible is transient)
  • Promoting 1 → 2 for cells the entity currently sees (and bumping 0 → 2 if freshly seen)

No separate visible and discovered fields. Avoids sync bugs and halves memory vs. two maps.

2. Python API: entity.perspective_map

Pairs with the existing GridView.perspective_entity / GridView.perspective_enabled terminology (see src/UIGridView.h:56-57). The symmetry reads as "GridView renders from this entity's perspective; the entity stores its perspective_map."

pmap = entity.perspective_map          # DiscreteMap, 3 states
pmap[x, y]                             # 0, 1, or 2
saved = pmap.to_bytes()                # serialize for grid transitions
restored = mcrfpy.DiscreteMap.from_bytes(saved, w, h)
entity.perspective_map = restored      # assign for grid restore

The property is lazy: first access on an entity that has a grid allocates a DiscreteMap sized to the grid (all zeros). Entities without a grid have perspective_map is None.

3. Removals (clean break — no deprecation shims)

  • UIEntity::gridstate member — src/UIEntity.h:69
  • UIEntity::ensureGridstate()src/UIEntity.cpp:39-
  • entity.gridstate Python property and its get_gridstate getter returning detached snapshots
  • UIGridPointState::get_point()src/UIGridPoint.h:76 (the visibility-gated grid point accessor)
  • The UIGridPointState struct itself — src/UIGridPoint.h:65-80 — becomes vestigial. Check for remaining readers before deletion; if any, migrate to entity.perspective_map[x, y] + grid.at(x, y).

No backward-compat alias. A future ticket may expose a layer-dict accessor on grid.at(x, y) returning {layer_name: value}, but that is out of scope here.

4. Perspective API: already correct

GridView::perspective_entity is already std::weak_ptr<UIEntity> (no tuple). The tuple-style grid.perspective = (entity, dmap) mentioned in the original issue description is rejected — the entity owns its own DiscreteMap, and the GridView just references the entity. No new Grid.perspective API is needed.

Existing code path:

  • GridView::perspective_enabled + perspective_entity stay as-is
  • Rendering in UIGridView.cpp:223 (if (perspective_enabled) { auto entity = perspective_entity.lock(); ... }) switches from reading entity->gridstate[...] to reading entity->perspective_map->values[...] with a comparison against >= 1 (discovered — render dimmed) or == 2 (visible — render normal)

5. C++ storage choice — implementer's call

PyDiscreteMap currently stores raw uint8_t* directly on the Python object (PyDiscreteMap.h:10-15) without a C++ backing class, unlike DijkstraMap which has a C++ class wrapped by PyDijkstraMapObject. Two viable options:

  • (a) Refactor DiscreteMap to match DijkstraMap's pattern: introduce a class DiscreteMap with shared ownership, make PyDiscreteMapObject hold a shared_ptr<DiscreteMap>, and UIEntity holds the same shared_ptr<DiscreteMap>. updateVisibility() can write in C++ without touching the Python type.
  • (b) UIEntity holds a PyObject* (refcounted) pointing to a PyDiscreteMapObject: no refactor needed, but updateVisibility() must either reach through the Python object to the uint8_t* (type-check + unwrap each time) or route through the Python API.

Recommend (a) for consistency with DijkstraMap and to keep updateVisibility() clean. Defer the final call to whoever picks this up.

Acceptance criteria

  • UIEntity stores perspective as one DiscreteMap (3 states)
  • updateVisibility() demotes 2 → 1 then promotes visible cells to 2 (and 0 → 2 on first sight)
  • Python property entity.perspective_map returns the DiscreteMap reference (NOT a snapshot copy)
  • Lazy allocation: first access on an entity with a grid creates a DiscreteMap sized to the grid
  • Assignment (entity.perspective_map = other_dmap) validates size against grid; ValueError on mismatch
  • Serialization round-trip: to_bytes()from_bytes() → assign → updateVisibility() still works
  • GridView fog-of-war rendering reads from entity.perspective_map instead of gridstate
  • entity.gridstate, UIGridPointState::get_point(), and ensureGridstate() removed
  • UIGridPointState struct removed if no remaining readers
  • Regression tests:
    • perspective_map returns same object across calls (identity, not copy)
    • to_bytes/from_bytes round-trip preserves all three states
    • Size-mismatch assignment raises ValueError
    • Grid transition: serialize perspective on grid A, restore on grid B (same size), FOV continues correctly
    • visible ⊆ discovered invariant holds after every updateVisibility() call

Key files

  • src/UIEntity.h / src/UIEntity.cpp — the member and its methods
  • src/UIGridPoint.hUIGridPointState struct removal
  • src/PyDiscreteMap.h / src/PyDiscreteMap.cpp — possible C++-class refactor (option a)
  • src/UIGridView.cpp:223 — fog-of-war rendering site

Not in scope (moved to #315)

  • EntityBehavior::dijkstra_map migration
  • SEEK/FLEE behaviors referencing DiscreteMap
  • HeightMap integration with behaviors
  • Multi-root Dijkstra, custom heuristics, PathProvider abstraction
## Implementation plan — design decisions settled 2026-04-17 Following a design review, this issue is **narrowed to perspective storage only**. Pathfinding/behavior integration (SEEK/FLEE referencing DiscreteMaps) has been moved to #315 and is explicitly out of scope here. ### Settled design decisions **1. Data model: one DiscreteMap, three states** ``` 0 = unknown (never seen) 1 = discovered (seen before, not currently visible) 2 = visible (in current FOV) ``` Invariant: `visible ⊆ discovered` — every cell that is currently visible must also be discovered. `updateVisibility()` enforces this by: - Demoting all `2 → 1` at the start of each FOV pass (discovered stays, visible is transient) - Promoting `1 → 2` for cells the entity currently sees (and bumping `0 → 2` if freshly seen) No separate `visible` and `discovered` fields. Avoids sync bugs and halves memory vs. two maps. **2. Python API: `entity.perspective_map`** Pairs with the existing `GridView.perspective_entity` / `GridView.perspective_enabled` terminology (see `src/UIGridView.h:56-57`). The symmetry reads as "GridView renders from this entity's perspective; the entity stores its perspective_map." ```python pmap = entity.perspective_map # DiscreteMap, 3 states pmap[x, y] # 0, 1, or 2 saved = pmap.to_bytes() # serialize for grid transitions restored = mcrfpy.DiscreteMap.from_bytes(saved, w, h) entity.perspective_map = restored # assign for grid restore ``` The property is **lazy**: first access on an entity that has a grid allocates a DiscreteMap sized to the grid (all zeros). Entities without a grid have `perspective_map is None`. **3. Removals (clean break — no deprecation shims)** - `UIEntity::gridstate` member — `src/UIEntity.h:69` - `UIEntity::ensureGridstate()` — `src/UIEntity.cpp:39-` - `entity.gridstate` Python property and its `get_gridstate` getter returning detached snapshots - `UIGridPointState::get_point()` — `src/UIGridPoint.h:76` (the visibility-gated grid point accessor) - The `UIGridPointState` struct itself — `src/UIGridPoint.h:65-80` — becomes vestigial. Check for remaining readers before deletion; if any, migrate to `entity.perspective_map[x, y]` + `grid.at(x, y)`. No backward-compat alias. A future ticket may expose a layer-dict accessor on `grid.at(x, y)` returning `{layer_name: value}`, but that is out of scope here. **4. Perspective API: already correct** `GridView::perspective_entity` is already `std::weak_ptr<UIEntity>` (no tuple). The tuple-style `grid.perspective = (entity, dmap)` mentioned in the original issue description is **rejected** — the entity owns its own DiscreteMap, and the GridView just references the entity. No new `Grid.perspective` API is needed. Existing code path: - `GridView::perspective_enabled` + `perspective_entity` stay as-is - Rendering in `UIGridView.cpp:223` (`if (perspective_enabled) { auto entity = perspective_entity.lock(); ... }`) switches from reading `entity->gridstate[...]` to reading `entity->perspective_map->values[...]` with a comparison against `>= 1` (discovered — render dimmed) or `== 2` (visible — render normal) **5. C++ storage choice — implementer's call** `PyDiscreteMap` currently stores raw `uint8_t*` directly on the Python object (`PyDiscreteMap.h:10-15`) without a C++ backing class, unlike `DijkstraMap` which has a C++ class wrapped by `PyDijkstraMapObject`. Two viable options: - **(a) Refactor DiscreteMap to match DijkstraMap's pattern**: introduce a `class DiscreteMap` with shared ownership, make `PyDiscreteMapObject` hold a `shared_ptr<DiscreteMap>`, and `UIEntity` holds the same `shared_ptr<DiscreteMap>`. `updateVisibility()` can write in C++ without touching the Python type. - **(b) UIEntity holds a `PyObject*` (refcounted) pointing to a PyDiscreteMapObject**: no refactor needed, but `updateVisibility()` must either reach through the Python object to the `uint8_t*` (type-check + unwrap each time) or route through the Python API. Recommend (a) for consistency with DijkstraMap and to keep `updateVisibility()` clean. Defer the final call to whoever picks this up. ### Acceptance criteria - [ ] `UIEntity` stores perspective as one DiscreteMap (3 states) - [ ] `updateVisibility()` demotes `2 → 1` then promotes visible cells to `2` (and `0 → 2` on first sight) - [ ] Python property `entity.perspective_map` returns the DiscreteMap reference (NOT a snapshot copy) - [ ] Lazy allocation: first access on an entity with a grid creates a DiscreteMap sized to the grid - [ ] Assignment (`entity.perspective_map = other_dmap`) validates size against grid; `ValueError` on mismatch - [ ] Serialization round-trip: `to_bytes()` → `from_bytes()` → assign → `updateVisibility()` still works - [ ] `GridView` fog-of-war rendering reads from `entity.perspective_map` instead of `gridstate` - [ ] `entity.gridstate`, `UIGridPointState::get_point()`, and `ensureGridstate()` removed - [ ] `UIGridPointState` struct removed if no remaining readers - [ ] Regression tests: - perspective_map returns same object across calls (identity, not copy) - to_bytes/from_bytes round-trip preserves all three states - Size-mismatch assignment raises `ValueError` - Grid transition: serialize perspective on grid A, restore on grid B (same size), FOV continues correctly - `visible ⊆ discovered` invariant holds after every `updateVisibility()` call ### Key files - `src/UIEntity.h` / `src/UIEntity.cpp` — the member and its methods - `src/UIGridPoint.h` — `UIGridPointState` struct removal - `src/PyDiscreteMap.h` / `src/PyDiscreteMap.cpp` — possible C++-class refactor (option a) - `src/UIGridView.cpp:223` — fog-of-war rendering site ### Not in scope (moved to #315) - `EntityBehavior::dijkstra_map` migration - SEEK/FLEE behaviors referencing DiscreteMap - HeightMap integration with behaviors - Multi-root Dijkstra, custom heuristics, PathProvider abstraction
john closed this issue 2026-04-18 09:44:14 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Depends on
Reference
john/McRogueFace#294
No description provided.