From 71c91e19a5dafae992f578dd8b8649310e00450d Mon Sep 17 00:00:00 2001 From: John McCardle Date: Mon, 22 Dec 2025 22:15:03 -0500 Subject: [PATCH] feat: Add consistent Scene API with module-level properties (closes #151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces module-level scene functions with more Pythonic OO interface: Scene class changes: - Add `scene.children` property (replaces get_ui() method) - Add `scene.on_key` getter/setter (matches on_click pattern) - Remove get_ui() method Module-level properties: - Add `mcrfpy.current_scene` (getter returns Scene, setter activates) - Add `mcrfpy.scenes` (read-only tuple of all Scene objects) Implementation uses custom module type (McRFPyModuleType) inheriting from PyModule_Type with tp_setattro for property assignment support. New usage: scene = mcrfpy.Scene("game") mcrfpy.current_scene = scene scene.on_key = handler ui = scene.children 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/McRFPy_API.cpp | 85 ++++++++++++- src/McRFPy_API.h | 5 + src/PySceneObject.cpp | 164 ++++++++++++++++++++++--- src/PySceneObject.h | 1 - tests/unit/test_issue_151_scene_api.py | 160 ++++++++++++++++++++++++ tests/unit/test_scene_properties.py | 18 +-- 6 files changed, 406 insertions(+), 27 deletions(-) create mode 100644 tests/unit/test_issue_151_scene_api.py diff --git a/src/McRFPy_API.cpp b/src/McRFPy_API.cpp index 7b71d7e..be70245 100644 --- a/src/McRFPy_API.cpp +++ b/src/McRFPy_API.cpp @@ -35,6 +35,71 @@ PyObject* McRFPy_API::mcrf_module; std::atomic McRFPy_API::exception_occurred{false}; std::atomic McRFPy_API::exit_code{0}; +// #151: Module-level __getattr__ for dynamic properties (current_scene, scenes) +static PyObject* mcrfpy_module_getattr(PyObject* self, PyObject* args) +{ + const char* name; + if (!PyArg_ParseTuple(args, "s", &name)) { + return NULL; + } + + if (strcmp(name, "current_scene") == 0) { + return McRFPy_API::api_get_current_scene(); + } + + if (strcmp(name, "scenes") == 0) { + return McRFPy_API::api_get_scenes(); + } + + // Attribute not found - raise AttributeError + PyErr_Format(PyExc_AttributeError, "module 'mcrfpy' has no attribute '%s'", name); + return NULL; +} + +// #151: Custom module type with __setattr__ support for current_scene +static int mcrfpy_module_setattro(PyObject* self, PyObject* name, PyObject* value) +{ + const char* name_str = PyUnicode_AsUTF8(name); + if (!name_str) return -1; + + if (strcmp(name_str, "current_scene") == 0) { + return McRFPy_API::api_set_current_scene(value); + } + + if (strcmp(name_str, "scenes") == 0) { + PyErr_SetString(PyExc_AttributeError, "'scenes' is read-only"); + return -1; + } + + // For other attributes, use default module setattr + return PyObject_GenericSetAttr(self, name, value); +} + +// Custom module type that inherits from PyModule_Type but has our __setattr__ +static PyTypeObject McRFPyModuleType = { + .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, + .tp_name = "mcrfpy.module", + .tp_basicsize = 0, // Inherited from base + .tp_itemsize = 0, + .tp_dealloc = NULL, + .tp_vectorcall_offset = 0, + .tp_getattr = NULL, + .tp_setattr = NULL, + .tp_as_async = NULL, + .tp_repr = NULL, + .tp_as_number = NULL, + .tp_as_sequence = NULL, + .tp_as_mapping = NULL, + .tp_hash = NULL, + .tp_call = NULL, + .tp_str = NULL, + .tp_getattro = NULL, + .tp_setattro = mcrfpy_module_setattro, + .tp_as_buffer = NULL, + .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .tp_doc = "McRogueFace module with property support", +}; + static PyMethodDef mcrfpyMethods[] = { {"createSoundBuffer", McRFPy_API::_createSoundBuffer, METH_VARARGS, @@ -254,6 +319,10 @@ static PyMethodDef mcrfpyMethods[] = { MCRF_NOTE("Messages appear in the 'logs' array of each frame in the output JSON.") )}, + // #151: Module-level attribute access for current_scene and scenes + {"__getattr__", mcrfpy_module_getattr, METH_VARARGS, + "Module-level __getattr__ for dynamic properties (current_scene, scenes)"}, + {NULL, NULL, 0, NULL} }; @@ -293,12 +362,22 @@ static PyModuleDef mcrfpyModule = { PyObject* PyInit_mcrfpy() { PyObject* m = PyModule_Create(&mcrfpyModule); - - if (m == NULL) + + if (m == NULL) { return NULL; } - + + // #151: Set up custom module type for current_scene/scenes property support + // The custom type inherits from PyModule_Type and adds __setattr__ handling + McRFPyModuleType.tp_base = &PyModule_Type; + if (PyType_Ready(&McRFPyModuleType) < 0) { + std::cout << "ERROR: PyType_Ready failed for McRFPyModuleType" << std::endl; + return NULL; + } + // Change the module's type to our custom type + Py_SET_TYPE(m, &McRFPyModuleType); + using namespace mcrfpydef; PyTypeObject* pytypes[] = { /*SFML exposed types*/ diff --git a/src/McRFPy_API.h b/src/McRFPy_API.h index c928961..4e5103a 100644 --- a/src/McRFPy_API.h +++ b/src/McRFPy_API.h @@ -98,6 +98,11 @@ public: static void updatePythonScenes(float dt); static void triggerResize(int width, int height); + // #151: Module-level scene property accessors + static PyObject* api_get_current_scene(); + static int api_set_current_scene(PyObject* value); + static PyObject* api_get_scenes(); + // Exception handling - signal game loop to exit on unhandled Python exceptions static std::atomic exception_occurred; static std::atomic exit_code; diff --git a/src/PySceneObject.cpp b/src/PySceneObject.cpp index 571d55a..04711f0 100644 --- a/src/PySceneObject.cpp +++ b/src/PySceneObject.cpp @@ -84,7 +84,8 @@ PyObject* PySceneClass::activate(PySceneObject* self, PyObject* args) return result; } -PyObject* PySceneClass::get_ui(PySceneObject* self, PyObject* args) +// children property getter (replaces get_ui method) +static PyObject* PySceneClass_get_children(PySceneObject* self, void* closure) { // Call the static method from McRFPy_API PyObject* py_args = Py_BuildValue("(s)", self->name.c_str()); @@ -99,15 +100,15 @@ PyObject* PySceneClass::register_keyboard(PySceneObject* self, PyObject* args) if (!PyArg_ParseTuple(args, "O", &callable)) { return NULL; } - + if (!PyCallable_Check(callable)) { PyErr_SetString(PyExc_TypeError, "Argument must be callable"); return NULL; } - + // Store the callable Py_INCREF(callable); - + // Get the current scene and set its key_callable GameEngine* game = McRFPy_API::game; if (game) { @@ -117,11 +118,58 @@ PyObject* PySceneClass::register_keyboard(PySceneObject* self, PyObject* args) game->currentScene()->key_callable = std::make_unique(callable); game->scene = old_scene; } - + Py_DECREF(callable); Py_RETURN_NONE; } +// on_key property getter +static PyObject* PySceneClass_get_on_key(PySceneObject* self, void* closure) +{ + GameEngine* game = McRFPy_API::game; + if (!game) { + Py_RETURN_NONE; + } + + auto scene = game->getScene(self->name); + if (!scene || !scene->key_callable) { + Py_RETURN_NONE; + } + + PyObject* callable = scene->key_callable->borrow(); + if (callable && callable != Py_None) { + Py_INCREF(callable); + return callable; + } + Py_RETURN_NONE; +} + +// on_key property setter +static int PySceneClass_set_on_key(PySceneObject* self, PyObject* value, void* closure) +{ + GameEngine* game = McRFPy_API::game; + if (!game) { + PyErr_SetString(PyExc_RuntimeError, "No game engine"); + return -1; + } + + auto scene = game->getScene(self->name); + if (!scene) { + PyErr_SetString(PyExc_RuntimeError, "Scene not found"); + return -1; + } + + if (value == Py_None || value == NULL) { + scene->key_unregister(); + } else if (PyCallable_Check(value)) { + scene->key_register(value); + } else { + PyErr_SetString(PyExc_TypeError, "on_key must be callable or None"); + return -1; + } + return 0; +} + PyObject* PySceneClass::get_name(PySceneObject* self, void* closure) { return PyUnicode_FromString(self->name.c_str()); @@ -394,6 +442,14 @@ PyGetSetDef PySceneClass::getsetters[] = { MCRF_PROPERTY(visible, "Scene visibility (bool). If False, scene is not rendered."), NULL}, {"opacity", (getter)PySceneClass_get_opacity, (setter)PySceneClass_set_opacity, MCRF_PROPERTY(opacity, "Scene opacity (0.0-1.0). Applied to all UI elements during rendering."), NULL}, + // #151: Consistent Scene API + {"children", (getter)PySceneClass_get_children, NULL, + MCRF_PROPERTY(children, "UI element collection for this scene (UICollection, read-only). " + "Use to add, remove, or iterate over UI elements. Changes are reflected immediately."), NULL}, + {"on_key", (getter)PySceneClass_get_on_key, (setter)PySceneClass_set_on_key, + MCRF_PROPERTY(on_key, "Keyboard event handler (callable or None). " + "Function receives (key: str, action: str) for keyboard events. " + "Set to None to remove the handler."), NULL}, {NULL} }; @@ -406,13 +462,6 @@ PyMethodDef PySceneClass::methods[] = { MCRF_RETURNS("None") MCRF_NOTE("Deactivates the current scene and activates this one. Scene transitions and lifecycle callbacks are triggered.") )}, - {"get_ui", (PyCFunction)get_ui, METH_NOARGS, - MCRF_METHOD(SceneClass, get_ui, - MCRF_SIG("()", "UICollection"), - MCRF_DESC("Get the UI element collection for this scene."), - MCRF_RETURNS("UICollection: Collection of UI elements (Frames, Captions, Sprites, Grids) in this scene") - MCRF_NOTE("Use to add, remove, or iterate over UI elements. Changes are reflected immediately.") - )}, {"register_keyboard", (PyCFunction)register_keyboard, METH_VARARGS, MCRF_METHOD(SceneClass, register_keyboard, MCRF_SIG("(callback: callable)", "None"), @@ -420,7 +469,7 @@ PyMethodDef PySceneClass::methods[] = { MCRF_ARGS_START MCRF_ARG("callback", "Function that receives (key: str, pressed: bool) when keyboard events occur") MCRF_RETURNS("None") - MCRF_NOTE("Alternative to overriding on_keypress() method. Handler is called for both key press and release events.") + MCRF_NOTE("Alternative to setting on_key property. Handler is called for both key press and release events.") )}, {NULL} }; @@ -456,9 +505,96 @@ void McRFPy_API::triggerResize(int width, int height) { GameEngine* game = McRFPy_API::game; if (!game) return; - + // Only notify the active scene if (python_scenes.count(game->scene) > 0) { PySceneClass::call_on_resize(python_scenes[game->scene], width, height); } +} + +// #151: Get the current scene as a Python Scene object +PyObject* McRFPy_API::api_get_current_scene() +{ + GameEngine* game = McRFPy_API::game; + if (!game) { + Py_RETURN_NONE; + } + + const std::string& current_name = game->scene; + if (python_scenes.count(current_name) > 0) { + PyObject* scene_obj = (PyObject*)python_scenes[current_name]; + Py_INCREF(scene_obj); + return scene_obj; + } + + // Scene exists but wasn't created via Python Scene class + Py_RETURN_NONE; +} + +// #151: Set the current scene from a Python Scene object +int McRFPy_API::api_set_current_scene(PyObject* value) +{ + GameEngine* game = McRFPy_API::game; + if (!game) { + PyErr_SetString(PyExc_RuntimeError, "No game engine"); + return -1; + } + + if (!value) { + PyErr_SetString(PyExc_ValueError, "value is NULL"); + return -1; + } + + // Accept Scene object or string name + const char* scene_name = nullptr; + + if (PyUnicode_Check(value)) { + scene_name = PyUnicode_AsUTF8(value); + } else { + // Check if value is a Scene or Scene subclass - use same pattern as rest of codebase + PyObject* scene_type = PyObject_GetAttrString(McRFPy_API::mcrf_module, "Scene"); + if (scene_type && PyObject_IsInstance(value, scene_type)) { + Py_DECREF(scene_type); + PySceneObject* scene_obj = (PySceneObject*)value; + scene_name = scene_obj->name.c_str(); + } else { + Py_XDECREF(scene_type); + PyErr_SetString(PyExc_TypeError, "current_scene must be a Scene object or scene name string"); + return -1; + } + } + + if (!scene_name) { + PyErr_SetString(PyExc_ValueError, "Invalid scene name"); + return -1; + } + + // Verify scene exists + if (!game->getScene(scene_name)) { + PyErr_Format(PyExc_KeyError, "Scene '%s' does not exist", scene_name); + return -1; + } + + std::string old_scene = game->scene; + game->scene = scene_name; + McRFPy_API::triggerSceneChange(old_scene, scene_name); + + return 0; +} + +// #151: Get all scenes as a tuple of Python Scene objects +PyObject* McRFPy_API::api_get_scenes() +{ + PyObject* tuple = PyTuple_New(python_scenes.size()); + if (!tuple) return NULL; + + Py_ssize_t i = 0; + for (auto& pair : python_scenes) { + PyObject* scene_obj = (PyObject*)pair.second; + Py_INCREF(scene_obj); + PyTuple_SET_ITEM(tuple, i, scene_obj); + i++; + } + + return tuple; } \ No newline at end of file diff --git a/src/PySceneObject.h b/src/PySceneObject.h index b504e5e..bc12ea3 100644 --- a/src/PySceneObject.h +++ b/src/PySceneObject.h @@ -27,7 +27,6 @@ public: // Scene methods static PyObject* activate(PySceneObject* self, PyObject* args); - static PyObject* get_ui(PySceneObject* self, PyObject* args); static PyObject* register_keyboard(PySceneObject* self, PyObject* args); // Properties diff --git a/tests/unit/test_issue_151_scene_api.py b/tests/unit/test_issue_151_scene_api.py new file mode 100644 index 0000000..cab8414 --- /dev/null +++ b/tests/unit/test_issue_151_scene_api.py @@ -0,0 +1,160 @@ +#!/usr/bin/env python3 +"""Test #151: Consistent Scene API - scene.children, scene.on_key, mcrfpy.current_scene, mcrfpy.scenes""" +import mcrfpy +import sys + +print("Testing Issue #151 - Consistent Scene API") +print("=" * 50) + +def test_scene_children(): + """Test scene.children property (replaces get_ui method)""" + print("\n1. Testing scene.children property...") + + class TestScene(mcrfpy.Scene): + def __init__(self, name): + super().__init__(name) + + scene = TestScene("children_test") + + # Get children + children = scene.children + assert children is not None, "children should not be None" + + # Add elements via children + frame = mcrfpy.Frame(pos=(10, 20), size=(100, 100)) + children.append(frame) + + caption = mcrfpy.Caption(text="Test", pos=(50, 50)) + children.append(caption) + + # Verify count + assert len(scene.children) == 2, f"Expected 2 children, got {len(scene.children)}" + + print(" PASS - scene.children works correctly") + return True + +def test_scene_on_key(): + """Test scene.on_key property (replaces keypressScene and register_keyboard)""" + print("\n2. Testing scene.on_key property...") + + class TestScene(mcrfpy.Scene): + def __init__(self, name): + super().__init__(name) + + scene = TestScene("on_key_test") + scene.activate() + + # Initial state should be None + # Note: might return None or the default empty callable + initial = scene.on_key + print(f" Initial on_key value: {initial}") + + # Define a handler + handler_called = [False] + def test_handler(key, action): + handler_called[0] = True + print(f" Handler called with key={key}, action={action}") + + # Set handler via property + scene.on_key = test_handler + + # Get handler back + retrieved = scene.on_key + assert retrieved is not None, "on_key should return the handler" + assert callable(retrieved), "on_key should be callable" + + # Clear handler + scene.on_key = None + cleared = scene.on_key + assert cleared is None, f"on_key should be None after clearing, got {cleared}" + + print(" PASS - scene.on_key works correctly") + return True + +def test_current_scene_property(): + """Test mcrfpy.current_scene property""" + print("\n3. Testing mcrfpy.current_scene property...") + + class TestScene(mcrfpy.Scene): + def __init__(self, name): + super().__init__(name) + + scene1 = TestScene("scene_prop_1") + scene2 = TestScene("scene_prop_2") + + # Set via property + mcrfpy.current_scene = scene1 + + # Get via property + current = mcrfpy.current_scene + assert current is not None, "current_scene should not be None" + assert current.name == "scene_prop_1", f"Expected scene_prop_1, got {current.name}" + + # Switch to scene2 + mcrfpy.current_scene = scene2 + current = mcrfpy.current_scene + assert current.name == "scene_prop_2", f"Expected scene_prop_2, got {current.name}" + + # Also test setting by string name + mcrfpy.current_scene = "scene_prop_1" + current = mcrfpy.current_scene + assert current.name == "scene_prop_1", f"Expected scene_prop_1 (set by string), got {current.name}" + + print(" PASS - mcrfpy.current_scene works correctly") + return True + +def test_scenes_collection(): + """Test mcrfpy.scenes collection""" + print("\n4. Testing mcrfpy.scenes collection...") + + # Get current scenes tuple + scenes = mcrfpy.scenes + assert scenes is not None, "scenes should not be None" + assert isinstance(scenes, tuple), f"scenes should be a tuple, got {type(scenes)}" + + print(f" Found {len(scenes)} scenes: {[s.name for s in scenes]}") + + # All items should be Scene objects + for scene in scenes: + assert hasattr(scene, 'name'), f"Scene should have name attribute: {scene}" + assert hasattr(scene, 'children'), f"Scene should have children attribute: {scene}" + + print(" PASS - mcrfpy.scenes works correctly") + return True + +def test_scenes_readonly(): + """Test that scenes is read-only""" + print("\n5. Testing mcrfpy.scenes is read-only...") + + try: + mcrfpy.scenes = () + print(" FAIL - should have raised AttributeError") + return False + except AttributeError as e: + print(f" Correctly raised AttributeError: {e}") + print(" PASS - mcrfpy.scenes is read-only") + return True + +# Run all tests +if __name__ == "__main__": + try: + all_passed = True + + all_passed &= test_scene_children() + all_passed &= test_scene_on_key() + all_passed &= test_current_scene_property() + all_passed &= test_scenes_collection() + all_passed &= test_scenes_readonly() + + print("\n" + "=" * 50) + if all_passed: + print("ALL TESTS PASSED!") + sys.exit(0) + else: + print("SOME TESTS FAILED") + sys.exit(1) + except Exception as e: + print(f"\nFAIL: {e}") + import traceback + traceback.print_exc() + sys.exit(1) diff --git a/tests/unit/test_scene_properties.py b/tests/unit/test_scene_properties.py index dde21b8..9557e54 100644 --- a/tests/unit/test_scene_properties.py +++ b/tests/unit/test_scene_properties.py @@ -132,9 +132,9 @@ def test_scene_active(): print(" - Scene active property: PASS") -def test_scene_get_ui(): - """Test Scene get_ui method""" - print("Testing scene get_ui method...") +def test_scene_children(): + """Test Scene children property (#151)""" + print("Testing scene children property...") class TestScene(mcrfpy.Scene): def __init__(self, name): @@ -142,18 +142,18 @@ def test_scene_get_ui(): scene = TestScene("ui_test_scene") - # Get UI collection - ui = scene.get_ui() - assert ui is not None, "get_ui() should return a collection" + # Get UI collection via children property + ui = scene.children + assert ui is not None, "children should return a collection" # Add some elements ui.append(mcrfpy.Frame(pos=(10, 20), size=(100, 100))) ui.append(mcrfpy.Caption(text="Test", pos=(50, 50))) # Verify length - assert len(ui) == 2, f"UI should have 2 elements, got {len(ui)}" + assert len(scene.children) == 2, f"children should have 2 elements, got {len(scene.children)}" - print(" - Scene get_ui method: PASS") + print(" - Scene children property: PASS") # Run all tests if __name__ == "__main__": @@ -163,7 +163,7 @@ if __name__ == "__main__": test_scene_opacity() test_scene_name() test_scene_active() - test_scene_get_ui() + test_scene_children() print("\n=== All Scene property tests passed! ===") sys.exit(0)