From 8699bba9e6f810d63045d4a50e12d285e052ed33 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Mon, 12 Jan 2026 07:02:54 -0500 Subject: [PATCH 1/2] BSP: add Binary Space Partitioning for procedural dungeon generation Implements #202, #203, #204, #205; partially implements #206: - BSP class: core tree structure with bounds, split_once, split_recursive, clear - BSPNode class: lightweight node reference with bounds, level, is_leaf, split_horizontal, split_position; navigation via left/right/parent/sibling; contains() and center() methods - Traversal enum: PRE_ORDER, IN_ORDER, POST_ORDER, LEVEL_ORDER, INVERTED_LEVEL_ORDER - BSP iteration: leaves() for leaf nodes only, traverse(order) for all nodes - BSP query: find(pos) returns deepest node containing position - BSP.to_heightmap(): converts BSP to HeightMap with select, shrink, value options Note: #206's BSPMap subclass deferred - to_heightmap returns plain HeightMap. The HeightMap already has all necessary operations (inverse, threshold, etc.) for procedural generation workflows. Co-Authored-By: Claude Opus 4.5 --- src/McRFPy_API.cpp | 19 + src/PyBSP.cpp | 1047 +++++++++++++++++++++++++++++++++ src/PyBSP.h | 201 +++++++ tests/unit/bsp_simple_test.py | 28 + tests/unit/bsp_test.py | 408 +++++++++++++ 5 files changed, 1703 insertions(+) create mode 100644 src/PyBSP.cpp create mode 100644 src/PyBSP.h create mode 100644 tests/unit/bsp_simple_test.py create mode 100644 tests/unit/bsp_test.py diff --git a/src/McRFPy_API.cpp b/src/McRFPy_API.cpp index 8fd857b..2761ef2 100644 --- a/src/McRFPy_API.cpp +++ b/src/McRFPy_API.cpp @@ -22,6 +22,7 @@ #include "PyMouse.h" #include "UIGridPathfinding.h" // AStarPath and DijkstraMap types #include "PyHeightMap.h" // Procedural generation heightmap (#193) +#include "PyBSP.h" // Procedural generation BSP (#202-206) #include "McRogueFaceVersion.h" #include "GameEngine.h" #include "ImGuiConsole.h" @@ -418,6 +419,7 @@ PyObject* PyInit_mcrfpy() /*procedural generation (#192)*/ &mcrfpydef::PyHeightMapType, + &mcrfpydef::PyBSPType, nullptr}; @@ -434,6 +436,10 @@ PyObject* PyInit_mcrfpy() /*pathfinding iterator - returned by AStarPath.__iter__() but not directly instantiable*/ &mcrfpydef::PyAStarPathIterType, + /*BSP internal types - returned by BSP methods but not directly instantiable*/ + &mcrfpydef::PyBSPNodeType, + &mcrfpydef::PyBSPIterType, + nullptr}; // Set up PyWindowType methods and getsetters before PyType_Ready @@ -448,6 +454,12 @@ PyObject* PyInit_mcrfpy() mcrfpydef::PyHeightMapType.tp_methods = PyHeightMap::methods; mcrfpydef::PyHeightMapType.tp_getset = PyHeightMap::getsetters; + // Set up PyBSPType and BSPNode methods and getsetters (#202-206) + mcrfpydef::PyBSPType.tp_methods = PyBSP::methods; + mcrfpydef::PyBSPType.tp_getset = PyBSP::getsetters; + mcrfpydef::PyBSPNodeType.tp_methods = PyBSPNode::methods; + mcrfpydef::PyBSPNodeType.tp_getset = PyBSPNode::getsetters; + // Set up weakref support for all types that need it PyTimerType.tp_weaklistoffset = offsetof(PyTimerObject, weakreflist); PyUIFrameType.tp_weaklistoffset = offsetof(PyUIFrameObject, weakreflist); @@ -555,6 +567,13 @@ PyObject* PyInit_mcrfpy() PyErr_Clear(); } + // Add Traversal enum class for BSP traversal (uses Python's IntEnum) + PyObject* traversal_class = PyTraversal::create_enum_class(m); + if (!traversal_class) { + // If enum creation fails, continue without it (non-fatal) + PyErr_Clear(); + } + // Add Key enum class for keyboard input PyObject* key_class = PyKey::create_enum_class(m); if (!key_class) { diff --git a/src/PyBSP.cpp b/src/PyBSP.cpp new file mode 100644 index 0000000..67f8c0d --- /dev/null +++ b/src/PyBSP.cpp @@ -0,0 +1,1047 @@ +#include "PyBSP.h" +#include "McRFPy_API.h" +#include "McRFPy_Doc.h" +#include "PyPositionHelper.h" +#include "PyHeightMap.h" +#include +#include +#include + +// Static storage for Traversal enum +PyObject* PyTraversal::traversal_enum_class = nullptr; + +// Traversal order constants +enum TraversalOrder { + TRAVERSAL_PRE_ORDER = 0, + TRAVERSAL_IN_ORDER = 1, + TRAVERSAL_POST_ORDER = 2, + TRAVERSAL_LEVEL_ORDER = 3, + TRAVERSAL_INVERTED_LEVEL_ORDER = 4, +}; + +// ==================== Traversal Enum ==================== + +PyObject* PyTraversal::create_enum_class(PyObject* module) { + // Import IntEnum from enum module + PyObject* enum_module = PyImport_ImportModule("enum"); + if (!enum_module) { + return NULL; + } + + PyObject* int_enum = PyObject_GetAttrString(enum_module, "IntEnum"); + Py_DECREF(enum_module); + if (!int_enum) { + return NULL; + } + + // Create dict of enum members + PyObject* members = PyDict_New(); + if (!members) { + Py_DECREF(int_enum); + return NULL; + } + + // Add traversal order members + struct { const char* name; int value; } entries[] = { + {"PRE_ORDER", TRAVERSAL_PRE_ORDER}, + {"IN_ORDER", TRAVERSAL_IN_ORDER}, + {"POST_ORDER", TRAVERSAL_POST_ORDER}, + {"LEVEL_ORDER", TRAVERSAL_LEVEL_ORDER}, + {"INVERTED_LEVEL_ORDER", TRAVERSAL_INVERTED_LEVEL_ORDER}, + }; + + for (const auto& entry : entries) { + PyObject* value = PyLong_FromLong(entry.value); + if (!value || PyDict_SetItemString(members, entry.name, value) < 0) { + Py_XDECREF(value); + Py_DECREF(members); + Py_DECREF(int_enum); + return NULL; + } + Py_DECREF(value); + } + + // Call IntEnum("Traversal", members) + PyObject* name = PyUnicode_FromString("Traversal"); + if (!name) { + Py_DECREF(members); + Py_DECREF(int_enum); + return NULL; + } + + PyObject* args = PyTuple_Pack(2, name, members); + Py_DECREF(name); + Py_DECREF(members); + if (!args) { + Py_DECREF(int_enum); + return NULL; + } + + PyObject* traversal_class = PyObject_Call(int_enum, args, NULL); + Py_DECREF(args); + Py_DECREF(int_enum); + + if (!traversal_class) { + return NULL; + } + + // Cache reference + traversal_enum_class = traversal_class; + Py_INCREF(traversal_enum_class); + + // Add to module + if (PyModule_AddObject(module, "Traversal", traversal_class) < 0) { + Py_DECREF(traversal_class); + traversal_enum_class = nullptr; + return NULL; + } + + return traversal_class; +} + +int PyTraversal::from_arg(PyObject* arg, int* out_order) { + // Accept None -> default to LEVEL_ORDER + if (arg == NULL || arg == Py_None) { + *out_order = TRAVERSAL_LEVEL_ORDER; + return 1; + } + + // Accept Traversal enum member + if (traversal_enum_class && PyObject_IsInstance(arg, traversal_enum_class)) { + PyObject* value = PyObject_GetAttrString(arg, "value"); + if (!value) { + return 0; + } + long val = PyLong_AsLong(value); + Py_DECREF(value); + if (val == -1 && PyErr_Occurred()) { + return 0; + } + if (val >= 0 && val <= TRAVERSAL_INVERTED_LEVEL_ORDER) { + *out_order = (int)val; + return 1; + } + PyErr_Format(PyExc_ValueError, "Invalid Traversal value: %ld", val); + return 0; + } + + // Accept int + if (PyLong_Check(arg)) { + long val = PyLong_AsLong(arg); + if (val == -1 && PyErr_Occurred()) { + return 0; + } + if (val >= 0 && val <= TRAVERSAL_INVERTED_LEVEL_ORDER) { + *out_order = (int)val; + return 1; + } + PyErr_Format(PyExc_ValueError, + "Invalid traversal value: %ld. Must be 0-4 or use mcrfpy.Traversal enum.", val); + return 0; + } + + // Accept string + if (PyUnicode_Check(arg)) { + const char* name = PyUnicode_AsUTF8(arg); + if (!name) { + return 0; + } + if (strcmp(name, "pre") == 0 || strcmp(name, "PRE_ORDER") == 0) { + *out_order = TRAVERSAL_PRE_ORDER; + return 1; + } + if (strcmp(name, "in") == 0 || strcmp(name, "IN_ORDER") == 0) { + *out_order = TRAVERSAL_IN_ORDER; + return 1; + } + if (strcmp(name, "post") == 0 || strcmp(name, "POST_ORDER") == 0) { + *out_order = TRAVERSAL_POST_ORDER; + return 1; + } + if (strcmp(name, "level") == 0 || strcmp(name, "LEVEL_ORDER") == 0) { + *out_order = TRAVERSAL_LEVEL_ORDER; + return 1; + } + if (strcmp(name, "level_inverted") == 0 || strcmp(name, "INVERTED_LEVEL_ORDER") == 0) { + *out_order = TRAVERSAL_INVERTED_LEVEL_ORDER; + return 1; + } + PyErr_Format(PyExc_ValueError, + "Unknown traversal order: '%s'. Use mcrfpy.Traversal enum or: " + "'pre', 'in', 'post', 'level', 'level_inverted'", name); + return 0; + } + + PyErr_SetString(PyExc_TypeError, + "Traversal order must be mcrfpy.Traversal enum, string, int, or None"); + return 0; +} + +// ==================== BSP Property Definitions ==================== + +PyGetSetDef PyBSP::getsetters[] = { + {"bounds", (getter)PyBSP::get_bounds, NULL, + MCRF_PROPERTY(bounds, "Root node bounds as ((x, y), (w, h)). Read-only."), NULL}, + {"root", (getter)PyBSP::get_root, NULL, + MCRF_PROPERTY(root, "Reference to the root BSPNode. Read-only."), NULL}, + {NULL} +}; + +// ==================== BSP Method Definitions ==================== + +PyMethodDef PyBSP::methods[] = { + {"split_once", (PyCFunction)PyBSP::split_once, METH_VARARGS | METH_KEYWORDS, + MCRF_METHOD(BSP, split_once, + MCRF_SIG("(horizontal: bool, position: int)", "BSP"), + MCRF_DESC("Split the root node once at the specified position."), + MCRF_ARGS_START + MCRF_ARG("horizontal", "True for horizontal split, False for vertical") + MCRF_ARG("position", "Split coordinate (y for horizontal, x for vertical)") + MCRF_RETURNS("BSP: self, for method chaining") + )}, + {"split_recursive", (PyCFunction)PyBSP::split_recursive, METH_VARARGS | METH_KEYWORDS, + MCRF_METHOD(BSP, split_recursive, + MCRF_SIG("(depth: int, min_size: tuple[int, int], max_ratio: float = 1.5, seed: int = None)", "BSP"), + MCRF_DESC("Recursively split to the specified depth."), + MCRF_ARGS_START + MCRF_ARG("depth", "Maximum recursion depth. Creates up to 2^depth leaves.") + MCRF_ARG("min_size", "Minimum (width, height) for a node to be split.") + MCRF_ARG("max_ratio", "Maximum aspect ratio before forcing split direction. Default: 1.5.") + MCRF_ARG("seed", "Random seed. None for random.") + MCRF_RETURNS("BSP: self, for method chaining") + )}, + {"clear", (PyCFunction)PyBSP::clear, METH_NOARGS, + MCRF_METHOD(BSP, clear, + MCRF_SIG("()", "BSP"), + MCRF_DESC("Remove all children, keeping only the root node with original bounds."), + MCRF_RETURNS("BSP: self, for method chaining") + )}, + {"leaves", (PyCFunction)PyBSP::leaves, METH_NOARGS, + MCRF_METHOD(BSP, leaves, + MCRF_SIG("()", "Iterator[BSPNode]"), + MCRF_DESC("Iterate all leaf nodes (the actual rooms)."), + MCRF_RETURNS("Iterator yielding BSPNode objects") + )}, + {"traverse", (PyCFunction)PyBSP::traverse, METH_VARARGS | METH_KEYWORDS, + MCRF_METHOD(BSP, traverse, + MCRF_SIG("(order: Traversal = Traversal.LEVEL_ORDER)", "Iterator[BSPNode]"), + MCRF_DESC("Iterate all nodes in the specified order."), + MCRF_ARGS_START + MCRF_ARG("order", "Traversal order from Traversal enum. Default: LEVEL_ORDER.") + MCRF_RETURNS("Iterator yielding BSPNode objects") + MCRF_NOTE("Orders: PRE_ORDER, IN_ORDER, POST_ORDER, LEVEL_ORDER, INVERTED_LEVEL_ORDER") + )}, + {"find", (PyCFunction)PyBSP::find, METH_VARARGS | METH_KEYWORDS, + MCRF_METHOD(BSP, find, + MCRF_SIG("(pos: tuple[int, int])", "BSPNode | None"), + MCRF_DESC("Find the smallest (deepest) node containing the position."), + MCRF_ARGS_START + MCRF_ARG("pos", "Position as (x, y) tuple, list, or Vector") + MCRF_RETURNS("BSPNode if found, None if position is outside bounds") + )}, + {"to_heightmap", (PyCFunction)PyBSP::to_heightmap, METH_VARARGS | METH_KEYWORDS, + MCRF_METHOD(BSP, to_heightmap, + MCRF_SIG("(size: tuple[int, int] = None, select: str = 'leaves', shrink: int = 0, value: float = 1.0)", "HeightMap"), + MCRF_DESC("Convert BSP node selection to a HeightMap."), + MCRF_ARGS_START + MCRF_ARG("size", "Output size (width, height). Default: bounds size.") + MCRF_ARG("select", "'leaves', 'all', or 'internal'. Default: 'leaves'.") + MCRF_ARG("shrink", "Pixels to shrink from each node's bounds. Default: 0.") + MCRF_ARG("value", "Value inside selected regions. Default: 1.0.") + MCRF_RETURNS("HeightMap with selected regions filled") + )}, + {NULL} +}; + +// ==================== BSP Implementation ==================== + +PyObject* PyBSP::pynew(PyTypeObject* type, PyObject* args, PyObject* kwds) +{ + PyBSPObject* self = (PyBSPObject*)type->tp_alloc(type, 0); + if (self) { + self->root = nullptr; + self->orig_x = 0; + self->orig_y = 0; + self->orig_w = 0; + self->orig_h = 0; + } + return (PyObject*)self; +} + +int PyBSP::init(PyBSPObject* self, PyObject* args, PyObject* kwds) +{ + static const char* keywords[] = {"bounds", nullptr}; + PyObject* bounds_obj = nullptr; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "O", const_cast(keywords), + &bounds_obj)) { + return -1; + } + + // Parse bounds: ((x, y), (w, h)) + if (!PyTuple_Check(bounds_obj) || PyTuple_Size(bounds_obj) != 2) { + PyErr_SetString(PyExc_TypeError, "bounds must be ((x, y), (w, h)) tuple"); + return -1; + } + + PyObject* pos_obj = PyTuple_GetItem(bounds_obj, 0); + PyObject* size_obj = PyTuple_GetItem(bounds_obj, 1); + + if (!PyTuple_Check(pos_obj) || PyTuple_Size(pos_obj) != 2 || + !PyTuple_Check(size_obj) || PyTuple_Size(size_obj) != 2) { + PyErr_SetString(PyExc_TypeError, "bounds must be ((x, y), (w, h)) tuple"); + return -1; + } + + int x = (int)PyLong_AsLong(PyTuple_GetItem(pos_obj, 0)); + int y = (int)PyLong_AsLong(PyTuple_GetItem(pos_obj, 1)); + int w = (int)PyLong_AsLong(PyTuple_GetItem(size_obj, 0)); + int h = (int)PyLong_AsLong(PyTuple_GetItem(size_obj, 1)); + + if (PyErr_Occurred()) { + return -1; + } + + if (w <= 0 || h <= 0) { + PyErr_SetString(PyExc_ValueError, "width and height must be positive"); + return -1; + } + + // Clean up any existing BSP + if (self->root) { + TCOD_bsp_delete(self->root); + } + + // Create new BSP with size + self->root = TCOD_bsp_new_with_size(x, y, w, h); + if (!self->root) { + PyErr_SetString(PyExc_MemoryError, "Failed to allocate BSP"); + return -1; + } + + // Store original bounds for clear() + self->orig_x = x; + self->orig_y = y; + self->orig_w = w; + self->orig_h = h; + + return 0; +} + +void PyBSP::dealloc(PyBSPObject* self) +{ + if (self->root) { + TCOD_bsp_delete(self->root); + self->root = nullptr; + } + Py_TYPE(self)->tp_free((PyObject*)self); +} + +PyObject* PyBSP::repr(PyObject* obj) +{ + PyBSPObject* self = (PyBSPObject*)obj; + std::ostringstream ss; + + if (self->root) { + // Count leaves + int leaf_count = 0; + TCOD_bsp_traverse_pre_order(self->root, [](TCOD_bsp_t* node, void* data) -> bool { + if (TCOD_bsp_is_leaf(node)) { + (*(int*)data)++; + } + return true; + }, &leaf_count); + + ss << "root->w << " x " << self->root->h << "), " + << leaf_count << " leaves>"; + } else { + ss << ""; + } + + return PyUnicode_FromString(ss.str().c_str()); +} + +// Property: bounds +PyObject* PyBSP::get_bounds(PyBSPObject* self, void* closure) +{ + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + return Py_BuildValue("((ii)(ii))", + self->root->x, self->root->y, + self->root->w, self->root->h); +} + +// Property: root +PyObject* PyBSP::get_root(PyBSPObject* self, void* closure) +{ + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + return PyBSPNode::create(self->root, (PyObject*)self); +} + +// Method: split_once(horizontal, position) -> BSP +PyObject* PyBSP::split_once(PyBSPObject* self, PyObject* args, PyObject* kwds) +{ + static const char* keywords[] = {"horizontal", "position", nullptr}; + int horizontal; + int position; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "pi", const_cast(keywords), + &horizontal, &position)) { + return nullptr; + } + + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + + TCOD_bsp_split_once(self->root, horizontal ? true : false, position); + + Py_INCREF(self); + return (PyObject*)self; +} + +// Method: split_recursive(depth, min_size, max_ratio=1.5, seed=None) -> BSP +PyObject* PyBSP::split_recursive(PyBSPObject* self, PyObject* args, PyObject* kwds) +{ + static const char* keywords[] = {"depth", "min_size", "max_ratio", "seed", nullptr}; + int depth; + PyObject* min_size_obj = nullptr; + float max_ratio = 1.5f; + PyObject* seed_obj = nullptr; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "iO|fO", const_cast(keywords), + &depth, &min_size_obj, &max_ratio, &seed_obj)) { + return nullptr; + } + + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + + // Parse min_size tuple + if (!PyTuple_Check(min_size_obj) || PyTuple_Size(min_size_obj) != 2) { + PyErr_SetString(PyExc_TypeError, "min_size must be (width, height) tuple"); + return nullptr; + } + + int min_w = (int)PyLong_AsLong(PyTuple_GetItem(min_size_obj, 0)); + int min_h = (int)PyLong_AsLong(PyTuple_GetItem(min_size_obj, 1)); + + if (PyErr_Occurred()) { + return nullptr; + } + + if (depth < 0) { + PyErr_SetString(PyExc_ValueError, "depth must be non-negative"); + return nullptr; + } + + if (min_w <= 0 || min_h <= 0) { + PyErr_SetString(PyExc_ValueError, "min_size values must be positive"); + return nullptr; + } + + // Create random generator if seed provided + TCOD_Random* rnd = nullptr; + if (seed_obj != nullptr && seed_obj != Py_None) { + if (!PyLong_Check(seed_obj)) { + PyErr_SetString(PyExc_TypeError, "seed must be an integer or None"); + return nullptr; + } + uint32_t seed = (uint32_t)PyLong_AsUnsignedLong(seed_obj); + if (PyErr_Occurred()) { + return nullptr; + } + rnd = TCOD_random_new_from_seed(TCOD_RNG_MT, seed); + } + + TCOD_bsp_split_recursive(self->root, rnd, depth, min_w, min_h, max_ratio, max_ratio); + + if (rnd) { + TCOD_random_delete(rnd); + } + + Py_INCREF(self); + return (PyObject*)self; +} + +// Method: clear() -> BSP +PyObject* PyBSP::clear(PyBSPObject* self, PyObject* Py_UNUSED(args)) +{ + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + + TCOD_bsp_remove_sons(self->root); + + // Restore original bounds + TCOD_bsp_resize(self->root, self->orig_x, self->orig_y, self->orig_w, self->orig_h); + + Py_INCREF(self); + return (PyObject*)self; +} + +// ==================== BSP Iteration ==================== + +// Traversal callback to collect nodes +struct TraversalData { + std::vector* nodes; + bool leaves_only; +}; + +static bool collect_callback(TCOD_bsp_t* node, void* userData) { + TraversalData* data = (TraversalData*)userData; + if (!data->leaves_only || TCOD_bsp_is_leaf(node)) { + data->nodes->push_back(node); + } + return true; // Continue traversal +} + +// Method: leaves() -> Iterator[BSPNode] +PyObject* PyBSP::leaves(PyBSPObject* self, PyObject* Py_UNUSED(args)) +{ + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + + // Create iterator object + PyBSPIterObject* iter = (PyBSPIterObject*)mcrfpydef::PyBSPIterType.tp_alloc( + &mcrfpydef::PyBSPIterType, 0); + if (!iter) { + return nullptr; + } + + // Collect all leaf nodes + iter->nodes = new std::vector(); + TraversalData data = {iter->nodes, true}; // leaves_only = true + TCOD_bsp_traverse_level_order(self->root, collect_callback, &data); + + iter->index = 0; + iter->bsp_owner = (PyObject*)self; + Py_INCREF(self); + + return (PyObject*)iter; +} + +// Method: traverse(order) -> Iterator[BSPNode] +PyObject* PyBSP::traverse(PyBSPObject* self, PyObject* args, PyObject* kwds) +{ + static const char* keywords[] = {"order", nullptr}; + PyObject* order_obj = nullptr; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "|O", const_cast(keywords), + &order_obj)) { + return nullptr; + } + + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + + int order; + if (!PyTraversal::from_arg(order_obj, &order)) { + return nullptr; + } + + // Create iterator object + PyBSPIterObject* iter = (PyBSPIterObject*)mcrfpydef::PyBSPIterType.tp_alloc( + &mcrfpydef::PyBSPIterType, 0); + if (!iter) { + return nullptr; + } + + // Collect all nodes in the specified order + iter->nodes = new std::vector(); + TraversalData data = {iter->nodes, false}; // leaves_only = false + + switch (order) { + case TRAVERSAL_PRE_ORDER: + TCOD_bsp_traverse_pre_order(self->root, collect_callback, &data); + break; + case TRAVERSAL_IN_ORDER: + TCOD_bsp_traverse_in_order(self->root, collect_callback, &data); + break; + case TRAVERSAL_POST_ORDER: + TCOD_bsp_traverse_post_order(self->root, collect_callback, &data); + break; + case TRAVERSAL_LEVEL_ORDER: + TCOD_bsp_traverse_level_order(self->root, collect_callback, &data); + break; + case TRAVERSAL_INVERTED_LEVEL_ORDER: + TCOD_bsp_traverse_inverted_level_order(self->root, collect_callback, &data); + break; + } + + iter->index = 0; + iter->bsp_owner = (PyObject*)self; + Py_INCREF(self); + + return (PyObject*)iter; +} + +// Method: find(pos) -> BSPNode | None +PyObject* PyBSP::find(PyBSPObject* self, PyObject* args, PyObject* kwds) +{ + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + + int x, y; + if (!PyPosition_ParseInt(args, kwds, &x, &y)) { + return nullptr; + } + + TCOD_bsp_t* found = TCOD_bsp_find_node(self->root, x, y); + if (!found) { + Py_RETURN_NONE; + } + + return PyBSPNode::create(found, (PyObject*)self); +} + +// Method: to_heightmap(...) -> HeightMap +PyObject* PyBSP::to_heightmap(PyBSPObject* self, PyObject* args, PyObject* kwds) +{ + static const char* keywords[] = {"size", "select", "shrink", "value", nullptr}; + PyObject* size_obj = nullptr; + const char* select = "leaves"; + int shrink = 0; + float value = 1.0f; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "|Osif", const_cast(keywords), + &size_obj, &select, &shrink, &value)) { + return nullptr; + } + + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + + // Determine output size + int width = self->root->w; + int height = self->root->h; + if (size_obj != nullptr && size_obj != Py_None) { + if (!PyTuple_Check(size_obj) || PyTuple_Size(size_obj) != 2) { + PyErr_SetString(PyExc_TypeError, "size must be (width, height) tuple"); + return nullptr; + } + width = (int)PyLong_AsLong(PyTuple_GetItem(size_obj, 0)); + height = (int)PyLong_AsLong(PyTuple_GetItem(size_obj, 1)); + if (PyErr_Occurred()) { + return nullptr; + } + } + + if (width <= 0 || height <= 0) { + PyErr_SetString(PyExc_ValueError, "size values must be positive"); + return nullptr; + } + + if (shrink < 0) { + PyErr_SetString(PyExc_ValueError, "shrink must be non-negative"); + return nullptr; + } + + // Determine which nodes to select + bool select_leaves = false; + bool select_internal = false; + if (strcmp(select, "leaves") == 0) { + select_leaves = true; + } else if (strcmp(select, "all") == 0) { + select_leaves = true; + select_internal = true; + } else if (strcmp(select, "internal") == 0) { + select_internal = true; + } else { + PyErr_Format(PyExc_ValueError, + "select must be 'leaves', 'all', or 'internal', got '%s'", select); + return nullptr; + } + + // Create HeightMap + PyObject* heightmap_type = PyObject_GetAttrString(McRFPy_API::mcrf_module, "HeightMap"); + if (!heightmap_type) { + PyErr_SetString(PyExc_RuntimeError, "HeightMap type not found"); + return nullptr; + } + + PyObject* size_tuple = Py_BuildValue("(ii)", width, height); + PyObject* hmap_args = PyTuple_Pack(1, size_tuple); + Py_DECREF(size_tuple); + + PyHeightMapObject* hmap = (PyHeightMapObject*)PyObject_Call(heightmap_type, hmap_args, nullptr); + Py_DECREF(hmap_args); + Py_DECREF(heightmap_type); + + if (!hmap) { + return nullptr; + } + + // Fill selected nodes + struct FillData { + TCOD_heightmap_t* heightmap; + int shrink; + float value; + bool select_leaves; + bool select_internal; + int bsp_x, bsp_y; // BSP origin for offset calculation + int hmap_w, hmap_h; + }; + + FillData fill_data = { + hmap->heightmap, + shrink, + value, + select_leaves, + select_internal, + self->root->x, + self->root->y, + width, + height + }; + + TCOD_bsp_traverse_level_order(self->root, [](TCOD_bsp_t* node, void* userData) -> bool { + FillData* data = (FillData*)userData; + bool is_leaf = TCOD_bsp_is_leaf(node); + + // Check if this node should be selected + if ((is_leaf && !data->select_leaves) || (!is_leaf && !data->select_internal)) { + return true; // Continue but don't fill + } + + // Calculate bounds with shrink + int x1 = node->x - data->bsp_x + data->shrink; + int y1 = node->y - data->bsp_y + data->shrink; + int x2 = node->x - data->bsp_x + node->w - data->shrink; + int y2 = node->y - data->bsp_y + node->h - data->shrink; + + // Clamp to heightmap bounds + if (x1 < 0) x1 = 0; + if (y1 < 0) y1 = 0; + if (x2 > data->hmap_w) x2 = data->hmap_w; + if (y2 > data->hmap_h) y2 = data->hmap_h; + + // Fill the region + for (int y = y1; y < y2; y++) { + for (int x = x1; x < x2; x++) { + TCOD_heightmap_set_value(data->heightmap, x, y, data->value); + } + } + + return true; + }, &fill_data); + + return (PyObject*)hmap; +} + +// ==================== BSPNode Property Definitions ==================== + +PyGetSetDef PyBSPNode::getsetters[] = { + {"bounds", (getter)PyBSPNode::get_bounds, NULL, + MCRF_PROPERTY(bounds, "Node bounds as ((x, y), (w, h)). Read-only."), NULL}, + {"level", (getter)PyBSPNode::get_level, NULL, + MCRF_PROPERTY(level, "Depth in tree (0 for root). Read-only."), NULL}, + {"is_leaf", (getter)PyBSPNode::get_is_leaf, NULL, + MCRF_PROPERTY(is_leaf, "True if this node has no children. Read-only."), NULL}, + {"split_horizontal", (getter)PyBSPNode::get_split_horizontal, NULL, + MCRF_PROPERTY(split_horizontal, "Split orientation, None if leaf. Read-only."), NULL}, + {"split_position", (getter)PyBSPNode::get_split_position, NULL, + MCRF_PROPERTY(split_position, "Split coordinate, None if leaf. Read-only."), NULL}, + {"left", (getter)PyBSPNode::get_left, NULL, + MCRF_PROPERTY(left, "Left child, or None if leaf. Read-only."), NULL}, + {"right", (getter)PyBSPNode::get_right, NULL, + MCRF_PROPERTY(right, "Right child, or None if leaf. Read-only."), NULL}, + {"parent", (getter)PyBSPNode::get_parent, NULL, + MCRF_PROPERTY(parent, "Parent node, or None if root. Read-only."), NULL}, + {"sibling", (getter)PyBSPNode::get_sibling, NULL, + MCRF_PROPERTY(sibling, "Other child of parent, or None. Read-only."), NULL}, + {NULL} +}; + +// ==================== BSPNode Method Definitions ==================== + +PyMethodDef PyBSPNode::methods[] = { + {"contains", (PyCFunction)PyBSPNode::contains, METH_VARARGS | METH_KEYWORDS, + MCRF_METHOD(BSPNode, contains, + MCRF_SIG("(pos: tuple[int, int])", "bool"), + MCRF_DESC("Check if position is inside this node's bounds."), + MCRF_ARGS_START + MCRF_ARG("pos", "Position as (x, y) tuple, list, or Vector") + MCRF_RETURNS("bool: True if position is inside bounds") + )}, + {"center", (PyCFunction)PyBSPNode::center, METH_NOARGS, + MCRF_METHOD(BSPNode, center, + MCRF_SIG("()", "tuple[int, int]"), + MCRF_DESC("Return the center point of this node's bounds."), + MCRF_RETURNS("tuple[int, int]: Center position (x + w//2, y + h//2)") + )}, + {NULL} +}; + +// ==================== BSPNode Implementation ==================== + +PyObject* PyBSPNode::pynew(PyTypeObject* type, PyObject* args, PyObject* kwds) +{ + // BSPNode cannot be directly instantiated + PyErr_SetString(PyExc_TypeError, + "BSPNode cannot be instantiated directly. Use BSP methods to get nodes."); + return NULL; +} + +int PyBSPNode::init(PyBSPNodeObject* self, PyObject* args, PyObject* kwds) +{ + // This should never be called since pynew returns NULL + PyErr_SetString(PyExc_TypeError, "BSPNode cannot be instantiated directly"); + return -1; +} + +void PyBSPNode::dealloc(PyBSPNodeObject* self) +{ + // Don't delete the node - it's owned by the BSP tree + Py_XDECREF(self->bsp_owner); + Py_TYPE(self)->tp_free((PyObject*)self); +} + +PyObject* PyBSPNode::repr(PyObject* obj) +{ + PyBSPNodeObject* self = (PyBSPNodeObject*)obj; + std::ostringstream ss; + + if (self->node) { + const char* type = TCOD_bsp_is_leaf(self->node) ? "leaf" : "split"; + ss << "node->x << ", " << self->node->y + << ") size (" << self->node->w << " x " << self->node->h << ") level " + << (int)self->node->level << ">"; + } else { + ss << ""; + } + + return PyUnicode_FromString(ss.str().c_str()); +} + +// Helper to create a BSPNode wrapper +PyObject* PyBSPNode::create(TCOD_bsp_t* node, PyObject* bsp_owner) +{ + if (!node) { + Py_RETURN_NONE; + } + + PyBSPNodeObject* py_node = (PyBSPNodeObject*)mcrfpydef::PyBSPNodeType.tp_alloc( + &mcrfpydef::PyBSPNodeType, 0); + if (!py_node) { + return nullptr; + } + + py_node->node = node; + py_node->bsp_owner = bsp_owner; + Py_INCREF(bsp_owner); + + return (PyObject*)py_node; +} + +// Property: bounds +PyObject* PyBSPNode::get_bounds(PyBSPNodeObject* self, void* closure) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + return Py_BuildValue("((ii)(ii))", + self->node->x, self->node->y, + self->node->w, self->node->h); +} + +// Property: level +PyObject* PyBSPNode::get_level(PyBSPNodeObject* self, void* closure) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + return PyLong_FromLong(self->node->level); +} + +// Property: is_leaf +PyObject* PyBSPNode::get_is_leaf(PyBSPNodeObject* self, void* closure) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + return PyBool_FromLong(TCOD_bsp_is_leaf(self->node)); +} + +// Property: split_horizontal +PyObject* PyBSPNode::get_split_horizontal(PyBSPNodeObject* self, void* closure) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + if (TCOD_bsp_is_leaf(self->node)) { + Py_RETURN_NONE; + } + return PyBool_FromLong(self->node->horizontal); +} + +// Property: split_position +PyObject* PyBSPNode::get_split_position(PyBSPNodeObject* self, void* closure) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + if (TCOD_bsp_is_leaf(self->node)) { + Py_RETURN_NONE; + } + return PyLong_FromLong(self->node->position); +} + +// Property: left +PyObject* PyBSPNode::get_left(PyBSPNodeObject* self, void* closure) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + return PyBSPNode::create(TCOD_bsp_left(self->node), self->bsp_owner); +} + +// Property: right +PyObject* PyBSPNode::get_right(PyBSPNodeObject* self, void* closure) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + return PyBSPNode::create(TCOD_bsp_right(self->node), self->bsp_owner); +} + +// Property: parent +PyObject* PyBSPNode::get_parent(PyBSPNodeObject* self, void* closure) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + return PyBSPNode::create(TCOD_bsp_father(self->node), self->bsp_owner); +} + +// Property: sibling +PyObject* PyBSPNode::get_sibling(PyBSPNodeObject* self, void* closure) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + + TCOD_bsp_t* parent = TCOD_bsp_father(self->node); + if (!parent) { + Py_RETURN_NONE; + } + + TCOD_bsp_t* left = TCOD_bsp_left(parent); + TCOD_bsp_t* right = TCOD_bsp_right(parent); + + if (left == self->node) { + return PyBSPNode::create(right, self->bsp_owner); + } else { + return PyBSPNode::create(left, self->bsp_owner); + } +} + +// Method: contains(pos) -> bool +PyObject* PyBSPNode::contains(PyBSPNodeObject* self, PyObject* args, PyObject* kwds) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + + int x, y; + if (!PyPosition_ParseInt(args, kwds, &x, &y)) { + return nullptr; + } + + return PyBool_FromLong(TCOD_bsp_contains(self->node, x, y)); +} + +// Method: center() -> (x, y) +PyObject* PyBSPNode::center(PyBSPNodeObject* self, PyObject* Py_UNUSED(args)) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); + return nullptr; + } + + int cx = self->node->x + self->node->w / 2; + int cy = self->node->y + self->node->h / 2; + + return Py_BuildValue("(ii)", cx, cy); +} + +// ==================== BSP Iterator Implementation ==================== + +void PyBSPIter::dealloc(PyBSPIterObject* self) +{ + if (self->nodes) { + delete self->nodes; + self->nodes = nullptr; + } + Py_XDECREF(self->bsp_owner); + Py_TYPE(self)->tp_free((PyObject*)self); +} + +PyObject* PyBSPIter::iter(PyObject* self) +{ + Py_INCREF(self); + return self; +} + +PyObject* PyBSPIter::next(PyBSPIterObject* self) +{ + if (!self || !self->nodes) { + PyErr_SetString(PyExc_RuntimeError, "Iterator is invalid"); + return NULL; + } + + if (self->index >= self->nodes->size()) { + PyErr_SetNone(PyExc_StopIteration); + return NULL; + } + + TCOD_bsp_t* node = (*self->nodes)[self->index++]; + return PyBSPNode::create(node, self->bsp_owner); +} + +int PyBSPIter::init(PyBSPIterObject* self, PyObject* args, PyObject* kwds) +{ + PyErr_SetString(PyExc_TypeError, "BSPIter cannot be instantiated directly"); + return -1; +} + +PyObject* PyBSPIter::repr(PyObject* obj) +{ + PyBSPIterObject* self = (PyBSPIterObject*)obj; + std::ostringstream ss; + + if (self->nodes) { + ss << "index << "/" << self->nodes->size() << ">"; + } else { + ss << ""; + } + + return PyUnicode_FromString(ss.str().c_str()); +} diff --git a/src/PyBSP.h b/src/PyBSP.h new file mode 100644 index 0000000..36a599c --- /dev/null +++ b/src/PyBSP.h @@ -0,0 +1,201 @@ +#pragma once +#include "Common.h" +#include "Python.h" +#include +#include + +// Forward declarations +class PyBSP; +class PyBSPNode; + +// Python object structure for BSP tree (root owner) +typedef struct { + PyObject_HEAD + TCOD_bsp_t* root; // libtcod BSP root (owned, will be deleted) + int orig_x, orig_y; // Original bounds for clear() + int orig_w, orig_h; +} PyBSPObject; + +// Python object structure for BSPNode (lightweight reference) +typedef struct { + PyObject_HEAD + TCOD_bsp_t* node; // libtcod BSP node (NOT owned) + PyObject* bsp_owner; // Reference to PyBSPObject to prevent dangling +} PyBSPNodeObject; + +// BSP iterator for traverse() +typedef struct { + PyObject_HEAD + std::vector* nodes; // Pre-collected nodes + size_t index; + PyObject* bsp_owner; // Reference to PyBSPObject +} PyBSPIterObject; + +class PyBSP +{ +public: + // Python type interface + static PyObject* pynew(PyTypeObject* type, PyObject* args, PyObject* kwds); + static int init(PyBSPObject* self, PyObject* args, PyObject* kwds); + static void dealloc(PyBSPObject* self); + static PyObject* repr(PyObject* obj); + + // Properties + static PyObject* get_bounds(PyBSPObject* self, void* closure); + static PyObject* get_root(PyBSPObject* self, void* closure); + + // Splitting methods (#202) + static PyObject* split_once(PyBSPObject* self, PyObject* args, PyObject* kwds); + static PyObject* split_recursive(PyBSPObject* self, PyObject* args, PyObject* kwds); + static PyObject* clear(PyBSPObject* self, PyObject* Py_UNUSED(args)); + + // Iteration methods (#204) + static PyObject* leaves(PyBSPObject* self, PyObject* Py_UNUSED(args)); + static PyObject* traverse(PyBSPObject* self, PyObject* args, PyObject* kwds); + + // Query methods (#205) + static PyObject* find(PyBSPObject* self, PyObject* args, PyObject* kwds); + + // HeightMap conversion (#206) + static PyObject* to_heightmap(PyBSPObject* self, PyObject* args, PyObject* kwds); + + // Method and property definitions + static PyMethodDef methods[]; + static PyGetSetDef getsetters[]; +}; + +class PyBSPNode +{ +public: + // Python type interface + static PyObject* pynew(PyTypeObject* type, PyObject* args, PyObject* kwds); + static int init(PyBSPNodeObject* self, PyObject* args, PyObject* kwds); + static void dealloc(PyBSPNodeObject* self); + static PyObject* repr(PyObject* obj); + + // Properties (#203) + static PyObject* get_bounds(PyBSPNodeObject* self, void* closure); + static PyObject* get_level(PyBSPNodeObject* self, void* closure); + static PyObject* get_is_leaf(PyBSPNodeObject* self, void* closure); + static PyObject* get_split_horizontal(PyBSPNodeObject* self, void* closure); + static PyObject* get_split_position(PyBSPNodeObject* self, void* closure); + + // Navigation properties (#203) + static PyObject* get_left(PyBSPNodeObject* self, void* closure); + static PyObject* get_right(PyBSPNodeObject* self, void* closure); + static PyObject* get_parent(PyBSPNodeObject* self, void* closure); + static PyObject* get_sibling(PyBSPNodeObject* self, void* closure); + + // Methods (#203) + static PyObject* contains(PyBSPNodeObject* self, PyObject* args, PyObject* kwds); + static PyObject* center(PyBSPNodeObject* self, PyObject* Py_UNUSED(args)); + + // Helper to create a BSPNode from a TCOD_bsp_t* + static PyObject* create(TCOD_bsp_t* node, PyObject* bsp_owner); + + // Method and property definitions + static PyMethodDef methods[]; + static PyGetSetDef getsetters[]; +}; + +// BSP Iterator class +class PyBSPIter +{ +public: + static void dealloc(PyBSPIterObject* self); + static PyObject* iter(PyObject* self); + static PyObject* next(PyBSPIterObject* self); + static int init(PyBSPIterObject* self, PyObject* args, PyObject* kwds); + static PyObject* repr(PyObject* obj); +}; + +// Traversal enum creation +class PyTraversal +{ +public: + static PyObject* traversal_enum_class; + static PyObject* create_enum_class(PyObject* module); + static int from_arg(PyObject* arg, int* out_order); +}; + +namespace mcrfpydef { + // BSP - user-facing, exported + inline PyTypeObject PyBSPType = { + .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, + .tp_name = "mcrfpy.BSP", + .tp_basicsize = sizeof(PyBSPObject), + .tp_itemsize = 0, + .tp_dealloc = (destructor)PyBSP::dealloc, + .tp_repr = PyBSP::repr, + .tp_flags = Py_TPFLAGS_DEFAULT, + .tp_doc = PyDoc_STR( + "BSP(bounds: tuple[tuple[int, int], tuple[int, int]])\n\n" + "Binary Space Partitioning tree for procedural dungeon generation.\n\n" + "BSP recursively divides a rectangular region into smaller sub-regions, " + "creating a tree structure perfect for generating dungeon rooms and corridors.\n\n" + "Args:\n" + " bounds: ((x, y), (w, h)) - Position and size of the root region.\n\n" + "Properties:\n" + " bounds ((x, y), (w, h)): Read-only. Root node bounds.\n" + " root (BSPNode): Read-only. Reference to the root node.\n\n" + "Example:\n" + " bsp = mcrfpy.BSP(bounds=((0, 0), (80, 50)))\n" + " bsp.split_recursive(depth=4, min_size=(8, 8))\n" + " for leaf in bsp.leaves():\n" + " print(f'Room at {leaf.bounds}')\n" + ), + .tp_methods = nullptr, // Set in McRFPy_API.cpp + .tp_getset = nullptr, // Set in McRFPy_API.cpp + .tp_init = (initproc)PyBSP::init, + .tp_new = PyBSP::pynew, + }; + + // BSPNode - internal type (returned by BSP methods) + inline PyTypeObject PyBSPNodeType = { + .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, + .tp_name = "mcrfpy.BSPNode", + .tp_basicsize = sizeof(PyBSPNodeObject), + .tp_itemsize = 0, + .tp_dealloc = (destructor)PyBSPNode::dealloc, + .tp_repr = PyBSPNode::repr, + .tp_flags = Py_TPFLAGS_DEFAULT, + .tp_doc = PyDoc_STR( + "BSPNode - Lightweight reference to a node in a BSP tree.\n\n" + "BSPNode provides read-only access to node properties and navigation.\n" + "Nodes are created by BSP methods, not directly instantiated.\n\n" + "Properties:\n" + " bounds ((x, y), (w, h)): Position and size of this node.\n" + " level (int): Depth in tree (0 for root).\n" + " is_leaf (bool): True if this node has no children.\n" + " split_horizontal (bool | None): Split orientation, None if leaf.\n" + " split_position (int | None): Split coordinate, None if leaf.\n" + " left (BSPNode | None): Left child, or None if leaf.\n" + " right (BSPNode | None): Right child, or None if leaf.\n" + " parent (BSPNode | None): Parent node, or None if root.\n" + " sibling (BSPNode | None): Other child of parent, or None.\n" + ), + .tp_methods = nullptr, // Set in McRFPy_API.cpp + .tp_getset = nullptr, // Set in McRFPy_API.cpp + .tp_init = (initproc)PyBSPNode::init, + .tp_new = PyBSPNode::pynew, + }; + + // BSP Iterator - internal type + inline PyTypeObject PyBSPIterType = { + .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, + .tp_name = "mcrfpy.BSPIter", + .tp_basicsize = sizeof(PyBSPIterObject), + .tp_itemsize = 0, + .tp_dealloc = (destructor)PyBSPIter::dealloc, + .tp_repr = PyBSPIter::repr, + .tp_flags = Py_TPFLAGS_DEFAULT, + .tp_doc = PyDoc_STR("Iterator for BSP tree traversal."), + .tp_iter = PyBSPIter::iter, + .tp_iternext = (iternextfunc)PyBSPIter::next, + .tp_init = (initproc)PyBSPIter::init, + .tp_new = [](PyTypeObject* type, PyObject* args, PyObject* kwds) -> PyObject* { + PyErr_SetString(PyExc_TypeError, "BSPIter cannot be instantiated directly"); + return NULL; + }, + }; +} diff --git a/tests/unit/bsp_simple_test.py b/tests/unit/bsp_simple_test.py new file mode 100644 index 0000000..e96497f --- /dev/null +++ b/tests/unit/bsp_simple_test.py @@ -0,0 +1,28 @@ +"""Simple BSP test to identify crash.""" +import sys +import mcrfpy + +print("Step 1: Import complete") + +print("Step 2: Creating BSP...") +bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) +print("Step 2: BSP created:", bsp) + +print("Step 3: Getting bounds...") +bounds = bsp.bounds +print("Step 3: Bounds:", bounds) + +print("Step 4: Getting root...") +root = bsp.root +print("Step 4: Root:", root) + +print("Step 5: Split once...") +bsp.split_once(horizontal=True, position=40) +print("Step 5: Split complete") + +print("Step 6: Get leaves...") +leaves = list(bsp.leaves()) +print("Step 6: Leaves count:", len(leaves)) + +print("PASS") +sys.exit(0) diff --git a/tests/unit/bsp_test.py b/tests/unit/bsp_test.py new file mode 100644 index 0000000..96bd899 --- /dev/null +++ b/tests/unit/bsp_test.py @@ -0,0 +1,408 @@ +"""Unit tests for BSP (Binary Space Partitioning) procedural generation. + +Tests for issues #202-#206: +- #202: BSP core class with splitting +- #203: BSPNode lightweight node reference +- #204: BSP iteration (leaves, traverse) with Traversal enum +- #205: BSP query methods (find) +- #206: BSP.to_heightmap (returns HeightMap; BSPMap subclass deferred) +""" +import sys +import mcrfpy + +def test_bsp_construction(): + """Test BSP construction with bounds.""" + print("Testing BSP construction...") + + # Basic construction + bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + assert bsp is not None, "BSP should be created" + + # Check bounds property + bounds = bsp.bounds + assert bounds == ((0, 0), (100, 80)), f"Bounds should be ((0, 0), (100, 80)), got {bounds}" + + # Check root property + root = bsp.root + assert root is not None, "Root should not be None" + assert root.bounds == ((0, 0), (100, 80)), f"Root bounds mismatch" + + # Construction with offset + bsp2 = mcrfpy.BSP(bounds=((10, 20), (50, 40))) + assert bsp2.bounds == ((10, 20), (50, 40)), "Offset bounds not preserved" + + print(" BSP construction: PASS") + +def test_bsp_split_once(): + """Test single split operation (#202).""" + print("Testing BSP split_once...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + + # Before split, root should be a leaf + assert bsp.root.is_leaf, "Root should be leaf before split" + + # Horizontal split at y=40 + result = bsp.split_once(horizontal=True, position=40) + assert result is bsp, "split_once should return self for chaining" + + # After split, root should not be a leaf + root = bsp.root + assert not root.is_leaf, "Root should not be leaf after split" + assert root.split_horizontal == True, "Split should be horizontal" + assert root.split_position == 40, "Split position should be 40" + + # Check children exist + left = root.left + right = root.right + assert left is not None, "Left child should exist" + assert right is not None, "Right child should exist" + assert left.is_leaf, "Left child should be leaf" + assert right.is_leaf, "Right child should be leaf" + + print(" BSP split_once: PASS") + +def test_bsp_split_recursive(): + """Test recursive splitting (#202).""" + print("Testing BSP split_recursive...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + + # Recursive split with seed for reproducibility + result = bsp.split_recursive(depth=3, min_size=(8, 8), max_ratio=1.5, seed=42) + assert result is bsp, "split_recursive should return self" + + # Count leaves + leaves = list(bsp.leaves()) + assert len(leaves) > 1, f"Should have multiple leaves, got {len(leaves)}" + assert len(leaves) <= 8, f"Should have at most 2^3=8 leaves, got {len(leaves)}" + + # All leaves should be within bounds + for leaf in leaves: + x, y = leaf.bounds[0] + w, h = leaf.bounds[1] + assert x >= 0 and y >= 0, f"Leaf position out of bounds: {leaf.bounds}" + assert x + w <= 80 and y + h <= 60, f"Leaf extends beyond bounds: {leaf.bounds}" + assert w >= 8 and h >= 8, f"Leaf smaller than min_size: {leaf.bounds}" + + print(" BSP split_recursive: PASS") + +def test_bsp_clear(): + """Test clear operation (#202).""" + print("Testing BSP clear...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + bsp.split_recursive(depth=4, min_size=(8, 8), seed=42) + + # Should have multiple leaves + leaves_before = len(list(bsp.leaves())) + assert leaves_before > 1, "Should have multiple leaves after split" + + # Clear + result = bsp.clear() + assert result is bsp, "clear should return self" + + # Should be back to single leaf + leaves_after = list(bsp.leaves()) + assert len(leaves_after) == 1, f"Should have 1 leaf after clear, got {len(leaves_after)}" + + # Root should be a leaf with original bounds + assert bsp.root.is_leaf, "Root should be leaf after clear" + assert bsp.bounds == ((0, 0), (100, 80)), "Bounds should be restored" + + print(" BSP clear: PASS") + +def test_bspnode_properties(): + """Test BSPNode properties (#203).""" + print("Testing BSPNode properties...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + bsp.split_recursive(depth=3, min_size=(8, 8), seed=42) + + root = bsp.root + + # Root properties + assert root.level == 0, f"Root level should be 0, got {root.level}" + assert root.parent is None, "Root parent should be None" + assert not root.is_leaf, "Split root should not be leaf" + + # Split properties (not leaf) + assert root.split_horizontal is not None, "Split horizontal should be bool for non-leaf" + assert root.split_position is not None, "Split position should be int for non-leaf" + + # Navigate to a leaf + current = root + while not current.is_leaf: + current = current.left + + # Leaf properties + assert current.is_leaf, "Should be a leaf" + assert current.split_horizontal is None, "Leaf split_horizontal should be None" + assert current.split_position is None, "Leaf split_position should be None" + assert current.level > 0, "Leaf level should be > 0" + + # Test center method + bounds = current.bounds + cx, cy = current.center() + expected_cx = bounds[0][0] + bounds[1][0] // 2 + expected_cy = bounds[0][1] + bounds[1][1] // 2 + assert cx == expected_cx, f"Center x mismatch: {cx} != {expected_cx}" + assert cy == expected_cy, f"Center y mismatch: {cy} != {expected_cy}" + + print(" BSPNode properties: PASS") + +def test_bspnode_navigation(): + """Test BSPNode navigation (#203).""" + print("Testing BSPNode navigation...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + bsp.split_once(horizontal=True, position=40) + + root = bsp.root + left = root.left + right = root.right + + # Parent navigation + assert left.parent is not None, "Left parent should exist" + assert left.parent.bounds == root.bounds, "Left parent should be root" + + # Sibling navigation + assert left.sibling is not None, "Left sibling should exist" + assert left.sibling.bounds == right.bounds, "Left sibling should be right" + assert right.sibling is not None, "Right sibling should exist" + assert right.sibling.bounds == left.bounds, "Right sibling should be left" + + # Root has no parent or sibling + assert root.parent is None, "Root parent should be None" + assert root.sibling is None, "Root sibling should be None" + + print(" BSPNode navigation: PASS") + +def test_bspnode_contains(): + """Test BSPNode contains method (#203).""" + print("Testing BSPNode contains...") + + bsp = mcrfpy.BSP(bounds=((10, 20), (50, 40))) # x: 10-60, y: 20-60 + + root = bsp.root + + # Inside + assert root.contains((30, 40)), "Center should be inside" + assert root.contains((10, 20)), "Top-left corner should be inside" + assert root.contains((59, 59)), "Near bottom-right should be inside" + + # Outside + assert not root.contains((5, 40)), "Left of bounds should be outside" + assert not root.contains((65, 40)), "Right of bounds should be outside" + assert not root.contains((30, 15)), "Above bounds should be outside" + assert not root.contains((30, 65)), "Below bounds should be outside" + + print(" BSPNode contains: PASS") + +def test_bsp_leaves_iteration(): + """Test leaves iteration (#204).""" + print("Testing BSP leaves iteration...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + bsp.split_recursive(depth=3, min_size=(8, 8), seed=42) + + # Iterate leaves + leaves = list(bsp.leaves()) + assert len(leaves) > 0, "Should have at least one leaf" + + # All should be leaves + for leaf in leaves: + assert leaf.is_leaf, f"Should be leaf: {leaf}" + + # Can convert to list multiple times + leaves2 = list(bsp.leaves()) + assert len(leaves) == len(leaves2), "Multiple iterations should yield same count" + + print(" BSP leaves iteration: PASS") + +def test_bsp_traverse(): + """Test traverse with different orders (#204).""" + print("Testing BSP traverse...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + bsp.split_recursive(depth=2, min_size=(8, 8), seed=42) + + # Test all traversal orders + orders = [ + mcrfpy.Traversal.PRE_ORDER, + mcrfpy.Traversal.IN_ORDER, + mcrfpy.Traversal.POST_ORDER, + mcrfpy.Traversal.LEVEL_ORDER, + mcrfpy.Traversal.INVERTED_LEVEL_ORDER, + ] + + for order in orders: + nodes = list(bsp.traverse(order=order)) + assert len(nodes) > 0, f"Should have nodes for {order}" + # All traversals should visit same number of nodes + assert len(nodes) == len(list(bsp.traverse())), f"Node count mismatch for {order}" + + # Default should be LEVEL_ORDER + default_nodes = list(bsp.traverse()) + level_nodes = list(bsp.traverse(mcrfpy.Traversal.LEVEL_ORDER)) + assert len(default_nodes) == len(level_nodes), "Default should match LEVEL_ORDER" + + # PRE_ORDER: root first + pre_nodes = list(bsp.traverse(mcrfpy.Traversal.PRE_ORDER)) + assert pre_nodes[0].bounds == bsp.root.bounds, "PRE_ORDER should start with root" + + # POST_ORDER: root last + post_nodes = list(bsp.traverse(mcrfpy.Traversal.POST_ORDER)) + assert post_nodes[-1].bounds == bsp.root.bounds, "POST_ORDER should end with root" + + print(" BSP traverse: PASS") + +def test_bsp_find(): + """Test find method (#205).""" + print("Testing BSP find...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + bsp.split_recursive(depth=3, min_size=(8, 8), seed=42) + + # Find a point inside bounds + node = bsp.find((40, 30)) + assert node is not None, "Should find node for point inside" + assert node.is_leaf, "Found node should be a leaf (deepest)" + assert node.contains((40, 30)), "Found node should contain the point" + + # Find at corner + corner_node = bsp.find((0, 0)) + assert corner_node is not None, "Should find node at corner" + assert corner_node.contains((0, 0)), "Corner node should contain (0,0)" + + # Find outside bounds + outside = bsp.find((100, 100)) + assert outside is None, "Should return None for point outside" + + print(" BSP find: PASS") + +def test_bsp_to_heightmap(): + """Test to_heightmap conversion (#206).""" + print("Testing BSP to_heightmap...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (50, 40))) + bsp.split_recursive(depth=2, min_size=(8, 8), seed=42) + + # Basic conversion + hmap = bsp.to_heightmap() + assert hmap is not None, "Should create heightmap" + assert hmap.size == (50, 40), f"Size should match bounds, got {hmap.size}" + + # Check that leaves are filled + leaves = list(bsp.leaves()) + for leaf in leaves: + lx, ly = leaf.bounds[0] + val = hmap[lx, ly] + assert val == 1.0, f"Leaf interior should be 1.0, got {val}" + + # Test with shrink + hmap_shrink = bsp.to_heightmap(shrink=2) + assert hmap_shrink is not None, "Should create with shrink" + + # Test with select='internal' + hmap_internal = bsp.to_heightmap(select='internal') + assert hmap_internal is not None, "Should create with select=internal" + + # Test with select='all' + hmap_all = bsp.to_heightmap(select='all') + assert hmap_all is not None, "Should create with select=all" + + # Test with custom size + hmap_sized = bsp.to_heightmap(size=(100, 80)) + assert hmap_sized.size == (100, 80), f"Custom size should work, got {hmap_sized.size}" + + # Test with custom value + hmap_val = bsp.to_heightmap(value=0.5) + leaves = list(bsp.leaves()) + leaf = leaves[0] + lx, ly = leaf.bounds[0] + val = hmap_val[lx, ly] + assert val == 0.5, f"Custom value should be 0.5, got {val}" + + print(" BSP to_heightmap: PASS") + +def test_traversal_enum(): + """Test Traversal enum (#204).""" + print("Testing Traversal enum...") + + # Check enum exists + assert hasattr(mcrfpy, 'Traversal'), "Traversal enum should exist" + + # Check all members + assert mcrfpy.Traversal.PRE_ORDER.value == 0 + assert mcrfpy.Traversal.IN_ORDER.value == 1 + assert mcrfpy.Traversal.POST_ORDER.value == 2 + assert mcrfpy.Traversal.LEVEL_ORDER.value == 3 + assert mcrfpy.Traversal.INVERTED_LEVEL_ORDER.value == 4 + + print(" Traversal enum: PASS") + +def test_bsp_chaining(): + """Test method chaining.""" + print("Testing BSP method chaining...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + + # Chain multiple operations + result = bsp.split_recursive(depth=2, min_size=(8, 8), seed=42).clear().split_once(True, 30) + assert result is bsp, "Chaining should return self" + + print(" BSP chaining: PASS") + +def test_bsp_repr(): + """Test repr output.""" + print("Testing BSP repr...") + + bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + repr_str = repr(bsp) + assert "BSP" in repr_str, f"repr should contain BSP: {repr_str}" + assert "80" in repr_str and "60" in repr_str, f"repr should contain size: {repr_str}" + + bsp.split_recursive(depth=2, min_size=(8, 8), seed=42) + repr_str2 = repr(bsp) + assert "leaves" in repr_str2, f"repr should mention leaves: {repr_str2}" + + # BSPNode repr + node_repr = repr(bsp.root) + assert "BSPNode" in node_repr, f"BSPNode repr: {node_repr}" + + print(" BSP repr: PASS") + +def run_all_tests(): + """Run all BSP tests.""" + print("\n=== BSP Unit Tests ===\n") + + try: + test_bsp_construction() + test_bsp_split_once() + test_bsp_split_recursive() + test_bsp_clear() + test_bspnode_properties() + test_bspnode_navigation() + test_bspnode_contains() + test_bsp_leaves_iteration() + test_bsp_traverse() + test_bsp_find() + test_bsp_to_heightmap() + test_traversal_enum() + test_bsp_chaining() + test_bsp_repr() + + print("\n=== ALL BSP TESTS PASSED ===\n") + sys.exit(0) + except AssertionError as e: + print(f"\nTEST FAILED: {e}") + sys.exit(1) + except Exception as e: + print(f"\nUNEXPECTED ERROR: {type(e).__name__}: {e}") + import traceback + traceback.print_exc() + sys.exit(1) + +if __name__ == "__main__": + run_all_tests() From 6caf3dcd0551db7db4dff086e71e8753a5ac3942 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Mon, 12 Jan 2026 07:59:31 -0500 Subject: [PATCH 2/2] BSP: add safety features and API improvements (closes #202, #203, #204, #205, #206) Safety improvements: - Generation counter detects stale BSPNode references after clear()/split_recursive() - GRID_MAX validation prevents oversized BSP trees - Depth parameter capped at 16 to prevent resource exhaustion - Iterator checks generation to detect invalidation during mutation API improvements: - Changed constructor from bounds=((x,y),(w,h)) to pos=(x,y), size=(w,h) - Added pos and size properties alongside bounds - BSPNode __eq__ compares underlying pointers for identity - BSP __iter__ as shorthand for leaves() - BSP __len__ returns leaf count Tests: - Added tests for stale node detection, GRID_MAX validation, depth cap - Added tests for __len__, __iter__, and BSPNode equality Co-Authored-By: Claude Opus 4.5 --- src/PyBSP.cpp | 308 ++++++++++++++++++++++++---------- src/PyBSP.h | 53 +++++- tests/unit/bsp_simple_test.py | 2 +- tests/unit/bsp_test.py | 197 ++++++++++++++++++++-- 4 files changed, 442 insertions(+), 118 deletions(-) diff --git a/src/PyBSP.cpp b/src/PyBSP.cpp index 67f8c0d..a387a1d 100644 --- a/src/PyBSP.cpp +++ b/src/PyBSP.cpp @@ -85,11 +85,10 @@ PyObject* PyTraversal::create_enum_class(PyObject* module) { return NULL; } - // Cache reference + // Cache reference (borrowed by module after AddObject) traversal_enum_class = traversal_class; - Py_INCREF(traversal_enum_class); - // Add to module + // Add to module (steals reference) if (PyModule_AddObject(module, "Traversal", traversal_class) < 0) { Py_DECREF(traversal_class); traversal_enum_class = nullptr; @@ -99,6 +98,12 @@ PyObject* PyTraversal::create_enum_class(PyObject* module) { return traversal_class; } +void PyTraversal::cleanup() { + // The enum class is owned by the module after PyModule_AddObject + // We just clear our pointer; the module handles cleanup + traversal_enum_class = nullptr; +} + int PyTraversal::from_arg(PyObject* arg, int* out_order) { // Accept None -> default to LEVEL_ORDER if (arg == NULL || arg == Py_None) { @@ -182,29 +187,41 @@ int PyTraversal::from_arg(PyObject* arg, int* out_order) { PyGetSetDef PyBSP::getsetters[] = { {"bounds", (getter)PyBSP::get_bounds, NULL, MCRF_PROPERTY(bounds, "Root node bounds as ((x, y), (w, h)). Read-only."), NULL}, + {"pos", (getter)PyBSP::get_pos, NULL, + MCRF_PROPERTY(pos, "Top-left position (x, y). Read-only."), NULL}, + {"size", (getter)PyBSP::get_size, NULL, + MCRF_PROPERTY(size, "Dimensions (width, height). Read-only."), NULL}, {"root", (getter)PyBSP::get_root, NULL, MCRF_PROPERTY(root, "Reference to the root BSPNode. Read-only."), NULL}, {NULL} }; +// Sequence methods for len() and iteration +PySequenceMethods PyBSP::sequence_methods = { + .sq_length = (lenfunc)PyBSP::len, +}; + // ==================== BSP Method Definitions ==================== PyMethodDef PyBSP::methods[] = { {"split_once", (PyCFunction)PyBSP::split_once, METH_VARARGS | METH_KEYWORDS, MCRF_METHOD(BSP, split_once, MCRF_SIG("(horizontal: bool, position: int)", "BSP"), - MCRF_DESC("Split the root node once at the specified position."), + MCRF_DESC("Split the root node once at the specified position. " + "horizontal=True creates a horizontal divider, producing top/bottom rooms. " + "horizontal=False creates a vertical divider, producing left/right rooms."), MCRF_ARGS_START - MCRF_ARG("horizontal", "True for horizontal split, False for vertical") + MCRF_ARG("horizontal", "True for horizontal divider (top/bottom), False for vertical (left/right)") MCRF_ARG("position", "Split coordinate (y for horizontal, x for vertical)") MCRF_RETURNS("BSP: self, for method chaining") )}, {"split_recursive", (PyCFunction)PyBSP::split_recursive, METH_VARARGS | METH_KEYWORDS, MCRF_METHOD(BSP, split_recursive, MCRF_SIG("(depth: int, min_size: tuple[int, int], max_ratio: float = 1.5, seed: int = None)", "BSP"), - MCRF_DESC("Recursively split to the specified depth."), + MCRF_DESC("Recursively split to the specified depth. " + "WARNING: Invalidates all existing BSPNode references from this tree."), MCRF_ARGS_START - MCRF_ARG("depth", "Maximum recursion depth. Creates up to 2^depth leaves.") + MCRF_ARG("depth", "Maximum recursion depth (1-16). Creates up to 2^depth leaves.") MCRF_ARG("min_size", "Minimum (width, height) for a node to be split.") MCRF_ARG("max_ratio", "Maximum aspect ratio before forcing split direction. Default: 1.5.") MCRF_ARG("seed", "Random seed. None for random.") @@ -213,13 +230,14 @@ PyMethodDef PyBSP::methods[] = { {"clear", (PyCFunction)PyBSP::clear, METH_NOARGS, MCRF_METHOD(BSP, clear, MCRF_SIG("()", "BSP"), - MCRF_DESC("Remove all children, keeping only the root node with original bounds."), + MCRF_DESC("Remove all children, keeping only the root node with original bounds. " + "WARNING: Invalidates all existing BSPNode references from this tree."), MCRF_RETURNS("BSP: self, for method chaining") )}, {"leaves", (PyCFunction)PyBSP::leaves, METH_NOARGS, MCRF_METHOD(BSP, leaves, MCRF_SIG("()", "Iterator[BSPNode]"), - MCRF_DESC("Iterate all leaf nodes (the actual rooms)."), + MCRF_DESC("Iterate all leaf nodes (the actual rooms). Same as iterating the BSP directly."), MCRF_RETURNS("Iterator yielding BSPNode objects") )}, {"traverse", (PyCFunction)PyBSP::traverse, METH_VARARGS | METH_KEYWORDS, @@ -264,41 +282,33 @@ PyObject* PyBSP::pynew(PyTypeObject* type, PyObject* args, PyObject* kwds) self->orig_y = 0; self->orig_w = 0; self->orig_h = 0; + self->generation = 0; } return (PyObject*)self; } int PyBSP::init(PyBSPObject* self, PyObject* args, PyObject* kwds) { - static const char* keywords[] = {"bounds", nullptr}; - PyObject* bounds_obj = nullptr; + static const char* keywords[] = {"pos", "size", nullptr}; + PyObject* pos_obj = nullptr; + PyObject* size_obj = nullptr; - if (!PyArg_ParseTupleAndKeywords(args, kwds, "O", const_cast(keywords), - &bounds_obj)) { + if (!PyArg_ParseTupleAndKeywords(args, kwds, "OO", const_cast(keywords), + &pos_obj, &size_obj)) { return -1; } - // Parse bounds: ((x, y), (w, h)) - if (!PyTuple_Check(bounds_obj) || PyTuple_Size(bounds_obj) != 2) { - PyErr_SetString(PyExc_TypeError, "bounds must be ((x, y), (w, h)) tuple"); + // Parse position using PyPositionHelper pattern + int x, y; + if (!PyPosition_FromObjectInt(pos_obj, &x, &y)) { + PyErr_SetString(PyExc_TypeError, "pos must be a tuple (x, y), list, or Vector"); return -1; } - PyObject* pos_obj = PyTuple_GetItem(bounds_obj, 0); - PyObject* size_obj = PyTuple_GetItem(bounds_obj, 1); - - if (!PyTuple_Check(pos_obj) || PyTuple_Size(pos_obj) != 2 || - !PyTuple_Check(size_obj) || PyTuple_Size(size_obj) != 2) { - PyErr_SetString(PyExc_TypeError, "bounds must be ((x, y), (w, h)) tuple"); - return -1; - } - - int x = (int)PyLong_AsLong(PyTuple_GetItem(pos_obj, 0)); - int y = (int)PyLong_AsLong(PyTuple_GetItem(pos_obj, 1)); - int w = (int)PyLong_AsLong(PyTuple_GetItem(size_obj, 0)); - int h = (int)PyLong_AsLong(PyTuple_GetItem(size_obj, 1)); - - if (PyErr_Occurred()) { + // Parse size using PyPositionHelper pattern + int w, h; + if (!PyPosition_FromObjectInt(size_obj, &w, &h)) { + PyErr_SetString(PyExc_TypeError, "size must be a tuple (w, h), list, or Vector"); return -1; } @@ -307,6 +317,14 @@ int PyBSP::init(PyBSPObject* self, PyObject* args, PyObject* kwds) return -1; } + // Validate against GRID_MAX like HeightMap does + if (w > GRID_MAX || h > GRID_MAX) { + PyErr_Format(PyExc_ValueError, + "BSP dimensions cannot exceed %d (got %dx%d)", + GRID_MAX, w, h); + return -1; + } + // Clean up any existing BSP if (self->root) { TCOD_bsp_delete(self->root); @@ -324,6 +342,7 @@ int PyBSP::init(PyBSPObject* self, PyObject* args, PyObject* kwds) self->orig_y = y; self->orig_w = w; self->orig_h = h; + self->generation = 0; return 0; } @@ -373,6 +392,26 @@ PyObject* PyBSP::get_bounds(PyBSPObject* self, void* closure) self->root->w, self->root->h); } +// Property: pos +PyObject* PyBSP::get_pos(PyBSPObject* self, void* closure) +{ + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + return Py_BuildValue("(ii)", self->root->x, self->root->y); +} + +// Property: size +PyObject* PyBSP::get_size(PyBSPObject* self, void* closure) +{ + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return nullptr; + } + return Py_BuildValue("(ii)", self->root->w, self->root->h); +} + // Property: root PyObject* PyBSP::get_root(PyBSPObject* self, void* closure) { @@ -400,6 +439,8 @@ PyObject* PyBSP::split_once(PyBSPObject* self, PyObject* args, PyObject* kwds) return nullptr; } + // Note: split_once only adds children, doesn't remove any nodes + // Root node pointer remains valid, so we don't increment generation TCOD_bsp_split_once(self->root, horizontal ? true : false, position); Py_INCREF(self); @@ -425,21 +466,22 @@ PyObject* PyBSP::split_recursive(PyBSPObject* self, PyObject* args, PyObject* kw return nullptr; } - // Parse min_size tuple - if (!PyTuple_Check(min_size_obj) || PyTuple_Size(min_size_obj) != 2) { - PyErr_SetString(PyExc_TypeError, "min_size must be (width, height) tuple"); + // Parse min_size using PyPositionHelper pattern + int min_w, min_h; + if (!PyPosition_FromObjectInt(min_size_obj, &min_w, &min_h)) { + PyErr_SetString(PyExc_TypeError, "min_size must be (width, height) tuple, list, or Vector"); return nullptr; } - int min_w = (int)PyLong_AsLong(PyTuple_GetItem(min_size_obj, 0)); - int min_h = (int)PyLong_AsLong(PyTuple_GetItem(min_size_obj, 1)); - - if (PyErr_Occurred()) { + if (depth < 1) { + PyErr_SetString(PyExc_ValueError, "depth must be at least 1"); return nullptr; } - if (depth < 0) { - PyErr_SetString(PyExc_ValueError, "depth must be non-negative"); + if (depth > BSP_MAX_DEPTH) { + PyErr_Format(PyExc_ValueError, + "depth cannot exceed %d (got %d) to prevent memory exhaustion", + BSP_MAX_DEPTH, depth); return nullptr; } @@ -462,6 +504,9 @@ PyObject* PyBSP::split_recursive(PyBSPObject* self, PyObject* args, PyObject* kw rnd = TCOD_random_new_from_seed(TCOD_RNG_MT, seed); } + // Increment generation BEFORE splitting - invalidates existing nodes + self->generation++; + TCOD_bsp_split_recursive(self->root, rnd, depth, min_w, min_h, max_ratio, max_ratio); if (rnd) { @@ -480,6 +525,9 @@ PyObject* PyBSP::clear(PyBSPObject* self, PyObject* Py_UNUSED(args)) return nullptr; } + // Increment generation BEFORE clearing - invalidates existing nodes + self->generation++; + TCOD_bsp_remove_sons(self->root); // Restore original bounds @@ -489,6 +537,31 @@ PyObject* PyBSP::clear(PyBSPObject* self, PyObject* Py_UNUSED(args)) return (PyObject*)self; } +// Sequence protocol: len(bsp) returns leaf count +Py_ssize_t PyBSP::len(PyBSPObject* self) +{ + if (!self->root) { + PyErr_SetString(PyExc_RuntimeError, "BSP not initialized"); + return -1; + } + + int leaf_count = 0; + TCOD_bsp_traverse_pre_order(self->root, [](TCOD_bsp_t* node, void* data) -> bool { + if (TCOD_bsp_is_leaf(node)) { + (*(int*)data)++; + } + return true; + }, &leaf_count); + + return (Py_ssize_t)leaf_count; +} + +// __iter__ is shorthand for leaves() +PyObject* PyBSP::iter(PyBSPObject* self) +{ + return PyBSP::leaves(self, nullptr); +} + // ==================== BSP Iteration ==================== // Traversal callback to collect nodes @@ -527,6 +600,7 @@ PyObject* PyBSP::leaves(PyBSPObject* self, PyObject* Py_UNUSED(args)) iter->index = 0; iter->bsp_owner = (PyObject*)self; + iter->generation = self->generation; // Capture generation for validity check Py_INCREF(self); return (PyObject*)iter; @@ -584,6 +658,7 @@ PyObject* PyBSP::traverse(PyBSPObject* self, PyObject* args, PyObject* kwds) iter->index = 0; iter->bsp_owner = (PyObject*)self; + iter->generation = self->generation; // Capture generation for validity check Py_INCREF(self); return (PyObject*)iter; @@ -633,13 +708,8 @@ PyObject* PyBSP::to_heightmap(PyBSPObject* self, PyObject* args, PyObject* kwds) int width = self->root->w; int height = self->root->h; if (size_obj != nullptr && size_obj != Py_None) { - if (!PyTuple_Check(size_obj) || PyTuple_Size(size_obj) != 2) { - PyErr_SetString(PyExc_TypeError, "size must be (width, height) tuple"); - return nullptr; - } - width = (int)PyLong_AsLong(PyTuple_GetItem(size_obj, 0)); - height = (int)PyLong_AsLong(PyTuple_GetItem(size_obj, 1)); - if (PyErr_Occurred()) { + if (!PyPosition_FromObjectInt(size_obj, &width, &height)) { + PyErr_SetString(PyExc_TypeError, "size must be (width, height) tuple, list, or Vector"); return nullptr; } } @@ -751,6 +821,10 @@ PyObject* PyBSP::to_heightmap(PyBSPObject* self, PyObject* args, PyObject* kwds) PyGetSetDef PyBSPNode::getsetters[] = { {"bounds", (getter)PyBSPNode::get_bounds, NULL, MCRF_PROPERTY(bounds, "Node bounds as ((x, y), (w, h)). Read-only."), NULL}, + {"pos", (getter)PyBSPNode::get_pos, NULL, + MCRF_PROPERTY(pos, "Top-left position (x, y). Read-only."), NULL}, + {"size", (getter)PyBSPNode::get_size, NULL, + MCRF_PROPERTY(size, "Dimensions (width, height). Read-only."), NULL}, {"level", (getter)PyBSPNode::get_level, NULL, MCRF_PROPERTY(level, "Depth in tree (0 for root). Read-only."), NULL}, {"is_leaf", (getter)PyBSPNode::get_is_leaf, NULL, @@ -792,6 +866,30 @@ PyMethodDef PyBSPNode::methods[] = { // ==================== BSPNode Implementation ==================== +// Validity check - returns false and sets error if node is stale +bool PyBSPNode::checkValid(PyBSPNodeObject* self) +{ + if (!self->node) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid (null pointer)"); + return false; + } + + if (!self->bsp_owner) { + PyErr_SetString(PyExc_RuntimeError, "BSPNode has no parent BSP"); + return false; + } + + PyBSPObject* bsp = (PyBSPObject*)self->bsp_owner; + if (self->generation != bsp->generation) { + PyErr_SetString(PyExc_RuntimeError, + "BSPNode is stale: parent BSP was modified (clear() or split_recursive() called). " + "Re-fetch nodes from the BSP object."); + return false; + } + + return true; +} + PyObject* PyBSPNode::pynew(PyTypeObject* type, PyObject* args, PyObject* kwds) { // BSPNode cannot be directly instantiated @@ -817,20 +915,52 @@ void PyBSPNode::dealloc(PyBSPNodeObject* self) PyObject* PyBSPNode::repr(PyObject* obj) { PyBSPNodeObject* self = (PyBSPNodeObject*)obj; + + // Check validity without raising error for repr + PyBSPObject* bsp = self->bsp_owner ? (PyBSPObject*)self->bsp_owner : nullptr; + bool is_valid = self->node && bsp && self->generation == bsp->generation; + std::ostringstream ss; - if (self->node) { + if (is_valid) { const char* type = TCOD_bsp_is_leaf(self->node) ? "leaf" : "split"; ss << "node->x << ", " << self->node->y << ") size (" << self->node->w << " x " << self->node->h << ") level " << (int)self->node->level << ">"; } else { - ss << ""; + ss << ""; } return PyUnicode_FromString(ss.str().c_str()); } +// Rich comparison for BSPNode - compares underlying pointers +PyObject* PyBSPNode::richcompare(PyObject* self, PyObject* other, int op) +{ + // Only support == and != + if (op != Py_EQ && op != Py_NE) { + Py_RETURN_NOTIMPLEMENTED; + } + + // Check if other is also a BSPNode + if (!PyObject_TypeCheck(other, &mcrfpydef::PyBSPNodeType)) { + if (op == Py_EQ) Py_RETURN_FALSE; + else Py_RETURN_TRUE; + } + + PyBSPNodeObject* self_node = (PyBSPNodeObject*)self; + PyBSPNodeObject* other_node = (PyBSPNodeObject*)other; + + // Compare wrapped pointers + bool equal = (self_node->node == other_node->node); + + if (op == Py_EQ) { + return PyBool_FromLong(equal); + } else { + return PyBool_FromLong(!equal); + } +} + // Helper to create a BSPNode wrapper PyObject* PyBSPNode::create(TCOD_bsp_t* node, PyObject* bsp_owner) { @@ -846,6 +976,7 @@ PyObject* PyBSPNode::create(TCOD_bsp_t* node, PyObject* bsp_owner) py_node->node = node; py_node->bsp_owner = bsp_owner; + py_node->generation = ((PyBSPObject*)bsp_owner)->generation; Py_INCREF(bsp_owner); return (PyObject*)py_node; @@ -854,42 +985,44 @@ PyObject* PyBSPNode::create(TCOD_bsp_t* node, PyObject* bsp_owner) // Property: bounds PyObject* PyBSPNode::get_bounds(PyBSPNodeObject* self, void* closure) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; return Py_BuildValue("((ii)(ii))", self->node->x, self->node->y, self->node->w, self->node->h); } +// Property: pos +PyObject* PyBSPNode::get_pos(PyBSPNodeObject* self, void* closure) +{ + if (!checkValid(self)) return nullptr; + return Py_BuildValue("(ii)", self->node->x, self->node->y); +} + +// Property: size +PyObject* PyBSPNode::get_size(PyBSPNodeObject* self, void* closure) +{ + if (!checkValid(self)) return nullptr; + return Py_BuildValue("(ii)", self->node->w, self->node->h); +} + // Property: level PyObject* PyBSPNode::get_level(PyBSPNodeObject* self, void* closure) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; return PyLong_FromLong(self->node->level); } // Property: is_leaf PyObject* PyBSPNode::get_is_leaf(PyBSPNodeObject* self, void* closure) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; return PyBool_FromLong(TCOD_bsp_is_leaf(self->node)); } // Property: split_horizontal PyObject* PyBSPNode::get_split_horizontal(PyBSPNodeObject* self, void* closure) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; if (TCOD_bsp_is_leaf(self->node)) { Py_RETURN_NONE; } @@ -899,10 +1032,7 @@ PyObject* PyBSPNode::get_split_horizontal(PyBSPNodeObject* self, void* closure) // Property: split_position PyObject* PyBSPNode::get_split_position(PyBSPNodeObject* self, void* closure) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; if (TCOD_bsp_is_leaf(self->node)) { Py_RETURN_NONE; } @@ -912,40 +1042,28 @@ PyObject* PyBSPNode::get_split_position(PyBSPNodeObject* self, void* closure) // Property: left PyObject* PyBSPNode::get_left(PyBSPNodeObject* self, void* closure) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; return PyBSPNode::create(TCOD_bsp_left(self->node), self->bsp_owner); } // Property: right PyObject* PyBSPNode::get_right(PyBSPNodeObject* self, void* closure) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; return PyBSPNode::create(TCOD_bsp_right(self->node), self->bsp_owner); } // Property: parent PyObject* PyBSPNode::get_parent(PyBSPNodeObject* self, void* closure) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; return PyBSPNode::create(TCOD_bsp_father(self->node), self->bsp_owner); } // Property: sibling PyObject* PyBSPNode::get_sibling(PyBSPNodeObject* self, void* closure) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; TCOD_bsp_t* parent = TCOD_bsp_father(self->node); if (!parent) { @@ -965,10 +1083,7 @@ PyObject* PyBSPNode::get_sibling(PyBSPNodeObject* self, void* closure) // Method: contains(pos) -> bool PyObject* PyBSPNode::contains(PyBSPNodeObject* self, PyObject* args, PyObject* kwds) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; int x, y; if (!PyPosition_ParseInt(args, kwds, &x, &y)) { @@ -981,10 +1096,7 @@ PyObject* PyBSPNode::contains(PyBSPNodeObject* self, PyObject* args, PyObject* k // Method: center() -> (x, y) PyObject* PyBSPNode::center(PyBSPNodeObject* self, PyObject* Py_UNUSED(args)) { - if (!self->node) { - PyErr_SetString(PyExc_RuntimeError, "BSPNode is invalid"); - return nullptr; - } + if (!checkValid(self)) return nullptr; int cx = self->node->x + self->node->w / 2; int cy = self->node->y + self->node->h / 2; @@ -1017,6 +1129,16 @@ PyObject* PyBSPIter::next(PyBSPIterObject* self) return NULL; } + // Check for tree modification during iteration + if (self->bsp_owner) { + PyBSPObject* bsp = (PyBSPObject*)self->bsp_owner; + if (self->generation != bsp->generation) { + PyErr_SetString(PyExc_RuntimeError, + "BSP tree was modified during iteration (clear() or split_recursive() called)"); + return NULL; + } + } + if (self->index >= self->nodes->size()) { PyErr_SetNone(PyExc_StopIteration); return NULL; diff --git a/src/PyBSP.h b/src/PyBSP.h index 36a599c..430092b 100644 --- a/src/PyBSP.h +++ b/src/PyBSP.h @@ -3,17 +3,23 @@ #include "Python.h" #include #include +#include // Forward declarations class PyBSP; class PyBSPNode; +// Maximum recursion depth to prevent memory exhaustion +// 2^16 = 65536 potential leaf nodes, which is already excessive +constexpr int BSP_MAX_DEPTH = 16; + // Python object structure for BSP tree (root owner) typedef struct { PyObject_HEAD TCOD_bsp_t* root; // libtcod BSP root (owned, will be deleted) int orig_x, orig_y; // Original bounds for clear() int orig_w, orig_h; + uint64_t generation; // Incremented on structural changes (clear, split) } PyBSPObject; // Python object structure for BSPNode (lightweight reference) @@ -21,6 +27,7 @@ typedef struct { PyObject_HEAD TCOD_bsp_t* node; // libtcod BSP node (NOT owned) PyObject* bsp_owner; // Reference to PyBSPObject to prevent dangling + uint64_t generation; // Generation at time of creation (for validity check) } PyBSPNodeObject; // BSP iterator for traverse() @@ -29,6 +36,7 @@ typedef struct { std::vector* nodes; // Pre-collected nodes size_t index; PyObject* bsp_owner; // Reference to PyBSPObject + uint64_t generation; // Generation at iterator creation } PyBSPIterObject; class PyBSP @@ -42,6 +50,8 @@ public: // Properties static PyObject* get_bounds(PyBSPObject* self, void* closure); + static PyObject* get_pos(PyBSPObject* self, void* closure); + static PyObject* get_size(PyBSPObject* self, void* closure); static PyObject* get_root(PyBSPObject* self, void* closure); // Splitting methods (#202) @@ -59,9 +69,14 @@ public: // HeightMap conversion (#206) static PyObject* to_heightmap(PyBSPObject* self, PyObject* args, PyObject* kwds); + // Sequence protocol + static Py_ssize_t len(PyBSPObject* self); + static PyObject* iter(PyBSPObject* self); + // Method and property definitions static PyMethodDef methods[]; static PyGetSetDef getsetters[]; + static PySequenceMethods sequence_methods; }; class PyBSPNode @@ -73,8 +88,13 @@ public: static void dealloc(PyBSPNodeObject* self); static PyObject* repr(PyObject* obj); + // Comparison + static PyObject* richcompare(PyObject* self, PyObject* other, int op); + // Properties (#203) static PyObject* get_bounds(PyBSPNodeObject* self, void* closure); + static PyObject* get_pos(PyBSPNodeObject* self, void* closure); + static PyObject* get_size(PyBSPNodeObject* self, void* closure); static PyObject* get_level(PyBSPNodeObject* self, void* closure); static PyObject* get_is_leaf(PyBSPNodeObject* self, void* closure); static PyObject* get_split_horizontal(PyBSPNodeObject* self, void* closure); @@ -93,6 +113,9 @@ public: // Helper to create a BSPNode from a TCOD_bsp_t* static PyObject* create(TCOD_bsp_t* node, PyObject* bsp_owner); + // Validity check - returns false and sets error if node is stale + static bool checkValid(PyBSPNodeObject* self); + // Method and property definitions static PyMethodDef methods[]; static PyGetSetDef getsetters[]; @@ -116,6 +139,8 @@ public: static PyObject* traversal_enum_class; static PyObject* create_enum_class(PyObject* module); static int from_arg(PyObject* arg, int* out_order); + // Cleanup for module finalization (optional) + static void cleanup(); }; namespace mcrfpydef { @@ -127,23 +152,31 @@ namespace mcrfpydef { .tp_itemsize = 0, .tp_dealloc = (destructor)PyBSP::dealloc, .tp_repr = PyBSP::repr, + .tp_as_sequence = &PyBSP::sequence_methods, .tp_flags = Py_TPFLAGS_DEFAULT, .tp_doc = PyDoc_STR( - "BSP(bounds: tuple[tuple[int, int], tuple[int, int]])\n\n" + "BSP(pos: tuple[int, int], size: tuple[int, int])\n\n" "Binary Space Partitioning tree for procedural dungeon generation.\n\n" "BSP recursively divides a rectangular region into smaller sub-regions, " "creating a tree structure perfect for generating dungeon rooms and corridors.\n\n" "Args:\n" - " bounds: ((x, y), (w, h)) - Position and size of the root region.\n\n" + " pos: (x, y) - Top-left position of the root region.\n" + " size: (w, h) - Width and height of the root region.\n\n" "Properties:\n" - " bounds ((x, y), (w, h)): Read-only. Root node bounds.\n" + " pos (tuple[int, int]): Read-only. Top-left position (x, y).\n" + " size (tuple[int, int]): Read-only. Dimensions (width, height).\n" + " bounds ((pos), (size)): Read-only. Combined position and size.\n" " root (BSPNode): Read-only. Reference to the root node.\n\n" + "Iteration:\n" + " for leaf in bsp: # Iterates over leaf nodes (rooms)\n" + " len(bsp) # Returns number of leaf nodes\n\n" "Example:\n" - " bsp = mcrfpy.BSP(bounds=((0, 0), (80, 50)))\n" + " bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 50))\n" " bsp.split_recursive(depth=4, min_size=(8, 8))\n" - " for leaf in bsp.leaves():\n" - " print(f'Room at {leaf.bounds}')\n" + " for leaf in bsp:\n" + " print(f'Room at {leaf.pos}, size {leaf.size}')\n" ), + .tp_iter = (getiterfunc)PyBSP::iter, .tp_methods = nullptr, // Set in McRFPy_API.cpp .tp_getset = nullptr, // Set in McRFPy_API.cpp .tp_init = (initproc)PyBSP::init, @@ -163,8 +196,13 @@ namespace mcrfpydef { "BSPNode - Lightweight reference to a node in a BSP tree.\n\n" "BSPNode provides read-only access to node properties and navigation.\n" "Nodes are created by BSP methods, not directly instantiated.\n\n" + "WARNING: BSPNode references become invalid after BSP.clear() or\n" + "BSP.split_recursive(). Accessing properties of an invalid node\n" + "raises RuntimeError.\n\n" "Properties:\n" - " bounds ((x, y), (w, h)): Position and size of this node.\n" + " pos (tuple[int, int]): Top-left position (x, y).\n" + " size (tuple[int, int]): Dimensions (width, height).\n" + " bounds ((pos), (size)): Combined position and size.\n" " level (int): Depth in tree (0 for root).\n" " is_leaf (bool): True if this node has no children.\n" " split_horizontal (bool | None): Split orientation, None if leaf.\n" @@ -174,6 +212,7 @@ namespace mcrfpydef { " parent (BSPNode | None): Parent node, or None if root.\n" " sibling (BSPNode | None): Other child of parent, or None.\n" ), + .tp_richcompare = PyBSPNode::richcompare, .tp_methods = nullptr, // Set in McRFPy_API.cpp .tp_getset = nullptr, // Set in McRFPy_API.cpp .tp_init = (initproc)PyBSPNode::init, diff --git a/tests/unit/bsp_simple_test.py b/tests/unit/bsp_simple_test.py index e96497f..242927d 100644 --- a/tests/unit/bsp_simple_test.py +++ b/tests/unit/bsp_simple_test.py @@ -5,7 +5,7 @@ import mcrfpy print("Step 1: Import complete") print("Step 2: Creating BSP...") -bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) +bsp = mcrfpy.BSP(pos=(0, 0), size=(100, 80)) print("Step 2: BSP created:", bsp) print("Step 3: Getting bounds...") diff --git a/tests/unit/bsp_test.py b/tests/unit/bsp_test.py index 96bd899..da51ee6 100644 --- a/tests/unit/bsp_test.py +++ b/tests/unit/bsp_test.py @@ -11,14 +11,18 @@ import sys import mcrfpy def test_bsp_construction(): - """Test BSP construction with bounds.""" + """Test BSP construction with pos/size.""" print("Testing BSP construction...") - # Basic construction - bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + # Basic construction with pos/size kwargs + bsp = mcrfpy.BSP(pos=(0, 0), size=(100, 80)) assert bsp is not None, "BSP should be created" - # Check bounds property + # Check pos and size properties + assert bsp.pos == (0, 0), f"pos should be (0, 0), got {bsp.pos}" + assert bsp.size == (100, 80), f"size should be (100, 80), got {bsp.size}" + + # Check bounds property (combines pos and size) bounds = bsp.bounds assert bounds == ((0, 0), (100, 80)), f"Bounds should be ((0, 0), (100, 80)), got {bounds}" @@ -28,7 +32,7 @@ def test_bsp_construction(): assert root.bounds == ((0, 0), (100, 80)), f"Root bounds mismatch" # Construction with offset - bsp2 = mcrfpy.BSP(bounds=((10, 20), (50, 40))) + bsp2 = mcrfpy.BSP(pos=(10, 20), size=(50, 40)) assert bsp2.bounds == ((10, 20), (50, 40)), "Offset bounds not preserved" print(" BSP construction: PASS") @@ -37,7 +41,7 @@ def test_bsp_split_once(): """Test single split operation (#202).""" print("Testing BSP split_once...") - bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(100, 80)) # Before split, root should be a leaf assert bsp.root.is_leaf, "Root should be leaf before split" @@ -66,7 +70,7 @@ def test_bsp_split_recursive(): """Test recursive splitting (#202).""" print("Testing BSP split_recursive...") - bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) # Recursive split with seed for reproducibility result = bsp.split_recursive(depth=3, min_size=(8, 8), max_ratio=1.5, seed=42) @@ -91,7 +95,7 @@ def test_bsp_clear(): """Test clear operation (#202).""" print("Testing BSP clear...") - bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(100, 80)) bsp.split_recursive(depth=4, min_size=(8, 8), seed=42) # Should have multiple leaves @@ -116,7 +120,7 @@ def test_bspnode_properties(): """Test BSPNode properties (#203).""" print("Testing BSPNode properties...") - bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(100, 80)) bsp.split_recursive(depth=3, min_size=(8, 8), seed=42) root = bsp.root @@ -155,7 +159,7 @@ def test_bspnode_navigation(): """Test BSPNode navigation (#203).""" print("Testing BSPNode navigation...") - bsp = mcrfpy.BSP(bounds=((0, 0), (100, 80))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(100, 80)) bsp.split_once(horizontal=True, position=40) root = bsp.root @@ -182,7 +186,7 @@ def test_bspnode_contains(): """Test BSPNode contains method (#203).""" print("Testing BSPNode contains...") - bsp = mcrfpy.BSP(bounds=((10, 20), (50, 40))) # x: 10-60, y: 20-60 + bsp = mcrfpy.BSP(pos=(10, 20), size=(50, 40)) # x: 10-60, y: 20-60 root = bsp.root @@ -203,7 +207,7 @@ def test_bsp_leaves_iteration(): """Test leaves iteration (#204).""" print("Testing BSP leaves iteration...") - bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) bsp.split_recursive(depth=3, min_size=(8, 8), seed=42) # Iterate leaves @@ -224,7 +228,7 @@ def test_bsp_traverse(): """Test traverse with different orders (#204).""" print("Testing BSP traverse...") - bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) bsp.split_recursive(depth=2, min_size=(8, 8), seed=42) # Test all traversal orders @@ -261,7 +265,7 @@ def test_bsp_find(): """Test find method (#205).""" print("Testing BSP find...") - bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) bsp.split_recursive(depth=3, min_size=(8, 8), seed=42) # Find a point inside bounds @@ -285,7 +289,7 @@ def test_bsp_to_heightmap(): """Test to_heightmap conversion (#206).""" print("Testing BSP to_heightmap...") - bsp = mcrfpy.BSP(bounds=((0, 0), (50, 40))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(50, 40)) bsp.split_recursive(depth=2, min_size=(8, 8), seed=42) # Basic conversion @@ -346,7 +350,7 @@ def test_bsp_chaining(): """Test method chaining.""" print("Testing BSP method chaining...") - bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) # Chain multiple operations result = bsp.split_recursive(depth=2, min_size=(8, 8), seed=42).clear().split_once(True, 30) @@ -358,7 +362,7 @@ def test_bsp_repr(): """Test repr output.""" print("Testing BSP repr...") - bsp = mcrfpy.BSP(bounds=((0, 0), (80, 60))) + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) repr_str = repr(bsp) assert "BSP" in repr_str, f"repr should contain BSP: {repr_str}" assert "80" in repr_str and "60" in repr_str, f"repr should contain size: {repr_str}" @@ -373,6 +377,159 @@ def test_bsp_repr(): print(" BSP repr: PASS") +def test_bsp_stale_node_detection(): + """Test that stale nodes are detected after clear()/split_recursive() (#review).""" + print("Testing BSP stale node detection...") + + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) + bsp.split_recursive(depth=2, min_size=(8, 8), seed=42) + + # Save reference to a node + old_root = bsp.root + old_leaf = list(bsp.leaves())[0] + + # Clear invalidates all nodes + bsp.clear() + + # Accessing stale node should raise RuntimeError + try: + _ = old_root.bounds + assert False, "Accessing stale root bounds should raise RuntimeError" + except RuntimeError as e: + assert "stale" in str(e).lower() or "invalid" in str(e).lower(), \ + f"Error should mention staleness: {e}" + + try: + _ = old_leaf.is_leaf + assert False, "Accessing stale leaf should raise RuntimeError" + except RuntimeError as e: + pass # Expected + + # split_recursive also invalidates (rebuilds tree) + bsp2 = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) + bsp2.split_recursive(depth=2, min_size=(8, 8), seed=42) + saved_node = bsp2.root + bsp2.split_recursive(depth=3, min_size=(8, 8), seed=99) + + try: + _ = saved_node.bounds + assert False, "Accessing node after split_recursive should raise RuntimeError" + except RuntimeError: + pass # Expected + + print(" BSP stale node detection: PASS") + +def test_bsp_grid_max_validation(): + """Test GRID_MAX validation (#review).""" + print("Testing BSP GRID_MAX validation...") + + # Should succeed with valid size + bsp = mcrfpy.BSP(pos=(0, 0), size=(1000, 1000)) + assert bsp is not None + + # Should fail with size exceeding GRID_MAX (8192) + try: + bsp_too_big = mcrfpy.BSP(pos=(0, 0), size=(10000, 100)) + assert False, "Should raise ValueError for size > GRID_MAX" + except ValueError as e: + assert "8192" in str(e) or "exceed" in str(e).lower(), f"Error should mention limit: {e}" + + try: + bsp_too_big = mcrfpy.BSP(pos=(0, 0), size=(100, 10000)) + assert False, "Should raise ValueError for height > GRID_MAX" + except ValueError as e: + pass # Expected + + print(" BSP GRID_MAX validation: PASS") + +def test_bsp_depth_cap(): + """Test depth is capped at 16 (#review).""" + print("Testing BSP depth cap...") + + bsp = mcrfpy.BSP(pos=(0, 0), size=(1000, 1000)) + + # Should cap depth at 16 or raise ValueError + try: + bsp.split_recursive(depth=20, min_size=(1, 1), seed=42) + # If it succeeds, verify reasonable number of leaves (not 2^20) + leaves = list(bsp.leaves()) + assert len(leaves) <= 2**16, f"Too many leaves: {len(leaves)}" + except ValueError as e: + # It's also acceptable to reject excessive depth + assert "16" in str(e) or "depth" in str(e).lower(), f"Error should mention depth limit: {e}" + + print(" BSP depth cap: PASS") + +def test_bsp_len(): + """Test __len__ returns leaf count (#review).""" + print("Testing BSP __len__...") + + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) + assert len(bsp) == 1, f"Unsplit BSP should have 1 leaf, got {len(bsp)}" + + bsp.split_once(horizontal=True, position=30) + assert len(bsp) == 2, f"After one split should have 2 leaves, got {len(bsp)}" + + bsp.clear() + bsp.split_recursive(depth=3, min_size=(8, 8), seed=42) + expected = len(list(bsp.leaves())) + assert len(bsp) == expected, f"len() mismatch: {len(bsp)} != {expected}" + + print(" BSP __len__: PASS") + +def test_bsp_iter(): + """Test __iter__ as shorthand for leaves() (#review).""" + print("Testing BSP __iter__...") + + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) + bsp.split_recursive(depth=2, min_size=(8, 8), seed=42) + + # Direct iteration should yield same results as leaves() + iter_list = list(bsp) + leaves_list = list(bsp.leaves()) + + assert len(iter_list) == len(leaves_list), \ + f"Iterator count mismatch: {len(iter_list)} != {len(leaves_list)}" + + # All items should be leaves + for node in bsp: + assert node.is_leaf, f"Iterator should yield leaves: {node}" + + # Can iterate multiple times + count1 = sum(1 for _ in bsp) + count2 = sum(1 for _ in bsp) + assert count1 == count2, "Should be able to iterate multiple times" + + print(" BSP __iter__: PASS") + +def test_bspnode_equality(): + """Test BSPNode __eq__ comparison (#review).""" + print("Testing BSPNode equality...") + + bsp = mcrfpy.BSP(pos=(0, 0), size=(80, 60)) + bsp.split_recursive(depth=2, min_size=(8, 8), seed=42) + + # Same node should be equal + root1 = bsp.root + root2 = bsp.root + assert root1 == root2, "Same node should be equal" + + # Different nodes should not be equal + leaves = list(bsp.leaves()) + assert len(leaves) >= 2, "Need at least 2 leaves for comparison" + assert leaves[0] != leaves[1], "Different nodes should not be equal" + + # Parent vs child should not be equal + root = bsp.root + left = root.left + assert root != left, "Parent and child should not be equal" + + # Not equal to non-BSPNode + assert not (root == "not a node"), "BSPNode should not equal string" + assert not (root == 42), "BSPNode should not equal int" + + print(" BSPNode equality: PASS") + def run_all_tests(): """Run all BSP tests.""" print("\n=== BSP Unit Tests ===\n") @@ -392,6 +549,12 @@ def run_all_tests(): test_traversal_enum() test_bsp_chaining() test_bsp_repr() + test_bsp_stale_node_detection() + test_bsp_grid_max_validation() + test_bsp_depth_cap() + test_bsp_len() + test_bsp_iter() + test_bspnode_equality() print("\n=== ALL BSP TESTS PASSED ===\n") sys.exit(0)