Preserve Python subclass identity for entities in grids (reopens #266)

The Phase 3 fix for #266 removed UIEntity::self which prevented
tp_dealloc from ever running. However, this also allowed Python
subclass wrappers (GameEntity, ZoneExit, etc.) to be GC'd while
the C++ entity lived on in a grid. Later access via grid.entities
returned a base Entity wrapper, losing all subclass methods.

Fix: Add UIEntity::pyobject field that holds a strong reference to
the Python wrapper. Set in init(), cleared when the entity leaves
a grid (die(), set_grid(None), collection removal). This keeps
subclass identity alive while in a grid, but allows proper GC when
the entity is removed. Added releasePyIdentity() helper called at
all grid exit points.

Regression test exercises Liber Noster patterns: subclass hierarchy,
isinstance() checks, combat mixins, tooltip/send methods, GC
survival, die(), pop(), remove(), and stress test with 20 entities.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
John McCardle 2026-03-09 00:24:26 -04:00
commit 836a0584df
4 changed files with 291 additions and 2 deletions

View file

@ -26,6 +26,7 @@ UIEntity::UIEntity()
}
UIEntity::~UIEntity() {
releasePyIdentity();
if (serial_number != 0) {
PythonObjectCache::getInstance().remove(serial_number);
}
@ -230,7 +231,7 @@ int UIEntity::init(PyUIEntityObject* self, PyObject* args, PyObject* kwds) {
// Initialize weak reference list
self->weakreflist = NULL;
// Register in Python object cache
// Register in Python object cache
if (self->data->serial_number == 0) {
self->data->serial_number = PythonObjectCache::getInstance().assignSerial();
PyObject* weakref = PyWeakref_NewRef((PyObject*)self, NULL);
@ -239,6 +240,13 @@ int UIEntity::init(PyUIEntityObject* self, PyObject* args, PyObject* kwds) {
Py_DECREF(weakref); // Cache owns the reference now
}
}
// Hold a strong reference to preserve Python subclass identity.
// Without this, the Python wrapper can be GC'd while the C++ entity
// lives on in a grid, and later access returns a base Entity wrapper
// that lacks subclass methods. Cleared in die() and set_grid(None).
self->data->pyobject = (PyObject*)self;
Py_INCREF(self);
// Set texture and sprite index
if (texture_ptr) {
@ -660,6 +668,9 @@ int UIEntity::set_grid(PyUIEntityObject* self, PyObject* value, void* closure)
entities->erase(it);
}
self->data->grid.reset();
// Release identity strong ref — entity left grid
self->data->releasePyIdentity();
}
return 0;
}
@ -762,6 +773,9 @@ PyObject* UIEntity::die(PyUIEntityObject* self, PyObject* Py_UNUSED(ignored))
entities->erase(it);
// Clear the grid reference
self->data->grid.reset();
// Release identity strong ref — entity is no longer in a grid
self->data->releasePyIdentity();
}
Py_RETURN_NONE;

View file

@ -61,6 +61,7 @@ class UIEntity
{
public:
uint64_t serial_number = 0; // For Python object cache
PyObject* pyobject = nullptr; // Strong ref: preserves Python subclass identity while in grid
std::shared_ptr<UIGrid> grid;
std::vector<UIGridPointState> gridstate;
UISprite sprite;
@ -70,7 +71,17 @@ public:
UIEntity();
~UIEntity();
// Release the strong reference that preserves Python subclass identity.
// Called when entity leaves a grid (die, set_grid, collection removal).
void releasePyIdentity() {
if (pyobject) {
PyObject* tmp = pyobject;
pyobject = nullptr;
Py_DECREF(tmp);
}
}
// Visibility methods
void ensureGridstate(); // Resize gridstate to match current grid dimensions
void updateVisibility(); // Update gridstate from current FOV
@ -136,6 +147,8 @@ namespace mcrfpydef {
.tp_itemsize = 0,
.tp_dealloc = [](PyObject* obj) {
auto* self = (PyUIEntityObject*)obj;
// Clear the identity ref without DECREF - we ARE this object
if (self->data) self->data->pyobject = nullptr;
if (self->weakreflist) PyObject_ClearWeakRefs(obj);
self->data.reset();
Py_TYPE(obj)->tp_free(obj);

View file

@ -156,6 +156,7 @@ int UIEntityCollection::setitem(PyUIEntityCollectionObject* self, Py_ssize_t ind
if (self->grid) {
self->grid->spatial_hash.remove(*it);
}
(*it)->releasePyIdentity();
(*it)->grid = nullptr;
list->erase(it);
return 0;
@ -181,6 +182,7 @@ int UIEntityCollection::setitem(PyUIEntityCollectionObject* self, Py_ssize_t ind
}
// Clear grid reference from the old entity
(*it)->releasePyIdentity();
(*it)->grid = nullptr;
// Replace the element and set grid reference
@ -409,6 +411,7 @@ int UIEntityCollection::ass_subscript(PyUIEntityCollectionObject* self, PyObject
if (self->grid) {
self->grid->spatial_hash.remove(*it);
}
(*it)->releasePyIdentity();
(*it)->grid = nullptr;
}
self->data->erase(start_it, stop_it);
@ -426,6 +429,7 @@ int UIEntityCollection::ass_subscript(PyUIEntityCollectionObject* self, PyObject
if (self->grid) {
self->grid->spatial_hash.remove(*it);
}
(*it)->releasePyIdentity();
(*it)->grid = nullptr;
self->data->erase(it);
}
@ -479,6 +483,7 @@ int UIEntityCollection::ass_subscript(PyUIEntityCollectionObject* self, PyObject
if (self->grid) {
self->grid->spatial_hash.remove(*it);
}
(*it)->releasePyIdentity();
(*it)->grid = nullptr;
}
@ -610,6 +615,7 @@ PyObject* UIEntityCollection::remove(PyUIEntityCollectionObject* self, PyObject*
if (self->grid) {
self->grid->spatial_hash.remove(*it);
}
(*it)->releasePyIdentity();
(*it)->grid = nullptr;
list->erase(it);
Py_RETURN_NONE;
@ -727,6 +733,20 @@ PyObject* UIEntityCollection::pop(PyUIEntityCollectionObject* self, PyObject* ar
entity->grid = nullptr;
list->erase(it);
// Return cached Python object if available (preserves subclass identity)
if (entity->serial_number != 0) {
PyObject* cached = PythonObjectCache::getInstance().lookup(entity->serial_number);
if (cached) {
// Release identity ref — entity is leaving the grid
// The caller now holds a strong ref via 'cached'
entity->releasePyIdentity();
return cached;
}
}
// Release identity ref (no cached object to return)
entity->releasePyIdentity();
// Create Python object for the entity
PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType;