From 327da3622a1692c71bbfe6735d303ad6a433d34d Mon Sep 17 00:00:00 2001
From: John McCardle <mccardle.john@gmail.com>
Date: Thu, 23 Oct 2025 13:17:45 -0400
Subject: [PATCH] feat: Change EntityCollection.remove() to accept Entity
 objects
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, EntityCollection.remove() required an integer index, which was
inconsistent with Python's list.remove(item) behavior and the broader
Python ecosystem conventions.

Changes:
- remove() now accepts Entity object directly instead of index
- Searches collection by comparing C++ shared_ptr identity
- Raises ValueError if entity not found in collection
- More Pythonic API matching Python's list.remove() semantics

This aligns with Issue #73 and improves API consistency across the
collection system.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 src/UIGrid.cpp | 51 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/src/UIGrid.cpp b/src/UIGrid.cpp
index 62e8d22..dafc6f8 100644
--- a/src/UIGrid.cpp
+++ b/src/UIGrid.cpp
@@ -1772,33 +1772,46 @@ PyObject* UIEntityCollection::append(PyUIEntityCollectionObject* self, PyObject*
 
 PyObject* UIEntityCollection::remove(PyUIEntityCollectionObject* self, PyObject* o)
 {
-    if (!PyLong_Check(o))
+    // Type checking - must be an Entity
+    if (!PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Entity")))
     {
-        PyErr_SetString(PyExc_TypeError, "EntityCollection.remove requires an integer index to remove");
+        PyErr_SetString(PyExc_TypeError, "EntityCollection.remove requires an Entity object");
         return NULL;
     }
-    long index = PyLong_AsLong(o);
     
-    // Handle negative indexing
-    while (index < 0) index += self->data->size();
-    
-    if (index >= self->data->size())
-    {
-        PyErr_SetString(PyExc_ValueError, "Index out of range");
+    // Get the C++ object from the Python object
+    PyUIEntityObject* entity = (PyUIEntityObject*)o;
+    if (!entity->data) {
+        PyErr_SetString(PyExc_RuntimeError, "Invalid Entity object");
         return NULL;
     }
-
-    // Get iterator to the entity to remove
-    auto it = self->data->begin();
-    std::advance(it, index);
     
-    // Clear grid reference before removing
-    (*it)->grid = nullptr;
+    // Get the underlying list
+    auto list = self->data.get();
+    if (!list) {
+        PyErr_SetString(PyExc_RuntimeError, "The collection store returned a null pointer");
+        return NULL;
+    }
     
-    // release the shared pointer at correct part of the list
-    self->data->erase(it);
-    Py_INCREF(Py_None);
-    return Py_None;
+    // Search for the entity by comparing C++ pointers
+    auto it = list->begin();
+    while (it != list->end()) {
+        if (it->get() == entity->data.get()) {
+            // Found it - clear grid reference before removing
+            (*it)->grid = nullptr;
+            
+            // Remove from the list
+            self->data->erase(it);
+            
+            Py_INCREF(Py_None);
+            return Py_None;
+        }
+        ++it;
+    }
+    
+    // Entity not found - raise ValueError
+    PyErr_SetString(PyExc_ValueError, "Entity not in EntityCollection");
+    return NULL;
 }
 
 PyObject* UIEntityCollection::extend(PyUIEntityCollectionObject* self, PyObject* o)