HeightMap: improve dig_hill/dig_bezier API clarity

Rename parameters for clearer semantics:
- dig_hill: depth -> target_height
- dig_bezier: start_depth/end_depth -> start_height/end_height

The libtcod "dig" functions set terrain TO a target height, not
relative to current values. "target_height" makes this clearer.

Also add warnings for likely user errors:
- add_hill/dig_hill/dig_bezier with radius <= 0 (no-op)
- smooth with iterations <= 0 already raises ValueError

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

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_SIG("(center, radius: float, target_height: float)", "HeightMap"),
-         MCRF_DESC("Dig a smooth crater at the specified position. Use negative depth to dig below current terrain."),
+         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_SIG("(points: tuple, start_radius: float, end_radius: float, start_height: float, end_height: float)", "HeightMap"),
-         MCRF_DESC("Carve a path along a cubic Bezier curve. Use negative depths to dig below current terrain."),
+         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("start_height", "Target height at start of path")
-         MCRF_ARG("end_depth", "Target depth at end (use negative to dig)")
+         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<char**>(keywords),
-                                     &center_obj, &radius, &depth)) {
+                                     &center_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<char**>(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;

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
+    # dig_hill constructs a pit with target_height at center
-    # to max(current, target_depth) where target curves from center
+    # Only lowers cells - cells below target_height are unchanged
-    hmap.dig_hill((25, 25), radius=15.0, depth=-0.3)
+    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
+    # Construct a canal from corner to corner
-    # Use negative depths to actually dig below current terrain
+    # 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)
+    # Start and end should be at the target height (lowered)
-    assert hmap[0, 0] < 0.5, f"Start should be carved, got {hmap[0, 0]}"
+    assert hmap[0, 0] < 0.5, f"Start should be lowered, got {hmap[0, 0]}"
-    assert hmap[0, 0] < 0, f"Start should be negative, 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
+    # dig_bezier constructs a river valley at target heights
-    hmap.dig_bezier(((0, 32), (20, 20), (45, 45), (64, 32)), 3.0, 2.0, -0.3, -0.2)
+    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()