From c73bb03103b2781a41f9a5f52246f7b7b5b1de53 Mon Sep 17 00:00:00 2001 From: Antamansid Date: Wed, 15 Jul 2026 17:58:07 +0300 Subject: [PATCH 1/4] fix(HitTest): don't hang waitUsableRectUpdate when graph stabilizes via processQueue drain --- src/services/HitTest.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/services/HitTest.ts b/src/services/HitTest.ts index 731b42e6..367db64e 100644 --- a/src/services/HitTest.ts +++ b/src/services/HitTest.ts @@ -226,13 +226,15 @@ export class HitTest extends Emitter { let cleaned = false; let unsubRect: () => void = noop; let unsubPending: () => void = noop; + let check: () => void = noop; const cleanup = () => { if (cleaned) return; cleaned = true; unsubRect(); unsubPending(); + this.off("update", check); }; - const check = () => { + check = () => { if (!this.isUnstable) { cleanup(); // eslint-disable-next-line callback-return @@ -241,6 +243,14 @@ export class HitTest extends Emitter { }; unsubRect = this.$usableRect.subscribe(check); unsubPending = this.$pendingEntitiesUpdate.subscribe(check); + // isUnstable also depends on the processing queue (processQueue.isScheduled() / queue.size), + // which is not a signal. A re-entrant second processQueue batch can drain the queue and + // stabilize the graph WITHOUT changing $usableRect or $pendingEntitiesUpdate (their final + // values are already set — updateUsableRect early-returns on an unchanged rect and the + // pending flag is already false), so neither subscription would ever re-fire and the + // callback would hang forever. processQueue emits "update" at the end of every run, so + // re-check stability there too. + this.on("update", check); return cleanup; } callback(this.$usableRect.value); From 401adde285f66368da32600ee735f4d7308292c8 Mon Sep 17 00:00:00 2001 From: Antamansid Date: Wed, 15 Jul 2026 18:05:03 +0300 Subject: [PATCH 2/4] chore: add test --- src/services/HitTest.test.ts | 61 ++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/services/HitTest.test.ts b/src/services/HitTest.test.ts index 63f83682..3a21695b 100644 --- a/src/services/HitTest.test.ts +++ b/src/services/HitTest.test.ts @@ -49,6 +49,44 @@ function triggerProcessQueue(ht: HitTest): void { (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 bbox = { minX: 0, minY: 0, maxX: 100, maxY: 100, x: 0, y: 0 }; + const second = { + affectsUsableRect: true, + destroyed: false, + ...bbox, + updateRect(_bbox: unknown): void { + // no-op + }, + } as unknown as HitBox; + const first = { + affectsUsableRect: true, + destroyed: false, + ...bbox, + updateRect(_bbox: unknown): void { + if (!reentered) { + reentered = true; + // Re-entrant update while processQueue is running → schedules a second batch. + ht.update(second, { ...bbox }); + } + }, + } as unknown as HitBox; + + ht.update(first, { ...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", () => { it("makes isUnstable true immediately after call", () => { const ht = makeHitTest(true); @@ -86,4 +124,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 + }); }); From e3dc48053bed1bfb25755929dde3599c42c21335 Mon Sep 17 00:00:00 2001 From: Antamansid Date: Wed, 15 Jul 2026 18:25:35 +0300 Subject: [PATCH 3/4] chore: no comments --- src/services/HitTest.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/services/HitTest.ts b/src/services/HitTest.ts index 367db64e..a5990080 100644 --- a/src/services/HitTest.ts +++ b/src/services/HitTest.ts @@ -243,13 +243,6 @@ export class HitTest extends Emitter { }; unsubRect = this.$usableRect.subscribe(check); unsubPending = this.$pendingEntitiesUpdate.subscribe(check); - // isUnstable also depends on the processing queue (processQueue.isScheduled() / queue.size), - // which is not a signal. A re-entrant second processQueue batch can drain the queue and - // stabilize the graph WITHOUT changing $usableRect or $pendingEntitiesUpdate (their final - // values are already set — updateUsableRect early-returns on an unchanged rect and the - // pending flag is already false), so neither subscription would ever re-fire and the - // callback would hang forever. processQueue emits "update" at the end of every run, so - // re-check stability there too. this.on("update", check); return cleanup; } From cf517f72c4bdaf0ee471003c5632ac96337bfe37 Mon Sep 17 00:00:00 2001 From: Antamansid Date: Wed, 15 Jul 2026 18:33:49 +0300 Subject: [PATCH 4/4] chore: fix unknown and eslint errors --- src/services/HitTest.test.ts | 59 ++++++++++++------------------------ src/services/HitTest.ts | 15 ++++----- 2 files changed, 26 insertions(+), 48 deletions(-) diff --git a/src/services/HitTest.test.ts b/src/services/HitTest.test.ts index 3a21695b..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,20 +37,7 @@ 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, - updateRect(_bbox: unknown): void { - // no-op for test fake - }, - } as unknown as HitBox; - ht.update(fakeHitBox, { 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(); } @@ -59,29 +51,18 @@ function triggerProcessQueue(ht: HitTest): void { */ function triggerReentrantProcessQueue(ht: HitTest): void { let reentered = false; - const bbox = { minX: 0, minY: 0, maxX: 100, maxY: 100, x: 0, y: 0 }; - const second = { - affectsUsableRect: true, - destroyed: false, - ...bbox, - updateRect(_bbox: unknown): void { - // no-op - }, - } as unknown as HitBox; - const first = { - affectsUsableRect: true, - destroyed: false, - ...bbox, + const second = makeHitBox(); + const first = makeHitBox({ updateRect(_bbox: unknown): void { if (!reentered) { reentered = true; // Re-entrant update while processQueue is running → schedules a second batch. - ht.update(second, { ...bbox }); + ht.update(second, { ...STUB_BBOX }); } }, - } as unknown as HitBox; + }); - ht.update(first, { ...bbox }); + 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 diff --git a/src/services/HitTest.ts b/src/services/HitTest.ts index a5990080..ce3e1096 100644 --- a/src/services/HitTest.ts +++ b/src/services/HitTest.ts @@ -224,26 +224,23 @@ export class HitTest extends Emitter { if (this.isUnstable) { let cleaned = false; - let unsubRect: () => void = noop; - let unsubPending: () => void = noop; - let check: () => void = noop; + const unsubscribers: Array<() => void> = []; const cleanup = () => { if (cleaned) return; cleaned = true; - unsubRect(); - unsubPending(); - this.off("update", check); + unsubscribers.forEach((unsubscribe) => unsubscribe()); }; - check = () => { + const check = () => { if (!this.isUnstable) { cleanup(); // eslint-disable-next-line callback-return 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);