john/McRogueFace

Fork 0

[Refactoring] Migrate UIEntity::grid from shared_ptr<UIGrid> to shared_ptr<GridData> #313

New issue

Open

opened 2026-04-17 03:25:40 +00:00 by john · 0 comments

john commented

2026-04-17 03:25:40 +00:00

Owner

Context

The Grid / GridView overhaul (#252) extracted GridData as the pure data layer. UIGrid now inherits from both UIDrawable and GridData. UIGridView holds a std::shared_ptr<GridData> and renders from it. UIGridPoint, GridChunk, and GridLayer were all converted to hold GridData* parent_grid instead of UIGrid*.

The one remaining holdout is Entity. UIEntity::grid is still std::shared_ptr<UIGrid>:

// src/UIEntity.h:68
std::shared_ptr<UIGrid> grid;

shared_ptr<UIGrid> also appears in UIEntityCollection.h, GridLayers.cpp, UIGridPoint.h, and a few call sites in UIEntity.cpp. These are the last places where entity logic is coupled to the rendering type instead of the data layer.

Why migrate

Architectural consistency — the data/view split is already made everywhere except Entity. Completing it retires the last piece of #252.
Headless ergonomics — an entity tied to GridData rather than UIGrid means game logic works correctly in contexts where rendering isn't loaded.
Forcing function for 1.0 API decisions — settles what entity.grid means: is it the rendering-coupled mcrfpy.Grid or a pure data view?
Enables #294 — Entity.gridstate as DiscreteMap reference depends on this data-layer plumbing existing cleanly.

Scope

C++ changes

Change UIEntity::grid from std::shared_ptr<UIGrid> to std::shared_ptr<GridData>
Update call sites (entity->grid->getTexture(), etc.) to go through GridData methods (most already exist on GridData; any UIGrid-only method needs to be hoisted or accessed via a different path)
Update UIEntityCollection, GridLayers, UIGridPoint references

Python API decisions

Option A (preserve surface): entity.grid still returns the mcrfpy.Grid Python object. The C++ GridData* has to be upcast to UIGrid for Python exposure. Requires that any GridData owned by a UIGrid can reach back to its UIGrid, OR that Python-side storage keeps both pointers.
Option B (split surface): entity.grid returns the UIGrid (as today) when one exists; a new entity.grid_data returns a data-only handle. Explicit dual API.
Option C (data-only): entity.grid starts returning a new mcrfpy.GridData Python type. Breaking change, but cleanest.

Recommend Option A for 1.0 unless there's a game-dev reason to split.

Tests

Regression test: entity survives its UIGrid being destroyed (only the GridData is kept alive)
Existing grid/entity tests should pass unchanged under Option A

#294 (Entity.gridstate as DiscreteMap reference) — naturally chains onto this
#252 (closed, Grid/GridView overhaul) — this is the final phase
Alpha Release Requirement if we want it locked before 1.0

Estimate

Medium. The type change is mechanical; the interesting work is the Python API decision and the corresponding update to docs / stubs.

## Context The Grid / GridView overhaul (#252) extracted `GridData` as the pure data layer. `UIGrid` now inherits from both `UIDrawable` and `GridData`. `UIGridView` holds a `std::shared_ptr<GridData>` and renders from it. `UIGridPoint`, `GridChunk`, and `GridLayer` were all converted to hold `GridData* parent_grid` instead of `UIGrid*`. **The one remaining holdout is Entity.** `UIEntity::grid` is still `std::shared_ptr<UIGrid>`: ```cpp // src/UIEntity.h:68 std::shared_ptr<UIGrid> grid; ``` `shared_ptr<UIGrid>` also appears in `UIEntityCollection.h`, `GridLayers.cpp`, `UIGridPoint.h`, and a few call sites in `UIEntity.cpp`. These are the last places where entity logic is coupled to the rendering type instead of the data layer. ## Why migrate 1. **Architectural consistency** — the data/view split is already made everywhere except Entity. Completing it retires the last piece of #252. 2. **Headless ergonomics** — an entity tied to `GridData` rather than `UIGrid` means game logic works correctly in contexts where rendering isn't loaded. 3. **Forcing function for 1.0 API decisions** — settles what `entity.grid` means: is it the rendering-coupled `mcrfpy.Grid` or a pure data view? 4. **Enables #294** — `Entity.gridstate as DiscreteMap reference` depends on this data-layer plumbing existing cleanly. ## Scope ### C++ changes - Change `UIEntity::grid` from `std::shared_ptr<UIGrid>` to `std::shared_ptr<GridData>` - Update call sites (`entity->grid->getTexture()`, etc.) to go through `GridData` methods (most already exist on `GridData`; any UIGrid-only method needs to be hoisted or accessed via a different path) - Update `UIEntityCollection`, `GridLayers`, `UIGridPoint` references ### Python API decisions - **Option A (preserve surface)**: `entity.grid` still returns the `mcrfpy.Grid` Python object. The C++ `GridData*` has to be upcast to `UIGrid` for Python exposure. Requires that any GridData owned by a UIGrid can reach back to its UIGrid, OR that Python-side storage keeps both pointers. - **Option B (split surface)**: `entity.grid` returns the UIGrid (as today) when one exists; a new `entity.grid_data` returns a data-only handle. Explicit dual API. - **Option C (data-only)**: `entity.grid` starts returning a new `mcrfpy.GridData` Python type. Breaking change, but cleanest. Recommend Option A for 1.0 unless there's a game-dev reason to split. ### Tests - Regression test: entity survives its UIGrid being destroyed (only the GridData is kept alive) - Existing grid/entity tests should pass unchanged under Option A ## Blocks / Related - #294 (Entity.gridstate as DiscreteMap reference) — naturally chains onto this - #252 (closed, Grid/GridView overhaul) — this is the final phase - Alpha Release Requirement if we want it locked before 1.0 ## Estimate Medium. The type change is mechanical; the interesting work is the Python API decision and the corresponding update to docs / stubs.

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.

Dependencies

No dependencies set.

Reference

john/McRogueFace#313

No description provided.

Rows
Columns

[Refactoring] Migrate UIEntity::grid from shared_ptr&lt;UIGrid&gt; to shared_ptr&lt;GridData&gt; #313