From 9e2444da69d484138bc6744b89b2ab42841437a7 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Sat, 7 Feb 2026 20:15:55 -0500 Subject: [PATCH] Add pop/find/extend to EntityCollection3D, closes #243 EntityCollection3D now has API parity with UIEntityCollection: - pop(index=-1): Remove and return entity at index - find(name): Search by entity name, return Entity3D or None - extend(iterable): Append multiple Entity3D objects Also adds `name` property to Entity3D for use with find(). Co-Authored-By: Claude Opus 4.6 --- src/3d/Entity3D.h | 7 + src/3d/EntityCollection3D.cpp | 128 ++++++++++++++++++ src/3d/EntityCollection3D.h | 3 + .../unit/test_entity_collection3d_methods.py | 97 +++++++++++++ 4 files changed, 235 insertions(+) create mode 100644 tests/unit/test_entity_collection3d_methods.py diff --git a/src/3d/Entity3D.h b/src/3d/Entity3D.h index ce1d69a..f089e2c 100644 --- a/src/3d/Entity3D.h +++ b/src/3d/Entity3D.h @@ -88,6 +88,10 @@ public: bool isVisible() const { return visible_; } void setVisible(bool v) { visible_ = v; } + // Name (for find() lookup) + const std::string& getName() const { return name_; } + void setName(const std::string& n) { name_ = n; } + // Color for placeholder cube rendering sf::Color getColor() const { return color_; } void setColor(const sf::Color& c) { color_ = c; } @@ -228,6 +232,8 @@ public: static PyObject* repr(PyEntity3DObject* self); // Property getters/setters + static PyObject* get_name(PyEntity3DObject* self, void* closure); + static int set_name(PyEntity3DObject* self, PyObject* value, void* closure); static PyObject* get_pos(PyEntity3DObject* self, void* closure); static int set_pos(PyEntity3DObject* self, PyObject* value, void* closure); static PyObject* get_world_pos(PyEntity3DObject* self, void* closure); @@ -297,6 +303,7 @@ private: vec3 scale_ = vec3(1.0f, 1.0f, 1.0f); // Appearance + std::string name_; // For find() lookup bool visible_ = true; sf::Color color_ = sf::Color(200, 100, 50); // Default orange int sprite_index_ = 0; diff --git a/src/3d/EntityCollection3D.cpp b/src/3d/EntityCollection3D.cpp index 08eef9d..f38ad04 100644 --- a/src/3d/EntityCollection3D.cpp +++ b/src/3d/EntityCollection3D.cpp @@ -196,6 +196,125 @@ PyObject* EntityCollection3D::clear(PyEntityCollection3DObject* self, PyObject* Py_RETURN_NONE; } +PyObject* EntityCollection3D::pop(PyEntityCollection3DObject* self, PyObject* args, PyObject* kwds) +{ + static const char* kwlist[] = {"index", NULL}; + Py_ssize_t index = -1; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "|n", const_cast(kwlist), &index)) { + return NULL; + } + + if (!self->data || self->data->empty()) { + PyErr_SetString(PyExc_IndexError, "pop from empty EntityCollection3D"); + return NULL; + } + + Py_ssize_t size = static_cast(self->data->size()); + if (index < 0) index += size; + if (index < 0 || index >= size) { + PyErr_SetString(PyExc_IndexError, "EntityCollection3D pop index out of range"); + return NULL; + } + + // Iterate to the index + auto it = self->data->begin(); + std::advance(it, index); + + auto entity = *it; + + // Clear viewport reference before removing + entity->setViewport(nullptr); + self->data->erase(it); + + // Create Python wrapper for the removed entity + auto type = &mcrfpydef::PyEntity3DType; + auto obj = (PyEntity3DObject*)type->tp_alloc(type, 0); + if (!obj) return NULL; + + new (&obj->data) std::shared_ptr(entity); + obj->weakreflist = nullptr; + + return (PyObject*)obj; +} + +PyObject* EntityCollection3D::extend(PyEntityCollection3DObject* self, PyObject* o) +{ + if (!self->data || !self->viewport) { + PyErr_SetString(PyExc_RuntimeError, "Collection has no data"); + return NULL; + } + + PyObject* iterator = PyObject_GetIter(o); + if (!iterator) { + return NULL; + } + + // First pass: validate all items are Entity3D + std::vector> to_add; + PyObject* item; + while ((item = PyIter_Next(iterator)) != NULL) { + if (!PyObject_IsInstance(item, (PyObject*)&mcrfpydef::PyEntity3DType)) { + Py_DECREF(item); + Py_DECREF(iterator); + PyErr_SetString(PyExc_TypeError, "extend() requires an iterable of Entity3D objects"); + return NULL; + } + auto entity_obj = (PyEntity3DObject*)item; + if (!entity_obj->data) { + Py_DECREF(item); + Py_DECREF(iterator); + PyErr_SetString(PyExc_ValueError, "Entity3D has no data"); + return NULL; + } + to_add.push_back(entity_obj->data); + Py_DECREF(item); + } + Py_DECREF(iterator); + + if (PyErr_Occurred()) { + return NULL; + } + + // Second pass: append all validated entities + for (auto& entity : to_add) { + self->data->push_back(entity); + entity->setViewport(self->viewport); + } + + Py_RETURN_NONE; +} + +PyObject* EntityCollection3D::find(PyEntityCollection3DObject* self, PyObject* args, PyObject* kwds) +{ + static const char* kwlist[] = {"name", NULL}; + const char* name = nullptr; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "s", const_cast(kwlist), &name)) { + return NULL; + } + + if (!self->data) { + PyErr_SetString(PyExc_RuntimeError, "Collection has no data"); + return NULL; + } + + for (const auto& entity : *self->data) { + if (entity->getName() == name) { + auto type = &mcrfpydef::PyEntity3DType; + auto obj = (PyEntity3DObject*)type->tp_alloc(type, 0); + if (!obj) return NULL; + + new (&obj->data) std::shared_ptr(entity); + obj->weakreflist = nullptr; + + return (PyObject*)obj; + } + } + + Py_RETURN_NONE; +} + PyMethodDef EntityCollection3D::methods[] = { {"append", (PyCFunction)EntityCollection3D::append, METH_O, "append(entity)\n\n" @@ -206,6 +325,15 @@ PyMethodDef EntityCollection3D::methods[] = { {"clear", (PyCFunction)EntityCollection3D::clear, METH_NOARGS, "clear()\n\n" "Remove all entities from the collection."}, + {"pop", (PyCFunction)EntityCollection3D::pop, METH_VARARGS | METH_KEYWORDS, + "pop(index=-1) -> Entity3D\n\n" + "Remove and return Entity3D at index (default: last)."}, + {"extend", (PyCFunction)EntityCollection3D::extend, METH_O, + "extend(iterable)\n\n" + "Add all Entity3D objects from iterable to the collection."}, + {"find", (PyCFunction)EntityCollection3D::find, METH_VARARGS | METH_KEYWORDS, + "find(name) -> Entity3D or None\n\n" + "Find an Entity3D by name. Returns None if not found."}, {NULL} // Sentinel }; diff --git a/src/3d/EntityCollection3D.h b/src/3d/EntityCollection3D.h index f236de2..b4d415a 100644 --- a/src/3d/EntityCollection3D.h +++ b/src/3d/EntityCollection3D.h @@ -43,6 +43,9 @@ public: static PyObject* append(PyEntityCollection3DObject* self, PyObject* o); static PyObject* remove(PyEntityCollection3DObject* self, PyObject* o); static PyObject* clear(PyEntityCollection3DObject* self, PyObject* args); + static PyObject* pop(PyEntityCollection3DObject* self, PyObject* args, PyObject* kwds); + static PyObject* extend(PyEntityCollection3DObject* self, PyObject* o); + static PyObject* find(PyEntityCollection3DObject* self, PyObject* args, PyObject* kwds); static PyMethodDef methods[]; // Python type slots diff --git a/tests/unit/test_entity_collection3d_methods.py b/tests/unit/test_entity_collection3d_methods.py new file mode 100644 index 0000000..b9d6d90 --- /dev/null +++ b/tests/unit/test_entity_collection3d_methods.py @@ -0,0 +1,97 @@ +"""Test EntityCollection3D pop/find/extend (issue #243)""" +import mcrfpy +import sys + +errors = [] + +vp = mcrfpy.Viewport3D(pos=(0,0), size=(100,100)) +vp.set_grid_size(16, 16) + +# Setup: add 5 named entities +for i in range(5): + e = mcrfpy.Entity3D(pos=(i, i), scale=1.0) + e.name = f"entity_{i}" + vp.entities.append(e) + +# Test find - existing +found = vp.entities.find("entity_2") +if found is None: + errors.append("find('entity_2') returned None") +elif found.name != "entity_2": + errors.append(f"find returned wrong entity: {found.name}") + +# Test find - missing +missing = vp.entities.find("nonexistent") +if missing is not None: + errors.append("find('nonexistent') should return None") + +# Test pop() - default (last element) +count_before = len(vp.entities) +popped = vp.entities.pop() +if popped.name != "entity_4": + errors.append(f"pop() should return last, got {popped.name}") +if len(vp.entities) != count_before - 1: + errors.append(f"Length should decrease after pop") + +# Test pop(0) - first element +popped0 = vp.entities.pop(0) +if popped0.name != "entity_0": + errors.append(f"pop(0) should return first, got {popped0.name}") + +# Test pop(1) - middle element +popped1 = vp.entities.pop(1) +if popped1.name != "entity_2": + errors.append(f"pop(1) should return index 1, got {popped1.name}") + +# Current state: [entity_1, entity_3] +if len(vp.entities) != 2: + errors.append(f"Expected 2 remaining, got {len(vp.entities)}") + +# Test pop - out of range +try: + vp.entities.pop(99) + errors.append("pop(99) should raise IndexError") +except IndexError: + pass + +# Test extend +new_entities = [] +for i in range(3): + e = mcrfpy.Entity3D(pos=(10+i, 10+i), scale=1.0) + e.name = f"new_{i}" + new_entities.append(e) +vp.entities.extend(new_entities) +if len(vp.entities) != 5: + errors.append(f"After extend, expected 5, got {len(vp.entities)}") + +# Verify extended entities are findable +if vp.entities.find("new_1") is None: + errors.append("Extended entity not findable") + +# Test extend with invalid type +try: + vp.entities.extend([42]) + errors.append("extend with non-Entity3D should raise TypeError") +except TypeError: + pass + +# Test extend with non-iterable +try: + vp.entities.extend(42) + errors.append("extend with non-iterable should raise TypeError") +except TypeError: + pass + +# Test name property +e_named = mcrfpy.Entity3D(pos=(0,0), scale=1.0) +e_named.name = "test_name" +if e_named.name != "test_name": + errors.append(f"name property: expected 'test_name', got '{e_named.name}'") + +if errors: + for err in errors: + print(f"FAIL: {err}", file=sys.stderr) + sys.exit(1) +else: + print("PASS: EntityCollection3D pop/find/extend (issue #243)", file=sys.stderr) + sys.exit(0)