From deb5d81ab69f00d6e18fb46a12ec0b926668d753 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Wed, 26 Nov 2025 05:24:55 -0500 Subject: [PATCH 1/5] feat: Add .find() method to UICollection and EntityCollection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements name-based search for UI elements and entities: - Exact match returns single element or None - Wildcard patterns (prefix*, *suffix, *contains*) return list - Recursive search for nested Frame children (UICollection only) API: ui.find("player_frame") # exact match ui.find("enemy*") # starts with ui.find("*_button", recursive=True) # recursive search grid.entities.find("*goblin*") # entity search Closes #41, closes #40 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/UICollection.cpp | 156 ++++++++++++++++++++- src/UICollection.h | 1 + src/UIGrid.cpp | 125 ++++++++++++++++- src/UIGrid.h | 1 + tests/unit/collection_find_test.py | 209 +++++++++++++++++++++++++++++ 5 files changed, 482 insertions(+), 10 deletions(-) create mode 100644 tests/unit/collection_find_test.py diff --git a/src/UICollection.cpp b/src/UICollection.cpp index 7e91e41..36b2e64 100644 --- a/src/UICollection.cpp +++ b/src/UICollection.cpp @@ -905,12 +905,158 @@ PyObject* UICollection::count(PyUICollectionObject* self, PyObject* value) { return PyLong_FromSsize_t(count); } +// Helper function to match names with optional wildcard support +static bool matchName(const std::string& name, const std::string& pattern) { + // Check for wildcard pattern + if (pattern.find('*') != std::string::npos) { + // Simple wildcard matching: only support * at start, end, or both + if (pattern == "*") { + return true; // Match everything + } else if (pattern.front() == '*' && pattern.back() == '*' && pattern.length() > 2) { + // *substring* - contains match + std::string substring = pattern.substr(1, pattern.length() - 2); + return name.find(substring) != std::string::npos; + } else if (pattern.front() == '*') { + // *suffix - ends with + std::string suffix = pattern.substr(1); + return name.length() >= suffix.length() && + name.compare(name.length() - suffix.length(), suffix.length(), suffix) == 0; + } else if (pattern.back() == '*') { + // prefix* - starts with + std::string prefix = pattern.substr(0, pattern.length() - 1); + return name.compare(0, prefix.length(), prefix) == 0; + } + // For more complex patterns, fall back to exact match + return name == pattern; + } + // Exact match + return name == pattern; +} + +PyObject* UICollection::find(PyUICollectionObject* self, PyObject* args, PyObject* kwds) { + const char* name = nullptr; + int recursive = 0; + + static const char* kwlist[] = {"name", "recursive", NULL}; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "s|p", const_cast(kwlist), + &name, &recursive)) { + return NULL; + } + + auto vec = self->data.get(); + if (!vec) { + PyErr_SetString(PyExc_RuntimeError, "Collection data is null"); + return NULL; + } + + std::string pattern(name); + bool has_wildcard = (pattern.find('*') != std::string::npos); + + if (has_wildcard) { + // Return list of all matches + PyObject* results = PyList_New(0); + if (!results) return NULL; + + for (auto& drawable : *vec) { + if (matchName(drawable->name, pattern)) { + PyObject* py_drawable = convertDrawableToPython(drawable); + if (!py_drawable) { + Py_DECREF(results); + return NULL; + } + if (PyList_Append(results, py_drawable) < 0) { + Py_DECREF(py_drawable); + Py_DECREF(results); + return NULL; + } + Py_DECREF(py_drawable); // PyList_Append increfs + } + + // Recursive search into Frame children + if (recursive && drawable->derived_type() == PyObjectsEnum::UIFRAME) { + auto frame = std::static_pointer_cast(drawable); + // Create temporary collection object for recursive call + PyTypeObject* collType = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "UICollection"); + if (collType) { + PyUICollectionObject* child_coll = (PyUICollectionObject*)collType->tp_alloc(collType, 0); + if (child_coll) { + child_coll->data = frame->children; + PyObject* child_results = find(child_coll, args, kwds); + if (child_results && PyList_Check(child_results)) { + // Extend results with child results + for (Py_ssize_t i = 0; i < PyList_Size(child_results); i++) { + PyObject* item = PyList_GetItem(child_results, i); + Py_INCREF(item); + PyList_Append(results, item); + Py_DECREF(item); + } + Py_DECREF(child_results); + } + Py_DECREF(child_coll); + } + Py_DECREF(collType); + } + } + } + + return results; + } else { + // Return first exact match or None + for (auto& drawable : *vec) { + if (drawable->name == pattern) { + return convertDrawableToPython(drawable); + } + + // Recursive search into Frame children + if (recursive && drawable->derived_type() == PyObjectsEnum::UIFRAME) { + auto frame = std::static_pointer_cast(drawable); + PyTypeObject* collType = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "UICollection"); + if (collType) { + PyUICollectionObject* child_coll = (PyUICollectionObject*)collType->tp_alloc(collType, 0); + if (child_coll) { + child_coll->data = frame->children; + PyObject* result = find(child_coll, args, kwds); + Py_DECREF(child_coll); + Py_DECREF(collType); + if (result && result != Py_None) { + return result; + } + Py_XDECREF(result); + } else { + Py_DECREF(collType); + } + } + } + } + + Py_RETURN_NONE; + } +} + PyMethodDef UICollection::methods[] = { - {"append", (PyCFunction)UICollection::append, METH_O}, - {"extend", (PyCFunction)UICollection::extend, METH_O}, - {"remove", (PyCFunction)UICollection::remove, METH_O}, - {"index", (PyCFunction)UICollection::index_method, METH_O}, - {"count", (PyCFunction)UICollection::count, METH_O}, + {"append", (PyCFunction)UICollection::append, METH_O, + "Add an element to the end of the collection"}, + {"extend", (PyCFunction)UICollection::extend, METH_O, + "Add all elements from an iterable to the collection"}, + {"remove", (PyCFunction)UICollection::remove, METH_O, + "Remove element at the given index"}, + {"index", (PyCFunction)UICollection::index_method, METH_O, + "Return the index of an element in the collection"}, + {"count", (PyCFunction)UICollection::count, METH_O, + "Count occurrences of an element in the collection"}, + {"find", (PyCFunction)UICollection::find, METH_VARARGS | METH_KEYWORDS, + "find(name, recursive=False) -> element or list\n\n" + "Find elements by name.\n\n" + "Args:\n" + " name (str): Name to search for. Supports wildcards:\n" + " - 'exact' for exact match (returns single element or None)\n" + " - 'prefix*' for starts-with match (returns list)\n" + " - '*suffix' for ends-with match (returns list)\n" + " - '*substring*' for contains match (returns list)\n" + " recursive (bool): If True, search in Frame children recursively.\n\n" + "Returns:\n" + " Single element if exact match, list if wildcard, None if not found."}, {NULL, NULL, 0, NULL} }; diff --git a/src/UICollection.h b/src/UICollection.h index bb8d254..b70fcf2 100644 --- a/src/UICollection.h +++ b/src/UICollection.h @@ -32,6 +32,7 @@ public: static PyObject* remove(PyUICollectionObject* self, PyObject* o); static PyObject* index_method(PyUICollectionObject* self, PyObject* value); static PyObject* count(PyUICollectionObject* self, PyObject* value); + static PyObject* find(PyUICollectionObject* self, PyObject* args, PyObject* kwds); static PyMethodDef methods[]; static PyObject* repr(PyUICollectionObject* self); static int init(PyUICollectionObject* self, PyObject* args, PyObject* kwds); diff --git a/src/UIGrid.cpp b/src/UIGrid.cpp index 751adcc..56c3197 100644 --- a/src/UIGrid.cpp +++ b/src/UIGrid.cpp @@ -2216,12 +2216,127 @@ PyMappingMethods UIEntityCollection::mpmethods = { .mp_ass_subscript = (objobjargproc)UIEntityCollection::ass_subscript }; +// Helper function for entity name matching with wildcards +static bool matchEntityName(const std::string& name, const std::string& pattern) { + if (pattern.find('*') != std::string::npos) { + if (pattern == "*") { + return true; + } else if (pattern.front() == '*' && pattern.back() == '*' && pattern.length() > 2) { + std::string substring = pattern.substr(1, pattern.length() - 2); + return name.find(substring) != std::string::npos; + } else if (pattern.front() == '*') { + std::string suffix = pattern.substr(1); + return name.length() >= suffix.length() && + name.compare(name.length() - suffix.length(), suffix.length(), suffix) == 0; + } else if (pattern.back() == '*') { + std::string prefix = pattern.substr(0, pattern.length() - 1); + return name.compare(0, prefix.length(), prefix) == 0; + } + return name == pattern; + } + return name == pattern; +} + +PyObject* UIEntityCollection::find(PyUIEntityCollectionObject* self, PyObject* args, PyObject* kwds) { + const char* name = nullptr; + + static const char* kwlist[] = {"name", NULL}; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "s", const_cast(kwlist), &name)) { + return NULL; + } + + auto list = self->data.get(); + if (!list) { + PyErr_SetString(PyExc_RuntimeError, "Collection data is null"); + return NULL; + } + + std::string pattern(name); + bool has_wildcard = (pattern.find('*') != std::string::npos); + + // Get the Entity type for creating Python objects + PyTypeObject* entityType = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Entity"); + if (!entityType) { + PyErr_SetString(PyExc_RuntimeError, "Could not find Entity type"); + return NULL; + } + + if (has_wildcard) { + // Return list of all matches + PyObject* results = PyList_New(0); + if (!results) { + Py_DECREF(entityType); + return NULL; + } + + for (auto& entity : *list) { + // Entity name is stored in sprite.name + if (matchEntityName(entity->sprite.name, pattern)) { + PyUIEntityObject* py_entity = (PyUIEntityObject*)entityType->tp_alloc(entityType, 0); + if (!py_entity) { + Py_DECREF(results); + Py_DECREF(entityType); + return NULL; + } + py_entity->data = entity; + py_entity->weakreflist = NULL; + + if (PyList_Append(results, (PyObject*)py_entity) < 0) { + Py_DECREF(py_entity); + Py_DECREF(results); + Py_DECREF(entityType); + return NULL; + } + Py_DECREF(py_entity); // PyList_Append increfs + } + } + + Py_DECREF(entityType); + return results; + } else { + // Return first exact match or None + for (auto& entity : *list) { + if (entity->sprite.name == pattern) { + PyUIEntityObject* py_entity = (PyUIEntityObject*)entityType->tp_alloc(entityType, 0); + if (!py_entity) { + Py_DECREF(entityType); + return NULL; + } + py_entity->data = entity; + py_entity->weakreflist = NULL; + Py_DECREF(entityType); + return (PyObject*)py_entity; + } + } + + Py_DECREF(entityType); + Py_RETURN_NONE; + } +} + PyMethodDef UIEntityCollection::methods[] = { - {"append", (PyCFunction)UIEntityCollection::append, METH_O}, - {"extend", (PyCFunction)UIEntityCollection::extend, METH_O}, - {"remove", (PyCFunction)UIEntityCollection::remove, METH_O}, - {"index", (PyCFunction)UIEntityCollection::index_method, METH_O}, - {"count", (PyCFunction)UIEntityCollection::count, METH_O}, + {"append", (PyCFunction)UIEntityCollection::append, METH_O, + "Add an entity to the collection"}, + {"extend", (PyCFunction)UIEntityCollection::extend, METH_O, + "Add all entities from an iterable"}, + {"remove", (PyCFunction)UIEntityCollection::remove, METH_O, + "Remove an entity from the collection"}, + {"index", (PyCFunction)UIEntityCollection::index_method, METH_O, + "Return the index of an entity"}, + {"count", (PyCFunction)UIEntityCollection::count, METH_O, + "Count occurrences of an entity"}, + {"find", (PyCFunction)UIEntityCollection::find, METH_VARARGS | METH_KEYWORDS, + "find(name) -> entity or list\n\n" + "Find entities by name.\n\n" + "Args:\n" + " name (str): Name to search for. Supports wildcards:\n" + " - 'exact' for exact match (returns single entity or None)\n" + " - 'prefix*' for starts-with match (returns list)\n" + " - '*suffix' for ends-with match (returns list)\n" + " - '*substring*' for contains match (returns list)\n\n" + "Returns:\n" + " Single entity if exact match, list if wildcard, None if not found."}, {NULL, NULL, 0, NULL} }; diff --git a/src/UIGrid.h b/src/UIGrid.h index bbf6b4e..f2c9633 100644 --- a/src/UIGrid.h +++ b/src/UIGrid.h @@ -143,6 +143,7 @@ public: static PyObject* remove(PyUIEntityCollectionObject* self, PyObject* o); static PyObject* index_method(PyUIEntityCollectionObject* self, PyObject* value); static PyObject* count(PyUIEntityCollectionObject* self, PyObject* value); + static PyObject* find(PyUIEntityCollectionObject* self, PyObject* args, PyObject* kwds); static PyMethodDef methods[]; static PyObject* repr(PyUIEntityCollectionObject* self); static int init(PyUIEntityCollectionObject* self, PyObject* args, PyObject* kwds); diff --git a/tests/unit/collection_find_test.py b/tests/unit/collection_find_test.py new file mode 100644 index 0000000..86a1733 --- /dev/null +++ b/tests/unit/collection_find_test.py @@ -0,0 +1,209 @@ +#!/usr/bin/env python3 +"""Test for UICollection.find() and EntityCollection.find() methods. + +Tests issue #40 (search and replace by name) and #41 (.find on collections). +""" + +import mcrfpy +import sys + +def test_uicollection_find(): + """Test UICollection.find() with exact and wildcard matches.""" + print("Testing UICollection.find()...") + + # Create a scene with named elements + mcrfpy.createScene("test_find") + ui = mcrfpy.sceneUI("test_find") + + # Create frames with names + frame1 = mcrfpy.Frame(pos=(0, 0), size=(100, 100)) + frame1.name = "main_frame" + ui.append(frame1) + + frame2 = mcrfpy.Frame(pos=(100, 0), size=(100, 100)) + frame2.name = "sidebar_frame" + ui.append(frame2) + + frame3 = mcrfpy.Frame(pos=(200, 0), size=(100, 100)) + frame3.name = "player_status" + ui.append(frame3) + + caption1 = mcrfpy.Caption(text="Hello", pos=(0, 200)) + caption1.name = "player_name" + ui.append(caption1) + + # Create an unnamed element + unnamed = mcrfpy.Caption(text="Unnamed", pos=(0, 250)) + ui.append(unnamed) + + # Test exact match - found + result = ui.find("main_frame") + assert result is not None, "Exact match should find element" + assert result.name == "main_frame", f"Found wrong element: {result.name}" + print(" [PASS] Exact match found") + + # Test exact match - not found + result = ui.find("nonexistent") + assert result is None, "Should return None when not found" + print(" [PASS] Not found returns None") + + # Test prefix wildcard (starts with) + results = ui.find("player*") + assert isinstance(results, list), "Wildcard should return list" + assert len(results) == 2, f"Expected 2 matches, got {len(results)}" + names = [r.name for r in results] + assert "player_status" in names, "player_status should match player*" + assert "player_name" in names, "player_name should match player*" + print(" [PASS] Prefix wildcard works") + + # Test suffix wildcard (ends with) + results = ui.find("*_frame") + assert isinstance(results, list), "Wildcard should return list" + assert len(results) == 2, f"Expected 2 matches, got {len(results)}" + names = [r.name for r in results] + assert "main_frame" in names + assert "sidebar_frame" in names + print(" [PASS] Suffix wildcard works") + + # Test contains wildcard + results = ui.find("*bar*") + assert isinstance(results, list), "Wildcard should return list" + assert len(results) == 1, f"Expected 1 match, got {len(results)}" + assert results[0].name == "sidebar_frame" + print(" [PASS] Contains wildcard works") + + # Test match all + results = ui.find("*") + # Should match all named elements (4 named + 1 unnamed with empty name) + assert isinstance(results, list), "Match all should return list" + assert len(results) == 5, f"Expected 5 matches, got {len(results)}" + print(" [PASS] Match all wildcard works") + + # Test empty pattern matches elements with empty names (unnamed elements) + result = ui.find("") + # The unnamed caption has an empty name, so exact match should find it + assert result is not None, "Empty name exact match should find the unnamed element" + print(" [PASS] Empty pattern finds unnamed elements") + + print("UICollection.find() tests passed!") + return True + + +def test_entitycollection_find(): + """Test EntityCollection.find() with exact and wildcard matches.""" + print("\nTesting EntityCollection.find()...") + + # Create a grid with entities + mcrfpy.createScene("test_entity_find") + ui = mcrfpy.sceneUI("test_entity_find") + + grid = mcrfpy.Grid(grid_size=(10, 10), pos=(0, 0), size=(400, 400)) + ui.append(grid) + + # Add named entities + player = mcrfpy.Entity(grid_pos=(1, 1)) + player.name = "player" + grid.entities.append(player) + + enemy1 = mcrfpy.Entity(grid_pos=(2, 2)) + enemy1.name = "enemy_goblin" + grid.entities.append(enemy1) + + enemy2 = mcrfpy.Entity(grid_pos=(3, 3)) + enemy2.name = "enemy_orc" + grid.entities.append(enemy2) + + item = mcrfpy.Entity(grid_pos=(4, 4)) + item.name = "item_sword" + grid.entities.append(item) + + # Test exact match + result = grid.entities.find("player") + assert result is not None, "Should find player" + assert result.name == "player" + print(" [PASS] Entity exact match works") + + # Test not found + result = grid.entities.find("boss") + assert result is None, "Should return None when not found" + print(" [PASS] Entity not found returns None") + + # Test prefix wildcard + results = grid.entities.find("enemy*") + assert isinstance(results, list) + assert len(results) == 2, f"Expected 2 enemies, got {len(results)}" + print(" [PASS] Entity prefix wildcard works") + + # Test suffix wildcard + results = grid.entities.find("*_orc") + assert isinstance(results, list) + assert len(results) == 1 + assert results[0].name == "enemy_orc" + print(" [PASS] Entity suffix wildcard works") + + print("EntityCollection.find() tests passed!") + return True + + +def test_recursive_find(): + """Test recursive find in nested Frame children.""" + print("\nTesting recursive find in nested frames...") + + mcrfpy.createScene("test_recursive") + ui = mcrfpy.sceneUI("test_recursive") + + # Create nested structure + parent = mcrfpy.Frame(pos=(0, 0), size=(400, 400)) + parent.name = "parent" + ui.append(parent) + + child = mcrfpy.Frame(pos=(10, 10), size=(200, 200)) + child.name = "child_frame" + parent.children.append(child) + + grandchild = mcrfpy.Caption(text="Deep", pos=(5, 5)) + grandchild.name = "deep_caption" + child.children.append(grandchild) + + # Non-recursive find should not find nested elements + result = ui.find("deep_caption") + assert result is None, "Non-recursive find should not find nested element" + print(" [PASS] Non-recursive doesn't find nested elements") + + # Recursive find should find nested elements + result = ui.find("deep_caption", recursive=True) + assert result is not None, "Recursive find should find nested element" + assert result.name == "deep_caption" + print(" [PASS] Recursive find locates nested elements") + + # Recursive wildcard should find all matches + results = ui.find("*_frame", recursive=True) + assert isinstance(results, list) + names = [r.name for r in results] + assert "child_frame" in names, "Should find child_frame" + print(" [PASS] Recursive wildcard finds nested matches") + + print("Recursive find tests passed!") + return True + + +if __name__ == "__main__": + try: + all_passed = True + all_passed &= test_uicollection_find() + all_passed &= test_entitycollection_find() + all_passed &= test_recursive_find() + + if all_passed: + print("\n" + "="*50) + print("All find() tests PASSED!") + print("="*50) + sys.exit(0) + else: + print("\nSome tests FAILED!") + sys.exit(1) + except Exception as e: + print(f"\nTest failed with exception: {e}") + import traceback + traceback.print_exc() + sys.exit(1) From afcb54d9fea6b9173eda71377c44d5c970079183 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Wed, 26 Nov 2025 08:08:43 -0500 Subject: [PATCH 2/5] fix: Make UICollection/EntityCollection match Python list semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Breaking change: UICollection.remove() now takes a value (element) instead of an index, matching Python's list.remove() behavior. New methods added to both UICollection and EntityCollection: - pop([index]) -> element: Remove and return element at index (default: last) - insert(index, element): Insert element at position Semantic fixes: - remove(element): Now removes first occurrence of element (was: remove by index) - All methods now have docstrings documenting behavior Note on z_index sorting: The collections are sorted by z_index before each render. Using index-based operations (pop, insert) with non-default z_index values may produce unexpected results. Use name-based .find() for stable element access when z_index sorting is in use. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/UICollection.cpp | 186 +++++++++++++-- src/UICollection.h | 2 + src/UIGrid.cpp | 142 ++++++++++- src/UIGrid.h | 2 + tests/unit/collection_list_methods_test.py | 263 +++++++++++++++++++++ 5 files changed, 564 insertions(+), 31 deletions(-) create mode 100644 tests/unit/collection_list_methods_test.py diff --git a/src/UICollection.cpp b/src/UICollection.cpp index 36b2e64..b29d22c 100644 --- a/src/UICollection.cpp +++ b/src/UICollection.cpp @@ -790,30 +790,151 @@ PyObject* UICollection::extend(PyUICollectionObject* self, PyObject* iterable) PyObject* UICollection::remove(PyUICollectionObject* self, PyObject* o) { - if (!PyLong_Check(o)) - { - PyErr_SetString(PyExc_TypeError, "UICollection.remove requires an integer index to remove"); - 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"); + auto vec = self->data.get(); + if (!vec) { + PyErr_SetString(PyExc_RuntimeError, "Collection data is null"); return NULL; } - // release the shared pointer at self->data[index]; - self->data->erase(self->data->begin() + index); - - // Mark scene as needing resort after removing element + // Type checking - must be a UIDrawable subclass + if (!PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Drawable"))) { + PyErr_SetString(PyExc_TypeError, + "UICollection.remove requires a UI element (Frame, Caption, Sprite, Grid)"); + return NULL; + } + + // Get the C++ object from the Python object + std::shared_ptr search_drawable = nullptr; + + if (PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Frame"))) { + search_drawable = ((PyUIFrameObject*)o)->data; + } else if (PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Caption"))) { + search_drawable = ((PyUICaptionObject*)o)->data; + } else if (PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Sprite"))) { + search_drawable = ((PyUISpriteObject*)o)->data; + } else if (PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Grid"))) { + search_drawable = ((PyUIGridObject*)o)->data; + } + + if (!search_drawable) { + PyErr_SetString(PyExc_TypeError, + "UICollection.remove requires a UI element (Frame, Caption, Sprite, Grid)"); + return NULL; + } + + // Search for the object and remove first occurrence + for (auto it = vec->begin(); it != vec->end(); ++it) { + if (it->get() == search_drawable.get()) { + vec->erase(it); + McRFPy_API::markSceneNeedsSort(); + Py_RETURN_NONE; + } + } + + PyErr_SetString(PyExc_ValueError, "element not in UICollection"); + return NULL; +} + +PyObject* UICollection::pop(PyUICollectionObject* self, PyObject* args) +{ + Py_ssize_t index = -1; // Default to last element + + if (!PyArg_ParseTuple(args, "|n", &index)) { + return NULL; + } + + auto vec = self->data.get(); + if (!vec) { + PyErr_SetString(PyExc_RuntimeError, "Collection data is null"); + return NULL; + } + + if (vec->empty()) { + PyErr_SetString(PyExc_IndexError, "pop from empty UICollection"); + return NULL; + } + + // Handle negative indexing + Py_ssize_t size = static_cast(vec->size()); + if (index < 0) { + index += size; + } + + if (index < 0 || index >= size) { + PyErr_SetString(PyExc_IndexError, "pop index out of range"); + return NULL; + } + + // Get the element before removing + std::shared_ptr drawable = (*vec)[index]; + + // Remove from vector + vec->erase(vec->begin() + index); + McRFPy_API::markSceneNeedsSort(); - - Py_INCREF(Py_None); - return Py_None; + + // Convert to Python object and return + return convertDrawableToPython(drawable); +} + +PyObject* UICollection::insert(PyUICollectionObject* self, PyObject* args) +{ + Py_ssize_t index; + PyObject* o; + + if (!PyArg_ParseTuple(args, "nO", &index, &o)) { + return NULL; + } + + auto vec = self->data.get(); + if (!vec) { + PyErr_SetString(PyExc_RuntimeError, "Collection data is null"); + return NULL; + } + + // Type checking - must be a UIDrawable subclass + if (!PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Drawable"))) { + PyErr_SetString(PyExc_TypeError, + "UICollection.insert requires a UI element (Frame, Caption, Sprite, Grid)"); + return NULL; + } + + // Get the C++ object from the Python object + std::shared_ptr drawable = nullptr; + + if (PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Frame"))) { + drawable = ((PyUIFrameObject*)o)->data; + } else if (PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Caption"))) { + drawable = ((PyUICaptionObject*)o)->data; + } else if (PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Sprite"))) { + drawable = ((PyUISpriteObject*)o)->data; + } else if (PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Grid"))) { + drawable = ((PyUIGridObject*)o)->data; + } + + if (!drawable) { + PyErr_SetString(PyExc_TypeError, + "UICollection.insert requires a UI element (Frame, Caption, Sprite, Grid)"); + return NULL; + } + + // Handle negative indexing and clamping (Python list.insert behavior) + Py_ssize_t size = static_cast(vec->size()); + if (index < 0) { + index += size; + if (index < 0) { + index = 0; + } + } else if (index > size) { + index = size; + } + + // Insert at position + vec->insert(vec->begin() + index, drawable); + + McRFPy_API::markSceneNeedsSort(); + + Py_RETURN_NONE; } PyObject* UICollection::index_method(PyUICollectionObject* self, PyObject* value) { @@ -1036,15 +1157,30 @@ PyObject* UICollection::find(PyUICollectionObject* self, PyObject* args, PyObjec PyMethodDef UICollection::methods[] = { {"append", (PyCFunction)UICollection::append, METH_O, - "Add an element to the end of the collection"}, + "append(element)\n\n" + "Add an element to the end of the collection."}, {"extend", (PyCFunction)UICollection::extend, METH_O, - "Add all elements from an iterable to the collection"}, + "extend(iterable)\n\n" + "Add all elements from an iterable to the collection."}, + {"insert", (PyCFunction)UICollection::insert, METH_VARARGS, + "insert(index, element)\n\n" + "Insert element at index. Like list.insert(), indices past the end append.\n\n" + "Note: If using z_index for sorting, insertion order may not persist after\n" + "the next render. Use name-based .find() for stable element access."}, {"remove", (PyCFunction)UICollection::remove, METH_O, - "Remove element at the given index"}, + "remove(element)\n\n" + "Remove first occurrence of element. Raises ValueError if not found."}, + {"pop", (PyCFunction)UICollection::pop, METH_VARARGS, + "pop([index]) -> element\n\n" + "Remove and return element at index (default: last element).\n\n" + "Note: If using z_index for sorting, indices may shift after render.\n" + "Use name-based .find() for stable element access."}, {"index", (PyCFunction)UICollection::index_method, METH_O, - "Return the index of an element in the collection"}, + "index(element) -> int\n\n" + "Return index of first occurrence of element. Raises ValueError if not found."}, {"count", (PyCFunction)UICollection::count, METH_O, - "Count occurrences of an element in the collection"}, + "count(element) -> int\n\n" + "Count occurrences of element in the collection."}, {"find", (PyCFunction)UICollection::find, METH_VARARGS | METH_KEYWORDS, "find(name, recursive=False) -> element or list\n\n" "Find elements by name.\n\n" diff --git a/src/UICollection.h b/src/UICollection.h index b70fcf2..a026ea9 100644 --- a/src/UICollection.h +++ b/src/UICollection.h @@ -30,6 +30,8 @@ public: static PyObject* append(PyUICollectionObject* self, PyObject* o); static PyObject* extend(PyUICollectionObject* self, PyObject* iterable); static PyObject* remove(PyUICollectionObject* self, PyObject* o); + static PyObject* pop(PyUICollectionObject* self, PyObject* args); + static PyObject* insert(PyUICollectionObject* self, PyObject* args); static PyObject* index_method(PyUICollectionObject* self, PyObject* value); static PyObject* count(PyUICollectionObject* self, PyObject* value); static PyObject* find(PyUICollectionObject* self, PyObject* args, PyObject* kwds); diff --git a/src/UIGrid.cpp b/src/UIGrid.cpp index 56c3197..8c57a3c 100644 --- a/src/UIGrid.cpp +++ b/src/UIGrid.cpp @@ -1953,13 +1953,132 @@ PyObject* UIEntityCollection::extend(PyUIEntityCollectionObject* self, PyObject* return Py_None; } +PyObject* UIEntityCollection::pop(PyUIEntityCollectionObject* self, PyObject* args) +{ + Py_ssize_t index = -1; // Default to last element + + if (!PyArg_ParseTuple(args, "|n", &index)) { + return NULL; + } + + auto list = self->data.get(); + if (!list) { + PyErr_SetString(PyExc_RuntimeError, "Collection data is null"); + return NULL; + } + + if (list->empty()) { + PyErr_SetString(PyExc_IndexError, "pop from empty EntityCollection"); + return NULL; + } + + // Handle negative indexing + Py_ssize_t size = static_cast(list->size()); + if (index < 0) { + index += size; + } + + if (index < 0 || index >= size) { + PyErr_SetString(PyExc_IndexError, "pop index out of range"); + return NULL; + } + + // Navigate to the element (std::list requires iteration) + auto it = list->begin(); + std::advance(it, index); + + // Get the entity before removing + std::shared_ptr entity = *it; + + // Clear grid reference and remove from list + entity->grid = nullptr; + list->erase(it); + + // Create Python object for the entity + PyTypeObject* entityType = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Entity"); + if (!entityType) { + PyErr_SetString(PyExc_RuntimeError, "Could not find Entity type"); + return NULL; + } + + PyUIEntityObject* py_entity = (PyUIEntityObject*)entityType->tp_alloc(entityType, 0); + Py_DECREF(entityType); + + if (!py_entity) { + return NULL; + } + + py_entity->data = entity; + py_entity->weakreflist = NULL; + + return (PyObject*)py_entity; +} + +PyObject* UIEntityCollection::insert(PyUIEntityCollectionObject* self, PyObject* args) +{ + Py_ssize_t index; + PyObject* o; + + if (!PyArg_ParseTuple(args, "nO", &index, &o)) { + return NULL; + } + + auto list = self->data.get(); + if (!list) { + PyErr_SetString(PyExc_RuntimeError, "Collection data is null"); + return NULL; + } + + // Type checking - must be an Entity + if (!PyObject_IsInstance(o, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Entity"))) { + PyErr_SetString(PyExc_TypeError, "EntityCollection.insert requires an Entity object"); + return NULL; + } + + PyUIEntityObject* entity = (PyUIEntityObject*)o; + if (!entity->data) { + PyErr_SetString(PyExc_RuntimeError, "Invalid Entity object"); + return NULL; + } + + // Handle negative indexing and clamping (Python list.insert behavior) + Py_ssize_t size = static_cast(list->size()); + if (index < 0) { + index += size; + if (index < 0) { + index = 0; + } + } else if (index > size) { + index = size; + } + + // Navigate to insert position + auto it = list->begin(); + std::advance(it, index); + + // Insert and set grid reference + list->insert(it, entity->data); + entity->data->grid = self->grid; + + // Initialize gridstate if needed + if (entity->data->gridstate.size() == 0 && self->grid) { + entity->data->gridstate.resize(self->grid->grid_x * self->grid->grid_y); + for (auto& state : entity->data->gridstate) { + state.visible = false; + state.discovered = false; + } + } + + Py_RETURN_NONE; +} + PyObject* UIEntityCollection::index_method(PyUIEntityCollectionObject* self, PyObject* value) { auto list = self->data.get(); if (!list) { PyErr_SetString(PyExc_RuntimeError, "the collection store returned a null pointer"); return NULL; } - + // Type checking - must be an Entity if (!PyObject_IsInstance(value, PyObject_GetAttrString(McRFPy_API::mcrf_module, "Entity"))) { PyErr_SetString(PyExc_TypeError, "EntityCollection.index requires an Entity object"); @@ -2317,15 +2436,26 @@ PyObject* UIEntityCollection::find(PyUIEntityCollectionObject* self, PyObject* a PyMethodDef UIEntityCollection::methods[] = { {"append", (PyCFunction)UIEntityCollection::append, METH_O, - "Add an entity to the collection"}, + "append(entity)\n\n" + "Add an entity to the end of the collection."}, {"extend", (PyCFunction)UIEntityCollection::extend, METH_O, - "Add all entities from an iterable"}, + "extend(iterable)\n\n" + "Add all entities from an iterable to the collection."}, + {"insert", (PyCFunction)UIEntityCollection::insert, METH_VARARGS, + "insert(index, entity)\n\n" + "Insert entity at index. Like list.insert(), indices past the end append."}, {"remove", (PyCFunction)UIEntityCollection::remove, METH_O, - "Remove an entity from the collection"}, + "remove(entity)\n\n" + "Remove first occurrence of entity. Raises ValueError if not found."}, + {"pop", (PyCFunction)UIEntityCollection::pop, METH_VARARGS, + "pop([index]) -> entity\n\n" + "Remove and return entity at index (default: last entity)."}, {"index", (PyCFunction)UIEntityCollection::index_method, METH_O, - "Return the index of an entity"}, + "index(entity) -> int\n\n" + "Return index of first occurrence of entity. Raises ValueError if not found."}, {"count", (PyCFunction)UIEntityCollection::count, METH_O, - "Count occurrences of an entity"}, + "count(entity) -> int\n\n" + "Count occurrences of entity in the collection."}, {"find", (PyCFunction)UIEntityCollection::find, METH_VARARGS | METH_KEYWORDS, "find(name) -> entity or list\n\n" "Find entities by name.\n\n" diff --git a/src/UIGrid.h b/src/UIGrid.h index f2c9633..c3835cf 100644 --- a/src/UIGrid.h +++ b/src/UIGrid.h @@ -141,6 +141,8 @@ public: static PyObject* append(PyUIEntityCollectionObject* self, PyObject* o); static PyObject* extend(PyUIEntityCollectionObject* self, PyObject* o); static PyObject* remove(PyUIEntityCollectionObject* self, PyObject* o); + static PyObject* pop(PyUIEntityCollectionObject* self, PyObject* args); + static PyObject* insert(PyUIEntityCollectionObject* self, PyObject* args); static PyObject* index_method(PyUIEntityCollectionObject* self, PyObject* value); static PyObject* count(PyUIEntityCollectionObject* self, PyObject* value); static PyObject* find(PyUIEntityCollectionObject* self, PyObject* args, PyObject* kwds); diff --git a/tests/unit/collection_list_methods_test.py b/tests/unit/collection_list_methods_test.py new file mode 100644 index 0000000..7035099 --- /dev/null +++ b/tests/unit/collection_list_methods_test.py @@ -0,0 +1,263 @@ +#!/usr/bin/env python3 +"""Test for Python list-like methods on UICollection and EntityCollection. + +Tests that remove(), pop(), insert(), index(), count() match Python list semantics. +""" + +import mcrfpy +import sys + + +def test_uicollection_remove(): + """Test UICollection.remove() takes a value, not an index.""" + print("Testing UICollection.remove()...") + + mcrfpy.createScene("test_remove") + ui = mcrfpy.sceneUI("test_remove") + + frame1 = mcrfpy.Frame(pos=(0, 0), size=(100, 100)) + frame2 = mcrfpy.Frame(pos=(100, 0), size=(100, 100)) + frame3 = mcrfpy.Frame(pos=(200, 0), size=(100, 100)) + + ui.append(frame1) + ui.append(frame2) + ui.append(frame3) + + assert len(ui) == 3 + + # Remove by value (like Python list) + ui.remove(frame2) + assert len(ui) == 2 + print(" [PASS] remove(element) works") + + # Verify frame2 is gone, but frame1 and frame3 remain + assert ui[0] is not None + assert ui[1] is not None + + # Try to remove something not in the list + try: + frame4 = mcrfpy.Frame(pos=(300, 0), size=(100, 100)) + ui.remove(frame4) + assert False, "Should have raised ValueError" + except ValueError as e: + assert "not in" in str(e).lower() + print(" [PASS] remove() raises ValueError when not found") + + # Try to pass an integer (should fail - no longer takes index) + try: + ui.remove(0) + assert False, "Should have raised TypeError" + except TypeError: + print(" [PASS] remove(int) raises TypeError (correct - takes element, not index)") + + print("UICollection.remove() tests passed!") + return True + + +def test_uicollection_pop(): + """Test UICollection.pop() removes and returns element at index.""" + print("\nTesting UICollection.pop()...") + + mcrfpy.createScene("test_pop") + ui = mcrfpy.sceneUI("test_pop") + + frame1 = mcrfpy.Frame(pos=(0, 0), size=(100, 100)) + frame1.name = "first" + frame2 = mcrfpy.Frame(pos=(100, 0), size=(100, 100)) + frame2.name = "second" + frame3 = mcrfpy.Frame(pos=(200, 0), size=(100, 100)) + frame3.name = "third" + + ui.append(frame1) + ui.append(frame2) + ui.append(frame3) + + # pop() with no args removes last + popped = ui.pop() + assert popped.name == "third", f"Expected 'third', got '{popped.name}'" + assert len(ui) == 2 + print(" [PASS] pop() removes last element") + + # pop(0) removes first + popped = ui.pop(0) + assert popped.name == "first", f"Expected 'first', got '{popped.name}'" + assert len(ui) == 1 + print(" [PASS] pop(0) removes first element") + + # pop(-1) is same as pop() + ui.append(mcrfpy.Frame(pos=(0, 0), size=(10, 10))) + ui[-1].name = "new_last" + popped = ui.pop(-1) + assert popped.name == "new_last" + print(" [PASS] pop(-1) removes last element") + + # pop from empty collection + ui.pop() # Remove last remaining element + try: + ui.pop() + assert False, "Should have raised IndexError" + except IndexError as e: + assert "empty" in str(e).lower() + print(" [PASS] pop() from empty raises IndexError") + + print("UICollection.pop() tests passed!") + return True + + +def test_uicollection_insert(): + """Test UICollection.insert() inserts at given index.""" + print("\nTesting UICollection.insert()...") + + mcrfpy.createScene("test_insert") + ui = mcrfpy.sceneUI("test_insert") + + frame1 = mcrfpy.Frame(pos=(0, 0), size=(100, 100)) + frame1.name = "first" + frame3 = mcrfpy.Frame(pos=(200, 0), size=(100, 100)) + frame3.name = "third" + + ui.append(frame1) + ui.append(frame3) + + # Insert in middle + frame2 = mcrfpy.Frame(pos=(100, 0), size=(100, 100)) + frame2.name = "second" + ui.insert(1, frame2) + + assert len(ui) == 3 + assert ui[0].name == "first" + assert ui[1].name == "second" + assert ui[2].name == "third" + print(" [PASS] insert(1, element) inserts at index 1") + + # Insert at beginning + frame0 = mcrfpy.Frame(pos=(0, 0), size=(50, 50)) + frame0.name = "zero" + ui.insert(0, frame0) + assert ui[0].name == "zero" + print(" [PASS] insert(0, element) inserts at beginning") + + # Insert at end (index > len) + frame_end = mcrfpy.Frame(pos=(0, 0), size=(50, 50)) + frame_end.name = "end" + ui.insert(100, frame_end) # Way past end + assert ui[-1].name == "end" + print(" [PASS] insert(100, element) appends when index > len") + + # Negative index + frame_neg = mcrfpy.Frame(pos=(0, 0), size=(50, 50)) + frame_neg.name = "negative" + current_len = len(ui) + ui.insert(-1, frame_neg) # Insert before last + assert ui[-2].name == "negative" + print(" [PASS] insert(-1, element) inserts before last") + + print("UICollection.insert() tests passed!") + return True + + +def test_entitycollection_pop_insert(): + """Test EntityCollection.pop() and insert().""" + print("\nTesting EntityCollection.pop() and insert()...") + + mcrfpy.createScene("test_entity_pop") + ui = mcrfpy.sceneUI("test_entity_pop") + + grid = mcrfpy.Grid(grid_size=(10, 10), pos=(0, 0), size=(400, 400)) + ui.append(grid) + + e1 = mcrfpy.Entity(grid_pos=(1, 1)) + e1.name = "first" + e2 = mcrfpy.Entity(grid_pos=(2, 2)) + e2.name = "second" + e3 = mcrfpy.Entity(grid_pos=(3, 3)) + e3.name = "third" + + grid.entities.append(e1) + grid.entities.append(e2) + grid.entities.append(e3) + + # Test pop() + popped = grid.entities.pop() + assert popped.name == "third" + assert len(grid.entities) == 2 + print(" [PASS] EntityCollection.pop() works") + + # Test pop(0) + popped = grid.entities.pop(0) + assert popped.name == "first" + assert len(grid.entities) == 1 + print(" [PASS] EntityCollection.pop(0) works") + + # Test insert + e_new = mcrfpy.Entity(grid_pos=(5, 5)) + e_new.name = "new" + grid.entities.insert(0, e_new) + assert grid.entities[0].name == "new" + print(" [PASS] EntityCollection.insert() works") + + print("EntityCollection pop/insert tests passed!") + return True + + +def test_index_and_count(): + """Test index() and count() methods.""" + print("\nTesting index() and count()...") + + mcrfpy.createScene("test_index_count") + ui = mcrfpy.sceneUI("test_index_count") + + frame1 = mcrfpy.Frame(pos=(0, 0), size=(100, 100)) + frame2 = mcrfpy.Frame(pos=(100, 0), size=(100, 100)) + + ui.append(frame1) + ui.append(frame2) + + # index() returns integer + idx = ui.index(frame1) + assert idx == 0, f"Expected 0, got {idx}" + assert isinstance(idx, int) + print(" [PASS] index() returns integer") + + idx = ui.index(frame2) + assert idx == 1 + print(" [PASS] index() finds correct position") + + # count() returns integer + cnt = ui.count(frame1) + assert cnt == 1 + assert isinstance(cnt, int) + print(" [PASS] count() returns integer") + + # count of element not in collection + frame3 = mcrfpy.Frame(pos=(200, 0), size=(100, 100)) + cnt = ui.count(frame3) + assert cnt == 0 + print(" [PASS] count() returns 0 for element not in collection") + + print("index() and count() tests passed!") + return True + + +if __name__ == "__main__": + try: + all_passed = True + all_passed &= test_uicollection_remove() + all_passed &= test_uicollection_pop() + all_passed &= test_uicollection_insert() + all_passed &= test_entitycollection_pop_insert() + all_passed &= test_index_and_count() + + if all_passed: + print("\n" + "="*50) + print("All list-like method tests PASSED!") + print("="*50) + sys.exit(0) + else: + print("\nSome tests FAILED!") + sys.exit(1) + except Exception as e: + print(f"\nTest failed with exception: {e}") + import traceback + traceback.print_exc() + sys.exit(1) From f041a0c8ca2f671ac3ac3fe75cec9cff8c89881a Mon Sep 17 00:00:00 2001 From: John McCardle Date: Wed, 26 Nov 2025 09:37:14 -0500 Subject: [PATCH 3/5] feat: Add Vector convenience features - indexing, tuple comparison, floor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements issue #109 improvements to mcrfpy.Vector: - Sequence protocol: v[0], v[1], v[-1], v[-2], len(v), tuple(v), x,y = v - Tuple comparison: v == (5, 6), v != (1, 2) works bidirectionally - .floor() method: returns new Vector with floored coordinates - .int property: returns (int(floor(x)), int(floor(y))) tuple for dict keys The sequence protocol enables unpacking and iteration, making Vector interoperable with code expecting tuples. The tuple comparison fixes compatibility issues where functions returning Vector broke code expecting tuple comparison (e.g., in Crypt of Sokoban). Closes #109 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/PyVector.cpp | 126 ++++++++++++++-- src/PyVector.h | 14 +- tests/unit/test_vector_convenience.py | 203 ++++++++++++++++++++++++++ 3 files changed, 329 insertions(+), 14 deletions(-) create mode 100644 tests/unit/test_vector_convenience.py diff --git a/src/PyVector.cpp b/src/PyVector.cpp index acb60c0..c8e92c6 100644 --- a/src/PyVector.cpp +++ b/src/PyVector.cpp @@ -8,6 +8,8 @@ PyGetSetDef PyVector::getsetters[] = { MCRF_PROPERTY(x, "X coordinate of the vector (float)"), (void*)0}, {"y", (getter)PyVector::get_member, (setter)PyVector::set_member, MCRF_PROPERTY(y, "Y coordinate of the vector (float)"), (void*)1}, + {"int", (getter)PyVector::get_int, NULL, + MCRF_PROPERTY(int, "Integer tuple (floor of x and y) for use as dict keys. Read-only."), NULL}, {NULL} }; @@ -60,6 +62,13 @@ PyMethodDef PyVector::methods[] = { MCRF_DESC("Create a copy of this vector."), MCRF_RETURNS("Vector: New Vector object with same x and y values") )}, + {"floor", (PyCFunction)PyVector::floor, METH_NOARGS, + MCRF_METHOD(Vector, floor, + MCRF_SIG("()", "Vector"), + MCRF_DESC("Return a new vector with floored (integer) coordinates."), + MCRF_RETURNS("Vector: New Vector with floor(x) and floor(y)") + MCRF_NOTE("Useful for grid-based positioning. For a hashable tuple, use the .int property instead.") + )}, {NULL} }; @@ -102,6 +111,19 @@ namespace mcrfpydef { .nb_matrix_multiply = 0, .nb_inplace_matrix_multiply = 0 }; + + PySequenceMethods PyVector_as_sequence = { + .sq_length = PyVector::sequence_length, + .sq_concat = 0, + .sq_repeat = 0, + .sq_item = PyVector::sequence_item, + .was_sq_slice = 0, + .sq_ass_item = 0, + .was_sq_ass_slice = 0, + .sq_contains = 0, + .sq_inplace_concat = 0, + .sq_inplace_repeat = 0 + }; } PyVector::PyVector(sf::Vector2f target) @@ -397,29 +419,65 @@ int PyVector::bool_check(PyObject* self) PyObject* PyVector::richcompare(PyObject* left, PyObject* right, int op) { auto type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Vector"); - - if (!PyObject_IsInstance(left, (PyObject*)type) || !PyObject_IsInstance(right, (PyObject*)type)) { + + float left_x, left_y, right_x, right_y; + + // Extract left operand values + if (PyObject_IsInstance(left, (PyObject*)type)) { + PyVectorObject* vec = (PyVectorObject*)left; + left_x = vec->data.x; + left_y = vec->data.y; + } else if (PyTuple_Check(left) && PyTuple_Size(left) == 2) { + PyObject* x_obj = PyTuple_GetItem(left, 0); + PyObject* y_obj = PyTuple_GetItem(left, 1); + if ((PyFloat_Check(x_obj) || PyLong_Check(x_obj)) && + (PyFloat_Check(y_obj) || PyLong_Check(y_obj))) { + left_x = (float)PyFloat_AsDouble(x_obj); + left_y = (float)PyFloat_AsDouble(y_obj); + } else { + Py_INCREF(Py_NotImplemented); + return Py_NotImplemented; + } + } else { Py_INCREF(Py_NotImplemented); return Py_NotImplemented; } - - PyVectorObject* vec1 = (PyVectorObject*)left; - PyVectorObject* vec2 = (PyVectorObject*)right; - + + // Extract right operand values + if (PyObject_IsInstance(right, (PyObject*)type)) { + PyVectorObject* vec = (PyVectorObject*)right; + right_x = vec->data.x; + right_y = vec->data.y; + } else if (PyTuple_Check(right) && PyTuple_Size(right) == 2) { + PyObject* x_obj = PyTuple_GetItem(right, 0); + PyObject* y_obj = PyTuple_GetItem(right, 1); + if ((PyFloat_Check(x_obj) || PyLong_Check(x_obj)) && + (PyFloat_Check(y_obj) || PyLong_Check(y_obj))) { + right_x = (float)PyFloat_AsDouble(x_obj); + right_y = (float)PyFloat_AsDouble(y_obj); + } else { + Py_INCREF(Py_NotImplemented); + return Py_NotImplemented; + } + } else { + Py_INCREF(Py_NotImplemented); + return Py_NotImplemented; + } + bool result = false; - + switch (op) { case Py_EQ: - result = (vec1->data.x == vec2->data.x && vec1->data.y == vec2->data.y); + result = (left_x == right_x && left_y == right_y); break; case Py_NE: - result = (vec1->data.x != vec2->data.x || vec1->data.y != vec2->data.y); + result = (left_x != right_x || left_y != right_y); break; default: Py_INCREF(Py_NotImplemented); return Py_NotImplemented; } - + if (result) Py_RETURN_TRUE; else @@ -500,10 +558,54 @@ PyObject* PyVector::copy(PyVectorObject* self, PyObject* Py_UNUSED(ignored)) { auto type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Vector"); auto result = (PyVectorObject*)type->tp_alloc(type, 0); - + if (result) { result->data = self->data; } - + return (PyObject*)result; } + +PyObject* PyVector::floor(PyVectorObject* self, PyObject* Py_UNUSED(ignored)) +{ + auto type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Vector"); + auto result = (PyVectorObject*)type->tp_alloc(type, 0); + + if (result) { + result->data = sf::Vector2f(std::floor(self->data.x), std::floor(self->data.y)); + } + + return (PyObject*)result; +} + +// Sequence protocol implementation +Py_ssize_t PyVector::sequence_length(PyObject* self) +{ + return 2; // Vectors always have exactly 2 elements +} + +PyObject* PyVector::sequence_item(PyObject* obj, Py_ssize_t index) +{ + PyVectorObject* self = (PyVectorObject*)obj; + + // Note: Python already handles negative index normalization when sq_length is defined + // So v[-1] arrives here as index=1, v[-2] as index=0 + // Out-of-range negative indices (like v[-3]) arrive as negative values (e.g., -1) + if (index == 0) { + return PyFloat_FromDouble(self->data.x); + } else if (index == 1) { + return PyFloat_FromDouble(self->data.y); + } else { + PyErr_SetString(PyExc_IndexError, "Vector index out of range (must be 0 or 1)"); + return NULL; + } +} + +// Property: .int - returns integer tuple for use as dict keys +PyObject* PyVector::get_int(PyObject* obj, void* closure) +{ + PyVectorObject* self = (PyVectorObject*)obj; + long ix = (long)std::floor(self->data.x); + long iy = (long)std::floor(self->data.y); + return Py_BuildValue("(ll)", ix, iy); +} diff --git a/src/PyVector.h b/src/PyVector.h index 0b4dc46..42c3ee3 100644 --- a/src/PyVector.h +++ b/src/PyVector.h @@ -45,15 +45,24 @@ public: static PyObject* distance_to(PyVectorObject*, PyObject*); static PyObject* angle(PyVectorObject*, PyObject*); static PyObject* copy(PyVectorObject*, PyObject*); + static PyObject* floor(PyVectorObject*, PyObject*); + + // Sequence protocol + static Py_ssize_t sequence_length(PyObject*); + static PyObject* sequence_item(PyObject*, Py_ssize_t); + + // Additional properties + static PyObject* get_int(PyObject*, void*); static PyGetSetDef getsetters[]; static PyMethodDef methods[]; }; namespace mcrfpydef { - // Forward declare the PyNumberMethods structure + // Forward declare the PyNumberMethods and PySequenceMethods structures extern PyNumberMethods PyVector_as_number; - + extern PySequenceMethods PyVector_as_sequence; + static PyTypeObject PyVectorType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Vector", @@ -61,6 +70,7 @@ namespace mcrfpydef { .tp_itemsize = 0, .tp_repr = PyVector::repr, .tp_as_number = &PyVector_as_number, + .tp_as_sequence = &PyVector_as_sequence, .tp_hash = PyVector::hash, .tp_flags = Py_TPFLAGS_DEFAULT, .tp_doc = PyDoc_STR("SFML Vector Object"), diff --git a/tests/unit/test_vector_convenience.py b/tests/unit/test_vector_convenience.py new file mode 100644 index 0000000..fd36686 --- /dev/null +++ b/tests/unit/test_vector_convenience.py @@ -0,0 +1,203 @@ +#!/usr/bin/env python3 +"""Unit tests for Vector convenience features (Issue #109) + +Tests: +- Sequence protocol: indexing, negative indexing, iteration, unpacking +- Tuple comparison: Vector == tuple, Vector != tuple +- Integer conversion: .floor() method, .int property +- Boolean check: falsey for (0, 0) +""" + +import mcrfpy +import sys + +def approx(a, b, epsilon=1e-5): + """Check if two floats are approximately equal (handles float32 precision)""" + return abs(a - b) < epsilon + +def test_indexing(): + """Test sequence protocol indexing""" + # Use values that are exact in float32: 3.5 = 7/2, 7.5 = 15/2 + v = mcrfpy.Vector(3.5, 7.5) + + # Positive indices + assert v[0] == 3.5, f"v[0] should be 3.5, got {v[0]}" + assert v[1] == 7.5, f"v[1] should be 7.5, got {v[1]}" + + # Negative indices + assert v[-1] == 7.5, f"v[-1] should be 7.5, got {v[-1]}" + assert v[-2] == 3.5, f"v[-2] should be 3.5, got {v[-2]}" + + # Out of bounds + try: + _ = v[2] + assert False, "v[2] should raise IndexError" + except IndexError: + pass + + try: + _ = v[-3] + assert False, "v[-3] should raise IndexError" + except IndexError: + pass + + print(" [PASS] Indexing") + +def test_length(): + """Test len() on Vector""" + v = mcrfpy.Vector(1, 2) + assert len(v) == 2, f"len(Vector) should be 2, got {len(v)}" + print(" [PASS] Length") + +def test_iteration(): + """Test iteration and unpacking""" + # Use values that are exact in float32 + v = mcrfpy.Vector(10.5, 20.5) + + # Iteration - use approximate comparison for float32 precision + values = list(v) + assert len(values) == 2, f"list(v) should have 2 elements" + assert approx(values[0], 10.5), f"list(v)[0] should be ~10.5, got {values[0]}" + assert approx(values[1], 20.5), f"list(v)[1] should be ~20.5, got {values[1]}" + + # Unpacking + x, y = v + assert approx(x, 10.5), f"Unpacked x should be ~10.5, got {x}" + assert approx(y, 20.5), f"Unpacked y should be ~20.5, got {y}" + + # tuple() conversion + t = tuple(v) + assert len(t) == 2 and approx(t[0], 10.5) and approx(t[1], 20.5), f"tuple(v) should be ~(10.5, 20.5), got {t}" + + print(" [PASS] Iteration and unpacking") + +def test_tuple_comparison(): + """Test comparison with tuples""" + # Use integer values which are exact in float32 + v = mcrfpy.Vector(5, 6) + + # Vector == tuple (integers are exact) + assert v == (5, 6), "Vector(5, 6) should equal (5, 6)" + assert v == (5.0, 6.0), "Vector(5, 6) should equal (5.0, 6.0)" + + # Vector != tuple + assert v != (5, 7), "Vector(5, 6) should not equal (5, 7)" + assert v != (4, 6), "Vector(5, 6) should not equal (4, 6)" + + # Tuple == Vector (reverse comparison) + assert (5, 6) == v, "(5, 6) should equal Vector(5, 6)" + assert (5, 7) != v, "(5, 7) should not equal Vector(5, 6)" + + # Edge cases + v_zero = mcrfpy.Vector(0, 0) + assert v_zero == (0, 0), "Vector(0, 0) should equal (0, 0)" + assert v_zero == (0.0, 0.0), "Vector(0, 0) should equal (0.0, 0.0)" + + # Negative values - use exact float32 values (x.5 are exact) + v_neg = mcrfpy.Vector(-3.5, -7.5) + assert v_neg == (-3.5, -7.5), "Vector(-3.5, -7.5) should equal (-3.5, -7.5)" + + print(" [PASS] Tuple comparison") + +def test_floor_method(): + """Test .floor() method""" + # Use values that clearly floor to different integers + v = mcrfpy.Vector(3.75, -2.25) # exact in float32 + floored = v.floor() + + assert isinstance(floored, mcrfpy.Vector), ".floor() should return a Vector" + assert floored.x == 3.0, f"floor(3.75) should be 3.0, got {floored.x}" + assert floored.y == -3.0, f"floor(-2.25) should be -3.0, got {floored.y}" + + # Positive values (use exact float32 values) + v2 = mcrfpy.Vector(5.875, 0.125) # exact in float32 + f2 = v2.floor() + assert f2 == (5.0, 0.0), f"floor(5.875, 0.125) should be (5.0, 0.0), got ({f2.x}, {f2.y})" + + # Already integers + v3 = mcrfpy.Vector(10.0, 20.0) + f3 = v3.floor() + assert f3 == (10.0, 20.0), f"floor(10.0, 20.0) should be (10.0, 20.0)" + + print(" [PASS] .floor() method") + +def test_int_property(): + """Test .int property""" + # Use exact float32 values + v = mcrfpy.Vector(3.75, -2.25) + int_tuple = v.int + + assert isinstance(int_tuple, tuple), ".int should return a tuple" + assert len(int_tuple) == 2, ".int tuple should have 2 elements" + assert int_tuple == (3, -3), f".int should be (3, -3), got {int_tuple}" + + # Check it's hashable (can be used as dict key) + d = {} + d[v.int] = "test" + assert d[(3, -3)] == "test", ".int tuple should work as dict key" + + # Positive values (use exact float32 values) + v2 = mcrfpy.Vector(5.875, 0.125) + assert v2.int == (5, 0), f".int should be (5, 0), got {v2.int}" + + print(" [PASS] .int property") + +def test_bool_check(): + """Test boolean conversion (already implemented, verify it works)""" + v_zero = mcrfpy.Vector(0, 0) + v_nonzero = mcrfpy.Vector(1, 0) + v_nonzero2 = mcrfpy.Vector(0, 1) + + assert not bool(v_zero), "Vector(0, 0) should be falsey" + assert bool(v_nonzero), "Vector(1, 0) should be truthy" + assert bool(v_nonzero2), "Vector(0, 1) should be truthy" + + # In if statement + if v_zero: + assert False, "Vector(0, 0) should not pass if check" + + if not v_nonzero: + assert False, "Vector(1, 0) should pass if check" + + print(" [PASS] Boolean check") + +def test_combined_operations(): + """Test that new features work together with existing operations""" + # Use exact float32 values + v1 = mcrfpy.Vector(3.5, 4.5) + v2 = mcrfpy.Vector(1.5, 2.5) + + # Arithmetic then tuple comparison (sums are exact) + result = v1 + v2 + assert result == (5.0, 7.0), f"(3.5+1.5, 4.5+2.5) should equal (5.0, 7.0), got ({result.x}, {result.y})" + + # Floor then use as dict key + floored = v1.floor() + positions = {floored.int: "player"} + assert (3, 4) in positions, "floored.int should work as dict key" + + # Unpack, modify, compare (products are exact) + x, y = v1 + v3 = mcrfpy.Vector(x * 2, y * 2) + assert v3 == (7.0, 9.0), f"Unpacking and creating new vector should work, got ({v3.x}, {v3.y})" + + print(" [PASS] Combined operations") + +def run_tests(): + """Run all tests""" + print("Testing Vector convenience features (Issue #109)...") + + test_indexing() + test_length() + test_iteration() + test_tuple_comparison() + test_floor_method() + test_int_property() + test_bool_check() + test_combined_operations() + + print("\n[ALL TESTS PASSED]") + sys.exit(0) + +# Run tests immediately (no game loop needed) +run_tests() From 9028bf485ef79bef56f9c7c3def3ab77bfb99ca6 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Wed, 26 Nov 2025 09:48:05 -0500 Subject: [PATCH 4/5] fix: Correct test to use del for index-based removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test was incorrectly using scene_ui.remove(-1) expecting it to remove the element at index -1. However, Python's list.remove(x) removes the first occurrence of VALUE x, not by index. Changed to use `del scene_ui[-1]` which is the correct Python idiom for removing an element by index. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- tests/unit/test_python_object_cache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_python_object_cache.py b/tests/unit/test_python_object_cache.py index 791cca3..e7c2831 100644 --- a/tests/unit/test_python_object_cache.py +++ b/tests/unit/test_python_object_cache.py @@ -92,9 +92,9 @@ def run_tests(runtime): test(retrieved_caption.caption_id == "test_caption", "Caption custom data preserved") # Test 8: Test removal and re-addition - #scene_ui.remove(frame) # TypeError: UICollection.remove requires an integer index to remove - seems like a C++ bug in the remove() implementation + # Use del to remove by index (Python standard), or .remove(element) to remove by value print(f"before remove: {len(scene_ui)=}") - scene_ui.remove(-1) + del scene_ui[-1] # Remove last element by index print(f"after remove: {len(scene_ui)=}") scene_ui.append(frame) From 19ded088b06edf093f789caa10e44faad3a6fb12 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Wed, 26 Nov 2025 10:26:30 -0500 Subject: [PATCH 5/5] feat: Exit on first Python callback exception (closes #133) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By default, McRogueFace now exits with code 1 on the first unhandled exception in timer, click, key, or animation callbacks. This prevents repeated exception output that wastes resources in AI-driven development. Changes: - Add exit_on_exception config flag (default: true) - Add --continue-after-exceptions CLI flag to preserve old behavior - Update exception handlers in Timer, PyCallable, and Animation - Signal game loop via McRFPy_API atomic flags - Return proper exit code from main() Before: Timer exceptions repeated 1000+ times until timeout After: Single traceback, clean exit with code 1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/Animation.cpp | 10 ++++-- src/CommandLineParser.cpp | 8 +++++ src/GameEngine.cpp | 5 +++ src/GameEngine.h | 1 + src/McRFPy_API.cpp | 22 ++++++++++++- src/McRFPy_API.h | 7 +++++ src/McRogueFaceConfig.h | 4 +++ src/PyCallable.cpp | 16 ++++++++-- src/Timer.cpp | 11 +++++-- src/main.cpp | 15 ++++++++- tests/notes/test_exception_exit.py | 31 ++++++++++++++++++ tests/notes/test_exception_exit_manual.sh | 38 +++++++++++++++++++++++ 12 files changed, 160 insertions(+), 8 deletions(-) create mode 100644 tests/notes/test_exception_exit.py create mode 100755 tests/notes/test_exception_exit_manual.sh diff --git a/src/Animation.cpp b/src/Animation.cpp index 4f74750..03f5b6f 100644 --- a/src/Animation.cpp +++ b/src/Animation.cpp @@ -3,6 +3,7 @@ #include "UIEntity.h" #include "PyAnimation.h" #include "McRFPy_API.h" +#include "GameEngine.h" #include "PythonObjectCache.h" #include #include @@ -368,9 +369,14 @@ void Animation::triggerCallback() { Py_DECREF(args); if (!result) { - // Print error but don't crash + std::cerr << "Animation callback raised an exception:" << std::endl; PyErr_Print(); - PyErr_Clear(); // Clear the error state + PyErr_Clear(); + + // Check if we should exit on exception + if (McRFPy_API::game && McRFPy_API::game->getConfig().exit_on_exception) { + McRFPy_API::signalPythonException(); + } } else { Py_DECREF(result); } diff --git a/src/CommandLineParser.cpp b/src/CommandLineParser.cpp index 3e69b1b..cad5398 100644 --- a/src/CommandLineParser.cpp +++ b/src/CommandLineParser.cpp @@ -121,6 +121,12 @@ CommandLineParser::ParseResult CommandLineParser::parse(McRogueFaceConfig& confi current_arg++; continue; } + + if (arg == "--continue-after-exceptions") { + config.exit_on_exception = false; + current_arg++; + continue; + } // If no flags matched, treat as positional argument (script name) if (arg[0] != '-') { @@ -160,6 +166,8 @@ void CommandLineParser::print_help() { << " --audio-off : disable audio\n" << " --audio-on : enable audio (even in headless mode)\n" << " --screenshot [path] : take a screenshot in headless mode\n" + << " --continue-after-exceptions : don't exit on Python callback exceptions\n" + << " (default: exit on first exception)\n" << "\n" << "Arguments:\n" << " file : program read from script file\n" diff --git a/src/GameEngine.cpp b/src/GameEngine.cpp index 3ac3cec..a012f26 100644 --- a/src/GameEngine.cpp +++ b/src/GameEngine.cpp @@ -289,6 +289,11 @@ void GameEngine::run() if (config.auto_exit_after_exec && timers.empty()) { running = false; } + + // Check if a Python exception has signaled exit + if (McRFPy_API::shouldExit()) { + running = false; + } } // Clean up before exiting the run loop diff --git a/src/GameEngine.h b/src/GameEngine.h index 4721bb8..c4a99ff 100644 --- a/src/GameEngine.h +++ b/src/GameEngine.h @@ -153,6 +153,7 @@ public: std::shared_ptr getTimer(const std::string& name); void setWindowScale(float); bool isHeadless() const { return headless; } + const McRogueFaceConfig& getConfig() const { return config; } void processEvent(const sf::Event& event); // Window property accessors diff --git a/src/McRFPy_API.cpp b/src/McRFPy_API.cpp index b095e52..b58df75 100644 --- a/src/McRFPy_API.cpp +++ b/src/McRFPy_API.cpp @@ -27,6 +27,9 @@ std::shared_ptr McRFPy_API::default_font; std::shared_ptr McRFPy_API::default_texture; PyObject* McRFPy_API::mcrf_module; +// Exception handling state +std::atomic McRFPy_API::exception_occurred{false}; +std::atomic McRFPy_API::exit_code{0}; static PyMethodDef mcrfpyMethods[] = { @@ -1144,6 +1147,23 @@ PyObject* McRFPy_API::_getMetrics(PyObject* self, PyObject* args) { // Add general metrics PyDict_SetItemString(dict, "current_frame", PyLong_FromLong(game->getFrame())); PyDict_SetItemString(dict, "runtime", PyFloat_FromDouble(game->runtime.getElapsedTime().asSeconds())); - + return dict; } + +// Exception handling implementation +void McRFPy_API::signalPythonException() { + // Check if we should exit on exception (consult config via game) + if (game && !game->isHeadless()) { + // In windowed mode, respect the config setting + // Access config through game engine - but we need to check the config + } + + // For now, always signal - the game loop will check the config + exception_occurred.store(true); + exit_code.store(1); +} + +bool McRFPy_API::shouldExit() { + return exception_occurred.load(); +} diff --git a/src/McRFPy_API.h b/src/McRFPy_API.h index 6b32dcf..6841fd2 100644 --- a/src/McRFPy_API.h +++ b/src/McRFPy_API.h @@ -2,6 +2,7 @@ #include "Common.h" #include "Python.h" #include +#include #include "PyFont.h" #include "PyTexture.h" @@ -85,4 +86,10 @@ public: static void triggerSceneChange(const std::string& from_scene, const std::string& to_scene); static void updatePythonScenes(float dt); static void triggerResize(int width, int height); + + // Exception handling - signal game loop to exit on unhandled Python exceptions + static std::atomic exception_occurred; + static std::atomic exit_code; + static void signalPythonException(); // Called by exception handlers + static bool shouldExit(); // Checked by game loop }; diff --git a/src/McRogueFaceConfig.h b/src/McRogueFaceConfig.h index 9534df6..ed4e9d2 100644 --- a/src/McRogueFaceConfig.h +++ b/src/McRogueFaceConfig.h @@ -31,6 +31,10 @@ struct McRogueFaceConfig { // Auto-exit when no timers remain (for --headless --exec automation) bool auto_exit_after_exec = false; + + // Exception handling: exit on first Python callback exception (default: true) + // Use --continue-after-exceptions to disable + bool exit_on_exception = true; }; #endif // MCROGUEFACE_CONFIG_H \ No newline at end of file diff --git a/src/PyCallable.cpp b/src/PyCallable.cpp index 0a3fb53..6fed830 100644 --- a/src/PyCallable.cpp +++ b/src/PyCallable.cpp @@ -1,4 +1,6 @@ #include "PyCallable.h" +#include "McRFPy_API.h" +#include "GameEngine.h" PyCallable::PyCallable(PyObject* _target) { @@ -51,9 +53,14 @@ void PyClickCallable::call(sf::Vector2f mousepos, std::string button, std::strin PyObject* retval = PyCallable::call(args, NULL); if (!retval) { - std::cout << "ClickCallable has raised an exception. It's going to STDERR and being dropped:" << std::endl; + std::cerr << "Click callback raised an exception:" << std::endl; PyErr_Print(); PyErr_Clear(); + + // Check if we should exit on exception + if (McRFPy_API::game && McRFPy_API::game->getConfig().exit_on_exception) { + McRFPy_API::signalPythonException(); + } } else if (retval != Py_None) { std::cout << "ClickCallable returned a non-None value. It's not an error, it's just not being saved or used." << std::endl; @@ -81,9 +88,14 @@ void PyKeyCallable::call(std::string key, std::string action) PyObject* retval = PyCallable::call(args, NULL); if (!retval) { - std::cout << "KeyCallable has raised an exception. It's going to STDERR and being dropped:" << std::endl; + std::cerr << "Key callback raised an exception:" << std::endl; PyErr_Print(); PyErr_Clear(); + + // Check if we should exit on exception + if (McRFPy_API::game && McRFPy_API::game->getConfig().exit_on_exception) { + McRFPy_API::signalPythonException(); + } } else if (retval != Py_None) { std::cout << "KeyCallable returned a non-None value. It's not an error, it's just not being saved or used." << std::endl; diff --git a/src/Timer.cpp b/src/Timer.cpp index 0784f13..8873a39 100644 --- a/src/Timer.cpp +++ b/src/Timer.cpp @@ -1,6 +1,8 @@ #include "Timer.h" #include "PythonObjectCache.h" #include "PyCallable.h" +#include "McRFPy_API.h" +#include "GameEngine.h" Timer::Timer(PyObject* _target, int _interval, int now, bool _once) : callback(std::make_shared(_target)), interval(_interval), last_ran(now), @@ -51,10 +53,15 @@ bool Timer::test(int now) Py_DECREF(args); if (!retval) - { - std::cout << "Timer callback has raised an exception. It's going to STDERR and being dropped:" << std::endl; + { + std::cerr << "Timer callback raised an exception:" << std::endl; PyErr_Print(); PyErr_Clear(); + + // Check if we should exit on exception + if (McRFPy_API::game && McRFPy_API::game->getConfig().exit_on_exception) { + McRFPy_API::signalPythonException(); + } } else if (retval != Py_None) { std::cout << "Timer returned a non-None value. It's not an error, it's just not being saved or used." << std::endl; diff --git a/src/main.cpp b/src/main.cpp index 3652e6c..4908e8c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -44,6 +44,11 @@ int run_game_engine(const McRogueFaceConfig& config) if (Py_IsInitialized()) { McRFPy_API::api_shutdown(); } + + // Return exception exit code if a Python exception signaled exit + if (McRFPy_API::shouldExit()) { + return McRFPy_API::exit_code.load(); + } return 0; } @@ -181,9 +186,13 @@ int run_python_interpreter(const McRogueFaceConfig& config, int argc, char* argv // Run the game engine after script execution engine->run(); - + McRFPy_API::api_shutdown(); delete engine; + // Return exception exit code if signaled + if (McRFPy_API::shouldExit()) { + return McRFPy_API::exit_code.load(); + } return result; } else if (config.interactive_mode) { @@ -207,6 +216,10 @@ int run_python_interpreter(const McRogueFaceConfig& config, int argc, char* argv engine->run(); McRFPy_API::api_shutdown(); delete engine; + // Return exception exit code if signaled + if (McRFPy_API::shouldExit()) { + return McRFPy_API::exit_code.load(); + } return 0; } diff --git a/tests/notes/test_exception_exit.py b/tests/notes/test_exception_exit.py new file mode 100644 index 0000000..348c88b --- /dev/null +++ b/tests/notes/test_exception_exit.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 +"""Test for --continue-after-exceptions behavior (Issue #133) + +This test verifies that: +1. By default, unhandled exceptions in timer callbacks cause immediate exit with code 1 +2. With --continue-after-exceptions, exceptions are logged but execution continues +""" + +import mcrfpy +import sys + +def timer_that_raises(runtime): + """A timer callback that raises an exception""" + raise ValueError("Intentional test exception") + +# Create a test scene +mcrfpy.createScene("test") +mcrfpy.setScene("test") + +# Schedule the timer - it will fire after 50ms +mcrfpy.setTimer("raise_exception", timer_that_raises, 50) + +# This test expects: +# - Default behavior: exit with code 1 after first exception +# - With --continue-after-exceptions: continue running (would need timeout or explicit exit) +# +# The test runner should: +# 1. Run without --continue-after-exceptions and expect exit code 1 +# 2. Run with --continue-after-exceptions and expect it to not exit immediately + +print("Test initialized - timer will raise exception in 50ms") diff --git a/tests/notes/test_exception_exit_manual.sh b/tests/notes/test_exception_exit_manual.sh new file mode 100755 index 0000000..78c2d8b --- /dev/null +++ b/tests/notes/test_exception_exit_manual.sh @@ -0,0 +1,38 @@ +#!/bin/bash +# Manual test for --continue-after-exceptions feature (Issue #133) +# +# This test must be run manually because it verifies exit codes +# rather than test output. + +echo "Testing --continue-after-exceptions feature..." +echo + +cd "$(dirname "$0")/../../build" + +# Test 1: Default behavior - should exit with code 1 on first exception +echo "Test 1: Default behavior (exit on first exception)" +timeout 5 ./mcrogueface --headless --exec ../tests/notes/test_exception_exit.py 2>&1 +EXIT_CODE=$? +echo "Exit code: $EXIT_CODE" +if [ $EXIT_CODE -eq 1 ]; then + echo "[PASS] Exit code is 1 as expected" +else + echo "[FAIL] Expected exit code 1, got $EXIT_CODE" + exit 1 +fi +echo + +# Test 2: --continue-after-exceptions - should keep running until timeout +echo "Test 2: --continue-after-exceptions (continue after exception)" +timeout 1 ./mcrogueface --headless --continue-after-exceptions --exec ../tests/notes/test_exception_exit.py 2>&1 | tail -5 +EXIT_CODE=${PIPESTATUS[0]} +echo "Exit code: $EXIT_CODE" +if [ $EXIT_CODE -eq 124 ]; then + echo "[PASS] Timeout killed it (exit code 124) - continued running as expected" +else + echo "[FAIL] Expected exit code 124 (timeout), got $EXIT_CODE" + exit 1 +fi +echo + +echo "All tests PASSED!"