Add default CollisionHandler callback functions instead of None to better align API #272

viblo · viblo · commit bc90bf6636ba · 2025-02-25T22:25:19.000+01:00
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -7,6 +7,9 @@ Changelog
    Added Vec2d.length_squared, and depreacted Vec2d.get_length_sqrd()
    Added Vec2d.get_distance_squared(), and deprecated Vec2d.get_dist_sqrd()
    
+   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.
+
    New feature: ShapeFilter.rejects_collision()
    New feature: Added Vec2d.polar_tuple
    Optimized Vec2d.angle and Vec2d.angle_degrees (note that the optimized versions treat 0 length vectors with x and/or y equal to -0 slightly differently.)
diff --git a/pymunk/collision_handler.py b/pymunk/collision_handler.py
@@ -1,15 +1,15 @@
 __docformat__ = "reStructuredText"
 
-from typing import TYPE_CHECKING, Any, Callable, Dict, Optional
+from typing import TYPE_CHECKING, Any, Callable, Dict
 
 if TYPE_CHECKING:
     from .space import Space
 
 from ._chipmunk_cffi import ffi, lib
 from .arbiter import Arbiter
 
-_CollisionCallbackBool = Callable[[Arbiter, "Space", Any], bool]
-_CollisionCallbackNoReturn = Callable[[Arbiter, "Space", Any], None]
+_CollisionCallbackBool = Callable[[Arbiter, "Space", Dict[Any, Any]], bool]
+_CollisionCallbackNoReturn = Callable[[Arbiter, "Space", Dict[Any, Any]], None]
 
 
 class CollisionHandler(object):
@@ -42,25 +42,13 @@ def __init__(self, _handler: Any, space: "Space") -> None:
         self._handler.userData = self._userData
 
         self._space = space
-        self._begin: Optional[_CollisionCallbackBool] = None
-        self._pre_solve: Optional[_CollisionCallbackBool] = None
-        self._post_solve: Optional[_CollisionCallbackNoReturn] = None
-        self._separate: Optional[_CollisionCallbackNoReturn] = None
+        self._begin: _CollisionCallbackBool = CollisionHandler.always_collide
+        self._pre_solve: _CollisionCallbackBool = CollisionHandler.always_collide
+        self._post_solve: _CollisionCallbackNoReturn = CollisionHandler.do_nothing
+        self._separate: _CollisionCallbackNoReturn = CollisionHandler.do_nothing
 
         self._data: Dict[Any, Any] = {}
 
-    def _reset(self) -> None:
-        def allways_collide(arb: Arbiter, space: "Space", data: Any) -> bool:
-            return True
-
-        def do_nothing(arb: Arbiter, space: "Space", data: Any) -> None:
-            return
-
-        self.begin = allways_collide
-        self.pre_solve = allways_collide
-        self.post_solve = do_nothing
-        self.separate = do_nothing
-
     @property
     def data(self) -> Dict[Any, Any]:
         """Data property that get passed on into the
@@ -72,17 +60,9 @@ def data(self) -> Dict[Any, Any]:
         """
         return self._data
 
-    def _set_begin(self, func: _CollisionCallbackBool) -> None:
-        self._begin = func
-        self._handler.beginFunc = lib.ext_cpCollisionBeginFunc
-
-    def _get_begin(self) -> Optional[_CollisionCallbackBool]:
-        return self._begin
-
-    begin = property(
-        _get_begin,
-        _set_begin,
-        doc="""Two shapes just started touching for the first time this step.
+    @property
+    def begin(self) -> _CollisionCallbackBool:
+        """Two shapes just started touching for the first time this step.
 
         ``func(arbiter, space, data) -> bool``
 
@@ -91,20 +71,24 @@ def _get_begin(self) -> Optional[_CollisionCallbackBool]:
         false, the `pre_solve` and `post_solve` callbacks will never be run,
         but you will still recieve a separate event when the shapes stop
         overlapping.
-        """,
-    )
+        """
+        return self._begin
 
-    def _set_pre_solve(self, func: _CollisionCallbackBool) -> None:
-        self._pre_solve = func
-        self._handler.preSolveFunc = lib.ext_cpCollisionPreSolveFunc
+    @begin.setter
+    def begin(self, func: _CollisionCallbackBool) -> None:
+        assert (
+            func is not None
+        ), "To reset the begin callback, set handler.begin = CollisionHandler.always_collide"
+        self._begin = func
 
-    def _get_pre_solve(self) -> Optional[_CollisionCallbackBool]:
-        return self._pre_solve
+        if self._begin == CollisionHandler.always_collide:
+            self._handler.beginFunc = ffi.addressof(lib, "AlwaysCollide")
+        else:
+            self._handler.beginFunc = lib.ext_cpCollisionBeginFunc
 
-    pre_solve = property(
-        _get_pre_solve,
-        _set_pre_solve,
-        doc="""Two shapes are touching during this step.
+    @property
+    def pre_solve(self) -> _CollisionCallbackBool:
+        """Two shapes are touching during this step.
 
         ``func(arbiter, space, data) -> bool``
 
@@ -113,48 +97,90 @@ def _get_pre_solve(self) -> Optional[_CollisionCallbackBool]:
         override collision values using Arbiter.friction, Arbiter.elasticity
         or Arbiter.surfaceVelocity to provide custom friction, elasticity,
         or surface velocity values. See Arbiter for more info.
-        """,
-    )
-
-    def _set_post_solve(self, func: _CollisionCallbackNoReturn) -> None:
+        """
+        return self._pre_solve
 
-        self._post_solve = func
-        self._handler.postSolveFunc = lib.ext_cpCollisionPostSolveFunc
+    @pre_solve.setter
+    def pre_solve(self, func: _CollisionCallbackBool) -> None:
+        assert (
+            func is not None
+        ), "To reset the pre_solve callback, set handler.pre_solve = CollisionHandler.always_collide"
+        self._pre_solve = func
 
-    def _get_post_solve(self) -> Optional[_CollisionCallbackNoReturn]:
-        return self._post_solve
+        if self._pre_solve == CollisionHandler.always_collide:
+            self._handler.preSolveFunc = ffi.addressof(lib, "AlwaysCollide")
+        else:
+            self._handler.preSolveFunc = lib.ext_cpCollisionPreSolveFunc
 
-    post_solve = property(
-        _get_post_solve,
-        _set_post_solve,
-        doc="""Two shapes are touching and their collision response has been
+    @property
+    def post_solve(self) -> _CollisionCallbackNoReturn:
+        """Two shapes are touching and their collision response has been
         processed.
 
         ``func(arbiter, space, data)``
 
         You can retrieve the collision impulse or kinetic energy at this
         time if you want to use it to calculate sound volumes or damage
         amounts. See Arbiter for more info.
-        """,
-    )
+        """
+        return self._post_solve
 
-    def _set_separate(self, func: _CollisionCallbackNoReturn) -> None:
-        self._separate = func
-        self._handler.separateFunc = lib.ext_cpCollisionSeparateFunc
+    @post_solve.setter
+    def post_solve(self, func: _CollisionCallbackNoReturn) -> None:
+        assert (
+            func is not None
+        ), "To reset the post_solve callback, set handler.post_solve = CollisionHandler.do_nothing"
+        self._post_solve = func
 
-    def _get_separate(self) -> Optional[_CollisionCallbackNoReturn]:
-        return self._separate
+        if self._post_solve == CollisionHandler.do_nothing:
+            self._handler.postSolveFunc = ffi.addressof(lib, "DoNothing")
+        else:
+            self._handler.postSolveFunc = lib.ext_cpCollisionPostSolveFunc
 
-    separate = property(
-        _get_separate,
-        _set_separate,
-        doc="""Two shapes have just stopped touching for the first time this
+        self._handler.postSolveFunc = lib.ext_cpCollisionPostSolveFunc
+
+    @property
+    def separate(self) -> _CollisionCallbackNoReturn:
+        """Two shapes have just stopped touching for the first time this
         step.
 
         ``func(arbiter, space, data)``
 
         To ensure that begin()/separate() are always called in balanced
         pairs, it will also be called when removing a shape while its in
         contact with something or when de-allocating the space.
-        """,
-    )
+        """
+        return self._separate
+
+    @separate.setter
+    def separate(self, func: _CollisionCallbackNoReturn) -> None:
+        assert (
+            func is not None
+        ), "To reset the separate callback, set handler.separate = CollisionHandler.do_nothing"
+        self._separate = func
+
+        if self._separate == CollisionHandler.do_nothing:
+            self._handler.separateFunc = ffi.addressof(lib, "DoNothing")
+        else:
+            self._handler.separateFunc = lib.ext_cpCollisionSeparateFunc
+
+    @staticmethod
+    def do_nothing(arbiter: Arbiter, space: "Space", data: Dict[Any, Any]) -> None:
+        """The default do nothing method used for the post_solve and seprate
+        callbacks.
+
+        Note that its more efficient to set this method than to define your own
+        do nothing method.
+        """
+        return
+
+    @staticmethod
+    def always_collide(arbiter: Arbiter, space: "Space", data: Dict[Any, Any]) -> bool:
+        """The default method used for the begin and pre_solve callbacks.
+
+        It will always return True, meaning the collision should not be ignored.
+
+        Note that its more efficient to set this method than to define your own
+        return True method.
+        """
+        return True
diff --git a/pymunk/tests/test_batch.py b/pymunk/tests/test_batch.py
@@ -201,6 +201,9 @@ def test_get_arbiters(self) -> None:
         ints = memoryview(data.int_buf()).cast("P")
 
         def check_arb_data(arb: pymunk.Arbiter) -> None:
+            assert arb.shapes[0].body is not None
+            assert arb.shapes[1].body is not None
+
             a_id = arb.shapes[0].body.id
             b_id = arb.shapes[1].body.id
 
diff --git a/pymunk/tests/test_shape.py b/pymunk/tests/test_shape.py
@@ -195,8 +195,9 @@ def testPickle(self) -> None:
         self.assertEqual(c.surface_velocity, c2.surface_velocity)
         self.assertEqual(c.density, c2.density)
         self.assertEqual(c.mass, c2.mass)
+        assert c.body is not None
         self.assertEqual(c.body.mass, c2.body.mass)
-
+        self.assertIsNotNone
         c = p.Circle(None, 1)
         c.density = 3
 
diff --git a/pymunk/tests/test_space.py b/pymunk/tests/test_space.py
@@ -73,7 +73,7 @@ def testProperties(self) -> None:
         s.step(0.1)
         self.assertEqual(s.current_time_step, 0.1)
 
-        self.assertTrue(s.static_body != None)
+        self.assertTrue(s.static_body is not None)
         self.assertEqual(s.static_body.body_type, p.Body.STATIC)
 
         self.assertEqual(s.threads, 1)
@@ -196,7 +196,7 @@ def testPointQueryNearestWithShapeFilter(self) -> None:
             s1.filter = f1
             hit = s.point_query_nearest((0, 0), 0, f2)
             self.assertEqual(
-                hit != None,
+                hit is not None,
                 test["hit"],
                 "Got {}!=None, expected {} for test: {}".format(hit, test["hit"], test),
             )
@@ -647,6 +647,29 @@ def separate(*_: Any) -> None:
         s.step(1)
         s.remove(c1)
 
+    def testCollisionHandlerDefaultCallbacks(self) -> None:
+        s = p.Space()
+
+        b1 = p.Body(1, 1)
+        c1 = p.Circle(b1, 10)
+        b1.position = 9, 11
+
+        b2 = p.Body(body_type=p.Body.STATIC)
+        c2 = p.Circle(b2, 10)
+        b2.position = 0, 0
+
+        s.add(b1, c1, b2, c2)
+        s.gravity = 0, -100
+
+        h = s.add_default_collision_handler()
+        h.begin = h.always_collide
+        h.pre_solve = h.always_collide
+        h.post_solve = h.do_nothing
+        h.separate = h.do_nothing
+
+        for _ in range(10):
+            s.step(0.1)
+
     @unittest.skip("Existing bug in Pymunk. TODO: Fix bug and enable test")
     def testRemoveInSeparate(self) -> None:
         s = p.Space()
@@ -1057,26 +1080,26 @@ def _testCopyMethod(self, copy_func: Callable[[Space], Space]) -> None:
         # Assert collision handlers
         h2 = s2.add_default_collision_handler()
         self.assertIsNotNone(h2.begin)
-        self.assertIsNone(h2.pre_solve)
-        self.assertIsNone(h2.post_solve)
-        self.assertIsNone(h2.separate)
+        self.assertEqual(h2.pre_solve, p.CollisionHandler.always_collide)
+        self.assertEqual(h2.post_solve, p.CollisionHandler.do_nothing)
+        self.assertEqual(h2.separate, p.CollisionHandler.do_nothing)
 
         h2 = s2.add_wildcard_collision_handler(1)
-        self.assertIsNone(h2.begin)
+        self.assertEqual(h2.begin, p.CollisionHandler.always_collide)
         self.assertIsNotNone(h2.pre_solve)
-        self.assertIsNone(h2.post_solve)
-        self.assertIsNone(h2.separate)
+        self.assertEqual(h2.post_solve, p.CollisionHandler.do_nothing)
+        self.assertEqual(h2.separate, p.CollisionHandler.do_nothing)
 
         h2 = s2.add_collision_handler(1, 2)
-        self.assertIsNone(h2.begin)
-        self.assertIsNone(h2.pre_solve)
+        self.assertEqual(h2.begin, p.CollisionHandler.always_collide)
+        self.assertEqual(h2.pre_solve, p.CollisionHandler.always_collide)
         self.assertIsNotNone(h2.post_solve)
-        self.assertIsNone(h2.separate)
+        self.assertEqual(h2.separate, p.CollisionHandler.do_nothing)
 
         h2 = s2.add_collision_handler(3, 4)
-        self.assertIsNone(h2.begin)
-        self.assertIsNone(h2.pre_solve)
-        self.assertIsNone(h2.post_solve)
+        self.assertEqual(h2.begin, p.CollisionHandler.always_collide)
+        self.assertEqual(h2.pre_solve, p.CollisionHandler.always_collide)
+        self.assertEqual(h2.post_solve, p.CollisionHandler.do_nothing)
         self.assertIsNotNone(h2.separate)
 
     def testPickleCachedArbiters(self) -> None:
diff --git a/pymunk_cffi/extensions.c b/pymunk_cffi/extensions.c
@@ -488,3 +488,6 @@ void cpMessage(const char *condition, const char *file, int line, int isError, i
     snprintf(formattedMessage, sizeof(formattedMessage), "\tSource: %s:%d", file, line);
 	ext_pyLog(formattedMessage);	
 }
+
+static cpBool AlwaysCollide(cpArbiter *arb, cpSpace *space, cpDataPointer data){return cpTrue;}
+static void DoNothing(cpArbiter *arb, cpSpace *space, cpDataPointer data){}
diff --git a/pymunk_cffi/extensions_cdef.h b/pymunk_cffi/extensions_cdef.h
@@ -102,4 +102,7 @@ cpContact *cpContactArrAlloc(int count);
 
 cpFloat defaultSpringForce(cpDampedSpring *spring, cpFloat dist);
 
-cpFloat defaultSpringTorque(cpDampedRotarySpring *spring, cpFloat relativeAngle);
+cpFloat defaultSpringTorque(cpDampedRotarySpring *spring, cpFloat relativeAngle);
+
+static cpBool AlwaysCollide(cpArbiter *arb, cpSpace *space, cpDataPointer data);
+static void DoNothing(cpArbiter *arb, cpSpace *space, cpDataPointer data);

Original file line number	Diff line number	Diff line change
`@@ -488,3 +488,6 @@ void cpMessage(const char condition, const char file, int line, int isError, i`
`488`	`488`	`snprintf(formattedMessage, sizeof(formattedMessage), "\tSource: %s:%d", file, line);`
`489`	`489`	`ext_pyLog(formattedMessage);`
`490`	`490`	`}`
	`491`	`+`
	`492`	`+static cpBool AlwaysCollide(cpArbiter arb, cpSpace space, cpDataPointer data){return cpTrue;}`
	`493`	`+static void DoNothing(cpArbiter arb, cpSpace space, cpDataPointer data){}`