From 41a704a01088e703575177480d46c381103c7baf Mon Sep 17 00:00:00 2001 From: John McCardle Date: Thu, 27 Nov 2025 21:01:11 -0500 Subject: [PATCH] refactor: Use property setter pattern for parent assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of separate getParent()/setParent()/removeFromParent() methods, the parent property now supports the Pythonic getter/setter pattern: - child.parent # Get parent (or None) - child.parent = f # Set parent (adds to f.children) - child.parent = None # Remove from parent This matches the existing pattern used by the click property callback. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/UIBase.h | 7 ++- src/UIDrawable.cpp | 81 ++++++++++++++++++++++++++ src/UIDrawable.h | 1 + tests/unit/test_parent_child_system.py | 63 ++++++++++++++++++++ 4 files changed, 149 insertions(+), 3 deletions(-) diff --git a/src/UIBase.h b/src/UIBase.h index f1711b3..95a96dc 100644 --- a/src/UIBase.h +++ b/src/UIBase.h @@ -165,10 +165,11 @@ static int UIDrawable_set_opacity(T* self, PyObject* value, void* closure) // #122 & #102: Macro for parent/global_position properties (requires closure with type enum) // These need the PyObjectsEnum value in closure, so they're added separately in each class #define UIDRAWABLE_PARENT_GETSETTERS(type_enum) \ - {"parent", (getter)UIDrawable::get_parent, NULL, \ + {"parent", (getter)UIDrawable::get_parent, (setter)UIDrawable::set_parent, \ MCRF_PROPERTY(parent, \ - "Parent drawable (read-only). " \ - "Returns the parent Frame/Grid if nested, or None if at scene level." \ + "Parent drawable. " \ + "Get: Returns the parent Frame/Grid if nested, or None if at scene level. " \ + "Set: Assign a Frame/Grid to reparent, or None to remove from parent." \ ), (void*)type_enum}, \ {"global_position", (getter)UIDrawable::get_global_pos, NULL, \ MCRF_PROPERTY(global_position, \ diff --git a/src/UIDrawable.cpp b/src/UIDrawable.cpp index 448e712..204bbab 100644 --- a/src/UIDrawable.cpp +++ b/src/UIDrawable.cpp @@ -848,6 +848,87 @@ PyObject* UIDrawable::get_parent(PyObject* self, void* closure) { return obj; } +// Python API - set parent drawable (or None to remove from parent) +int UIDrawable::set_parent(PyObject* self, PyObject* value, void* closure) { + PyObjectsEnum objtype = static_cast(reinterpret_cast(closure)); + std::shared_ptr drawable = nullptr; + + // Get the shared_ptr for self + switch (objtype) { + case PyObjectsEnum::UIFRAME: + drawable = ((PyUIFrameObject*)self)->data; + break; + case PyObjectsEnum::UICAPTION: + drawable = ((PyUICaptionObject*)self)->data; + break; + case PyObjectsEnum::UISPRITE: + drawable = ((PyUISpriteObject*)self)->data; + break; + case PyObjectsEnum::UIGRID: + drawable = ((PyUIGridObject*)self)->data; + break; + case PyObjectsEnum::UILINE: + drawable = ((PyUILineObject*)self)->data; + break; + case PyObjectsEnum::UICIRCLE: + drawable = ((PyUICircleObject*)self)->data; + break; + case PyObjectsEnum::UIARC: + drawable = ((PyUIArcObject*)self)->data; + break; + default: + PyErr_SetString(PyExc_TypeError, "Invalid UIDrawable derived instance"); + return -1; + } + + // Handle None - remove from parent + if (value == Py_None) { + drawable->removeFromParent(); + return 0; + } + + // Value must be a Frame or Grid (things with children collections) + // Check if it's a Frame + PyTypeObject* frame_type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Frame"); + PyTypeObject* grid_type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Grid"); + + bool is_frame = frame_type && PyObject_IsInstance(value, (PyObject*)frame_type); + bool is_grid = grid_type && PyObject_IsInstance(value, (PyObject*)grid_type); + + Py_XDECREF(frame_type); + Py_XDECREF(grid_type); + + if (!is_frame && !is_grid) { + PyErr_SetString(PyExc_TypeError, "parent must be a Frame, Grid, or None"); + return -1; + } + + // Remove from old parent first + drawable->removeFromParent(); + + // Get the new parent's children collection and append + std::shared_ptr>>* children_ptr = nullptr; + std::shared_ptr new_parent = nullptr; + + if (is_frame) { + auto frame = ((PyUIFrameObject*)value)->data; + children_ptr = &frame->children; + new_parent = frame; + } else if (is_grid) { + auto grid = ((PyUIGridObject*)value)->data; + children_ptr = &grid->children; + new_parent = grid; + } + + if (children_ptr && *children_ptr) { + // Add to new parent's children + (*children_ptr)->push_back(drawable); + drawable->setParent(new_parent); + } + + return 0; +} + // Python API - get global position (read-only) PyObject* UIDrawable::get_global_pos(PyObject* self, void* closure) { PyObjectsEnum objtype = static_cast(reinterpret_cast(closure)); diff --git a/src/UIDrawable.h b/src/UIDrawable.h index 9aea7d0..b93059f 100644 --- a/src/UIDrawable.h +++ b/src/UIDrawable.h @@ -95,6 +95,7 @@ public: // Python API for parent/global_position static PyObject* get_parent(PyObject* self, void* closure); + static int set_parent(PyObject* self, PyObject* value, void* closure); static PyObject* get_global_pos(PyObject* self, void* closure); // New properties for Phase 1 diff --git a/tests/unit/test_parent_child_system.py b/tests/unit/test_parent_child_system.py index c336928..2cdd4c9 100644 --- a/tests/unit/test_parent_child_system.py +++ b/tests/unit/test_parent_child_system.py @@ -186,6 +186,67 @@ def test_all_drawable_types(): print(" - All drawable types: PASS") +def test_parent_setter(): + """Test parent property setter (assign parent directly)""" + print("Testing parent setter...") + + mcrfpy.createScene("test7") + ui = mcrfpy.sceneUI("test7") + + # Create parent frame and child + parent = mcrfpy.Frame(pos=(100, 100), size=(200, 200)) + child = mcrfpy.Caption(text="Child", pos=(10, 10)) + ui.append(parent) + + # Child has no parent initially + assert child.parent is None, "Child should have no parent initially" + + # Set parent via property assignment + child.parent = parent + assert child.parent is not None, "Child should have parent after assignment" + assert len(parent.children) == 1, f"Parent should have 1 child, has {len(parent.children)}" + + # Verify global position updates + global_pos = child.global_position + assert global_pos.x == 110.0, f"Global X should be 110, got {global_pos.x}" + assert global_pos.y == 110.0, f"Global Y should be 110, got {global_pos.y}" + + # Set parent to None to remove + child.parent = None + assert child.parent is None, "Child should have no parent after setting None" + assert len(parent.children) == 0, f"Parent should have 0 children, has {len(parent.children)}" + + print(" - Parent setter: PASS") + + +def test_reparenting_via_setter(): + """Test moving a child from one parent to another via setter""" + print("Testing reparenting via setter...") + + mcrfpy.createScene("test8") + ui = mcrfpy.sceneUI("test8") + + parent1 = mcrfpy.Frame(pos=(0, 0), size=(100, 100)) + parent2 = mcrfpy.Frame(pos=(200, 200), size=(100, 100)) + child = mcrfpy.Sprite(pos=(5, 5)) + + ui.append(parent1) + ui.append(parent2) + + # Add to first parent via setter + child.parent = parent1 + assert len(parent1.children) == 1, "Parent1 should have 1 child" + assert len(parent2.children) == 0, "Parent2 should have 0 children" + + # Move to second parent via setter (should auto-remove from parent1) + child.parent = parent2 + assert len(parent1.children) == 0, f"Parent1 should have 0 children after reparent, has {len(parent1.children)}" + assert len(parent2.children) == 1, f"Parent2 should have 1 child after reparent, has {len(parent2.children)}" + assert child.global_position.x == 205.0, f"Global X should be 205, got {child.global_position.x}" + + print(" - Reparenting via setter: PASS") + + if __name__ == "__main__": try: test_parent_property() @@ -194,6 +255,8 @@ if __name__ == "__main__": test_remove_clears_parent() test_scene_level_elements() test_all_drawable_types() + test_parent_setter() + test_reparenting_via_setter() print("\n=== All tests passed! ===") sys.exit(0)