perf!: Pool CollisionProspects and remove some list creations from the collision detection (#2625)

This change introduces a very simple pool for `CollisionProspect`s so
that those objects don't have to be re-created each tick.
It means that the `CollisionProspect` needs to be mutable though, so the
code becomes a little bit harder to read since sets can't be used
anymore.

examples/.metadata

@@ -4,7 +4,7 @@
 # This file should be version controlled.
 
 version:
-  revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
+  revision: f468f3366c26a5092eb964a230ce7892fda8f2f8
   channel: stable
 
 project_type: app
@@ -13,26 +13,11 @@ project_type: app
 migration:
   platforms:
     - platform: root
-      create_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-      base_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
+      create_revision: f468f3366c26a5092eb964a230ce7892fda8f2f8
+      base_revision: f468f3366c26a5092eb964a230ce7892fda8f2f8
     - platform: android
-      create_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-      base_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-    - platform: ios
-      create_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-      base_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-    - platform: linux
-      create_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-      base_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-    - platform: macos
-      create_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-      base_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-    - platform: web
-      create_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-      base_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-    - platform: windows
-      create_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
-      base_revision: 2ad6cd72c040113b47ee9055e722606a490ef0da
+      create_revision: f468f3366c26a5092eb964a230ce7892fda8f2f8
+      base_revision: f468f3366c26a5092eb964a230ce7892fda8f2f8
 
   # User provided section
 

examples/test/main_test.dart

@@ -1,6 +0,0 @@
-import 'package:examples/main.dart' as examples;
-import 'package:test/test.dart';
-
-void main() {
-  test('main', examples.main);
-}

packages/flame/lib/collisions.dart

@@ -1,4 +1,5 @@
 export 'src/collisions/broadphase/broadphase.dart';
+export 'src/collisions/broadphase/prospect_pool.dart';
 export 'src/collisions/broadphase/quadtree/has_quadtree_collision_detection.dart';
 export 'src/collisions/broadphase/quadtree/quad_tree_broadphase.dart';
 export 'src/collisions/broadphase/quadtree/quadtree.dart';

packages/flame/lib/src/collisions/broadphase/broadphase.dart

@@ -1,5 +1,4 @@
 import 'package:flame/collisions.dart';
-import 'package:meta/meta.dart';
 
 /// The [Broadphase] class is used to make collision detection more efficient
 /// by doing a rough estimation of which hitboxes that can collide before their
@@ -32,33 +31,56 @@ abstract class Broadphase<T extends Hitbox<T>> {
   /// detection system.
   void add(T item);
 
-  void addAll(Iterable<T> items) => items.forEach(add);
+  void addAll(Iterable<T> items) {
+    for (final item in items) {
+      add(item);
+    }
+  }
 
   /// Removes an item from the broadphase. Should be called in a
   /// [CollisionDetection] class while removing a hitbox from its collision
   /// detection system.
   void remove(T item);
 
-  void removeAll(Iterable<T> items) => items.forEach(remove);
+  void removeAll(Iterable<T> items) {
+    for (final item in items) {
+      remove(item);
+    }
+  }
 
   /// Returns the potential hitbox collisions
-  Set<CollisionProspect<T>> query();
+  Iterable<CollisionProspect<T>> query();
 }
 
 /// A [CollisionProspect] is a tuple that is used to contain two potentially
 /// colliding hitboxes.
-@immutable
 class CollisionProspect<T> {
-  final T a;
-  final T b;
+  T _a;
+  T _b;
 
-  const CollisionProspect(this.a, this.b);
+  T get a => _a;
+  T get b => _b;
 
-  @override
-  bool operator ==(Object other) =>
-      other is CollisionProspect &&
-      ((other.a == a && other.b == b) || (other.a == b && other.b == a));
+  int get hash => _hash;
+  int _hash;
 
-  @override
-  int get hashCode => Object.hashAllUnordered([a, b]);
+  CollisionProspect(this._a, this._b) : _hash = _a.hashCode ^ _b.hashCode;
+
+  /// Sets the prospect to contain [a] and [b] instead of what it previously
+  /// contained.
+  void set(T a, T b) {
+    _a = a;
+    _b = b;
+    _hash = a.hashCode ^ b.hashCode;
+  }
+
+  /// Sets the prospect to contain the content of [other].
+  void setFrom(CollisionProspect<T> other) {
+    _a = other._a;
+    _b = other._b;
+    _hash = other._hash;
+  }
+
+  /// Creates a new prospect object with the same content.
+  CollisionProspect<T> clone() => CollisionProspect(_a, _b);
 }

packages/flame/lib/src/collisions/broadphase/prospect_pool.dart

@@ -0,0 +1,25 @@
+import 'package:flame/src/collisions/broadphase/broadphase.dart';
+import 'package:flame/src/collisions/hitboxes/hitbox.dart';
+
+/// This pool is used to not create unnecessary [CollisionProspect] objects
+/// during collision detection, but to re-use the ones that have already been
+/// created.
+class ProspectPool<T extends Hitbox<T>> {
+  ProspectPool({this.incrementSize = 1000});
+
+  /// How much the pool should increase in size every time it needs to be made
+  /// larger.
+  final int incrementSize;
+  final _storage = <CollisionProspect<T>>[];
+  int get length => _storage.length;
+
+  /// The size of the pool will expand with [incrementSize] amount of
+  /// [CollisionProspect]s that are initially populated with two [dummyItem]s.
+  void expand(T dummyItem) {
+    for (var i = 0; i < incrementSize; i++) {
+      _storage.add(CollisionProspect<T>(dummyItem, dummyItem));
+    }
+  }
+
+  CollisionProspect<T> operator [](int index) => _storage[index];
+}

packages/flame/lib/src/collisions/broadphase/quadtree/has_quadtree_collision_detection.dart

@@ -16,7 +16,7 @@ import 'package:flame/game.dart';
 /// [initializeCollisionDetection] should be called in the game's [onLoad]
 /// method.
 mixin HasQuadTreeCollisionDetection on FlameGame
-    implements HasCollisionDetection<QuadTreeBroadphase<ShapeHitbox>> {
+    implements HasCollisionDetection<QuadTreeBroadphase> {
   late QuadTreeCollisionDetection _collisionDetection;
 
   @override
@@ -24,7 +24,7 @@ mixin HasQuadTreeCollisionDetection on FlameGame
 
   @override
   set collisionDetection(
-    CollisionDetection<ShapeHitbox, QuadTreeBroadphase<ShapeHitbox>> cd,
+    CollisionDetection<ShapeHitbox, QuadTreeBroadphase> cd,
   ) {
     if (cd is! QuadTreeCollisionDetection) {
       throw 'Must be QuadTreeCollisionDetection!';

packages/flame/lib/src/collisions/broadphase/quadtree/quad_tree_broadphase.dart

@@ -17,44 +17,43 @@ typedef ExternalMinDistanceCheck = bool Function(
 ///
 /// See [HasQuadTreeCollisionDetection.initializeCollisionDetection] for a
 /// detailed description of its initialization parameters.
-class QuadTreeBroadphase<T extends Hitbox<T>> extends Broadphase<T> {
+class QuadTreeBroadphase extends Broadphase<ShapeHitbox> {
   QuadTreeBroadphase({
     required Rect mainBoxSize,
     required this.broadphaseCheck,
     required this.minimumDistanceCheck,
     int maxObjects = 25,
     int maxDepth = 10,
-  }) : tree = QuadTree<T>(
+  }) : tree = QuadTree<ShapeHitbox>(
           mainBoxSize: mainBoxSize,
           maxObjects: maxObjects,
           maxDepth: maxDepth,
         );
 
-  final QuadTree<T> tree;
+  final QuadTree<ShapeHitbox> tree;
 
-  final activeCollisions = HashSet<T>();
+  final activeHitboxes = HashSet<ShapeHitbox>();
 
   ExternalBroadphaseCheck broadphaseCheck;
   ExternalMinDistanceCheck minimumDistanceCheck;
-  final _broadphaseCheckCache = <T, Map<T, bool>>{};
+  final _broadphaseCheckCache = <ShapeHitbox, Map<ShapeHitbox, bool>>{};
 
   final _cachedCenters = <ShapeHitbox, Vector2>{};
 
-  final _potentials = HashSet<CollisionProspect<T>>();
-  final _potentialsTmp = <List<ShapeHitbox>>[];
+  final _potentials = <int, CollisionProspect<ShapeHitbox>>{};
+  final _potentialsTmp = <ShapeHitbox>[];
+  final _prospectPool = ProspectPool<ShapeHitbox>();
 
   @override
-  List<T> get items => tree.hitboxes;
+  List<ShapeHitbox> get items => tree.hitboxes;
 
   @override
-  HashSet<CollisionProspect<T>> query() {
+  Iterable<CollisionProspect<ShapeHitbox>> query() {
     _potentials.clear();
     _potentialsTmp.clear();
 
-    for (final activeItem in activeCollisions) {
-      final asShapeItem = activeItem as ShapeHitbox;
-
-      if (asShapeItem.isRemoving || asShapeItem.parent == null) {
+    for (final activeItem in activeHitboxes) {
+      if (activeItem.isRemoving || !activeItem.isMounted) {
         tree.remove(activeItem);
         continue;
       }
@@ -70,63 +69,69 @@ class QuadTreeBroadphase<T extends Hitbox<T>> extends Broadphase<T> {
           continue;
         }
 
-        final asShapePotential = potential as ShapeHitbox;
-
-        if (asShapePotential.parent == asShapeItem.parent &&
-            asShapeItem.parent != null) {
+        if (!potential.allowSiblingCollision &&
+            potential.hitboxParent == activeItem.hitboxParent &&
+            potential.isMounted) {
           continue;
         }
 
         final distanceCloseEnough = minimumDistanceCheck.call(
           itemCenter,
-          _cacheCenterOfHitbox(asShapePotential),
+          _cacheCenterOfHitbox(potential),
         );
         if (distanceCloseEnough == false) {
           continue;
         }
 
-        _potentialsTmp.add([asShapeItem, asShapePotential]);
+        _potentialsTmp
+          ..add(activeItem)
+          ..add(potential);
       }
     }
 
     if (_potentialsTmp.isNotEmpty) {
-      for (var i = 0; i < _potentialsTmp.length; i++) {
-        final item0 = _potentialsTmp[i].first;
-        final item1 = _potentialsTmp[i].last;
+      for (var i = 0; i < _potentialsTmp.length; i += 2) {
+        final item0 = _potentialsTmp[i];
+        final item1 = _potentialsTmp[i + 1];
         if (broadphaseCheck(item0, item1)) {
-          _potentials.add(CollisionProspect(item0 as T, item1 as T));
-        } else {
-          if (_broadphaseCheckCache[item0 as T] == null) {
-            _broadphaseCheckCache[item0 as T] = {};
+          final CollisionProspect<ShapeHitbox> prospect;
+          if (_prospectPool.length <= i) {
+            _prospectPool.expand(item0);
           }
-          _broadphaseCheckCache[item0 as T]![item1 as T] = false;
+          prospect = _prospectPool[i]..set(item0, item1);
+          _potentials[prospect.hash] = prospect;
+        } else {
+          if (_broadphaseCheckCache[item0] == null) {
+            _broadphaseCheckCache[item0] = {};
+          }
+          _broadphaseCheckCache[item0]![item1] = false;
         }
       }
     }
-    return _potentials;
+    return _potentials.values;
   }
 
-  void updateTransform(T item) {
+  void updateTransform(ShapeHitbox item) {
     tree.remove(item, keepOldPosition: true);
-    _cacheCenterOfHitbox(item as ShapeHitbox);
+    _cacheCenterOfHitbox(item);
     tree.add(item);
   }
 
   @override
-  void add(T item) {
+  void add(ShapeHitbox item) {
     tree.add(item);
     if (item.collisionType == CollisionType.active) {
-      activeCollisions.add(item);
+      activeHitboxes.add(item);
     }
-    _cacheCenterOfHitbox(item as ShapeHitbox);
+    _cacheCenterOfHitbox(item);
   }
 
   @override
-  void remove(T item) {
+  void remove(ShapeHitbox item) {
     tree.remove(item);
     _cachedCenters.remove(item);
     if (item.collisionType == CollisionType.active) {
-      activeCollisions.remove(item);
+      activeHitboxes.remove(item);
     }
 
     final checkCache = _broadphaseCheckCache[item];
@@ -140,7 +145,7 @@ class QuadTreeBroadphase<T extends Hitbox<T>> extends Broadphase<T> {
 
   void clear() {
     tree.clear();
-    activeCollisions.clear();
+    activeHitboxes.clear();
     _broadphaseCheckCache.clear();
     _cachedCenters.clear();
   }

packages/flame/lib/src/collisions/broadphase/quadtree/quadtree_collision_detection.dart

@@ -6,7 +6,7 @@ import 'package:flutter/widgets.dart';
 /// Do not use standard [items] list for components. Instead adds all components
 /// into [QuadTreeBroadphase] class.
 class QuadTreeCollisionDetection
-    extends StandardCollisionDetection<QuadTreeBroadphase<ShapeHitbox>> {
+    extends StandardCollisionDetection<QuadTreeBroadphase> {
   QuadTreeCollisionDetection({
     required Rect mapDimensions,
     required ExternalBroadphaseCheck onComponentTypeCheck,
@@ -14,7 +14,7 @@ class QuadTreeCollisionDetection
     int maxObjects = 25,
     int maxDepth = 10,
   }) : super(
-          broadphase: QuadTreeBroadphase<ShapeHitbox>(
+          broadphase: QuadTreeBroadphase(
             mainBoxSize: mapDimensions,
             maxObjects: maxObjects,
             maxDepth: maxDepth,
@@ -29,16 +29,16 @@ class QuadTreeCollisionDetection
   @override
   void add(ShapeHitbox item) {
     item.onAabbChanged = () => _scheduledUpdate.add(item);
-    // ignore: prefer_function_declarations_over_variables
-    final listenerCollisionType = () {
+    void listenerCollisionType() {
       if (item.isMounted) {
         if (item.collisionType == CollisionType.active) {
-          broadphase.activeCollisions.add(item);
+          broadphase.activeHitboxes.add(item);
         } else {
-          broadphase.activeCollisions.remove(item);
+          broadphase.activeHitboxes.remove(item);
         }
       }
-    };
+    }
+
     item.collisionTypeNotifier.addListener(listenerCollisionType);
     _listenerCollisionType[item] = listenerCollisionType;
 
@@ -47,7 +47,9 @@ class QuadTreeCollisionDetection
 
   @override
   void addAll(Iterable<ShapeHitbox> items) {
-    items.forEach(add);
+    for (final item in items) {
+      add(item);
+    }
   }
 
   @override
@@ -65,14 +67,16 @@ class QuadTreeCollisionDetection
   @override
   void removeAll(Iterable<ShapeHitbox> items) {
     broadphase.clear();
-    items.forEach(remove);
+    for (final item in items) {
+      remove(item);
+    }
   }
 
   @override
   void run() {
-    _scheduledUpdate.forEach(
-      broadphase.updateTransform,
-    );
+    for (final hitbox in _scheduledUpdate) {
+      broadphase.updateTransform(hitbox);
+    }
     _scheduledUpdate.clear();
     super.run();
   }

packages/flame/lib/src/collisions/broadphase/sweep/sweep.dart

@@ -6,8 +6,9 @@ class Sweep<T extends Hitbox<T>> extends Broadphase<T> {
   @override
   final List<T> items;
 
-  late final List<T> _active = [];
-  late final Set<CollisionProspect<T>> _potentials = {};
+  final _active = <T>[];
+  final _potentials = <int, CollisionProspect<T>>{};
+  final _prospectPool = ProspectPool<T>();
 
   @override
   void add(T item) => items.add(item);
@@ -21,9 +22,10 @@ class Sweep<T extends Hitbox<T>> extends Broadphase<T> {
   }
 
   @override
-  Set<CollisionProspect<T>> query() {
+  Iterable<CollisionProspect<T>> query() {
     _active.clear();
     _potentials.clear();
+
     for (final item in items) {
       if (item.collisionType == CollisionType.inactive) {
         continue;
@@ -40,7 +42,12 @@ class Sweep<T extends Hitbox<T>> extends Broadphase<T> {
         if (activeBox.max.x >= currentMin) {
           if (item.collisionType == CollisionType.active ||
               activeItem.collisionType == CollisionType.active) {
-            _potentials.add(CollisionProspect<T>(item, activeItem));
+            if (_prospectPool.length <= _potentials.length) {
+              _prospectPool.expand(item);
+            }
+            final prospect = _prospectPool[_potentials.length]
+              ..set(item, activeItem);
+            _potentials[prospect.hash] = prospect;
           }
         } else {
           _active.remove(activeItem);
@@ -48,6 +55,6 @@ class Sweep<T extends Hitbox<T>> extends Broadphase<T> {
       }
       _active.add(item);
     }
-    return _potentials;
+    return _potentials.values;
   }
 }

packages/flame/lib/src/collisions/collision_detection.dart

@@ -12,7 +12,7 @@ abstract class CollisionDetection<T extends Hitbox<T>,
   final B broadphase;
 
   List<T> get items => broadphase.items;
-  final Set<CollisionProspect<T>> _lastPotentials = {};
+  final _lastPotentials = <CollisionProspect<T>>[];
 
   CollisionDetection({required this.broadphase});
 
@@ -32,9 +32,11 @@ abstract class CollisionDetection<T extends Hitbox<T>,
   void run() {
     broadphase.update();
     final potentials = broadphase.query();
-    potentials.forEach((tuple) {
-      final itemA = tuple.a;
-      final itemB = tuple.b;
+    final hashes = Set.unmodifiable(potentials.map((p) => p.hash));
+
+    for (final potential in potentials) {
+      final itemA = potential.a;
+      final itemB = potential.b;
 
       if (itemA.possiblyIntersects(itemB)) {
         final intersectionPoints = intersections(itemA, itemB);
@@ -49,18 +51,33 @@ abstract class CollisionDetection<T extends Hitbox<T>,
       } else if (itemA.collidingWith(itemB)) {
         handleCollisionEnd(itemA, itemB);
       }
-    });
+    }
 
     // Handles callbacks for an ended collision that the broadphase didn't
-    // reports as a potential collision anymore.
-    _lastPotentials.difference(potentials).forEach((tuple) {
-      if (tuple.a.collidingWith(tuple.b)) {
-        handleCollisionEnd(tuple.a, tuple.b);
+    // report as a potential collision anymore.
+    for (final prospect in _lastPotentials) {
+      if (!hashes.contains(prospect.hash) &&
+          prospect.a.collidingWith(prospect.b)) {
+        handleCollisionEnd(prospect.a, prospect.b);
       }
-    });
-    _lastPotentials
-      ..clear()
-      ..addAll(potentials);
+    }
+    _updateLastPotentials(potentials);
+  }
+
+  final _lastPotentialsPool = <CollisionProspect<T>>[];
+  void _updateLastPotentials(Iterable<CollisionProspect<T>> potentials) {
+    _lastPotentials.clear();
+    for (final potential in potentials) {
+      final CollisionProspect<T> lastPotential;
+      if (_lastPotentialsPool.length > _lastPotentials.length) {
+        lastPotential = _lastPotentialsPool[_lastPotentials.length]
+          ..setFrom(potential);
+      } else {
+        lastPotential = potential.clone();
+        _lastPotentialsPool.add(lastPotential);
+      }
+      _lastPotentials.add(lastPotential);
+    }
   }
 
   /// Check what the intersection points of two items are,

packages/flame/test/collisions/collision_callback_test.dart

@@ -405,14 +405,16 @@ void main() {
     },
     'component collision callbacks are not called with hitbox '
         'triggersParentCollision option': (game) async {
-      final utilityHitboxA = TestHitbox()..triggersParentCollision = false;
+      final utilityHitboxA = TestHitbox('hitboxA')
+        ..triggersParentCollision = false;
       final blockA = TestBlock(
         Vector2.all(10),
         Vector2.all(10),
       );
       blockA.add(utilityHitboxA);
 
-      final utilityHitboxB = TestHitbox()..triggersParentCollision = false;
+      final utilityHitboxB = TestHitbox('hitboxB')
+        ..triggersParentCollision = false;
       final blockB = TestBlock(
         Vector2.all(15),
         Vector2.all(10),

packages/flame/test/collisions/collision_test_helpers.dart

@@ -53,8 +53,9 @@ class TestHitbox extends RectangleHitbox {
   int startCounter = 0;
   int onCollisionCounter = 0;
   int endCounter = 0;
+  String? name;
 
-  TestHitbox() {
+  TestHitbox([this.name]) {
     onCollisionCallback = (_, __) {
       onCollisionCounter++;
     };
@@ -65,6 +66,13 @@ class TestHitbox extends RectangleHitbox {
       endCounter++;
     };
   }
+
+  @override
+  String toString() {
+    return name == null
+        ? '_TestHitbox[${identityHashCode(this)}]'
+        : '_TestHitbox[$name]';
+  }
 }
 
 class CompositeTestHitbox extends CompositeHitbox {

packages/flame_lint/lib/analysis_options.yaml

@@ -104,7 +104,6 @@ linter:
     - prefer_final_in_for_each
     - prefer_final_locals
     - prefer_for_elements_to_map_fromIterable
-    - prefer_foreach
     - prefer_function_declarations_over_variables
     - prefer_generic_function_type_aliases
     - prefer_if_elements_to_conditional_expressions