diff --git a/src/services/HitTest.test.ts b/src/services/HitTest.test.ts index 63f83682..8e8772b1 100644 --- a/src/services/HitTest.test.ts +++ b/src/services/HitTest.test.ts @@ -12,18 +12,23 @@ function makeHitTest(hasBlocks = false): HitTest { return new HitTest(mockGraph); } -function seedUsableRect(ht: HitTest): void { - const fakeHitBox = { +const STUB_BBOX = { minX: 0, minY: 0, maxX: 100, maxY: 100, x: 0, y: 0 }; + +/** Build a fake HitBox stub with sane defaults (a no-op updateRect and a 0..100 bbox). */ +function makeHitBox(overrides: Partial = {}): HitBox { + return { affectsUsableRect: true, destroyed: false, - minX: 0, - minY: 0, - maxX: 100, - maxY: 100, - x: 0, - y: 0, + ...STUB_BBOX, + updateRect(_bbox: unknown): void { + // no-op stub + }, + ...overrides, } as unknown as HitBox; - (ht as unknown as { usableRectTracker: { add(h: HitBox): void } }).usableRectTracker.add(fakeHitBox); +} + +function seedUsableRect(ht: HitTest): void { + (ht as unknown as { usableRectTracker: { add(h: HitBox): void } }).usableRectTracker.add(makeHitBox()); (ht as unknown as { updateUsableRect(): void }).updateUsableRect(); } @@ -32,21 +37,35 @@ function seedUsableRect(ht: HitTest): void { * This mirrors real production code: hitbox updates always precede processQueue. */ function triggerProcessQueue(ht: HitTest): void { - const fakeHitBox = { - affectsUsableRect: true, - destroyed: false, - minX: 0, - minY: 0, - maxX: 100, - maxY: 100, - x: 0, - y: 0, + ht.update(makeHitBox(), { ...STUB_BBOX }); + (ht as unknown as { processQueue: { flush(): void } }).processQueue.flush(); +} + +/** + * Trigger a re-entrant, two-batch processQueue run: processing the first hitbox queues a second + * hitbox update, which schedules another processQueue batch. This mirrors real rendering where + * a component's hitbox registration triggers further hitbox updates. The dead spot appears here: + * the first batch clears `$pendingEntitiesUpdate` while a second batch is still scheduled, so the + * graph is still unstable purely because of the queue; the second batch then drains and stabilizes + * the graph WITHOUT changing `$usableRect` or `$pendingEntitiesUpdate` (both already final). + */ +function triggerReentrantProcessQueue(ht: HitTest): void { + let reentered = false; + const second = makeHitBox(); + const first = makeHitBox({ updateRect(_bbox: unknown): void { - // no-op for test fake + if (!reentered) { + reentered = true; + // Re-entrant update while processQueue is running → schedules a second batch. + ht.update(second, { ...STUB_BBOX }); + } }, - } as unknown as HitBox; - ht.update(fakeHitBox, { minX: 0, minY: 0, maxX: 100, maxY: 100, x: 0, y: 0 }); - (ht as unknown as { processQueue: { flush(): void } }).processQueue.flush(); + }); + + ht.update(first, { ...STUB_BBOX }); + const pq = (ht as unknown as { processQueue: { flush(): void; isScheduled(): boolean } }).processQueue; + pq.flush(); // batch #1: re-enters, schedules batch #2, clears pending while still scheduled + pq.flush(); // batch #2: drains the queue and stabilizes via the "update" event, not a signal } describe("HitTest.markPendingUpdate", () => { @@ -86,4 +105,27 @@ describe("HitTest.markPendingUpdate", () => { triggerProcessQueue(ht); expect(called).toBe(true); // resolved after flag cleared }); + + // Regression: with a re-entrant second processQueue batch, the graph becomes stable only once + // the queue drains — an event NOT reflected by the $usableRect / $pendingEntitiesUpdate signals. + // Subscribing the stability check to those two signals alone leaves the callback hanging forever + // (the "graph flies off-screen on open" bug). The fix also re-checks on the "update" event. + it("waitUsableRectUpdate resolves when the graph stabilizes via a re-entrant queue drain", () => { + const ht = makeHitTest(true); + seedUsableRect(ht); + + ht.markPendingUpdate(); + expect(ht.isUnstable).toBe(true); + + let called = false; + ht.waitUsableRectUpdate(() => { + called = true; + }); + expect(called).toBe(false); // deferred: still unstable + + triggerReentrantProcessQueue(ht); + + expect(ht.isUnstable).toBe(false); // graph did stabilize + expect(called).toBe(true); // ...and the callback must have fired + }); }); diff --git a/src/services/HitTest.ts b/src/services/HitTest.ts index 731b42e6..ce3e1096 100644 --- a/src/services/HitTest.ts +++ b/src/services/HitTest.ts @@ -224,13 +224,11 @@ export class HitTest extends Emitter { if (this.isUnstable) { let cleaned = false; - let unsubRect: () => void = noop; - let unsubPending: () => void = noop; + const unsubscribers: Array<() => void> = []; const cleanup = () => { if (cleaned) return; cleaned = true; - unsubRect(); - unsubPending(); + unsubscribers.forEach((unsubscribe) => unsubscribe()); }; const check = () => { if (!this.isUnstable) { @@ -239,8 +237,10 @@ export class HitTest extends Emitter { callback(this.$usableRect.value); } }; - unsubRect = this.$usableRect.subscribe(check); - unsubPending = this.$pendingEntitiesUpdate.subscribe(check); + unsubscribers.push(this.$usableRect.subscribe(check)); + unsubscribers.push(this.$pendingEntitiesUpdate.subscribe(check)); + this.on("update", check); + unsubscribers.push(() => this.off("update", check)); return cleanup; } callback(this.$usableRect.value);