Change ContactPointSet.points to tuple #279

Author: viblo

CHANGELOG.rst

@@ -12,7 +12,8 @@ Changelog
    Changed body.constraints to return a KeysView of the Constraints attached to the body. Note that its still weak references to the Constraints. 
    Reversed the dependency between bodies and shapes. Now the Body owns the connection, and the Shape only keeps a weak ref to the Body. That means that if you remove a Body, then any shapes not referenced anywhere else will also be removed. 
    Changed body.shapes to return a KeysView instead of a set of the shapes.
-   Changed Space.segment_query to return None in case the query did not hit the shape.
+   Changed Shape.segment_query to return None in case the query did not hit the shape.
+   Changed ContactPointSet.points to be a tuple and not list to make it clear its length is fixed.
 
    Added default do_nothing and always_collide callback functions to the CollisionHandler, so that its clear how to reset and align with other callbacks.
    If in old code you did handler.begin = None, you should now instead to handler.begin = CollisionHandler.always_collide etc.

pymunk/contact_point_set.py

@@ -1,6 +1,6 @@
 __docformat__ = "reStructuredText"
 
-from typing import TYPE_CHECKING, List
+from typing import TYPE_CHECKING, Tuple
 
 if TYPE_CHECKING:
     from ._chipmunk_cffi import ffi
@@ -31,8 +31,6 @@ def __init__(
         point_b: Vec2d,
         distance: float,
     ) -> None:
-        assert len(point_a) == 2
-        assert len(point_b) == 2
         self.point_a = point_a
         self.point_b = point_b
         self.distance = distance
@@ -52,12 +50,11 @@ class ContactPointSet(object):
     """
 
     normal: Vec2d
-    points: List[ContactPoint]
+    points: Tuple[ContactPoint, ...]
 
     __slots__ = ("normal", "points")
 
-    def __init__(self, normal: Vec2d, points: List[ContactPoint]) -> None:
-        assert len(normal) == 2
+    def __init__(self, normal: Vec2d, points: Tuple[ContactPoint, ...]) -> None:
         self.normal = normal
         self.points = points
 
@@ -68,14 +65,21 @@ def __repr__(self) -> str:
     def _from_cp(cls, _points: "ffi.CData") -> "ContactPointSet":
         normal = Vec2d(_points.normal.x, _points.normal.y)
 
-        points = []
-        for i in range(_points.count):
-            _p = _points.points[i]
-            p = ContactPoint(
-                Vec2d(_p.pointA.x, _p.pointA.y),
-                Vec2d(_p.pointB.x, _p.pointB.y),
-                _p.distance,
-            )
-            points.append(p)
-
-        return cls(normal, points)
+        assert _points.count in (1, 2), "This is likely a bug in Pymunk, please report."
+
+        _p1 = _points.points[0]
+        p1 = ContactPoint(
+            Vec2d(_p1.pointA.x, _p1.pointA.y),
+            Vec2d(_p1.pointB.x, _p1.pointB.y),
+            _p1.distance,
+        )
+        if _points.count == 1:
+            return cls(normal, (p1,))
+
+        _p2 = _points.points[1]
+        p2 = ContactPoint(
+            Vec2d(_p2.pointA.x, _p2.pointA.y),
+            Vec2d(_p2.pointB.x, _p2.pointB.y),
+            _p2.distance,
+        )
+        return cls(normal, (p1, p2))

pymunk/tests/test_arbiter.py

@@ -137,7 +137,7 @@ def pre_solve(arb: p.Arbiter, space: p.Space, data: Any) -> bool:
             self.assertAlmostEqual(p1.distance, -11)
 
             # check for length of points
-            ps2.points = []
+            ps2.points = ()
 
             def f() -> None:
                 arb.contact_point_set = ps2

pymunk/tests/test_space.py

@@ -1045,10 +1045,10 @@ def _testCopyMethod(self, copy_func: Callable[[Space], Space]) -> None:
         s.add(j1, j2)
 
         h = s.add_default_collision_handler()
-        h.begin = f1
+        h.begin = f2
 
         h = s.add_wildcard_collision_handler(1)
-        h.pre_solve = f1
+        h.pre_solve = f2
 
         h = s.add_collision_handler(1, 2)
         h.post_solve = f1
@@ -1192,3 +1192,7 @@ def testDeleteSpaceWithObjects(self) -> None:
 
 def f1(*args: Any, **kwargs: Any) -> None:
     pass
+
+
+def f2(*args: Any, **kwargs: Any) -> bool:
+    return True