From 923350137d148c56e617eae966467c77617c131b Mon Sep 17 00:00:00 2001 From: John McCardle Date: Thu, 3 Jul 2025 21:02:14 -0400 Subject: [PATCH] Implement Entity.index() method for Issue #73 Added index() method to Entity class that returns the entity's position in its parent grid's entity collection. This enables proper entity removal patterns using entity.index(). --- ROADMAP.md | 20 ++++-- quick_index_test.py | 12 ++++ src/UIEntity.cpp | 28 ++++++++ src/UIEntity.h | 1 + tests/issue73_entity_index_test.py | 101 +++++++++++++++++++++++++++++ tests/issue73_simple_index_test.py | 77 ++++++++++++++++++++++ 6 files changed, 234 insertions(+), 5 deletions(-) create mode 100644 quick_index_test.py create mode 100644 tests/issue73_entity_index_test.py create mode 100644 tests/issue73_simple_index_test.py diff --git a/ROADMAP.md b/ROADMAP.md index b6c629e..f8b199a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -189,11 +189,21 @@ Created comprehensive test suite with 13 tests covering all Python-exposed metho ### Phase 1: Foundation Stabilization (1-2 weeks) ``` -1. Fix Grid Segfault - CRITICAL: Unblocks all Grid functionality -2. Fix #78 Middle Mouse Click bug - Confirmed event handling issue -3. Fix Entity/Sprite property setters - Multiple Python binding errors -4. Critical Bugfixes (#77, #74, #37) - Fix remaining pain points -5. Alpha Blockers (#3, #2) - Remove deprecated methods +✅ COMPLETE AS OF 2025-01-03: +1. ✅ Fix Grid Segfault - Grid now supports None/null textures +2. ✅ Fix #78 Middle Mouse Click bug - Event type checking added +3. ✅ Fix Entity/Sprite property setters - PyVector conversion fixed +4. ✅ Fix #77 - Error message copy/paste bug fixed +5. ✅ Fix #74 - Grid.grid_y property added +6. ✅ Fix keypressScene() validation - Now rejects non-callable +7. ✅ Fix Sprite texture setter - No longer returns error without exception +8. ✅ Fix PyVector x/y properties - Were returning None + +REMAINING IN PHASE 1: +9. Fix #73 - Entity.index() method for removal +10. Fix #27 - EntityCollection.extend() method +11. Fix #33 - Sprite index validation +12. Alpha Blockers (#3, #2) - Remove deprecated methods ``` ### Phase 2: Alpha Release Preparation (4-6 weeks) diff --git a/quick_index_test.py b/quick_index_test.py new file mode 100644 index 0000000..0a09571 --- /dev/null +++ b/quick_index_test.py @@ -0,0 +1,12 @@ +import mcrfpy +t = mcrfpy.Texture("assets/kenney_ice.png", 16, 16) +g = mcrfpy.Grid(5, 5, t, (0, 0), (100, 100)) +e1 = mcrfpy.Entity((1, 1), t, 1, g) +e2 = mcrfpy.Entity((2, 2), t, 2, g) +g.entities.append(e1) +g.entities.append(e2) +print("Entity 1 index:", e1.index()) +print("Entity 2 index:", e2.index()) +# Test removal +g.entities.remove(e1.index()) +print("After removal, Entity 2 index:", e2.index()) \ No newline at end of file diff --git a/src/UIEntity.cpp b/src/UIEntity.cpp index 3b8950c..1a3ce03 100644 --- a/src/UIEntity.cpp +++ b/src/UIEntity.cpp @@ -36,6 +36,33 @@ PyObject* UIEntity::at(PyUIEntityObject* self, PyObject* o) { } +PyObject* UIEntity::index(PyUIEntityObject* self, PyObject* Py_UNUSED(ignored)) { + // Check if entity has an associated grid + if (!self->data || !self->data->grid) { + PyErr_SetString(PyExc_RuntimeError, "Entity is not associated with a grid"); + return NULL; + } + + // Get the grid's entity collection + auto entities = self->data->grid->entities; + if (!entities) { + PyErr_SetString(PyExc_RuntimeError, "Grid has no entity collection"); + return NULL; + } + + // Find this entity in the collection + int index = 0; + for (auto it = entities->begin(); it != entities->end(); ++it, ++index) { + if (it->get() == self->data.get()) { + return PyLong_FromLong(index); + } + } + + // Entity not found in its grid's collection + PyErr_SetString(PyExc_ValueError, "Entity not found in its grid's entity collection"); + return NULL; +} + int UIEntity::init(PyUIEntityObject* self, PyObject* args, PyObject* kwds) { //static const char* keywords[] = { "x", "y", "texture", "sprite_index", "grid", nullptr }; //float x = 0.0f, y = 0.0f, scale = 1.0f; @@ -211,6 +238,7 @@ int UIEntity::set_spritenumber(PyUIEntityObject* self, PyObject* value, void* cl PyMethodDef UIEntity::methods[] = { {"at", (PyCFunction)UIEntity::at, METH_O}, + {"index", (PyCFunction)UIEntity::index, METH_NOARGS, "Return the index of this entity in its grid's entity collection"}, {NULL, NULL, 0, NULL} }; diff --git a/src/UIEntity.h b/src/UIEntity.h index 42ede28..8cee8b4 100644 --- a/src/UIEntity.h +++ b/src/UIEntity.h @@ -47,6 +47,7 @@ public: UIEntity(UIGrid&); static PyObject* at(PyUIEntityObject* self, PyObject* o); + static PyObject* index(PyUIEntityObject* self, PyObject* Py_UNUSED(ignored)); static int init(PyUIEntityObject* self, PyObject* args, PyObject* kwds); static PyObject* get_position(PyUIEntityObject* self, void* closure); diff --git a/tests/issue73_entity_index_test.py b/tests/issue73_entity_index_test.py new file mode 100644 index 0000000..18662ec --- /dev/null +++ b/tests/issue73_entity_index_test.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +""" +Test for Issue #73: Entity.index() method for removal + +Verifies that Entity objects can report their index in the grid's entity collection. +""" + +def test_entity_index(timer_name): + """Test that Entity.index() method works correctly""" + import mcrfpy + import sys + + print("Issue #73 test: Entity.index() method") + + # Create test scene and grid + mcrfpy.createScene("test") + ui = mcrfpy.sceneUI("test") + + # Create grid with texture + texture = mcrfpy.Texture("assets/kenney_ice.png", 16, 16) + grid = mcrfpy.Grid(10, 10, texture, (10, 10), (400, 400)) + ui.append(grid) + + # Create multiple entities + entities = [] + for i in range(5): + entity = mcrfpy.Entity((i, i), texture, i, grid) + entities.append(entity) + grid.entities.append(entity) + + print(f"✓ Created {len(entities)} entities") + + # Test 1: Check each entity knows its index + for expected_idx, entity in enumerate(entities): + try: + actual_idx = entity.index() + assert actual_idx == expected_idx, f"Expected index {expected_idx}, got {actual_idx}" + print(f"✓ Entity {expected_idx} correctly reports index {actual_idx}") + except Exception as e: + print(f"✗ Entity {expected_idx} index() failed: {e}") + raise + + # Test 2: Remove entity using index + entity_to_remove = entities[2] + remove_idx = entity_to_remove.index() + grid.entities.remove(remove_idx) + print(f"✓ Removed entity at index {remove_idx}") + + # Test 3: Verify indices updated after removal + for i, entity in enumerate(entities): + if i == 2: + # This entity was removed, should raise error + try: + idx = entity.index() + print(f"✗ Removed entity still reports index {idx}") + except ValueError as e: + print(f"✓ Removed entity correctly raises error: {e}") + elif i < 2: + # These entities should keep their indices + idx = entity.index() + assert idx == i, f"Entity before removal has wrong index: {idx}" + else: + # These entities should have shifted down by 1 + idx = entity.index() + assert idx == i - 1, f"Entity after removal has wrong index: {idx}" + + # Test 4: Entity without grid + orphan_entity = mcrfpy.Entity((0, 0), texture, 0, None) + try: + idx = orphan_entity.index() + print(f"✗ Orphan entity should raise error but returned {idx}") + except RuntimeError as e: + print(f"✓ Orphan entity correctly raises error: {e}") + + # Test 5: Use index() in practical removal pattern + # Add some new entities + for i in range(3): + entity = mcrfpy.Entity((7+i, 7+i), texture, 10+i, grid) + grid.entities.append(entity) + + # Remove entities with sprite_number > 10 + removed_count = 0 + i = 0 + while i < len(grid.entities): + entity = grid.entities[i] + if entity.sprite_number > 10: + grid.entities.remove(entity.index()) + removed_count += 1 + # Don't increment i, as entities shifted down + else: + i += 1 + + print(f"✓ Removed {removed_count} entities using index() in loop") + assert len(grid.entities) == 5, f"Expected 5 entities remaining, got {len(grid.entities)}" + + print("\n✅ Issue #73 test PASSED - Entity.index() method works correctly") + sys.exit(0) + +# Execute the test after a short delay +import mcrfpy +mcrfpy.setTimer("test", test_entity_index, 100) \ No newline at end of file diff --git a/tests/issue73_simple_index_test.py b/tests/issue73_simple_index_test.py new file mode 100644 index 0000000..a206f65 --- /dev/null +++ b/tests/issue73_simple_index_test.py @@ -0,0 +1,77 @@ +#!/usr/bin/env python3 +""" +Simple test for Issue #73: Entity.index() method +""" + +def test_entity_index(timer_name): + """Test that Entity.index() method works correctly""" + import mcrfpy + import sys + + print("Testing Entity.index() method...") + + # Create test scene and grid + mcrfpy.createScene("test") + ui = mcrfpy.sceneUI("test") + + # Create grid with texture + texture = mcrfpy.Texture("assets/kenney_ice.png", 16, 16) + grid = mcrfpy.Grid(10, 10, texture, (10, 10), (400, 400)) + ui.append(grid) + + # Clear any existing entities + while len(grid.entities) > 0: + grid.entities.remove(0) + + # Create entities + entity1 = mcrfpy.Entity((1, 1), texture, 1, grid) + entity2 = mcrfpy.Entity((2, 2), texture, 2, grid) + entity3 = mcrfpy.Entity((3, 3), texture, 3, grid) + + grid.entities.append(entity1) + grid.entities.append(entity2) + grid.entities.append(entity3) + + print(f"Created {len(grid.entities)} entities") + + # Test index() method + idx1 = entity1.index() + idx2 = entity2.index() + idx3 = entity3.index() + + print(f"Entity 1 index: {idx1}") + print(f"Entity 2 index: {idx2}") + print(f"Entity 3 index: {idx3}") + + assert idx1 == 0, f"Entity 1 should be at index 0, got {idx1}" + assert idx2 == 1, f"Entity 2 should be at index 1, got {idx2}" + assert idx3 == 2, f"Entity 3 should be at index 2, got {idx3}" + + print("✓ All entities report correct indices") + + # Test removal using index + remove_idx = entity2.index() + grid.entities.remove(remove_idx) + print(f"✓ Removed entity at index {remove_idx}") + + # Check remaining entities + assert len(grid.entities) == 2 + assert entity1.index() == 0 + assert entity3.index() == 1 # Should have shifted down + + print("✓ Indices updated correctly after removal") + + # Test entity not in grid + orphan = mcrfpy.Entity((5, 5), texture, 5, None) + try: + idx = orphan.index() + print(f"✗ Orphan entity should raise error but returned {idx}") + except RuntimeError as e: + print(f"✓ Orphan entity correctly raises error") + + print("\n✅ Entity.index() test PASSED") + sys.exit(0) + +# Execute the test after a short delay +import mcrfpy +mcrfpy.setTimer("test", test_entity_index, 100) \ No newline at end of file