diff --git a/src/PyHeightMap.cpp b/src/PyHeightMap.cpp index f435893..7b56e9c 100644 --- a/src/PyHeightMap.cpp +++ b/src/PyHeightMap.cpp @@ -160,14 +160,14 @@ PyMethodDef PyHeightMap::methods[] = { )}, {"dig_hill", (PyCFunction)PyHeightMap::dig_hill, METH_VARARGS | METH_KEYWORDS, MCRF_METHOD(HeightMap, dig_hill, - MCRF_SIG("(center, radius: float, depth: float)", "HeightMap"), - MCRF_DESC("Dig a smooth crater at the specified position. Use negative depth to dig below current terrain."), + MCRF_SIG("(center, radius: float, target_height: float)", "HeightMap"), + MCRF_DESC("Construct a pit or crater with the specified center height."), MCRF_ARGS_START MCRF_ARG("center", "Center position as (x, y) tuple, list, or Vector") MCRF_ARG("radius", "Radius of the crater in cells") - MCRF_ARG("depth", "Target depth (use negative to dig below current values)") + MCRF_ARG("target_height", "Height at the center of the pit") MCRF_RETURNS("HeightMap: self, for method chaining") - MCRF_NOTE("Only modifies cells where current value exceeds target depth") + MCRF_NOTE("Only lowers cells; cells below target_height are unchanged") )}, {"add_voronoi", (PyCFunction)PyHeightMap::add_voronoi, METH_VARARGS | METH_KEYWORDS, MCRF_METHOD(HeightMap, add_voronoi, @@ -202,16 +202,16 @@ PyMethodDef PyHeightMap::methods[] = { )}, {"dig_bezier", (PyCFunction)PyHeightMap::dig_bezier, METH_VARARGS | METH_KEYWORDS, MCRF_METHOD(HeightMap, dig_bezier, - MCRF_SIG("(points: tuple, start_radius: float, end_radius: float, start_depth: float, end_depth: float)", "HeightMap"), - MCRF_DESC("Carve a path along a cubic Bezier curve. Use negative depths to dig below current terrain."), + MCRF_SIG("(points: tuple, start_radius: float, end_radius: float, start_height: float, end_height: float)", "HeightMap"), + MCRF_DESC("Construct a canal along a cubic Bezier curve with specified heights."), MCRF_ARGS_START MCRF_ARG("points", "Four control points as ((x0,y0), (x1,y1), (x2,y2), (x3,y3))") MCRF_ARG("start_radius", "Radius at start of path") MCRF_ARG("end_radius", "Radius at end of path") - MCRF_ARG("start_depth", "Target depth at start (use negative to dig)") - MCRF_ARG("end_depth", "Target depth at end (use negative to dig)") + MCRF_ARG("start_height", "Target height at start of path") + MCRF_ARG("end_height", "Target height at end of path") MCRF_RETURNS("HeightMap: self, for method chaining") - MCRF_NOTE("Only modifies cells where current value exceeds target depth") + MCRF_NOTE("Only lowers cells; cells below target height are unchanged") )}, {"smooth", (PyCFunction)PyHeightMap::smooth, METH_VARARGS | METH_KEYWORDS, MCRF_METHOD(HeightMap, smooth, @@ -880,21 +880,29 @@ PyObject* PyHeightMap::add_hill(PyHeightMapObject* self, PyObject* args, PyObjec return nullptr; } + // Warn on zero/negative radius (no-op but likely user error) + if (radius <= 0) { + if (PyErr_WarnEx(PyExc_UserWarning, + "add_hill called with radius <= 0; no cells will be modified", 1) < 0) { + return nullptr; + } + } + TCOD_heightmap_add_hill(self->heightmap, cx, cy, radius, height); Py_INCREF(self); return (PyObject*)self; } -// Method: dig_hill(center, radius, depth) -> HeightMap +// Method: dig_hill(center, radius, target_height) -> HeightMap PyObject* PyHeightMap::dig_hill(PyHeightMapObject* self, PyObject* args, PyObject* kwds) { - static const char* keywords[] = {"center", "radius", "depth", nullptr}; + static const char* keywords[] = {"center", "radius", "target_height", nullptr}; PyObject* center_obj = nullptr; - float radius, depth; + float radius, target_height; if (!PyArg_ParseTupleAndKeywords(args, kwds, "Off", const_cast(keywords), - ¢er_obj, &radius, &depth)) { + ¢er_obj, &radius, &target_height)) { return nullptr; } @@ -908,7 +916,15 @@ PyObject* PyHeightMap::dig_hill(PyHeightMapObject* self, PyObject* args, PyObjec return nullptr; } - TCOD_heightmap_dig_hill(self->heightmap, cx, cy, radius, depth); + // Warn on zero/negative radius (no-op but likely user error) + if (radius <= 0) { + if (PyErr_WarnEx(PyExc_UserWarning, + "dig_hill called with radius <= 0; no cells will be modified", 1) < 0) { + return nullptr; + } + } + + TCOD_heightmap_dig_hill(self->heightmap, cx, cy, radius, target_height); Py_INCREF(self); return (PyObject*)self; @@ -1066,15 +1082,15 @@ PyObject* PyHeightMap::rain_erosion(PyHeightMapObject* self, PyObject* args, PyO return (PyObject*)self; } -// Method: dig_bezier(points, start_radius, end_radius, start_depth, end_depth) -> HeightMap +// Method: dig_bezier(points, start_radius, end_radius, start_height, end_height) -> HeightMap PyObject* PyHeightMap::dig_bezier(PyHeightMapObject* self, PyObject* args, PyObject* kwds) { - static const char* keywords[] = {"points", "start_radius", "end_radius", "start_depth", "end_depth", nullptr}; + static const char* keywords[] = {"points", "start_radius", "end_radius", "start_height", "end_height", nullptr}; PyObject* points_obj = nullptr; - float start_radius, end_radius, start_depth, end_depth; + float start_radius, end_radius, start_height, end_height; if (!PyArg_ParseTupleAndKeywords(args, kwds, "Offff", const_cast(keywords), - &points_obj, &start_radius, &end_radius, &start_depth, &end_depth)) { + &points_obj, &start_radius, &end_radius, &start_height, &end_height)) { return nullptr; } @@ -1107,7 +1123,15 @@ PyObject* PyHeightMap::dig_bezier(PyHeightMapObject* self, PyObject* args, PyObj py[i] = y; } - TCOD_heightmap_dig_bezier(self->heightmap, px, py, start_radius, start_depth, end_radius, end_depth); + // Warn on zero/negative radii (no-op but likely user error) + if (start_radius <= 0 || end_radius <= 0) { + if (PyErr_WarnEx(PyExc_UserWarning, + "dig_bezier called with radius <= 0; some or all cells may not be modified", 1) < 0) { + return nullptr; + } + } + + TCOD_heightmap_dig_bezier(self->heightmap, px, py, start_radius, start_height, end_radius, end_height); Py_INCREF(self); return (PyObject*)self; diff --git a/tests/unit/test_heightmap_terrain.py b/tests/unit/test_heightmap_terrain.py index a49af36..f33d265 100644 --- a/tests/unit/test_heightmap_terrain.py +++ b/tests/unit/test_heightmap_terrain.py @@ -48,13 +48,13 @@ def test_add_hill_flexible_center(): def test_dig_hill_basic(): - """dig_hill() creates depression at center using negative depth""" + """dig_hill() creates depression at center using target_height""" hmap = mcrfpy.HeightMap((50, 50), fill=0.5) - # dig_hill with negative depth creates a crater by setting values - # to max(current, target_depth) where target curves from center - hmap.dig_hill((25, 25), radius=15.0, depth=-0.3) + # dig_hill constructs a pit with target_height at center + # Only lowers cells - cells below target_height are unchanged + hmap.dig_hill((25, 25), radius=15.0, target_height=-0.3) - # Center should have lowest value (set to the negative depth) + # Center should have lowest value (set to the target_height) center_val = hmap[25, 25] edge_val = hmap[0, 0] @@ -192,25 +192,27 @@ def test_rain_erosion_invalid_drops(): def test_dig_bezier_basic(): - """dig_bezier() carves a path using negative depths""" + """dig_bezier() constructs a canal with target heights""" hmap = mcrfpy.HeightMap((50, 50), fill=0.5) - # Carve a path from corner to corner - # Use negative depths to actually dig below current terrain + # Construct a canal from corner to corner + # target_height sets the center height of the canal points = ((0, 0), (10, 25), (40, 25), (49, 49)) hmap.dig_bezier(points, start_radius=5.0, end_radius=5.0, - start_depth=-0.3, end_depth=-0.3) + start_height=-0.3, end_height=-0.3) - # Start and end should be lower than original (carved out) - assert hmap[0, 0] < 0.5, f"Start should be carved, got {hmap[0, 0]}" - assert hmap[0, 0] < 0, f"Start should be negative, got {hmap[0, 0]}" + # Start and end should be at the target height (lowered) + assert hmap[0, 0] < 0.5, f"Start should be lowered, got {hmap[0, 0]}" + assert hmap[0, 0] < 0, f"Start should be at target height, got {hmap[0, 0]}" print("PASS: test_dig_bezier_basic") def test_dig_bezier_returns_self(): """dig_bezier() returns self for chaining""" hmap = mcrfpy.HeightMap((20, 20)) - result = hmap.dig_bezier(((0, 0), (5, 10), (15, 10), (19, 19)), 2.0, 2.0, 0.3, 0.3) + result = hmap.dig_bezier(((0, 0), (5, 10), (15, 10), (19, 19)), + start_radius=2.0, end_radius=2.0, + start_height=0.3, end_height=0.3) assert result is hmap print("PASS: test_dig_bezier_returns_self") @@ -220,7 +222,9 @@ def test_dig_bezier_wrong_point_count(): hmap = mcrfpy.HeightMap((20, 20)) try: - hmap.dig_bezier(((0, 0), (5, 5), (10, 10)), 2.0, 2.0, 0.3, 0.3) # Only 3 points + hmap.dig_bezier(((0, 0), (5, 5), (10, 10)), + start_radius=2.0, end_radius=2.0, + start_height=0.3, end_height=0.3) # Only 3 points print("FAIL: should have raised ValueError for 3 points") sys.exit(1) except ValueError as e: @@ -233,7 +237,8 @@ def test_dig_bezier_accepts_list(): """dig_bezier() accepts list of points""" hmap = mcrfpy.HeightMap((20, 20), fill=0.5) points = [[0, 0], [5, 10], [15, 10], [19, 19]] # List instead of tuple - hmap.dig_bezier(points, 2.0, 2.0, 0.3, 0.3) + hmap.dig_bezier(points, start_radius=2.0, end_radius=2.0, + start_height=0.3, end_height=0.3) # Should complete without error print("PASS: test_dig_bezier_accepts_list") @@ -276,9 +281,62 @@ def test_smooth_invalid_iterations(): except ValueError: pass + try: + hmap.smooth(iterations=-5) + print("FAIL: should have raised ValueError for iterations=-5") + sys.exit(1) + except ValueError: + pass + print("PASS: test_smooth_invalid_iterations") +def test_add_hill_zero_radius_warning(): + """add_hill() warns on zero/negative radius""" + import warnings + hmap = mcrfpy.HeightMap((20, 20), fill=0.5) + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + hmap.add_hill((10, 10), radius=0.0, height=1.0) + assert len(w) == 1 + assert "radius <= 0" in str(w[0].message) + + # Map should be unchanged + assert abs(hmap[10, 10] - 0.5) < 0.001 + print("PASS: test_add_hill_zero_radius_warning") + + +def test_dig_hill_zero_radius_warning(): + """dig_hill() warns on zero/negative radius""" + import warnings + hmap = mcrfpy.HeightMap((20, 20), fill=0.5) + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + hmap.dig_hill((10, 10), radius=-1.0, target_height=0.0) + assert len(w) == 1 + assert "radius <= 0" in str(w[0].message) + + print("PASS: test_dig_hill_zero_radius_warning") + + +def test_dig_bezier_zero_radius_warning(): + """dig_bezier() warns on zero/negative radius""" + import warnings + hmap = mcrfpy.HeightMap((20, 20), fill=0.5) + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + hmap.dig_bezier(((0, 0), (5, 10), (15, 10), (19, 19)), + start_radius=0.0, end_radius=2.0, + start_height=0.3, end_height=0.3) + assert len(w) == 1 + assert "radius <= 0" in str(w[0].message) + + print("PASS: test_dig_bezier_zero_radius_warning") + + def test_chaining_terrain_methods(): """Terrain methods can be chained together""" hmap = mcrfpy.HeightMap((50, 50), fill=0.0) @@ -307,8 +365,10 @@ def test_terrain_pipeline(): # Add features hmap.add_hill((32, 32), 20.0, 0.3) # Mountain - # dig_bezier with negative depths to carve a river valley - hmap.dig_bezier(((0, 32), (20, 20), (45, 45), (64, 32)), 3.0, 2.0, -0.3, -0.2) + # dig_bezier constructs a river valley at target heights + hmap.dig_bezier(((0, 32), (20, 20), (45, 45), (64, 32)), + start_radius=3.0, end_radius=2.0, + start_height=-0.3, end_height=-0.2) # Apply erosion and smoothing hmap.rain_erosion(500, seed=123) @@ -350,6 +410,9 @@ def run_all_tests(): test_smooth_basic() test_smooth_returns_self() test_smooth_invalid_iterations() + test_add_hill_zero_radius_warning() + test_dig_hill_zero_radius_warning() + test_dig_bezier_zero_radius_warning() test_chaining_terrain_methods() test_terrain_pipeline()