Merge pull request #198 from antonsoo/fix/automatic-diagonal-heuristic

Bound automatic diagonal heuristics by two cardinal steps

Author: HexDecimal

CHANGELOG.md

@@ -8,6 +8,7 @@ This project adheres to [Semantic Versioning](https://semver.org/) since version
 
 ### Fixed
 
+- `tcod.path.SimpleGraph` caps its automatic diagonal heuristic at two cardinal steps when those are cheaper.
 - Many-to-many `tcod.path.path2d` queries retain weighted predecessors when reconstructing their route.
 - Copying or pickling Perlin and Simplex noise no longer advances a shared random generator while preparing an unused wavelet tile.
 - `tcod.image.Image.rotate90` updates the width and height of rectangular images, keeping NumPy array conversions consistent with the rotated pixels.

tcod/path.py

@@ -1066,7 +1066,10 @@ def __init__(self, *, cost: ArrayLike, cardinal: int, diagonal: int, greed: int
         self._shape = self._subgraph._shape[0], self._subgraph._shape[1]
         self._shape_c = self._subgraph._shape_c
         self._subgraph.add_edges(edge_map=edge_map, cost=cost)
-        self.set_heuristic(cardinal=cardinal * greed, diagonal=diagonal * greed)
+        self.set_heuristic(
+            cardinal=cardinal * greed,
+            diagonal=(min(diagonal, cardinal * 2) if cardinal else diagonal) * greed,
+        )
 
     @property
     def ndim(self) -> int:
@@ -1091,7 +1094,8 @@ def set_heuristic(self, *, cardinal: int, diagonal: int) -> None:
         `cardinal` and `diagonal` are weights for the heuristic.
         Higher values are more greedy.
         The default values are set to ``cardinal * greed`` and
-        ``diagonal * greed`` when the :any:`SimpleGraph` is created.
+        ``diagonal * greed`` when the :any:`SimpleGraph` is created, with the
+        diagonal estimate capped at two cardinal steps if cardinal movement is enabled.
         """
         self._subgraph.set_heuristic(cardinal=cardinal, diagonal=diagonal)
 

tests/test_simplegraph_heuristic.py

@@ -0,0 +1,34 @@
+"""Automatic heuristics must account for cheaper cardinal alternatives."""
+
+from typing import Literal
+
+import numpy as np
+import pytest
+
+import tcod.path
+
+
+@pytest.mark.parametrize("order", ["C", "F"])
+def test_expensive_diagonal_heuristic(order: Literal["C", "F"]) -> None:
+    """Eight cardinal steps beat the tempting nine-cost mixed route."""
+    cost = np.array(
+        [[1, 0, 0, 0, 1], [1, 1, 1, 1, 1], [1, 0, 1, 0, 1], [0, 1, 1, 1, 1], [0, 0, 1, 1, 1]],
+        dtype=np.int16,
+        order=order,
+    )
+    graph = tcod.path.SimpleGraph(cost=cost, cardinal=1, diagonal=3)
+    pathfinder = tcod.path.Pathfinder(graph)
+    pathfinder.add_root((0, 0))
+    path = pathfinder.path_to((4, 4))
+    total = sum(3 if np.all(a != b) else 1 for a, b in zip(path, path[1:]))
+    assert total == 8
+    assert pathfinder.distance[4, 4] == 8
+
+
+def test_diagonal_only_graph() -> None:
+    """Disabling cardinal movement retains the diagonal estimate and edges."""
+    graph = tcod.path.SimpleGraph(cost=np.ones((3, 3), dtype=np.int16), cardinal=0, diagonal=3)
+    pathfinder = tcod.path.Pathfinder(graph)
+    pathfinder.add_root((0, 0))
+    np.testing.assert_array_equal(pathfinder.path_to((2, 2)), [[0, 0], [1, 1], [2, 2]])
+    assert pathfinder.distance[2, 2] == 6