Remove entity self-reference cycle

UIEntity::init() stored self->data->self = (PyObject*)self with
Py_INCREF(self), creating a reference cycle that prevented entities
from ever being freed. The matching Py_DECREF never existed.

Fix: Remove the `self` field from UIEntity entirely. Replace all
read sites (iter next, getitem, get_perspective, entities_in_radius)
with PythonObjectCache lookups using serial_number, which uses weak
references and doesn't prevent garbage collection.

Also adds tp_dealloc to PyUIEntityType to properly clean up the
shared_ptr and weak references when the Python wrapper is freed.

Closes #266, closes #275

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

src/UIEntity.cpp

@@ -19,7 +19,7 @@
 
 
 UIEntity::UIEntity()
-: self(nullptr), grid(nullptr), position(0.0f, 0.0f), sprite_offset(0.0f, 0.0f)
+: grid(nullptr), position(0.0f, 0.0f), sprite_offset(0.0f, 0.0f)
 {
     // Initialize sprite with safe defaults (sprite has its own safe constructor now)
     // gridstate vector starts empty - will be lazily initialized when needed
@@ -241,10 +241,6 @@ int UIEntity::init(PyUIEntityObject* self, PyObject* args, PyObject* kwds) {
         }
     }
     
-    // Store reference to Python object (legacy - to be removed)
-    self->data->self = (PyObject*)self;
-    Py_INCREF(self);
-
     // Set texture and sprite index
     if (texture_ptr) {
         self->data->sprite = UISprite(texture_ptr, sprite_index, sf::Vector2f(0,0), 1.0);

src/UIEntity.h

@@ -60,7 +60,6 @@ PyObject* UIGridPointStateVector_to_PyList(const std::vector<UIGridPointState>&
 class UIEntity
 {
 public:
-    PyObject* self = nullptr;  // Reference to the Python object (if created from Python)
     uint64_t serial_number = 0;  // For Python object cache
     std::shared_ptr<UIGrid> grid;
     std::vector<UIGridPointState> gridstate;
@@ -135,6 +134,12 @@ namespace mcrfpydef {
         .tp_name = "mcrfpy.Entity",
         .tp_basicsize = sizeof(PyUIEntityObject),
         .tp_itemsize = 0,
+        .tp_dealloc = [](PyObject* obj) {
+            auto* self = (PyUIEntityObject*)obj;
+            if (self->weakreflist) PyObject_ClearWeakRefs(obj);
+            self->data.reset();
+            Py_TYPE(obj)->tp_free(obj);
+        },
         .tp_repr = (reprfunc)UIEntity::repr,
         .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
         .tp_doc = PyDoc_STR("Entity(grid_pos=None, texture=None, sprite_index=0, **kwargs)\n\n"

src/UIEntityCollection.cpp

@@ -41,10 +41,12 @@ PyObject* UIEntityCollectionIter::next(PyUIEntityCollectionIterObject* self)
     auto target = *self->current;
     ++self->current;
 
-    // Return the stored Python object if it exists (preserves derived types)
-    if (target->self != nullptr) {
-        Py_INCREF(target->self);
-        return target->self;
+    // Check cache first to preserve derived class identity
+    if (target->serial_number != 0) {
+        PyObject* cached = PythonObjectCache::getInstance().lookup(target->serial_number);
+        if (cached) {
+            return cached;  // Already INCREF'd by lookup
+        }
     }
 
     // Otherwise create and return a new Python Entity object
@@ -107,13 +109,7 @@ PyObject* UIEntityCollection::getitem(PyUIEntityCollectionObject* self, Py_ssize
         }
     }
 
-    // Legacy: If the entity has a stored Python object reference, return that
-    if (target->self != nullptr) {
-        Py_INCREF(target->self);
-        return target->self;
-    }
-
-    // Otherwise, create a new base Entity object
+    // Create a new base Entity object
     PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType;
 
     auto o = (PyUIEntityObject*)entity_type->tp_alloc(entity_type, 0);

src/UIGrid.cpp

@@ -1442,13 +1442,7 @@ PyObject* UIGrid::get_perspective(PyUIGridObject* self, void* closure)
             }
         }
         
-        // Legacy: If the entity has a stored Python object reference
-        if (locked->self != nullptr) {
-            Py_INCREF(locked->self);
-            return locked->self;
-        }
-        
-        // Otherwise, create a new base Entity object
+        // Create a new base Entity object
         auto type = &mcrfpydef::PyUIEntityType;
         auto o = (PyUIEntityObject*)type->tp_alloc(type, 0);
         if (o) {
@@ -1936,11 +1930,12 @@ PyObject* UIGrid::py_entities_in_radius(PyUIGridObject* self, PyObject* args, Py
     for (size_t i = 0; i < entities.size(); i++) {
         auto& entity = entities[i];
 
-        // Return stored Python object if it exists
-        if (entity->self != nullptr) {
-            Py_INCREF(entity->self);
-            PyList_SET_ITEM(result, i, entity->self);
-        } else {
+        // Check cache first to preserve derived class identity
+        PyObject* py_entity = nullptr;
+        if (entity->serial_number != 0) {
+            py_entity = PythonObjectCache::getInstance().lookup(entity->serial_number);
+        }
+        if (!py_entity) {
             // Create new Python Entity wrapper
             auto pyEntity = (PyUIEntityObject*)entity_type->tp_alloc(entity_type, 0);
             if (!pyEntity) {
@@ -1949,8 +1944,9 @@ PyObject* UIGrid::py_entities_in_radius(PyUIGridObject* self, PyObject* args, Py
             }
             pyEntity->data = entity;
             pyEntity->weakreflist = NULL;
-            PyList_SET_ITEM(result, i, (PyObject*)pyEntity);
+            py_entity = (PyObject*)pyEntity;
         }
+        PyList_SET_ITEM(result, i, py_entity);
     }
 
     return result;

tests/regression/issue_266_entity_lifecycle_test.py

@@ -0,0 +1,100 @@
+"""Regression test: entity self-reference cycle (#266, #275).
+
+Bug: UIEntity::init() stored a reference to the Python wrapper via
+self->data->self = (PyObject*)self and Py_INCREF(self). This created
+a reference cycle that prevented the entity from ever being freed,
+even after removing it from all grids and dropping all Python references.
+
+Fix: Remove the self field entirely. Use PythonObjectCache (weak refs)
+for Python object identity preservation instead.
+"""
+import mcrfpy
+import gc
+import sys
+
+def test_entity_accessible_via_grid():
+    """Entities remain accessible through grid.entities after Python ref dropped"""
+    grid = mcrfpy.Grid(grid_size=(10, 10))
+    entity = mcrfpy.Entity(grid_pos=(5, 5), grid=grid)
+    entity_id = id(entity)
+
+    # Drop the Python reference
+    del entity
+    gc.collect()
+
+    # Entity should still be accessible via grid
+    assert len(grid.entities) == 1, f"Expected 1 entity, got {len(grid.entities)}"
+    retrieved = grid.entities[0]
+    assert retrieved.grid_x == 5.0, f"Expected x=5.0, got {retrieved.grid_x}"
+    print("  PASS: entity_accessible_via_grid")
+
+def test_entity_removed_from_grid():
+    """Entity can be removed from grid cleanly"""
+    grid = mcrfpy.Grid(grid_size=(10, 10))
+    entity = mcrfpy.Entity(grid_pos=(3, 3), grid=grid)
+
+    assert len(grid.entities) == 1
+    grid.entities.remove(entity)
+    assert len(grid.entities) == 0
+    print("  PASS: entity_removed_from_grid")
+
+def test_multiple_entities():
+    """Multiple entities can be created, accessed, and removed"""
+    grid = mcrfpy.Grid(grid_size=(20, 20))
+    entities = []
+    for i in range(10):
+        e = mcrfpy.Entity(grid_pos=(i, i), grid=grid)
+        entities.append(e)
+
+    assert len(grid.entities) == 10
+
+    # Drop all Python references
+    del entities
+    gc.collect()
+
+    # All should still be in grid
+    assert len(grid.entities) == 10
+
+    # Access each one
+    for i in range(10):
+        e = grid.entities[i]
+        assert e.grid_x == float(i), f"Entity {i} x={e.grid_x}, expected {float(i)}"
+
+    print("  PASS: multiple_entities")
+
+def test_entity_transfer_preserves_identity():
+    """Entity identity preserved when transferring between grids"""
+    grid1 = mcrfpy.Grid(grid_size=(10, 10))
+    grid2 = mcrfpy.Grid(grid_size=(20, 20))
+    entity = mcrfpy.Entity(grid_pos=(5, 5), grid=grid1)
+
+    entity.grid = grid2
+    assert len(grid1.entities) == 0
+    assert len(grid2.entities) == 1
+
+    retrieved = grid2.entities[0]
+    assert retrieved.grid_x == 5.0
+    print("  PASS: entity_transfer_preserves_identity")
+
+def test_iteration_after_gc():
+    """Iterating grid.entities works after GC of Python wrappers"""
+    grid = mcrfpy.Grid(grid_size=(10, 10))
+    for i in range(5):
+        mcrfpy.Entity(grid_pos=(i, 0), grid=grid)
+
+    gc.collect()
+
+    count = 0
+    for e in grid.entities:
+        count += 1
+    assert count == 5, f"Expected 5 entities in iteration, got {count}"
+    print("  PASS: iteration_after_gc")
+
+print("Testing entity lifecycle (self-reference removal)...")
+test_entity_accessible_via_grid()
+test_entity_removed_from_grid()
+test_multiple_entities()
+test_entity_transfer_preserves_identity()
+test_iteration_after_gc()
+print("PASS: all entity lifecycle tests passed")
+sys.exit(0)