From d7e0b9fb2942ac6fd042b1eba157ba9e740be9cf Mon Sep 17 00:00:00 2001
From: Paul <36696816+pushfoo@users.noreply.github.com>
Date: Sat, 29 Jun 2024 00:44:56 -0400
Subject: [PATCH] Optimize & document collision circular check in physics
 engines (#2155)

* Clarify _circular_check doc, signature, and typing

* Add None return annotation

* Rename player argment to moving

* Reformat docstring

* Use Sprite.position in _circular_check insted of .center_*

* Optimize _circular_check with reduced ops

* Use array instead of putting GC pressure onto it

* Use strided iteration over the 16-length array of floats

* Update the debug print statements

* Clarify algorithm and rename function

* Rename to _wiggle_until_free

* Update docstring

* Rename variable

* Stop black auto-formatter from turning readable 4 x 4 grid into a tower

* Fix array slice assigment issue and touch-up types

* Use a list instead of an array.array since only ctypes ctypes arrays allow slice assignment from iterables

* Adjust some formatting and comment locations

* Clean up physics_engines imports
---
 arcade/physics_engines.py | 78 ++++++++++++++++++++++++++-------------
 1 file changed, 52 insertions(+), 26 deletions(-)

diff --git a/arcade/physics_engines.py b/arcade/physics_engines.py
index ace8f686e..1dcf58c5e 100644
--- a/arcade/physics_engines.py
+++ b/arcade/physics_engines.py
@@ -6,6 +6,7 @@
 
 # pylint: disable=too-many-arguments, too-many-locals, too-few-public-methods
 import math
+
 from typing import Iterable, Optional, Union
 
 from arcade import (
@@ -22,36 +23,61 @@
 from arcade.utils import copy_dunders_unimplemented
 
 
-def _circular_check(player: Sprite, walls: list[SpriteList]) -> None:
-    """
-    This is a horrible kludge to 'guess' our way out of a collision
+def _wiggle_until_free(colliding: Sprite, walls: list[SpriteList]) -> None:
+    """Kludge to 'guess' a colliding sprite out of a collision.
+
+    It works by iterating over increasing wiggle sizes of 8 points
+    around the ``colliding`` sprite's original center position. Each
+    time it fails to find a free position. Although the wiggle distance
+    starts at 1, it grows quickly since each failed iteration multiplies
+    wiggle distance by two.
+
+    :param colliding: A sprite to move out of the given list of SpriteLists.
+    :param walls: A list of walls to guess our way out of.
     """
-    original_x = player.center_x
-    original_y = player.center_y
 
-    vary = 1
+    # Original x & y of the moving object
+    o_x, o_y = colliding.position
+
+    # fmt: off
+    try_list = [  # Allocate once so we don't recreate or gc
+        0.0, 0.0, 0.0, 0.0,
+        0.0, 0.0, 0.0, 0.0,
+        0.0, 0.0, 0.0, 0.0,
+        0.0, 0.0, 0.0, 0.0,
+    ]
+
+    wiggle_distance = 1
     while True:
-        try_list = [
-            [original_x, original_y + vary],
-            [original_x, original_y - vary],
-            [original_x + vary, original_y],
-            [original_x - vary, original_y],
-            [original_x + vary, original_y + vary],
-            [original_x + vary, original_y - vary],
-            [original_x - vary, original_y + vary],
-            [original_x - vary, original_y - vary],
-        ]
-
-        for my_item in try_list:
-            x, y = my_item
-            player.center_x = x
-            player.center_y = y
-            check_hit_list = check_for_collision_with_lists(player, walls)
-            # print(f"Vary {vary} ({self.player_sprite.center_x} {self.player_sprite.center_y}) "
+        # Cache our variant dimensions
+        o_x_plus  = o_x + wiggle_distance
+        o_y_plus  = o_y + wiggle_distance
+        o_x_minus = o_x - wiggle_distance
+        o_y_minus = o_y - wiggle_distance
+
+        # Burst setting of no-gc region is cheaper than nested lists
+        try_list[:] = (
+            o_x       , o_y_plus ,
+            o_x       , o_y_minus,
+            o_x_plus  , o_y      ,
+            o_x_minus , o_y      ,
+            o_x_plus  , o_y_plus ,
+            o_x_plus  , o_y_minus,
+            o_x_minus , o_y_plus ,
+            o_x_minus , o_y_minus
+        )
+        # fmt: on
+
+        # Iterate and slice the try_list
+        for strided_index in range(0, 16, 2):
+            x, y = try_list[strided_index:strided_index + 2]
+            colliding.position = x, y
+            check_hit_list = check_for_collision_with_lists(colliding, walls)
+            # print(f"Vary {vary} ({trapped.center_x} {trapped.center_y}) "
             #       f"= {len(check_hit_list)}")
             if len(check_hit_list) == 0:
                 return
-        vary *= 2
+        wiggle_distance *= 2
 
 
 def _move_sprite(
@@ -60,7 +86,7 @@ def _move_sprite(
 
     # See if we are starting this turn with a sprite already colliding with us.
     if len(check_for_collision_with_lists(moving_sprite, walls)) > 0:
-        _circular_check(moving_sprite, walls)
+        _wiggle_until_free(moving_sprite, walls)
 
     original_x = moving_sprite.center_x
     original_y = moving_sprite.center_y
@@ -81,7 +107,7 @@ def _move_sprite(
             max_distance = (moving_sprite.width + moving_sprite.height) / 2
 
             # Resolve any collisions by this weird kludge
-            _circular_check(moving_sprite, walls)
+            _wiggle_until_free(moving_sprite, walls)
             if (
                 get_distance(original_x, original_y, moving_sprite.center_x, moving_sprite.center_y)
                 > max_distance