Name the bad point when a draw function gets malformed points

CHANGELOG.md

@@ -73,6 +73,7 @@ Arcade [PyPi Release History](https://pypi.org/project/arcade/#history) page.
 - Added `arcade.sweep_line(start, end, sprite_list)`, which returns a `SweepInfo` for the first sprite a line hits (its `fraction`, `distance`, and surface `normal`), or `None`. It's `sweep_sprite` for a point: for lasers, hitscan weapons, or seeing what's in the way. Added the `sprite_laser_mirrors` example, where a laser bounces off rotating mirrors using the normal.
 
 ### Misc Changes
+- `draw_lines`, `draw_points`, `draw_line_strip`, `draw_polygon_filled` and `draw_polygon_outline` raise a `ValueError` naming the first point that isn't 2 numbers, such as `point_list[2] is 7, but each point must be 2 numbers, such as (x, y)` ([#2215](https://github.com/pythonarcade/arcade/issues/2215)). Before, a bad point raised a confusing error like `'int' object is not iterable`, and in `draw_lines`, `draw_points` and `draw_line_strip` a point with 1 or 3 numbers silently shifted every number after it. The check costs one length comparison per call, and the points are now converted faster: drawing 1,000 lines or points went from about 137 to 90 µs per call.
 - The card game tutorial no longer passes `hit_box_algorithm="None"` to `Sprite`, an Arcade 2 argument that `Sprite` silently ignores. Its text no longer says hit box calculation is slow: loading all 52 cards with their default hit boxes takes about 40 ms.
 - Removed the docs build's workaround for Sphinx not copying changed CSS files (`util/sphinx_static_file_temp_fix.py` and its `.ENABLE_DEVMACHINE_SPHINX_STATIC_FIX` switch). Sphinx fixed it upstream, and the pinned Sphinx 9.1.0 copies changed CSS on incremental builds and with `make.py serve` ([#2266](https://github.com/pythonarcade/arcade/issues/2266)).
 - Docs fixes:

arcade/draw/helpers.py

@@ -1,11 +1,52 @@
 import array
 import math
+from itertools import chain
 
 from arcade import gl
 from arcade.types import Color, Point2, Point2List, RGBOrA255
 from arcade.window_commands import get_window
 
 
+def _bad_point_error(point_list: Point2List) -> ValueError | None:
+    """Return an error naming the first point that isn't 2 numbers, if any.
+
+    Only called after something went wrong, so it can be slow.
+    """
+    for index, point in enumerate(point_list):
+        try:
+            x, y = point
+            array.array("f", (x, y))
+        except (TypeError, ValueError, OverflowError):
+            return ValueError(
+                f"point_list[{index}] is {point!r}, but each point must be 2 numbers, "
+                "such as (x, y)"
+            )
+    return None
+
+
+def _flatten_points(point_list: Point2List) -> array.array:
+    """Flatten 2D points into an array of floats: x0, y0, x1, y1, ...
+
+    Raises a ValueError naming the first point that isn't 2 numbers.
+    Checking costs one length comparison, so it doesn't slow down drawing.
+    """
+    try:
+        data = array.array("f", tuple(chain.from_iterable(point_list)))
+    except (TypeError, ValueError, OverflowError) as error:
+        raise (_bad_point_error(point_list) or error) from None
+    # A point with 1 or 3 numbers would shift every number after it
+    if len(data) != 2 * len(point_list):
+        raise _bad_point_error(point_list) or ValueError("Each point must be 2 numbers")
+    return data
+
+
+def _check_points_after(error: Exception, point_list: Point2List) -> None:
+    """Raise a clearer error than ``error`` if a point isn't 2 numbers."""
+    clearer = _bad_point_error(point_list)
+    if clearer is not None:
+        raise clearer from None
+
+
 def get_points_for_thick_line(
     start_x: float, start_y: float, end_x: float, end_y: float, line_width: float
 ) -> tuple[Point2, Point2, Point2, Point2]:
@@ -72,7 +113,7 @@ def _generic_draw_line_strip(
 
     # Translate Python objects into types Arcade's Buffer objects accept
     color_array = array.array("B", rgba * num_vertices)
-    vertex_array = array.array("f", tuple(item for sublist in point_list for item in sublist))
+    vertex_array = _flatten_points(point_list)
     geometry.num_vertices = num_vertices
 
     # Double buffer sizes until they can hold all our data

arcade/draw/line.py

@@ -4,7 +4,12 @@
 from arcade.types import Color, Point2, Point2List, RGBOrA255
 from arcade.window_commands import get_window
 
-from .helpers import _generic_draw_line_strip, get_points_for_thick_line
+from .helpers import (
+    _check_points_after,
+    _flatten_points,
+    _generic_draw_line_strip,
+    get_points_for_thick_line,
+)
 
 
 def draw_line_strip(point_list: Point2List, color: RGBOrA255, line_width: float = 1) -> None:
@@ -26,14 +31,18 @@ def draw_line_strip(point_list: Point2List, color: RGBOrA255, line_width: float
         triangle_point_list: list[Point2] = []
         # FIXME: This needs a lot of improvement
         last_point = None
-        for point in point_list:
-            if last_point is not None:
-                points = get_points_for_thick_line(
-                    last_point[0], last_point[1], point[0], point[1], line_width
-                )
-                reordered_points = points[1], points[0], points[2], points[3]
-                triangle_point_list.extend(reordered_points)
-            last_point = point
+        try:
+            for point in point_list:
+                if last_point is not None:
+                    points = get_points_for_thick_line(
+                        last_point[0], last_point[1], point[0], point[1], line_width
+                    )
+                    reordered_points = points[1], points[0], points[2], points[3]
+                    triangle_point_list.extend(reordered_points)
+                last_point = point
+        except (TypeError, ValueError, IndexError) as error:
+            _check_points_after(error, point_list)
+            raise
         _generic_draw_line_strip(triangle_point_list, color, gl.TRIANGLE_STRIP)
 
 
@@ -110,7 +119,7 @@ def draw_lines(point_list: Point2List, color: RGBOrA255, line_width: float = 1)
     # Validate & normalize to a pass the shader an RGBA float uniform
     color_normalized = Color.from_iterable(color).normalized
 
-    line_pos_array = array.array("f", (v for point in point_list for v in point))
+    line_pos_array = _flatten_points(point_list)
     num_points = len(point_list)
     if num_points == 0:
         return

arcade/draw/point.py

@@ -1,9 +1,8 @@
-import array
-
 from arcade.types import Color, Point2List, RGBOrA255
 from arcade.types.rect import XYWH
 from arcade.window_commands import get_window
 
+from .helpers import _flatten_points
 from .rect import draw_rect_filled
 
 
@@ -66,7 +65,7 @@ def draw_points(point_list: Point2List, color: RGBOrA255, size: float = 1.0) ->
     num_points = len(point_list)
     if num_points == 0:
         return
-    point_array = array.array("f", (v for point in point_list for v in point))
+    point_array = _flatten_points(point_list)
 
     # Resize buffer
     data_size = num_points * 8

arcade/draw/polygon.py

@@ -2,7 +2,7 @@
 from arcade.earclip import earclip
 from arcade.types import Point2, Point2List, RGBOrA255
 
-from .helpers import _generic_draw_line_strip, get_points_for_thick_line
+from .helpers import _check_points_after, _generic_draw_line_strip, get_points_for_thick_line
 
 
 def draw_polygon_filled(point_list: Point2List, color: RGBOrA255) -> None:
@@ -16,7 +16,11 @@ def draw_polygon_filled(point_list: Point2List, color: RGBOrA255) -> None:
         color:
             The color, specified in RGB or RGBA format.
     """
-    triangle_points = earclip(point_list)
+    try:
+        triangle_points = earclip(point_list)
+    except (TypeError, ValueError, IndexError) as error:
+        _check_points_after(error, point_list)
+        raise
     flattened_list = tuple(i for g in triangle_points for i in g)
     _generic_draw_line_strip(flattened_list, color, gl.TRIANGLES)
 
@@ -35,29 +39,33 @@ def draw_polygon_outline(point_list: Point2List, color: RGBOrA255, line_width: f
         line_width:
             Width of the line in pixels.
     """
-    # Convert to modifiable list & close the loop
-    new_point_list = list(point_list)
-    new_point_list.append(point_list[0])
-
-    # Create a place to store the triangles we'll use to thicken the line
-    triangle_point_list: list[Point2] = []
-
-    # This needs a lot of improvement
-    last_point = None
-    for point in new_point_list:
-        if last_point is not None:
-            # Calculate triangles, then re-order to link up the quad?
-            points = get_points_for_thick_line(*last_point, *point, line_width)
-            reordered_points = points[1], points[0], points[2], points[3]
-
-            triangle_point_list.extend(reordered_points)
-        last_point = point
-
-    # Use first two points of new list to close the loop
-    new_start, new_next = new_point_list[:2]
-    s_x, s_y = new_start
-    n_x, n_y = new_next
-    points = get_points_for_thick_line(s_x, s_y, n_x, n_y, line_width)
-    triangle_point_list.append(points[1])
+    try:
+        # Convert to modifiable list & close the loop
+        new_point_list = list(point_list)
+        new_point_list.append(point_list[0])
+
+        # Create a place to store the triangles we'll use to thicken the line
+        triangle_point_list: list[Point2] = []
+
+        # This needs a lot of improvement
+        last_point = None
+        for point in new_point_list:
+            if last_point is not None:
+                # Calculate triangles, then re-order to link up the quad?
+                points = get_points_for_thick_line(*last_point, *point, line_width)
+                reordered_points = points[1], points[0], points[2], points[3]
+
+                triangle_point_list.extend(reordered_points)
+            last_point = point
+
+        # Use first two points of new list to close the loop
+        new_start, new_next = new_point_list[:2]
+        s_x, s_y = new_start
+        n_x, n_y = new_next
+        points = get_points_for_thick_line(s_x, s_y, n_x, n_y, line_width)
+        triangle_point_list.append(points[1])
+    except (TypeError, ValueError, IndexError) as error:
+        _check_points_after(error, point_list)
+        raise
 
     _generic_draw_line_strip(triangle_point_list, color, gl.TRIANGLE_STRIP)

tests/unit/draw/test_point_validation.py

@@ -0,0 +1,52 @@
+"""Draw functions that take a list of points name the first bad point."""
+
+import array
+
+import pytest
+from pyglet.math import Vec2
+
+import arcade
+from arcade.draw.helpers import _flatten_points
+
+GOOD = [(0, 0), (10, 10), (20, 0), (30, 10)]
+
+
+def test_flatten_points():
+    assert _flatten_points(GOOD) == array.array("f", [0, 0, 10, 10, 20, 0, 30, 10])
+    assert _flatten_points([Vec2(1, 2), (3, 4)]) == array.array("f", [1, 2, 3, 4])
+    assert _flatten_points([]) == array.array("f")
+
+
+@pytest.mark.parametrize(
+    "points, bad_index",
+    [
+        # The example from the issue: used to raise "'int' object is not iterable"
+        ([(1, 1), 5, (2, 2)], 1),
+        # Points with 1 or 3 numbers used to shift every number after them
+        ([(1,), (2, 2), (3, 3)], 0),
+        ([(1, 1), (2, 2, 2), (3, 3)], 1),
+        ([(1, 1), ("a", 2)], 1),
+    ],
+)
+def test_flatten_points_names_bad_point(points, bad_index):
+    with pytest.raises(ValueError, match=rf"point_list\[{bad_index}\] is .*2 numbers"):
+        _flatten_points(points)
+
+
+@pytest.mark.parametrize(
+    "draw",
+    [
+        lambda points: arcade.draw_lines(points, arcade.color.RED),
+        lambda points: arcade.draw_points(points, arcade.color.RED),
+        lambda points: arcade.draw_line_strip(points, arcade.color.RED),
+        lambda points: arcade.draw_line_strip(points, arcade.color.RED, line_width=3),
+        lambda points: arcade.draw_polygon_filled(points, arcade.color.RED),
+        lambda points: arcade.draw_polygon_outline(points, arcade.color.RED, line_width=3),
+    ],
+    ids=["lines", "points", "line_strip", "thick_line_strip", "polygon_filled", "polygon_outline"],
+)
+def test_draw_functions_name_bad_point(window, draw):
+    draw(GOOD)
+    draw([Vec2(*point) for point in GOOD])
+    with pytest.raises(ValueError, match=r"point_list\[2\] is 7"):
+        draw([(0, 0), (10, 10), 7, (30, 10)])