From 5173eaf7f588e227520aeda2127e2f1630330f9d Mon Sep 17 00:00:00 2001 From: John McCardle Date: Fri, 10 Apr 2026 01:40:29 -0400 Subject: [PATCH] Update spatial hash in animation setProperty for entity position, closes #256 UIEntity::setProperty() now calls spatial_hash.update() when draw_x/draw_y change, matching the Python property setter behavior. Added enable_shared_from_this to support shared_from_this() in the setProperty path. Co-Authored-By: Claude Opus 4.6 --- src/UIEntity.cpp | 16 +++-- src/UIEntity.h | 2 +- .../issue_256_animation_spatial_hash_test.py | 71 +++++++++++++++++++ 3 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 tests/regression/issue_256_animation_spatial_hash_test.py diff --git a/src/UIEntity.cpp b/src/UIEntity.cpp index e48199e..fbd9487 100644 --- a/src/UIEntity.cpp +++ b/src/UIEntity.cpp @@ -1640,15 +1640,23 @@ PyObject* UIEntity::repr(PyUIEntityObject* self) { // "x" and "y" are kept as aliases for backwards compatibility bool UIEntity::setProperty(const std::string& name, float value) { if (name == "draw_x" || name == "x") { // #176 - draw_x is preferred, x is alias + float old_x = position.x; + float old_y = position.y; position.x = value; - // Don't update sprite position here - UIGrid::render() handles the pixel positioning - if (grid) grid->markCompositeDirty(); // #144 - Propagate to parent grid for texture caching + if (grid) { + grid->markCompositeDirty(); + grid->spatial_hash.update(shared_from_this(), old_x, old_y); // #256 + } return true; } else if (name == "draw_y" || name == "y") { // #176 - draw_y is preferred, y is alias + float old_x = position.x; + float old_y = position.y; position.y = value; - // Don't update sprite position here - UIGrid::render() handles the pixel positioning - if (grid) grid->markCompositeDirty(); // #144 - Propagate to parent grid for texture caching + if (grid) { + grid->markCompositeDirty(); + grid->spatial_hash.update(shared_from_this(), old_x, old_y); // #256 + } return true; } else if (name == "sprite_scale") { diff --git a/src/UIEntity.h b/src/UIEntity.h index 858fa83..1e5e9df 100644 --- a/src/UIEntity.h +++ b/src/UIEntity.h @@ -60,7 +60,7 @@ sf::Vector2f PyObject_to_sfVector2f(PyObject* obj); PyObject* UIGridPointState_to_PyObject(const UIGridPointState& state); PyObject* UIGridPointStateVector_to_PyList(const std::vector& vec); -class UIEntity +class UIEntity : public std::enable_shared_from_this { public: uint64_t serial_number = 0; // For Python object cache diff --git a/tests/regression/issue_256_animation_spatial_hash_test.py b/tests/regression/issue_256_animation_spatial_hash_test.py new file mode 100644 index 0000000..9a16b26 --- /dev/null +++ b/tests/regression/issue_256_animation_spatial_hash_test.py @@ -0,0 +1,71 @@ +"""Regression test: Animation system bypasses spatial hash (#256). + +Bug: When entity position was changed via animate(), the spatial hash +was not updated. This caused grid.entities_in_radius() to return stale +results based on the last Python-assigned position, not the animated one. + +Fix: UIEntity::setProperty() (called by the animation system) now calls +spatial_hash.update() for position property changes, matching the +behavior of Python property setters. +""" +import mcrfpy +import sys + +def test_animate_updates_spatial_hash(): + """Entity animated to new position is found by spatial queries""" + scene = mcrfpy.Scene("test_anim_spatial") + grid = mcrfpy.Grid(grid_size=(50, 50), pos=(0, 0), size=(500, 500)) + scene.children.append(grid) + + entity = mcrfpy.Entity(grid_pos=(5, 5), grid=grid) + + # Verify entity is at initial position + found = grid.entities_in_radius((5.0, 5.0), 1.0) + assert len(found) == 1, f"Expected 1 entity at (5,5), found {len(found)}" + + # Animate to a far-away position using draw_x/draw_y + entity.animate("draw_x", 40.0, 0.01, mcrfpy.Easing.LINEAR) + entity.animate("draw_y", 40.0, 0.01, mcrfpy.Easing.LINEAR) + + # Step the game loop enough to complete the animation + for _ in range(20): + mcrfpy.step(0.01) + + # Verify entity position has changed + pos = entity.draw_pos + assert abs(pos.x - 40.0) < 0.1, f"draw_pos.x should be ~40, got {pos.x}" + assert abs(pos.y - 40.0) < 0.1, f"draw_pos.y should be ~40, got {pos.y}" + + # Spatial hash should now find the entity near (40, 40), not (5, 5) + found_new = grid.entities_in_radius((40.0, 40.0), 2.0) + assert len(found_new) == 1, f"Expected 1 entity near (40,40), found {len(found_new)}" + + found_old = grid.entities_in_radius((5.0, 5.0), 1.0) + assert len(found_old) == 0, f"Expected 0 entities at old pos (5,5), found {len(found_old)}" + + print(" PASS: animate_updates_spatial_hash") + +def test_property_set_still_works(): + """Direct property assignment still updates spatial hash""" + grid = mcrfpy.Grid(grid_size=(50, 50), pos=(0, 0), size=(500, 500)) + entity = mcrfpy.Entity(grid_pos=(10, 10), grid=grid) + + found = grid.entities_in_radius((10.0, 10.0), 1.0) + assert len(found) == 1 + + entity.draw_pos = mcrfpy.Vector(30.0, 30.0) + + found_new = grid.entities_in_radius((30.0, 30.0), 1.0) + assert len(found_new) == 1, f"Expected 1 entity at (30,30), found {len(found_new)}" + + found_old = grid.entities_in_radius((10.0, 10.0), 1.0) + assert len(found_old) == 0, f"Expected 0 at old position, found {len(found_old)}" + + print(" PASS: property_set_still_works") + + +print("Testing animation spatial hash update (#256)...") +test_animate_updates_spatial_hash() +test_property_set_still_works() +print("PASS: all animation spatial hash tests passed") +sys.exit(0)