diff --git a/CHANGELOG.md b/CHANGELOG.md index f8917fd8f..3237e2822 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Added +- Added additional data `side` and `lastContact` to `onCollisionEnd` and `collisionend` events +- Added configuration option to `ex.PhysicsConfig` to configure composite collider onCollisionStart/End behavior - Added configuration option to `ex.TileMap({ meshingLookBehind: Infinity })` which allows users to configure how far the TileMap looks behind for matching colliders (default is 10). - Added Arcade Collision Solver bias to help mitigate seams in geometry that can cause problems for certain games. - `ex.ContactSolveBias.None` No bias, current default behavior collisions are solved in the default distance order @@ -203,6 +205,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- Fixes an issue where a collider that was part of a contact that was deleted did not fire a collision end event, this was unexpected +- Fixes an issue where you may want to have composite colliders behave as constituent colliders for the purposes of start/end collision events. A new property is added to physics config, the current behavior is the default which is `'together'`, this means the whole composite collider is treated as 1 collider for onCollisionStart/onCollisionEnd. Now you can configure a `separate` which will fire onCollisionStart/onCollisionEnd for every separate collider included in the composite (useful if you are building levels or things with gaps that you need to disambiguate). You can also configure this on a per composite level to mix and match `CompositeCollider.compositeStrategy` - Fixed issue where particles would have an errant draw if using a particle sprite - Fixed issue where a null/undefined graphics group member graphic would cause a crash, now logs a warning. - Fixed issue where Actor built in components could not be extended because of the way the Actor based type was built. diff --git a/sandbox/src/game.ts b/sandbox/src/game.ts index e2ee4a5a0..c082f0e1c 100644 --- a/sandbox/src/game.ts +++ b/sandbox/src/game.ts @@ -53,12 +53,15 @@ var game = new ex.Engine({ pointerScope: ex.PointerScope.Canvas, displayMode: ex.DisplayMode.FitScreenAndFill, snapToPixel: false, - fixedUpdateFps: 30, + fixedUpdateFps: 60, maxFps: 60, antialiasing: false, uvPadding: 0, physics: { - solver: ex.SolverStrategy.Realistic, + colliders: { + compositeStrategy: 'together' + }, + solver: ex.SolverStrategy.Arcade, gravity: ex.vec(0, 20), arcade: { contactSolveBias: ex.ContactSolveBias.VerticalFirst @@ -578,8 +581,8 @@ player.on('collisionstart', () => { console.log('collision start'); }); -player.on('collisionend', () => { - console.log('collision end'); +player.on('collisionend', (e) => { + console.log('collision end', e.other.collider); }); // Health bar example diff --git a/src/engine/Actor.ts b/src/engine/Actor.ts index 77f32532d..e2c12520c 100644 --- a/src/engine/Actor.ts +++ b/src/engine/Actor.ts @@ -973,8 +973,10 @@ export class Actor extends Entity implements Eventable, PointerEvents, CanInitia * Fires once when 2 entities with a ColliderComponent separate after having been in contact. * @param self * @param other + * @param side + * @param lastContact */ - public onCollisionEnd(self: Collider, other: Collider) { + public onCollisionEnd(self: Collider, other: Collider, side: Side, lastContact: CollisionContact) { // Override me } diff --git a/src/engine/Collision/ColliderComponent.ts b/src/engine/Collision/ColliderComponent.ts index d9d40b216..b497e3430 100644 --- a/src/engine/Collision/ColliderComponent.ts +++ b/src/engine/Collision/ColliderComponent.ts @@ -58,18 +58,25 @@ export class ColliderComponent extends Component { return collider; } + private _collidersToRemove: Collider[] = []; /** * Remove collider geometry from collider component */ public clear() { if (this._collider) { - this._collider.events.unpipe(this.events); - this.$colliderRemoved.notifyAll(this._collider); - this._collider.owner = null; + this._collidersToRemove.push(this._collider); this._collider = null; } } + public processColliderRemoval() { + for (const collider of this._collidersToRemove) { + collider.events.unpipe(this.events); + this.$colliderRemoved.notifyAll(collider); + collider.owner = null; + } + } + public clone(): ColliderComponent { const clone = new ColliderComponent(this._collider.clone()); @@ -187,9 +194,9 @@ export class ColliderComponent extends Component { }); this.events.on('collisionend', (evt: any) => { const end = evt as CollisionEndEvent; - entity.events.emit('collisionend', new CollisionEndEvent(end.target.owner, end.other.owner)); + entity.events.emit('collisionend', new CollisionEndEvent(end.target.owner, end.other.owner, end.side, end.lastContact)); if (entity instanceof Actor) { - entity.onCollisionEnd(end.target, end.other); + entity.onCollisionEnd(end.target, end.other, end.side, end.lastContact); } }); } diff --git a/src/engine/Collision/Colliders/Collider.ts b/src/engine/Collision/Colliders/Collider.ts index b7420dcef..444cf1dce 100644 --- a/src/engine/Collision/Colliders/Collider.ts +++ b/src/engine/Collision/Colliders/Collider.ts @@ -12,6 +12,7 @@ import { ExcaliburGraphicsContext } from '../../Graphics/Context/ExcaliburGraphi import { Transform } from '../../Math/transform'; import { EventEmitter } from '../../EventEmitter'; import { RayCastHit } from '../Detection/RayCastHit'; +import { CompositeCollider } from './CompositeCollider'; /** * A collision collider specifies the geometry that can detect when other collision colliders intersect @@ -21,11 +22,11 @@ export abstract class Collider implements Clonable { private static _ID = 0; public readonly id: Id<'collider'> = createId('collider', Collider._ID++); /** - * Excalibur uses this to signal to the [[CollisionSystem]] this is part of a composite collider - * @internal - * @hidden + * Composite collider if any this collider is attached to + * + * **WARNING** do not tamper with this property */ - public __compositeColliderId: Id<'collider'> | null = null; + public composite: CompositeCollider | null = null; public events = new EventEmitter(); /** diff --git a/src/engine/Collision/Colliders/CompositeCollider.ts b/src/engine/Collision/Colliders/CompositeCollider.ts index 75360b026..f1bd397d5 100644 --- a/src/engine/Collision/Colliders/CompositeCollider.ts +++ b/src/engine/Collision/Colliders/CompositeCollider.ts @@ -21,6 +21,26 @@ export class CompositeCollider extends Collider { private _dynamicAABBTree = new DynamicTree(DefaultPhysicsConfig.dynamicTree); private _colliders: Collider[] = []; + private _compositeStrategy?: 'separate' | 'together'; + /** + * Treat composite collider's member colliders as either separate colliders for the purposes of onCollisionStart/onCollision + * or as a single collider together. + * + * This property can be overridden on individual [[CompositeColliders]]. + * + * For composites without gaps or small groups of colliders, you probably want 'together' + * + * For composites with deliberate gaps, like a platforming level layout, you probably want 'separate' + * + * Default is 'together' if unset + */ + public set compositeStrategy(value: 'separate' | 'together') { + this._compositeStrategy = value; + } + public get compositeStrategy() { + return this._compositeStrategy; + } + constructor(colliders: Collider[]) { super(); for (const c of colliders) { @@ -43,7 +63,7 @@ export class CompositeCollider extends Collider { // Flatten composites for (const c of colliders) { c.events.pipe(this.events); - c.__compositeColliderId = this.id; + c.composite = this; this._colliders.push(c); this._collisionProcessor.track(c); this._dynamicAABBTree.trackCollider(c); @@ -52,7 +72,7 @@ export class CompositeCollider extends Collider { removeCollider(collider: Collider) { collider.events.pipe(this.events); - collider.__compositeColliderId = null; + collider.composite = null; Util.removeItemFromArray(collider, this._colliders); this._collisionProcessor.untrack(collider); this._dynamicAABBTree.untrackCollider(collider); diff --git a/src/engine/Collision/Colliders/PolygonCollider.ts b/src/engine/Collision/Colliders/PolygonCollider.ts index 6395ac235..09bb4d838 100644 --- a/src/engine/Collision/Colliders/PolygonCollider.ts +++ b/src/engine/Collision/Colliders/PolygonCollider.ts @@ -43,16 +43,21 @@ export class PolygonCollider extends Collider { */ public offset: Vector; + public flagDirty() { + this._localBoundsDirty = true; + this._localSidesDirty = true; + this._sidesDirty = true; + } + private _points: Vector[]; + /** * Points in the polygon in order around the perimeter in local coordinates. These are relative from the body transform position. * Excalibur stores these in counter-clockwise order */ public set points(points: Vector[]) { - this._localBoundsDirty = true; - this._localSidesDirty = true; - this._sidesDirty = true; this._points = points; + this.flagDirty(); } /** diff --git a/src/engine/Collision/CollisionSystem.ts b/src/engine/Collision/CollisionSystem.ts index 31b14446f..e87ca5f4a 100644 --- a/src/engine/Collision/CollisionSystem.ts +++ b/src/engine/Collision/CollisionSystem.ts @@ -79,6 +79,9 @@ export class CollisionSystem extends System { colliderComp.update(); if (collider instanceof CompositeCollider) { const compositeColliders = collider.getColliders(); + if (!collider.compositeStrategy) { + collider.compositeStrategy = this._physics.config.colliders.compositeStrategy; + } colliders = colliders.concat(compositeColliders); } else { colliders.push(collider); @@ -127,6 +130,14 @@ export class CollisionSystem extends System { // Keep track of collisions contacts that have started or ended this._lastFrameContacts = new Map(this._currentFrameContacts); + + // Process deferred collider removals + for (const entity of this.query.entities) { + const collider = entity.get(ColliderComponent); + if (collider) { + collider.processColliderRemoval(); + } + } } getSolver(): CollisionSolver { @@ -143,7 +154,7 @@ export class CollisionSystem extends System { } public runContactStartEnd() { - // Composite collider collisions may have a duplicate id because we want to treat those as a singular start/end + // If composite colliders are 'together' collisions may have a duplicate id because we want to treat those as a singular start/end for (const [id, c] of this._currentFrameContacts) { // find all new contacts if (!this._lastFrameContacts.has(id)) { @@ -163,10 +174,12 @@ export class CollisionSystem extends System { if (!this._currentFrameContacts.has(id)) { const colliderA = c.colliderA; const colliderB = c.colliderB; - colliderA.events.emit('collisionend', new CollisionEndEvent(colliderA, colliderB)); - colliderA.events.emit('contactend', new ContactEndEvent(colliderA, colliderB) as any); - colliderB.events.emit('collisionend', new CollisionEndEvent(colliderB, colliderA)); - colliderB.events.emit('contactend', new ContactEndEvent(colliderB, colliderA) as any); + const side = Side.fromDirection(c.mtv); + const opposite = Side.getOpposite(side); + colliderA.events.emit('collisionend', new CollisionEndEvent(colliderA, colliderB, side, c)); + colliderA.events.emit('contactend', new ContactEndEvent(colliderA, colliderB, side, c) as any); + colliderB.events.emit('collisionend', new CollisionEndEvent(colliderB, colliderA, opposite, c)); + colliderB.events.emit('contactend', new ContactEndEvent(colliderB, colliderA, opposite, c) as any); } } } diff --git a/src/engine/Collision/Detection/CollisionContact.ts b/src/engine/Collision/Detection/CollisionContact.ts index c53f34d04..68abb707c 100644 --- a/src/engine/Collision/Detection/CollisionContact.ts +++ b/src/engine/Collision/Detection/CollisionContact.ts @@ -76,11 +76,11 @@ export class CollisionContact { this.localPoints = localPoints; this.info = info; this.id = Pair.calculatePairHash(colliderA.id, colliderB.id); - if (colliderA.__compositeColliderId || colliderB.__compositeColliderId) { - // Add on the parent composite pair for start/end contact - this.id += '|' + Pair.calculatePairHash( - colliderA.__compositeColliderId ?? colliderA.id, - colliderB.__compositeColliderId ?? colliderB.id); + if (colliderA.composite || colliderB.composite) { + // Add on the parent composite pair for start/end contact if 'together + const colliderAId = colliderA.composite?.compositeStrategy === 'separate' ? colliderA.id : colliderA.composite?.id ?? colliderA.id; + const colliderBId = colliderB.composite?.compositeStrategy === 'separate' ? colliderB.id : colliderB.composite?.id ?? colliderB.id; + this.id += '|' + Pair.calculatePairHash(colliderAId, colliderBId); } } diff --git a/src/engine/Collision/PhysicsConfig.ts b/src/engine/Collision/PhysicsConfig.ts index eaffdc478..1488198f9 100644 --- a/src/engine/Collision/PhysicsConfig.ts +++ b/src/engine/Collision/PhysicsConfig.ts @@ -30,6 +30,25 @@ export interface PhysicsConfig { */ solver?: SolverStrategy; + /** + * Configure colliders + */ + colliders?: { + /** + * Treat composite collider's member colliders as either separate colliders for the purposes of onCollisionStart/onCollision + * or as a single collider together. + * + * This property can be overridden on individual [[CompositeColliders]]. + * + * For composites without gaps or small groups of colliders, you probably want 'together' + * + * For composites with deliberate gaps, like a platforming level layout, you probably want 'separate' + * + * Default is 'together' if unset + */ + compositeStrategy?: 'separate' | 'together' + } + /** * Configure excalibur continuous collision (WIP) */ @@ -183,6 +202,9 @@ export const DefaultPhysicsConfig: DeepRequired = { enabled: true, gravity: vec(0, 0), solver: SolverStrategy.Arcade, + colliders: { + compositeStrategy: 'together' + }, continuous: { checkForFastBodies: true, disableMinimumSpeedForFastBody: false, @@ -224,6 +246,9 @@ export function DeprecatedStaticToConfig(): DeepRequired { disableMinimumSpeedForFastBody: Physics.disableMinimumSpeedForFastBody, surfaceEpsilon: Physics.surfaceEpsilon }, + colliders: { + compositeStrategy: 'together' + }, bodies: { canSleepByDefault: Physics.bodiesCanSleepByDefault, sleepEpsilon: Physics.sleepEpsilon, diff --git a/src/engine/Events.ts b/src/engine/Events.ts index 38b6e6003..73e27b4bb 100644 --- a/src/engine/Events.ts +++ b/src/engine/Events.ts @@ -451,7 +451,7 @@ export class ContactStartEvent { } export class ContactEndEvent { - constructor(public target: T, public other: T) {} + constructor(public target: T, public other: T, public side: Side, public lastContact: CollisionContact) {} } export class CollisionPreSolveEvent { @@ -494,7 +494,7 @@ export class CollisionEndEvent { expect(collider.owner).not.toBeNull(); comp.clear(); + comp.processColliderRemoval(); expect(comp.get()).toBeNull(); expect(collider.events.unpipe).toHaveBeenCalled(); diff --git a/src/spec/CollisionSpec.ts b/src/spec/CollisionSpec.ts index c78595be0..0b1fc97cc 100644 --- a/src/spec/CollisionSpec.ts +++ b/src/spec/CollisionSpec.ts @@ -25,6 +25,7 @@ describe('A Collision', () => { afterEach(() => { ex.Physics.collisionResolutionStrategy = ex.SolverStrategy.Arcade; engine.stop(); + engine.dispose(); engine = null; actor1 = null; actor2 = null; @@ -378,4 +379,50 @@ describe('A Collision', () => { expect(block2.onPreCollisionResolve).toHaveBeenCalled(); expect(block1.onCollisionStart).not.toHaveBeenCalled(); }); + + it('should collisionend for a deleted collider', async () => { + engine.stop(); + engine.dispose(); + engine = TestUtils.engine({ width: 600, height: 400, physics: { enabled: true, solver: ex.SolverStrategy.Arcade }}); + clock = engine.clock = engine.clock.toTestClock(); + await TestUtils.runToReady(engine); + const activeBlock = new ex.Actor({ + x: 200, + y: 200, + width: 50, + height: 50, + color: ex.Color.Red.clone(), + collisionType: ex.CollisionType.Active + }); + activeBlock.acc.x = 100; + engine.add(activeBlock); + + const fixedBlock = new ex.Actor({ + x: 400, + y: 200, + width: 50, + height: 50, + color: ex.Color.DarkGray.clone(), + collisionType: ex.CollisionType.Fixed + }); + engine.add(fixedBlock); + + const collisionStart = jasmine.createSpy('collisionstart'); + const collisionEnd = jasmine.createSpy('collisionend'); + + activeBlock.on('collisionstart', collisionStart); + activeBlock.on('collisionend', collisionEnd); + + clock.run(20, 100); + + expect(collisionStart).toHaveBeenCalled(); + expect(collisionEnd).not.toHaveBeenCalled(); + + activeBlock.collider.set(ex.Shape.Circle(5)); + + clock.run(10, 100); + + expect(collisionEnd).toHaveBeenCalledTimes(1); + + }); }); diff --git a/src/spec/CompositeColliderSpec.ts b/src/spec/CompositeColliderSpec.ts index 5ca5dad8c..5a847045c 100644 --- a/src/spec/CompositeColliderSpec.ts +++ b/src/spec/CompositeColliderSpec.ts @@ -158,6 +158,23 @@ describe('A CompositeCollider', () => { ex.Pair.calculatePairHash(compCollider.id, circle.id)); }); + it('creates contacts that have the don\'t have composite collider id when in separate mode', () => { + const compCollider = new ex.CompositeCollider([ex.Shape.Circle(50), ex.Shape.Box(200, 10, Vector.Half)]); + compCollider.compositeStrategy = 'separate'; + + const circle = ex.Shape.Circle(50); + const xf = new ex.Transform(); + xf.pos = vec(149, 0); + circle.update(xf); + + const contactBoxCircle = compCollider.collide(circle); + // Composite collisions have a special id that appends the "parent" id to the id to accurately track start/end + expect(contactBoxCircle[0].id).toBe( + ex.Pair.calculatePairHash(compCollider.getColliders()[1].id, circle.id) + + '|' + + ex.Pair.calculatePairHash(compCollider.getColliders()[1].id, circle.id)); + }); + it('can collide with other composite colliders', () => { const compCollider1 = new ex.CompositeCollider([ex.Shape.Circle(50), ex.Shape.Box(200, 10, Vector.Half)]);