From a87b8083ce176707a3c8b57bfc364d2bb6a95203 Mon Sep 17 00:00:00 2001
From: Erik Onarheim <erik.onarheim@gmail.com>
Date: Fri, 12 Jan 2024 20:34:02 -0600
Subject: [PATCH] fix: Flatten composite colliders (#2878)

This PR fixes an issue where `CompositeColliders` nested inside each other would cause a crash at runtime because the narrowphase doesn't know how to collide with composites, only pieces.

The fix flattens composites so they can only ever be 1 level deep.
---
 CHANGELOG.md                                  |  2 ++
 .../Collision/Colliders/CompositeCollider.ts  | 36 ++++++++++++-------
 src/spec/CompositeColliderSpec.ts             | 22 ++++++++++++
 3 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1f82d13b2..ba7ddbfcc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Fixed
 
+- Fixed issue where nesting `ex.CompositeColliders` inside one another would cause a crash on collision
+- Fixed issue where `ex.CompositeColliders` did not respect collider offset
 - Fixed issue where parenting a entity with fixed updates on would cause a drawing flicker, transform interpolation now is aware of changing parents so it interpolates drawing continuously to prevent any flickering
 - `ex.Animation.reset()` did not properly reset all internal state
 
diff --git a/src/engine/Collision/Colliders/CompositeCollider.ts b/src/engine/Collision/Colliders/CompositeCollider.ts
index 8cac3e256..e053b8313 100644
--- a/src/engine/Collision/Colliders/CompositeCollider.ts
+++ b/src/engine/Collision/Colliders/CompositeCollider.ts
@@ -26,19 +26,26 @@ export class CompositeCollider extends Collider {
     }
   }
 
-  // TODO composite offset
-
-
   clearColliders() {
     this._colliders = [];
   }
 
   addCollider(collider: Collider) {
-    collider.events.pipe(this.events);
-    collider.__compositeColliderId = this.id;
-    this._colliders.push(collider);
-    this._collisionProcessor.track(collider);
-    this._dynamicAABBTree.trackCollider(collider);
+    let colliders: Collider[];
+    if (collider instanceof CompositeCollider) {
+      colliders = collider.getColliders();
+      colliders.forEach(c => c.offset.addEqual(collider.offset));
+    } else {
+      colliders = [collider];
+    }
+    // Flatten composites
+    for (const c of colliders) {
+      c.events.pipe(this.events);
+      c.__compositeColliderId = this.id;
+      this._colliders.push(c);
+      this._collisionProcessor.track(c);
+      this._dynamicAABBTree.trackCollider(c);
+    }
   }
 
   removeCollider(collider: Collider) {
@@ -54,11 +61,11 @@ export class CompositeCollider extends Collider {
   }
 
   get worldPos(): Vector {
-    return this._transform?.pos ?? Vector.Zero;
+    return (this._transform?.pos ?? Vector.Zero).add(this.offset);
   }
 
   get center(): Vector {
-    return this._transform?.pos ?? Vector.Zero;
+    return (this._transform?.pos ?? Vector.Zero).add(this.offset);
   }
 
   get bounds(): BoundingBox {
@@ -69,7 +76,7 @@ export class CompositeCollider extends Collider {
       colliders[0]?.bounds ?? new BoundingBox().translate(this.worldPos)
     );
 
-    return results;
+    return results.translate(this.offset);
   }
 
   get localBounds(): BoundingBox {
@@ -240,12 +247,17 @@ export class CompositeCollider extends Collider {
 
   public debug(ex: ExcaliburGraphicsContext, color: Color,  options?: { lineWidth: number, pointSize: number }) {
     const colliders = this.getColliders();
+    ex.save();
+    ex.translate(this.offset.x, this.offset.y);
     for (const collider of colliders) {
       collider.debug(ex, color, options);
     }
+    ex.restore();
   }
 
   clone(): Collider {
-    return new CompositeCollider(this._colliders.map((c) => c.clone()));
+    const result = new CompositeCollider(this._colliders.map((c) => c.clone()));
+    result.offset = this.offset.clone();
+    return result;
   }
 }
diff --git a/src/spec/CompositeColliderSpec.ts b/src/spec/CompositeColliderSpec.ts
index 6f4a4bdfa..e2c1bc769 100644
--- a/src/spec/CompositeColliderSpec.ts
+++ b/src/spec/CompositeColliderSpec.ts
@@ -278,4 +278,26 @@ describe('A CompositeCollider', () => {
     dynamicTreeProcessor.untrack(compCollider);
     expect(dynamicTreeProcessor.getColliders().length).toBe(0);
   });
+
+  it('flattens composite colliders inside composite colliders with adjusted offset', () => {
+    const compCollider = new ex.CompositeCollider([ex.Shape.Circle(50), ex.Shape.Box(200, 10, Vector.Half)]);
+    compCollider.offset = ex.vec(50, 100);
+    expect(compCollider.getColliders()[0].offset).toBeVector(Vector.Zero);
+    expect(compCollider.getColliders()[1].offset).toBeVector(Vector.Zero);
+
+    const sut = new ex.CompositeCollider([]);
+
+    sut.addCollider(compCollider);
+
+    expect(sut.getColliders().length).toBe(2);
+    expect(sut.getColliders()[0].offset).toBeVector(compCollider.offset);
+    expect(sut.getColliders()[1].offset).toBeVector(compCollider.offset);
+  });
+
+  it('has the correct bounds when offset', () => {
+    const compCollider = new ex.CompositeCollider([ex.Shape.Circle(50), ex.Shape.Box(200, 10, Vector.Half)]);
+    expect(compCollider.bounds).toEqual(new ex.BoundingBox({left: -100, right: 100, top: -50, bottom: 50}));
+    compCollider.offset = ex.vec(50, 100);
+    expect(compCollider.bounds).toEqual(new ex.BoundingBox({left: -50, right: 150, top: 50, bottom: 150}));
+  });
 });