fix(grid): revive Grid input — move cell callbacks and hit-testing to GridView; N-view registry
Since the #252 Grid/GridView split, mcrfpy.Grid(...) constructs a UIGridView (PyObjectsEnum::UIGRIDVIEW), but every input dispatch site gated on derived_type() == UIGRID — the internal mcrfpy._GridData type that no scene-graph node is ever an instance of. All grid input was dead: cell click/enter/exit callbacks never fired, clicks on a grid's child drawables and entities never dispatched, and hover inside grids did nothing. The cell-callback family was split three ways, which is the actual bug: storage on GridData, firing on UIGrid (it needed UIDrawable::serial_number), and the camera required to resolve a screen pixel into a cell on UIGridView. So merely re-pointing the enum gate at UIGRIDVIEW would still report wrong cells for any panned or zoomed grid, and UIGrid::fireCellClick's subclass lookup resolved the serial of the throwaway _GridData wrapper that init_with_data DECREFs — meaning a Python subclass overriding on_cell_click as a method could never dispatch. Input ownership now lives entirely on UIGridView: - on_cell_click/enter/exit, hovered_cell, last_clicked_cell and the subclass dispatch cache move off GridData/UIGrid (hovered_cell must be per-view: two views over one GridData would otherwise ping-pong exit/enter). - UIGridView::click_at descends children (reverse z) -> entities -> cell. Children keep grid-world pixel coordinates: that is by design (#360), not an inconsistency with Frame's frame-local children. - The six derived_type()==UIGRID gates are replaced by virtuals on UIDrawable (dispatchCellClick, updateHover, asGridData) so no caller switches on grid-ness again. - Fixes a prerequisite bug: UIGridView never overrode onPositionChanged(), so box and position diverged and a repositioned Grid rendered at the stale spot while hit-testing against the new one. Any cell hit-test was meaningless until these agreed. - grid.perspective was silently dead (the FOV overlay gated on a UIGridView::perspective_enabled that nothing ever set); perspective state moves to the view with the rest. #359 is fused into this commit rather than sequenced after it: the shared-data back-reference is rewritten by the same hunks that move the callbacks off GridData, and separating them would mean authoring an intermediate GridData.h / UIGridView.cpp that never existed. GridData::owning_view (a single weak_ptr, for a relationship that is N views to one GridData) becomes a vector<weak_ptr> registry with register/unregister. The registry is kept rather than deleted: the markCompositeDirty push is NOT redundant with #351's content_generation poll, because it propagates bottom-up through each view's ancestor chain (e.g. a Frame(use_render_texture=True) wrapping a view), which a top-down render-time poll cannot do. Grid subclass method dispatch, panned/zoomed cell resolution, grid-child clicks, and cell hover are all covered by new regression tests. tests/unit/ test_grid_cell_events.py previously printed "PARTIAL (events may require interactive mode)" and exited 0 having fired zero events — that dishonest pass is why this shipped broken; it now asserts. Suite: 322/322. closes #355 closes #359 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
4c0953c56a
commit
85e4bbec07
21 changed files with 1941 additions and 898 deletions
191
tests/regression/issue_359_shared_griddata_test.py
Normal file
191
tests/regression/issue_359_shared_griddata_test.py
Normal file
|
|
@ -0,0 +1,191 @@
|
|||
"""Regression test for #359 - the shared-data grid path is unwired.
|
||||
|
||||
Two concrete defects, both in the `Grid(grid=other)` shared-GridData path
|
||||
(the whole point of the #252 split -- split-screen, minimap, multiple
|
||||
cameras over one dataset):
|
||||
|
||||
(a) UIGridView::init_explicit_view never registered the new view with the
|
||||
GridData it attaches to, so GridData::markDirty()/markCompositeDirty()
|
||||
(which push markContentDirty/markCompositeDirty up a view's OWN
|
||||
ancestor chain -- e.g. a Frame(cache_subtree=True) wrapping that view)
|
||||
never reached a secondary view's ancestors. Note this is NOT the same
|
||||
thing as the #351 render early-out itself: UIGridView::render() polls
|
||||
GridData::content_generation directly every time it actually runs, so
|
||||
a bare secondary view (no cached ancestor) already re-rasters
|
||||
correctly regardless of this bug. The bug only surfaces when a
|
||||
secondary view sits under a render-texture-caching ancestor, because
|
||||
that ancestor decides whether to invoke the child's render() AT ALL --
|
||||
a push notification, not a poll, is required to reach it. Test 3 below
|
||||
is the discriminator: it fails (stale cache) on the pre-fix code and
|
||||
passes once init_explicit_view registers the view.
|
||||
|
||||
(b) PyUIGridObject::view (the internal `_GridData.view` shim, meant to
|
||||
point back at "the" GridView) was placement-new'd but never assigned
|
||||
anywhere reachable from Python -- `_GridData.view` was always None.
|
||||
Given `_GridData` is internal (never exported to the mcrfpy module
|
||||
namespace), the fix deletes the field/getter outright rather than
|
||||
wiring it up (a single view back-pointer is structurally wrong once N
|
||||
views can share one GridData; see GridData::views / primaryView() for
|
||||
the replacement, exercised implicitly by test 2). Test 4 confirms the
|
||||
dead property is gone, not silently still None.
|
||||
|
||||
Design note carried over from the fix: GridData::views is a *list*, not a
|
||||
single pointer, specifically because N views can share one GridData. Index 0
|
||||
is guaranteed (by construction: a fresh GridData is only ever produced by
|
||||
Grid()'s factory path, `init_with_data`; `init_explicit_view` can only ATTACH
|
||||
to an already-existing GridData) to be the view that created the data, so
|
||||
UIEntity.grid identity (test 2) stays deterministic even with secondary
|
||||
views registered -- not an arbitrary pick from an unordered set.
|
||||
"""
|
||||
import sys
|
||||
import os
|
||||
import tempfile
|
||||
import mcrfpy
|
||||
from mcrfpy import automation
|
||||
|
||||
TMPDIR = tempfile.mkdtemp(prefix="mcrf359_")
|
||||
_counter = 0
|
||||
|
||||
|
||||
def shot():
|
||||
"""Render the active scene to PNG and return its bytes (headless
|
||||
screenshot is synchronous -- see #351's test for the same pattern)."""
|
||||
global _counter
|
||||
path = os.path.join(TMPDIR, "s%d.png" % _counter)
|
||||
_counter += 1
|
||||
automation.screenshot(path)
|
||||
with open(path, "rb") as f:
|
||||
return f.read()
|
||||
|
||||
|
||||
def test_1_two_views_render_shared_mutations():
|
||||
"""Two Grid views over one GridData, with different camera params, both
|
||||
reflect a mutation made through a third path (this is the exact scenario
|
||||
the #359 issue's own test description asks for)."""
|
||||
print("(1) two views over one GridData both render mutations...")
|
||||
texture = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||
|
||||
scene_a = mcrfpy.Scene("issue359_1a")
|
||||
grid_a = mcrfpy.Grid(grid_size=(10, 10), pos=(0, 0), size=(160, 160), texture=texture)
|
||||
scene_a.children.append(grid_a)
|
||||
|
||||
scene_b = mcrfpy.Scene("issue359_1b")
|
||||
grid_b = mcrfpy.Grid(grid=grid_a, pos=(0, 0), size=(160, 160))
|
||||
grid_b.zoom = 1.5
|
||||
grid_b.center = (64, 64)
|
||||
scene_b.children.append(grid_b)
|
||||
|
||||
mcrfpy.current_scene = scene_a
|
||||
a1 = shot()
|
||||
mcrfpy.current_scene = scene_b
|
||||
b1 = shot()
|
||||
|
||||
# Mutate through the shared data (add a visible entity at cell (3,3)).
|
||||
e = mcrfpy.Entity((3, 3), grid=grid_a)
|
||||
e.sprite_index = 5
|
||||
|
||||
mcrfpy.current_scene = scene_a
|
||||
a2 = shot()
|
||||
assert a2 != a1, "primary view (grid_a) did not reflect the mutation"
|
||||
|
||||
mcrfpy.current_scene = scene_b
|
||||
b2 = shot()
|
||||
assert b2 != b1, "secondary view (grid_b) did not reflect the mutation"
|
||||
print(" OK")
|
||||
|
||||
|
||||
def test_2_entity_grid_identity():
|
||||
"""entity.grid returns the SAME object as the Grid() that actually
|
||||
created the data (GridData::views.front()), not the secondary view, not
|
||||
None, and not a fresh throwaway wrapper -- even once a secondary view is
|
||||
registered on the same GridData."""
|
||||
print("(2) entity.grid identity with a secondary view registered...")
|
||||
grid_a = mcrfpy.Grid(grid_size=(5, 5))
|
||||
grid_b = mcrfpy.Grid(grid=grid_a, pos=(0, 0), size=(80, 80)) # noqa: F841 (keep alive)
|
||||
|
||||
e = mcrfpy.Entity((1, 1), grid=grid_a)
|
||||
|
||||
assert e.grid is grid_a, f"entity.grid identity broken: {e.grid!r} is not grid_a ({grid_a!r})"
|
||||
assert e.grid is not grid_b, "entity.grid returned the secondary view instead of the creator"
|
||||
print(" OK")
|
||||
|
||||
|
||||
def test_3_ancestor_cache_invalidation_through_secondary_view():
|
||||
"""The discriminating regression: a SECONDARY view nested inside a
|
||||
Frame(cache_subtree=True) must have its ancestor's cached subtree
|
||||
invalidated when the shared GridData mutates, even though the mutation
|
||||
happens through a DIFFERENT (primary) view/handle. This requires
|
||||
init_explicit_view to register the secondary view with GridData -- pure
|
||||
content_generation polling (#351) is not enough, because the caching
|
||||
Frame decides whether to call the child's render() at all."""
|
||||
print("(3) ancestor render-texture cache invalidated via secondary view...")
|
||||
texture = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||
|
||||
scene = mcrfpy.Scene("issue359_3")
|
||||
grid_a = mcrfpy.Grid(grid_size=(6, 6), pos=(200, 200), size=(96, 96), texture=texture)
|
||||
# grid_a deliberately NOT added to this scene -- it exists only to own
|
||||
# the data and be the mutation path; only the secondary view is visible.
|
||||
|
||||
grid_b = mcrfpy.Grid(grid=grid_a, pos=(0, 0), size=(96, 96))
|
||||
|
||||
frame = mcrfpy.Frame(pos=(0, 0), size=(96, 96))
|
||||
frame.cache_subtree = True
|
||||
frame.children.append(grid_b)
|
||||
scene.children.append(frame)
|
||||
mcrfpy.current_scene = scene
|
||||
|
||||
# First render bakes the Frame's cached subtree (render_dirty starts True).
|
||||
shot1 = shot()
|
||||
# Idle render: cache should be stable (sanity check the test methodology).
|
||||
shot2 = shot()
|
||||
assert shot1 == shot2, "cache_subtree Frame was not stable across an idle frame (test setup problem)"
|
||||
|
||||
# Mutate the shared GridData through the PRIMARY handle grid_a, NOT the
|
||||
# secondary view. If grid_b is registered on the GridData, this reaches
|
||||
# grid_b's own ancestor chain (the cache_subtree Frame) and invalidates it.
|
||||
e = mcrfpy.Entity((3, 3), grid=grid_a)
|
||||
e.sprite_index = 5
|
||||
|
||||
shot3 = shot()
|
||||
assert shot3 != shot2, (
|
||||
"Frame(cache_subtree=True) wrapping the SECONDARY view did not invalidate "
|
||||
"when the shared GridData mutated through the primary handle -- "
|
||||
"init_explicit_view is not registering the view with GridData (#359 defect a)"
|
||||
)
|
||||
print(" OK")
|
||||
|
||||
|
||||
def test_4_griddata_view_property_removed():
|
||||
"""_GridData.view (PyUIGridObject::view) is dead: nothing in the
|
||||
codebase ever assigned it, and _GridData is internal/not exported. The
|
||||
fix deletes the field and getter outright rather than wiring a
|
||||
structurally-wrong single back-pointer; confirm it's actually gone, not
|
||||
silently still None."""
|
||||
print("(4) _GridData.view property deleted (not left dangling)...")
|
||||
grid_a = mcrfpy.Grid(grid_size=(3, 3))
|
||||
gd = grid_a.grid_data
|
||||
assert not hasattr(gd, "view"), (
|
||||
"_GridData.view still exists -- #359 defect (b) says delete it "
|
||||
"(internal type, never wired, N-view sharing makes a single "
|
||||
"back-pointer wrong) rather than leave it dangling"
|
||||
)
|
||||
print(" OK")
|
||||
|
||||
|
||||
def main():
|
||||
test_1_two_views_render_shared_mutations()
|
||||
test_2_entity_grid_identity()
|
||||
test_3_ancestor_cache_invalidation_through_secondary_view()
|
||||
test_4_griddata_view_property_removed()
|
||||
print("PASS")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
try:
|
||||
main()
|
||||
sys.exit(0)
|
||||
except Exception as e:
|
||||
print(f"\nTEST FAILED: {e}")
|
||||
import traceback
|
||||
traceback.print_exc()
|
||||
sys.exit(1)
|
||||
Loading…
Add table
Add a link
Reference in a new issue