diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bb2bd75..914ff73c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- Fixed issue where clearSchedule during a scheduled callback could cause a cb to be skipped - Fixed issue where the Loader could run twice even if already loaded when included in the scene loader. - Fixed issue where pixel ratio was accidentally doubled during load if the loader was included in the scene loader. - Fixed issue where Slide trasition did not work properly when DisplayMode was FitScreenAndFill diff --git a/src/engine/Util/Clock.ts b/src/engine/Util/Clock.ts index 1f4ef621..f78bf6a9 100644 --- a/src/engine/Util/Clock.ts +++ b/src/engine/Util/Clock.ts @@ -109,15 +109,14 @@ export abstract class Clock { return id; } + private _idsToRemove: ScheduleId[] = []; /** * Clears a scheduled callback using the ID returned from {@apilink schedule} * @param id The ID of the scheduled callback to clear */ public clearSchedule(id: ScheduleId): void { - const index = this._scheduledCbs.findIndex(([scheduleId]) => scheduleId === id); - if (index !== -1) { - this._scheduledCbs.splice(index, 1); - } + // Deferred removal + this._idsToRemove.push(id); } /** * Called internally to trigger scheduled callbacks in the clock @@ -127,12 +126,24 @@ export abstract class Clock { public __runScheduledCbs(timing: ScheduledCallbackTiming = 'preframe') { // walk backwards to delete items as we loop for (let i = this._scheduledCbs.length - 1; i > -1; i--) { - const [_, callback, scheduledTime, callbackTiming] = this._scheduledCbs[i]; + const [scheduleId, callback, scheduledTime, callbackTiming] = this._scheduledCbs[i]; + if (this._idsToRemove.includes(scheduleId)) { + // skip canceled ids + continue; + } if (timing === callbackTiming && scheduledTime <= this._totalElapsed) { callback(this._elapsed); this._scheduledCbs.splice(i, 1); } } + + // deferred removal + for (const id of this._idsToRemove) { + const index = this._scheduledCbs.findIndex(([scheduleId]) => scheduleId === id); + if (index !== -1) { + this._scheduledCbs.splice(index, 1); + } + } } protected update(overrideUpdateMs?: number): void { diff --git a/src/spec/vitest/ClockSpec.ts b/src/spec/vitest/ClockSpec.ts index 9afebd24..4f2158e9 100644 --- a/src/spec/vitest/ClockSpec.ts +++ b/src/spec/vitest/ClockSpec.ts @@ -130,6 +130,46 @@ describe('Clocks', () => { expect(scheduledCb).not.toHaveBeenCalled(); }); + it('can clear scheduled callbacks during dispatch of callbacks', () => { + const testClock = new ex.TestClock({ + tick: () => { + /* nothing */ + }, + defaultUpdateMs: 1000 + }); + testClock.start(); + + const scheduledCb1 = vi.fn(); + const id1 = testClock.schedule(scheduledCb1, 1000); + + const scheduledCb2 = vi.fn(); + const id2 = testClock.schedule(scheduledCb2, 1000); + const scheduledCb3 = vi.fn(); + const id3 = testClock.schedule(scheduledCb3, 1000); + + const scheduledCb4 = vi.fn(() => { + testClock.clearSchedule(id2); + testClock.clearSchedule(id1); + }); + const id4 = testClock.schedule(scheduledCb4, 1000); + expect(scheduledCb1).not.toHaveBeenCalled(); + expect(scheduledCb2).not.toHaveBeenCalled(); + expect(scheduledCb3).not.toHaveBeenCalled(); + expect(scheduledCb4).not.toHaveBeenCalled(); + testClock.step(500); + + expect(scheduledCb1).not.toHaveBeenCalled(); + expect(scheduledCb2).not.toHaveBeenCalled(); + expect(scheduledCb3).not.toHaveBeenCalled(); + expect(scheduledCb4).not.toHaveBeenCalled(); + testClock.step(500); + + expect(scheduledCb1).not.toHaveBeenCalled(); + expect(scheduledCb2).not.toHaveBeenCalled(); + expect(scheduledCb3).toHaveBeenCalledOnce(); + expect(scheduledCb4).toHaveBeenCalledOnce(); + }); + it('can limit fps', () => { const tickSpy = vi.fn(); const clock = new ex.TestClock({