HeightMap: improve API consistency and add subscript support

Position argument flexibility:
- get(), get_interpolated(), get_slope(), get_normal() now accept:
  - Two separate args: hmap.get(5, 5)
  - Tuple: hmap.get((5, 5))
  - List: hmap.get([5, 5])
  - Vector: hmap.get(mcrfpy.Vector(5, 5))
- Uses PyPositionHelper for standardized parsing

Subscript support:
- Add __getitem__ as shorthand for get(): hmap[5, 5] or hmap[(5, 5)]

Range validation:
- count_in_range() now raises ValueError when min > max
- count_in_range() accepts both tuple and list

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

src/PyHeightMap.cpp

@@ -1,6 +1,7 @@
 #include "PyHeightMap.h"
 #include "McRFPy_API.h"
 #include "McRFPy_Doc.h"
+#include "PyPositionHelper.h"  // Standardized position argument parsing
 #include <sstream>
 
 // Property definitions
@@ -10,6 +11,13 @@ PyGetSetDef PyHeightMap::getsetters[] = {
     {NULL}
 };
 
+// Mapping methods for subscript support (hmap[x, y])
+PyMappingMethods PyHeightMap::mapping_methods = {
+    .mp_length = nullptr,  // __len__ not needed
+    .mp_subscript = (binaryfunc)PyHeightMap::subscript,  // __getitem__
+    .mp_ass_subscript = nullptr  // __setitem__ (read-only for now)
+};
+
 // Method definitions
 PyMethodDef PyHeightMap::methods[] = {
     {"fill", (PyCFunction)PyHeightMap::fill, METH_VARARGS,
@@ -61,38 +69,38 @@ PyMethodDef PyHeightMap::methods[] = {
          MCRF_RETURNS("HeightMap: self, for method chaining")
      )},
     // Query methods (#196)
-    {"get", (PyCFunction)PyHeightMap::get, METH_VARARGS,
+    {"get", (PyCFunction)PyHeightMap::get, METH_VARARGS | METH_KEYWORDS,
      MCRF_METHOD(HeightMap, get,
-         MCRF_SIG("(pos: tuple[int, int])", "float"),
+         MCRF_SIG("(x, y) or (pos)", "float"),
          MCRF_DESC("Get the height value at integer coordinates."),
          MCRF_ARGS_START
-         MCRF_ARG("pos", "Position as (x, y) tuple")
+         MCRF_ARG("x, y", "Position as two ints, tuple, list, or Vector")
          MCRF_RETURNS("float: Height value at that position")
          MCRF_RAISES("IndexError", "Position is out of bounds")
      )},
-    {"get_interpolated", (PyCFunction)PyHeightMap::get_interpolated, METH_VARARGS,
+    {"get_interpolated", (PyCFunction)PyHeightMap::get_interpolated, METH_VARARGS | METH_KEYWORDS,
      MCRF_METHOD(HeightMap, get_interpolated,
-         MCRF_SIG("(pos: tuple[float, float])", "float"),
+         MCRF_SIG("(x, y) or (pos)", "float"),
          MCRF_DESC("Get interpolated height value at non-integer coordinates."),
          MCRF_ARGS_START
-         MCRF_ARG("pos", "Position as (x, y) tuple with float coordinates")
+         MCRF_ARG("x, y", "Position as two floats, tuple, list, or Vector")
          MCRF_RETURNS("float: Bilinearly interpolated height value")
      )},
-    {"get_slope", (PyCFunction)PyHeightMap::get_slope, METH_VARARGS,
+    {"get_slope", (PyCFunction)PyHeightMap::get_slope, METH_VARARGS | METH_KEYWORDS,
      MCRF_METHOD(HeightMap, get_slope,
-         MCRF_SIG("(pos: tuple[int, int])", "float"),
+         MCRF_SIG("(x, y) or (pos)", "float"),
          MCRF_DESC("Get the slope at integer coordinates, from 0 (flat) to pi/2 (vertical)."),
          MCRF_ARGS_START
-         MCRF_ARG("pos", "Position as (x, y) tuple")
+         MCRF_ARG("x, y", "Position as two ints, tuple, list, or Vector")
          MCRF_RETURNS("float: Slope angle in radians (0 to pi/2)")
          MCRF_RAISES("IndexError", "Position is out of bounds")
      )},
     {"get_normal", (PyCFunction)PyHeightMap::get_normal, METH_VARARGS | METH_KEYWORDS,
      MCRF_METHOD(HeightMap, get_normal,
-         MCRF_SIG("(pos: tuple[float, float], water_level: float = 0.0)", "tuple[float, float, float]"),
+         MCRF_SIG("(x, y, water_level=0.0) or (pos, water_level=0.0)", "tuple[float, float, float]"),
          MCRF_DESC("Get the normal vector at given coordinates for lighting calculations."),
          MCRF_ARGS_START
-         MCRF_ARG("pos", "Position as (x, y) tuple with float coordinates")
+         MCRF_ARG("x, y", "Position as two floats, tuple, list, or Vector")
          MCRF_ARG("water_level", "Water level below which terrain is considered flat (default 0.0)")
          MCRF_RETURNS("tuple[float, float, float]: Normal vector (nx, ny, nz)")
      )},
@@ -107,8 +115,9 @@ PyMethodDef PyHeightMap::methods[] = {
          MCRF_SIG("(range: tuple[float, float])", "int"),
          MCRF_DESC("Count cells with values in the specified range (inclusive)."),
          MCRF_ARGS_START
-         MCRF_ARG("range", "Value range as (min, max) tuple")
+         MCRF_ARG("range", "Value range as (min, max) tuple or list")
          MCRF_RETURNS("int: Number of cells with values in range")
+         MCRF_RAISES("ValueError", "min > max")
      )},
     {NULL}
 };
@@ -353,29 +362,16 @@ PyObject* PyHeightMap::normalize(PyHeightMapObject* self, PyObject* args, PyObje
 
 // Query methods (#196)
 
-// Method: get(pos) -> float
+// Method: get(x, y) or get(pos) -> float
-PyObject* PyHeightMap::get(PyHeightMapObject* self, PyObject* args)
+PyObject* PyHeightMap::get(PyHeightMapObject* self, PyObject* args, PyObject* kwds)
 {
-    PyObject* pos_obj = nullptr;
-    if (!PyArg_ParseTuple(args, "O", &pos_obj)) {
-        return nullptr;
-    }
-
     if (!self->heightmap) {
         PyErr_SetString(PyExc_RuntimeError, "HeightMap not initialized");
         return nullptr;
     }
 
-    // Parse position tuple
+    int x, y;
-    if (!PyTuple_Check(pos_obj) || PyTuple_Size(pos_obj) != 2) {
+    if (!PyPosition_ParseInt(args, kwds, &x, &y)) {
-        PyErr_SetString(PyExc_TypeError, "pos must be a tuple of (x, y)");
-        return nullptr;
-    }
-
-    int x = (int)PyLong_AsLong(PyTuple_GetItem(pos_obj, 0));
-    int y = (int)PyLong_AsLong(PyTuple_GetItem(pos_obj, 1));
-
-    if (PyErr_Occurred()) {
         return nullptr;
     }
 
@@ -391,29 +387,16 @@ PyObject* PyHeightMap::get(PyHeightMapObject* self, PyObject* args)
     return PyFloat_FromDouble(value);
 }
 
-// Method: get_interpolated(pos) -> float
+// Method: get_interpolated(x, y) or get_interpolated(pos) -> float
-PyObject* PyHeightMap::get_interpolated(PyHeightMapObject* self, PyObject* args)
+PyObject* PyHeightMap::get_interpolated(PyHeightMapObject* self, PyObject* args, PyObject* kwds)
 {
-    PyObject* pos_obj = nullptr;
-    if (!PyArg_ParseTuple(args, "O", &pos_obj)) {
-        return nullptr;
-    }
-
     if (!self->heightmap) {
         PyErr_SetString(PyExc_RuntimeError, "HeightMap not initialized");
         return nullptr;
     }
 
-    // Parse position tuple (floats)
+    float x, y;
-    if (!PyTuple_Check(pos_obj) || PyTuple_Size(pos_obj) != 2) {
+    if (!PyPosition_ParseFloat(args, kwds, &x, &y)) {
-        PyErr_SetString(PyExc_TypeError, "pos must be a tuple of (x, y)");
-        return nullptr;
-    }
-
-    float x = (float)PyFloat_AsDouble(PyTuple_GetItem(pos_obj, 0));
-    float y = (float)PyFloat_AsDouble(PyTuple_GetItem(pos_obj, 1));
-
-    if (PyErr_Occurred()) {
         return nullptr;
     }
 
@@ -421,29 +404,16 @@ PyObject* PyHeightMap::get_interpolated(PyHeightMapObject* self, PyObject* args)
     return PyFloat_FromDouble(value);
 }
 
-// Method: get_slope(pos) -> float
+// Method: get_slope(x, y) or get_slope(pos) -> float
-PyObject* PyHeightMap::get_slope(PyHeightMapObject* self, PyObject* args)
+PyObject* PyHeightMap::get_slope(PyHeightMapObject* self, PyObject* args, PyObject* kwds)
 {
-    PyObject* pos_obj = nullptr;
-    if (!PyArg_ParseTuple(args, "O", &pos_obj)) {
-        return nullptr;
-    }
-
     if (!self->heightmap) {
         PyErr_SetString(PyExc_RuntimeError, "HeightMap not initialized");
         return nullptr;
     }
 
-    // Parse position tuple
+    int x, y;
-    if (!PyTuple_Check(pos_obj) || PyTuple_Size(pos_obj) != 2) {
+    if (!PyPosition_ParseInt(args, kwds, &x, &y)) {
-        PyErr_SetString(PyExc_TypeError, "pos must be a tuple of (x, y)");
-        return nullptr;
-    }
-
-    int x = (int)PyLong_AsLong(PyTuple_GetItem(pos_obj, 0));
-    int y = (int)PyLong_AsLong(PyTuple_GetItem(pos_obj, 1));
-
-    if (PyErr_Occurred()) {
         return nullptr;
     }
 
@@ -459,33 +429,32 @@ PyObject* PyHeightMap::get_slope(PyHeightMapObject* self, PyObject* args)
     return PyFloat_FromDouble(slope);
 }
 
-// Method: get_normal(pos, water_level=0.0) -> tuple[float, float, float]
+// Method: get_normal(x, y, water_level=0.0) or get_normal(pos, water_level=0.0) -> tuple[float, float, float]
 PyObject* PyHeightMap::get_normal(PyHeightMapObject* self, PyObject* args, PyObject* kwds)
 {
-    static const char* keywords[] = {"pos", "water_level", nullptr};
-    PyObject* pos_obj = nullptr;
-    float water_level = 0.0f;
-
-    if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|f", const_cast<char**>(keywords),
-                                     &pos_obj, &water_level)) {
-        return nullptr;
-    }
-
     if (!self->heightmap) {
         PyErr_SetString(PyExc_RuntimeError, "HeightMap not initialized");
         return nullptr;
     }
 
-    // Parse position tuple (floats)
+    // Check for water_level keyword argument
-    if (!PyTuple_Check(pos_obj) || PyTuple_Size(pos_obj) != 2) {
+    float water_level = 0.0f;
-        PyErr_SetString(PyExc_TypeError, "pos must be a tuple of (x, y)");
+    if (kwds) {
-        return nullptr;
+        PyObject* wl_obj = PyDict_GetItemString(kwds, "water_level");
+        if (wl_obj) {
+            if (PyFloat_Check(wl_obj)) {
+                water_level = (float)PyFloat_AsDouble(wl_obj);
+            } else if (PyLong_Check(wl_obj)) {
+                water_level = (float)PyLong_AsLong(wl_obj);
+            } else {
+                PyErr_SetString(PyExc_TypeError, "water_level must be a number");
+                return nullptr;
+            }
+        }
     }
 
-    float x = (float)PyFloat_AsDouble(PyTuple_GetItem(pos_obj, 0));
+    float x, y;
-    float y = (float)PyFloat_AsDouble(PyTuple_GetItem(pos_obj, 1));
+    if (!PyPosition_ParseFloat(args, kwds, &x, &y)) {
-
-    if (PyErr_Occurred()) {
         return nullptr;
     }
 
@@ -522,19 +491,66 @@ PyObject* PyHeightMap::count_in_range(PyHeightMapObject* self, PyObject* args)
         return nullptr;
     }
 
-    // Parse range tuple
+    // Parse range from tuple or list
-    if (!PyTuple_Check(range_obj) || PyTuple_Size(range_obj) != 2) {
+    float min_val, max_val;
-        PyErr_SetString(PyExc_TypeError, "range must be a tuple of (min, max)");
+    if (PyTuple_Check(range_obj) && PyTuple_Size(range_obj) == 2) {
+        PyObject* min_obj = PyTuple_GetItem(range_obj, 0);
+        PyObject* max_obj = PyTuple_GetItem(range_obj, 1);
+        if (PyFloat_Check(min_obj)) min_val = (float)PyFloat_AsDouble(min_obj);
+        else if (PyLong_Check(min_obj)) min_val = (float)PyLong_AsLong(min_obj);
+        else { PyErr_SetString(PyExc_TypeError, "range values must be numeric"); return nullptr; }
+        if (PyFloat_Check(max_obj)) max_val = (float)PyFloat_AsDouble(max_obj);
+        else if (PyLong_Check(max_obj)) max_val = (float)PyLong_AsLong(max_obj);
+        else { PyErr_SetString(PyExc_TypeError, "range values must be numeric"); return nullptr; }
+    } else if (PyList_Check(range_obj) && PyList_Size(range_obj) == 2) {
+        PyObject* min_obj = PyList_GetItem(range_obj, 0);
+        PyObject* max_obj = PyList_GetItem(range_obj, 1);
+        if (PyFloat_Check(min_obj)) min_val = (float)PyFloat_AsDouble(min_obj);
+        else if (PyLong_Check(min_obj)) min_val = (float)PyLong_AsLong(min_obj);
+        else { PyErr_SetString(PyExc_TypeError, "range values must be numeric"); return nullptr; }
+        if (PyFloat_Check(max_obj)) max_val = (float)PyFloat_AsDouble(max_obj);
+        else if (PyLong_Check(max_obj)) max_val = (float)PyLong_AsLong(max_obj);
+        else { PyErr_SetString(PyExc_TypeError, "range values must be numeric"); return nullptr; }
+    } else {
+        PyErr_SetString(PyExc_TypeError, "range must be a tuple or list of (min, max)");
         return nullptr;
     }
 
-    float min_val = (float)PyFloat_AsDouble(PyTuple_GetItem(range_obj, 0));
-    float max_val = (float)PyFloat_AsDouble(PyTuple_GetItem(range_obj, 1));
-
     if (PyErr_Occurred()) {
         return nullptr;
     }
 
+    // Validate range
+    if (min_val > max_val) {
+        PyErr_SetString(PyExc_ValueError, "range min must be less than or equal to max");
+        return nullptr;
+    }
+
     int count = TCOD_heightmap_count_cells(self->heightmap, min_val, max_val);
     return PyLong_FromLong(count);
 }
+
+// Subscript: hmap[x, y] -> float (shorthand for get())
+PyObject* PyHeightMap::subscript(PyHeightMapObject* self, PyObject* key)
+{
+    if (!self->heightmap) {
+        PyErr_SetString(PyExc_RuntimeError, "HeightMap not initialized");
+        return nullptr;
+    }
+
+    int x, y;
+    if (!PyPosition_FromObjectInt(key, &x, &y)) {
+        return nullptr;
+    }
+
+    // Bounds check
+    if (x < 0 || x >= self->heightmap->w || y < 0 || y >= self->heightmap->h) {
+        PyErr_Format(PyExc_IndexError,
+            "Position (%d, %d) out of bounds for HeightMap of size (%d, %d)",
+            x, y, self->heightmap->w, self->heightmap->h);
+        return nullptr;
+    }
+
+    float value = TCOD_heightmap_get_value(self->heightmap, x, y);
+    return PyFloat_FromDouble(value);
+}

src/PyHeightMap.h

@@ -33,13 +33,19 @@ public:
     static PyObject* normalize(PyHeightMapObject* self, PyObject* args, PyObject* kwds);
 
     // Query methods (#196)
-    static PyObject* get(PyHeightMapObject* self, PyObject* args);
+    static PyObject* get(PyHeightMapObject* self, PyObject* args, PyObject* kwds);
-    static PyObject* get_interpolated(PyHeightMapObject* self, PyObject* args);
+    static PyObject* get_interpolated(PyHeightMapObject* self, PyObject* args, PyObject* kwds);
-    static PyObject* get_slope(PyHeightMapObject* self, PyObject* args);
+    static PyObject* get_slope(PyHeightMapObject* self, PyObject* args, PyObject* kwds);
     static PyObject* get_normal(PyHeightMapObject* self, PyObject* args, PyObject* kwds);
     static PyObject* min_max(PyHeightMapObject* self, PyObject* Py_UNUSED(args));
     static PyObject* count_in_range(PyHeightMapObject* self, PyObject* args);
 
+    // Subscript support for hmap[x, y] syntax
+    static PyObject* subscript(PyHeightMapObject* self, PyObject* key);
+
+    // Mapping methods for subscript support
+    static PyMappingMethods mapping_methods;
+
     // Method and property definitions
     static PyMethodDef methods[];
     static PyGetSetDef getsetters[];
@@ -53,6 +59,7 @@ namespace mcrfpydef {
         .tp_itemsize = 0,
         .tp_dealloc = (destructor)PyHeightMap::dealloc,
         .tp_repr = PyHeightMap::repr,
+        .tp_as_mapping = &PyHeightMap::mapping_methods,  // hmap[x, y] subscript
         .tp_flags = Py_TPFLAGS_DEFAULT,
         .tp_doc = PyDoc_STR(
             "HeightMap(size: tuple[int, int], fill: float = 0.0)\n\n"
@@ -66,6 +73,7 @@ namespace mcrfpydef {
             "Example:\n"
             "    hmap = mcrfpy.HeightMap((100, 100))\n"
             "    hmap.fill(0.5).scale(2.0).clamp(0.0, 1.0)\n"
+            "    value = hmap[5, 5]  # Subscript shorthand for get()\n"
         ),
         .tp_methods = nullptr,  // Set in McRFPy_API.cpp before PyType_Ready
         .tp_getset = nullptr,   // Set in McRFPy_API.cpp before PyType_Ready

tests/unit/test_heightmap_query.py

@@ -45,18 +45,24 @@ def test_get_out_of_bounds():
     print("PASS: test_get_out_of_bounds")
 
 
-def test_get_invalid_type():
+def test_get_flexible_input():
-    """get() raises TypeError for invalid position"""
+    """get() accepts tuple, list, Vector, and two args"""
-    hmap = mcrfpy.HeightMap((10, 10))
+    hmap = mcrfpy.HeightMap((10, 10), fill=0.5)
 
-    try:
+    # Tuple works
-        hmap.get([5, 5])  # list instead of tuple
+    assert abs(hmap.get((5, 5)) - 0.5) < 0.001
-        print("FAIL: test_get_invalid_type - should have raised TypeError")
-        sys.exit(1)
-    except TypeError:
-        pass
 
-    print("PASS: test_get_invalid_type")
+    # List works
+    assert abs(hmap.get([5, 5]) - 0.5) < 0.001
+
+    # Two args work (no tuple needed)
+    assert abs(hmap.get(5, 5) - 0.5) < 0.001
+
+    # Vector works
+    vec = mcrfpy.Vector(5, 5)
+    assert abs(hmap.get(vec) - 0.5) < 0.001
+
+    print("PASS: test_get_flexible_input")
 
 
 def test_get_interpolated_basic():
@@ -177,18 +183,75 @@ def test_count_in_range_exact():
     print("PASS: test_count_in_range_exact")
 
 
-def test_count_in_range_invalid():
+def test_count_in_range_accepts_list():
-    """count_in_range() raises TypeError for invalid range"""
+    """count_in_range() accepts list or tuple"""
+    hmap = mcrfpy.HeightMap((10, 10), fill=0.5)
+
+    # Tuple works
+    count1 = hmap.count_in_range((0.0, 1.0))
+    assert count1 == 100
+
+    # List also works
+    count2 = hmap.count_in_range([0.0, 1.0])
+    assert count2 == 100
+
+    print("PASS: test_count_in_range_accepts_list")
+
+
+def test_count_in_range_invalid_range():
+    """count_in_range() raises ValueError when min > max"""
+    hmap = mcrfpy.HeightMap((10, 10), fill=0.5)
+
+    try:
+        hmap.count_in_range((1.0, 0.0))  # min > max
+        print("FAIL: test_count_in_range_invalid_range - should have raised ValueError")
+        sys.exit(1)
+    except ValueError as e:
+        assert "min" in str(e).lower()
+
+    print("PASS: test_count_in_range_invalid_range")
+
+
+def test_subscript_basic():
+    """hmap[x, y] works as shorthand for get()"""
+    hmap = mcrfpy.HeightMap((10, 10), fill=0.75)
+
+    # Subscript with tuple
+    value = hmap[5, 5]
+    assert abs(value - 0.75) < 0.001
+
+    print("PASS: test_subscript_basic")
+
+
+def test_subscript_flexible():
+    """hmap[] accepts tuple, list, Vector"""
+    hmap = mcrfpy.HeightMap((10, 10), fill=0.25)
+
+    # Tuple
+    assert abs(hmap[(3, 4)] - 0.25) < 0.001
+
+    # List
+    assert abs(hmap[[3, 4]] - 0.25) < 0.001
+
+    # Vector
+    vec = mcrfpy.Vector(3, 4)
+    assert abs(hmap[vec] - 0.25) < 0.001
+
+    print("PASS: test_subscript_flexible")
+
+
+def test_subscript_out_of_bounds():
+    """hmap[] raises IndexError for out-of-bounds"""
     hmap = mcrfpy.HeightMap((10, 10))
 
     try:
-        hmap.count_in_range([0.0, 1.0])  # list instead of tuple
+        _ = hmap[10, 5]
-        print("FAIL: test_count_in_range_invalid - should have raised TypeError")
+        print("FAIL: test_subscript_out_of_bounds - should have raised IndexError")
         sys.exit(1)
-    except TypeError:
+    except IndexError:
         pass
 
-    print("PASS: test_count_in_range_invalid")
+    print("PASS: test_subscript_out_of_bounds")
 
 
 def run_all_tests():
@@ -199,7 +262,7 @@ def run_all_tests():
     test_get_basic()
     test_get_corners()
     test_get_out_of_bounds()
-    test_get_invalid_type()
+    test_get_flexible_input()
     test_get_interpolated_basic()
     test_get_interpolated_at_integers()
     test_get_slope_flat()
@@ -211,7 +274,11 @@ def run_all_tests():
     test_count_in_range_all()
     test_count_in_range_none()
     test_count_in_range_exact()
-    test_count_in_range_invalid()
+    test_count_in_range_accepts_list()
+    test_count_in_range_invalid_range()
+    test_subscript_basic()
+    test_subscript_flexible()
+    test_subscript_out_of_bounds()
 
     print()
     print("All HeightMap query method tests PASSED!")