From 30b11756f0b5530395490b4d3c48214338f489f8 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sun, 1 Mar 2026 00:32:18 -0800 Subject: [PATCH] CODY-59 factory|feat: Improve representational quality of gates Rewrite `GateActor` as a stateful, animated actor so that nonblocking gates are visually low (2px vs 40px blocking) and the transition is animated over 1 second. The orchestrator waits at a gate until the animation completes before walking through. - GateActor uses `Rectangle` graphic with `onPreUpdate` frame-by-frame height interpolation, bottom-edge pinned to the platform surface. - Gates persist across `rebuildStaticElements` via `gateMap` (mirrors the existing `agentMap` pattern), with diffing in `updateGates`. - `waitForOpen()` promise coordination lets the orchestrator await gate transitions; resolvers flush unconditionally on animation completion to prevent leaks on direction reversal. - Spatial constants use `_PX` suffix to indicate units, matching the existing _MS convention on GATE_TRANSITION_DURATION_MS. --- .../src/client/game/actors/GateActor.ts | 84 ++++- .../game/actors/__tests__/GateActor.test.ts | 352 ++++++++++++++++-- .../src/client/game/scenes/FactoryScene.ts | 61 ++- .../scenes/__tests__/FactoryScene.test.ts | 172 ++++++++- 4 files changed, 617 insertions(+), 52 deletions(-) diff --git a/packages/factory/src/client/game/actors/GateActor.ts b/packages/factory/src/client/game/actors/GateActor.ts index b0151b4c..311a0852 100644 --- a/packages/factory/src/client/game/actors/GateActor.ts +++ b/packages/factory/src/client/game/actors/GateActor.ts @@ -1,15 +1,89 @@ -import { Actor, Color, type Vector } from 'excalibur'; +import { Actor, Color, type Engine, Rectangle, type Vector } from 'excalibur'; import { PALETTE } from '../../../shared/constants/palette.js'; +export const GATE_WIDTH_PX = 5; +export const GATE_BLOCKING_HEIGHT_PX = 40; +export const GATE_NONBLOCKING_HEIGHT_PX = 2; +export const GATE_TRANSITION_DURATION_MS = 1000; + export class GateActor extends Actor { + private isOpen: boolean; + private rectGraphic: Rectangle; + private platformSurfaceY: number; + private animationElapsed: number | undefined; + private startHeight: number; + private targetHeight: number; + private openResolvers: Array<() => void> = []; + constructor(open: boolean, position: Vector) { - const color = open ? PALETTE.green : PALETTE.red; + const initialHeight = open ? GATE_NONBLOCKING_HEIGHT_PX : GATE_BLOCKING_HEIGHT_PX; + const platformSurfaceY = position.y + GATE_BLOCKING_HEIGHT_PX / 2; + super({ - pos: position, - width: 5, - height: 40, + pos: position.clone(), + width: GATE_WIDTH_PX, + }); + + this.isOpen = open; + this.platformSurfaceY = platformSurfaceY; + this.startHeight = initialHeight; + this.targetHeight = initialHeight; + + const color = open ? PALETTE.green : PALETTE.red; + this.rectGraphic = new Rectangle({ + width: GATE_WIDTH_PX, + height: initialHeight, color: Color.fromHex(color), }); + + this.graphics.use(this.rectGraphic); + this.pos.y = this.platformSurfaceY - initialHeight / 2; + } + + setOpen(open: boolean): void { + if (open === this.isOpen) return; + + this.isOpen = open; + + const color = open ? PALETTE.green : PALETTE.red; + this.rectGraphic.color = Color.fromHex(color); + + this.startHeight = this.rectGraphic.height; + this.targetHeight = open ? GATE_NONBLOCKING_HEIGHT_PX : GATE_BLOCKING_HEIGHT_PX; + this.animationElapsed = 0; + } + + waitForOpen(): Promise { + if (this.isOpen && this.animationElapsed === undefined) { + return Promise.resolve(); + } + return new Promise((resolve) => { + this.openResolvers.push(resolve); + }); + } + + override onPreUpdate(_engine: Engine, elapsed: number): void { + if (this.animationElapsed === undefined) return; + + this.animationElapsed += elapsed; + const progress = Math.min(1, this.animationElapsed / GATE_TRANSITION_DURATION_MS); + const currentHeight = this.startHeight + (this.targetHeight - this.startHeight) * progress; + + this.rectGraphic.height = currentHeight; + this.pos.y = this.platformSurfaceY - currentHeight / 2; + + if (progress >= 1) { + this.animationElapsed = undefined; + + // Flush all pending resolvers regardless of direction. When closing, + // resolvers must still be drained to prevent permanently blocking callers. + // Callers that need to re-check gate state can do so after resolution. + const resolvers = this.openResolvers; + this.openResolvers = []; + for (const resolve of resolvers) { + resolve(); + } + } } } diff --git a/packages/factory/src/client/game/actors/__tests__/GateActor.test.ts b/packages/factory/src/client/game/actors/__tests__/GateActor.test.ts index e0d774b4..060ab487 100644 --- a/packages/factory/src/client/game/actors/__tests__/GateActor.test.ts +++ b/packages/factory/src/client/game/actors/__tests__/GateActor.test.ts @@ -1,17 +1,28 @@ -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { mockActorConstructor } = vi.hoisted(() => { +const { mockActorConstructor, mockGraphicsUse, mockRectangleConstructor } = vi.hoisted(() => { return { mockActorConstructor: vi.fn(), + mockGraphicsUse: vi.fn(), + mockRectangleConstructor: vi.fn(), }; }); vi.mock('excalibur', () => { class MockActor { config: Record; + graphics = { use: mockGraphicsUse }; + pos = { x: 0, y: 0, clone: () => ({ x: 0, y: 0 }) }; constructor(config: Record) { mockActorConstructor(config); this.config = config; + const pos = config.pos; + if (typeof pos === 'object' && pos !== null && 'x' in pos && 'y' in pos) { + const { x, y } = pos; + const nx = typeof x === 'number' ? x : 0; + const ny = typeof y === 'number' ? y : 0; + this.pos = { x: nx, y: ny, clone: () => ({ x: nx, y: ny }) }; + } } } @@ -25,57 +36,328 @@ vi.mock('excalibur', () => { } } + class MockRectangle { + width: number; + height: number; + color: unknown; + constructor(options: { width: number; height: number; color: unknown }) { + mockRectangleConstructor(options); + this.width = options.width; + this.height = options.height; + this.color = options.color; + } + } + return { Actor: MockActor, Color: MockColor, - vec: (x: number, y: number) => ({ x, y }), + Rectangle: MockRectangle, + vec: (x: number, y: number) => ({ x, y, clone: () => ({ x, y }) }), }; }); -const { GateActor } = await import('../GateActor.js'); +const { GateActor, GATE_WIDTH_PX, GATE_BLOCKING_HEIGHT_PX, GATE_NONBLOCKING_HEIGHT_PX, GATE_TRANSITION_DURATION_MS } = + await import('../GateActor.js'); const { Color, vec } = await import('excalibur'); const { PALETTE } = await import('../../../../shared/constants/palette.js'); +interface MockRect { + width: number; + height: number; + color: unknown; +} + +function isMockRect(value: unknown): value is MockRect { + return typeof value === 'object' && value !== null && 'width' in value && 'height' in value && 'color' in value; +} + +/** Get the Rectangle graphic passed to `graphics.use()` in the most recent call. */ +function getUsedRect(): MockRect { + const lastCall: unknown[] | undefined = mockGraphicsUse.mock.lastCall; + if (lastCall === undefined) throw new Error('graphics.use() was never called'); + const arg: unknown = lastCall[0]; + if (!isMockRect(arg)) throw new Error('Unexpected argument to graphics.use()'); + return arg; +} + +/** Linear interpolation between two values. */ +function lerp(start: number, end: number, t: number): number { + return start + (end - start) * t; +} + +const POSITION_Y = 370; +const platformSurfaceY = POSITION_Y + GATE_BLOCKING_HEIGHT_PX / 2; + describe('GateActor', () => { - it('uses green color when gate is open', () => { - new GateActor(true, vec(100, 200)); - - expect(mockActorConstructor).toHaveBeenCalledWith( - expect.objectContaining({ - color: Color.fromHex(PALETTE.green), - }), - ); + beforeEach(() => { + mockActorConstructor.mockClear(); + mockGraphicsUse.mockClear(); + mockRectangleConstructor.mockClear(); }); - it('uses red color when gate is closed', () => { - new GateActor(false, vec(100, 200)); + describe('constructor', () => { + it('creates a rectangle with green color and nonblocking height when open', () => { + new GateActor(true, vec(100, POSITION_Y)); + + expect(mockRectangleConstructor).toHaveBeenCalledWith( + expect.objectContaining({ + width: GATE_WIDTH_PX, + height: GATE_NONBLOCKING_HEIGHT_PX, + color: Color.fromHex(PALETTE.green), + }), + ); + }); + + it('creates a rectangle with red color and blocking height when closed', () => { + new GateActor(false, vec(100, POSITION_Y)); + + expect(mockRectangleConstructor).toHaveBeenCalledWith( + expect.objectContaining({ + width: GATE_WIDTH_PX, + height: GATE_BLOCKING_HEIGHT_PX, + color: Color.fromHex(PALETTE.red), + }), + ); + }); + + it('uses the rectangle graphic via graphics.use()', () => { + new GateActor(true, vec(100, POSITION_Y)); + + expect(mockGraphicsUse).toHaveBeenCalled(); + }); + + it('pins bottom edge of open gate to platform surface', () => { + const gate = new GateActor(true, vec(100, POSITION_Y)); + + expect(gate.pos.y).toBe(platformSurfaceY - GATE_NONBLOCKING_HEIGHT_PX / 2); + }); + + it('pins bottom edge of closed gate to platform surface', () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + + expect(gate.pos.y).toBe(platformSurfaceY - GATE_BLOCKING_HEIGHT_PX / 2); + }); + + it('passes only width (not height or color) to the Actor constructor', () => { + new GateActor(true, vec(100, POSITION_Y)); + + expect(mockActorConstructor).toHaveBeenCalledWith(expect.objectContaining({ width: GATE_WIDTH_PX })); + expect(mockActorConstructor).toHaveBeenCalledWith(expect.not.objectContaining({ height: expect.anything() })); + expect(mockActorConstructor).toHaveBeenCalledWith(expect.not.objectContaining({ color: expect.anything() })); + }); + }); + + describe('setOpen', () => { + it('changes color to green when opening a closed gate', () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + const rect = getUsedRect(); + + gate.setOpen(true); + + expect(rect.color).toEqual(Color.fromHex(PALETTE.green)); + }); + + it('changes color to red when closing an open gate', () => { + const gate = new GateActor(true, vec(100, POSITION_Y)); + const rect = getUsedRect(); + + gate.setOpen(false); + + expect(rect.color).toEqual(Color.fromHex(PALETTE.red)); + }); + + it('is a no-op when setting same state', () => { + const gate = new GateActor(true, vec(100, POSITION_Y)); + const rect = getUsedRect(); + const colorBefore = rect.color; + + gate.setOpen(true); + + expect(rect.color).toBe(colorBefore); + }); + + it('starts animation when state changes (waitForOpen returns pending promise)', async () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + + gate.setOpen(true); + + // waitForOpen should not resolve immediately because animation is in progress + let resolved = false; + void gate.waitForOpen().then(() => { + resolved = true; + return undefined; + }); + + // Drain microtasks + await Promise.resolve(); + expect(resolved).toBe(false); + }); + + it('uses current interpolated height as startHeight when reversing direction mid-animation', () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + const rect = getUsedRect(); + + gate.setOpen(true); + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS / 2); + const midHeight = lerp(GATE_BLOCKING_HEIGHT_PX, GATE_NONBLOCKING_HEIGHT_PX, 0.5); + expect(rect.height).toBe(midHeight); - expect(mockActorConstructor).toHaveBeenCalledWith( - expect.objectContaining({ - color: Color.fromHex(PALETTE.red), - }), - ); + // Reverse direction: close the gate mid-animation + gate.setOpen(false); + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS / 2); + expect(rect.height).toBe(lerp(midHeight, GATE_BLOCKING_HEIGHT_PX, 0.5)); + + // Complete new animation: height should reach blocking height + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS / 2); + expect(rect.height).toBe(GATE_BLOCKING_HEIGHT_PX); + }); + + it('flushes pending waitForOpen resolvers when direction reverses to closed', async () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + + gate.setOpen(true); + const promise = gate.waitForOpen(); + + // Advance partway through opening animation + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, 300); + + // Reverse direction: close the gate + gate.setOpen(false); + + // Complete the closing animation + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS); + + // Resolver should be flushed to prevent the caller from hanging + await expect(promise).resolves.toBeUndefined(); + }); + + it('maintains consistent pos.y throughout mid-animation reversal', () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + + gate.setOpen(true); + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS / 2); + const midHeight = lerp(GATE_BLOCKING_HEIGHT_PX, GATE_NONBLOCKING_HEIGHT_PX, 0.5); + expect(gate.pos.y).toBe(platformSurfaceY - midHeight / 2); + + // Reverse direction + gate.setOpen(false); + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS / 2); + const reversedMidHeight = lerp(midHeight, GATE_BLOCKING_HEIGHT_PX, 0.5); + expect(gate.pos.y).toBe(platformSurfaceY - reversedMidHeight / 2); + }); }); - it('sets correct dimensions', () => { - new GateActor(true, vec(50, 75)); + describe('onPreUpdate', () => { + it('interpolates height to halfway at half of transition duration', () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + const rect = getUsedRect(); + + gate.setOpen(true); + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS / 2); + + expect(rect.height).toBe(lerp(GATE_BLOCKING_HEIGHT_PX, GATE_NONBLOCKING_HEIGHT_PX, 0.5)); + }); + + it('reaches target height when elapsed exceeds transition duration', () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + const rect = getUsedRect(); + + gate.setOpen(true); + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS * 1.2); + + expect(rect.height).toBe(GATE_NONBLOCKING_HEIGHT_PX); + }); + + it('updates pos.y to keep bottom edge pinned during animation', () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + + gate.setOpen(true); + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS / 2); + + const midHeight = lerp(GATE_BLOCKING_HEIGHT_PX, GATE_NONBLOCKING_HEIGHT_PX, 0.5); + expect(gate.pos.y).toBe(platformSurfaceY - midHeight / 2); + }); - expect(mockActorConstructor).toHaveBeenCalledWith( - expect.objectContaining({ - width: 5, - height: 40, - }), - ); + it('does nothing when no animation is in progress', () => { + const gate = new GateActor(true, vec(100, POSITION_Y)); + const yBefore = gate.pos.y; + + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, 100); + + expect(gate.pos.y).toBe(yBefore); + }); + + it('resolves waitForOpen promises when animation completes toward open', async () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + + gate.setOpen(true); + const promise = gate.waitForOpen(); + + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS); + + await expect(promise).resolves.toBeUndefined(); + }); + + it('resolves waitForOpen promises when animation completes toward closed', async () => { + const gate = new GateActor(true, vec(100, POSITION_Y)); + + gate.setOpen(false); + const promise = gate.waitForOpen(); + + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS); + + // Resolvers are flushed to prevent permanently blocking callers + await expect(promise).resolves.toBeUndefined(); + }); + + it('resolves all concurrent waitForOpen callers when animation completes', async () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + + gate.setOpen(true); + const promise1 = gate.waitForOpen(); + const promise2 = gate.waitForOpen(); + const promise3 = gate.waitForOpen(); + + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- test-only: passing dummy engine to onPreUpdate + gate.onPreUpdate(undefined as never, GATE_TRANSITION_DURATION_MS); + + await expect(promise1).resolves.toBeUndefined(); + await expect(promise2).resolves.toBeUndefined(); + await expect(promise3).resolves.toBeUndefined(); + }); }); - it('passes position to Actor constructor', () => { - const pos = vec(300, 400); - new GateActor(false, pos); + describe('waitForOpen', () => { + it('resolves immediately when gate is already open with no animation', async () => { + const gate = new GateActor(true, vec(100, POSITION_Y)); + + await expect(gate.waitForOpen()).resolves.toBeUndefined(); + }); + + it('returns pending promise on freshly constructed closed gate before any setOpen call', async () => { + const gate = new GateActor(false, vec(100, POSITION_Y)); + + let resolved = false; + void gate.waitForOpen().then(() => { + resolved = true; + return undefined; + }); - expect(mockActorConstructor).toHaveBeenCalledWith( - expect.objectContaining({ - pos, - }), - ); + await Promise.resolve(); + expect(resolved).toBe(false); + }); }); }); diff --git a/packages/factory/src/client/game/scenes/FactoryScene.ts b/packages/factory/src/client/game/scenes/FactoryScene.ts index 988aadd3..167f6596 100644 --- a/packages/factory/src/client/game/scenes/FactoryScene.ts +++ b/packages/factory/src/client/game/scenes/FactoryScene.ts @@ -11,7 +11,7 @@ import { StationActor } from '../actors/StationActor.js'; import type { LayoutResult } from '../layout/platform-layout.js'; import { computeLayout, REVIEW_STATION_INDEX } from '../layout/platform-layout.js'; import { computeWalkPath, type Waypoint } from '../layout/walk-path.js'; -import type { AgentConfig, SceneConfig } from '../mappers/run-to-scene.js'; +import type { AgentConfig, GateConfig, SceneConfig } from '../mappers/run-to-scene.js'; import { createSceneConfig } from '../mappers/run-to-scene.js'; import { diffAgents } from '../state/agent-differ.js'; import { resolveAgentStates } from '../state/agent-state-resolver.js'; @@ -31,6 +31,8 @@ function walkWithWarning(actor: AgentActor, role: string, waypoints: ReadonlyArr export class FactoryScene extends Scene { private agentMap = new Map(); private prevAgentConfigs: AgentConfig[] = []; + private gateMap = new Map(); + private prevGateConfigs: GateConfig[] = []; private status: CanonicalRunStatus; private layout: LayoutResult | undefined; @@ -49,6 +51,7 @@ export class FactoryScene extends Scene { this.status = status; const config = createSceneConfig(this.status); this.rebuildStaticElements(config); + this.updateGates(config); this.updateAgents(config); this.fitCamera(); } @@ -97,6 +100,8 @@ export class FactoryScene extends Scene { } private addGates(config: SceneConfig, layout: LayoutResult): void { + if (this.gateMap.size > 0) return; + if (layout.gatePositions.length !== config.gates.length) { console.error( `[FactoryScene] Gate count mismatch: ${String(config.gates.length)} gates but ${String(layout.gatePositions.length)} positions`, @@ -106,8 +111,11 @@ export class FactoryScene extends Scene { const gate = config.gates[i]; const pos = layout.gatePositions[i]; if (gate === undefined || pos === undefined) continue; - this.add(new GateActor(gate.open, vec(pos.x, pos.y))); + const gateActor = new GateActor(gate.open, vec(pos.x, pos.y)); + this.gateMap.set(i, gateActor); + this.add(gateActor); } + this.prevGateConfigs = config.gates; } /** Remove all actors and rebuild static elements (platform, stations, gates, artifacts). */ @@ -120,9 +128,31 @@ export class FactoryScene extends Scene { this.add(agentActor); } + // Re-add persistent gate actors + for (const gateActor of this.gateMap.values()) { + this.add(gateActor); + } + this.buildStaticElements(config); } + /** Diff gate open/closed states and call setOpen on changed gates. */ + private updateGates(config: SceneConfig): void { + for (let i = 0; i < config.gates.length; i++) { + const gateConfig = config.gates[i]; + const prevConfig = this.prevGateConfigs[i]; + if (gateConfig !== undefined && gateConfig.open !== prevConfig?.open) { + const actor = this.gateMap.get(i); + if (actor === undefined) { + console.warn(`[FactoryScene] updateGates: no actor for gate index ${String(i)}`); + continue; + } + actor.setOpen(gateConfig.open); + } + } + this.prevGateConfigs = config.gates; + } + /** * Fade an agent to transparent over 300ms, then remove it from the scene. * Fade errors are intentionally ignored because the actor may already have been @@ -204,15 +234,26 @@ export class FactoryScene extends Scene { if (artifact !== undefined) { actor.showArtifactIndicator(artifact.type); } + + // For multi-station jumps, only the nearest gate is awaited. Intermediate + // gates are guaranteed open because updateGates runs before updateAgents + // and all intervening stations are already active at that point. + const gateIndex = Math.min(prev.stationIndex, next.stationIndex); + const gate = this.gateMap.get(gateIndex); const generationAtStart = actor.walkGeneration; - void walkWithWarning(actor, next.role, waypoints) - .then(() => delay(300)) - .finally(() => { - actor.hideArtifactIndicator(); - if (actor.walkGeneration === generationAtStart) { - actor.setFacing(next.approaching === true ? 'right' : 'left'); - } - }); + + void (async () => { + if (gate !== undefined) { + await gate.waitForOpen(); + } + await walkWithWarning(actor, next.role, waypoints); + await delay(300); + })().finally(() => { + actor.hideArtifactIndicator(); + if (actor.walkGeneration === generationAtStart) { + actor.setFacing(next.approaching === true ? 'right' : 'left'); + } + }); } else { void walkWithWarning(actor, next.role, waypoints); } diff --git a/packages/factory/src/client/game/scenes/__tests__/FactoryScene.test.ts b/packages/factory/src/client/game/scenes/__tests__/FactoryScene.test.ts index a6b00932..79279cbf 100644 --- a/packages/factory/src/client/game/scenes/__tests__/FactoryScene.test.ts +++ b/packages/factory/src/client/game/scenes/__tests__/FactoryScene.test.ts @@ -14,6 +14,8 @@ const { mockShowArtifactIndicator, mockHideArtifactIndicator, mockSetFacing, + mockGateSetOpen, + mockGateWaitForOpen, } = vi.hoisted(() => { const kill = vi.fn(); const fade = vi.fn(() => ({ @@ -31,6 +33,8 @@ const { mockShowArtifactIndicator: vi.fn(), mockHideArtifactIndicator: vi.fn(), mockSetFacing: vi.fn(), + mockGateSetOpen: vi.fn(), + mockGateWaitForOpen: vi.fn(() => Promise.resolve()), }; }); @@ -115,6 +119,8 @@ vi.mock('../../../game/actors/ArtifactActor.js', () => ({ vi.mock('../../../game/actors/GateActor.js', () => ({ GateActor: class GateActor { kind = 'gate'; + setOpen = mockGateSetOpen; + waitForOpen = mockGateWaitForOpen; constructor( public open: boolean, public position: unknown, @@ -177,6 +183,8 @@ describe('FactoryScene', () => { mockHideArtifactIndicator.mockClear(); mockKill.mockClear(); mockFade.mockClear(); + mockGateSetOpen.mockClear(); + mockGateWaitForOpen.mockClear(); mockCamera.zoom = 1; mockCamera.pos = { x: 0, y: 0 }; }); @@ -749,7 +757,7 @@ describe('FactoryScene', () => { expect(mockWalkPath).toHaveBeenCalled(); }); - it('shows artifact indicator on orchestrator before walk when artifact exists at previous station', () => { + it('shows artifact indicator on orchestrator before walk when artifact exists at previous station', async () => { // Initial: arch + planning completed → implementation inferred as current → orchestrator at station 2 const status = createMockRunStatus({ status: 'in_progress', @@ -776,8 +784,12 @@ describe('FactoryScene', () => { }); scene.updateStatus(updatedStatus); + // showArtifactIndicator is called synchronously before the async doWalk expect(mockShowArtifactIndicator).toHaveBeenCalledWith('code'); - expect(mockWalkPath).toHaveBeenCalled(); + // walkPath is called after awaiting gate.waitForOpen() (async) + await vi.waitFor(() => { + expect(mockWalkPath).toHaveBeenCalled(); + }); }); it('hides artifact indicator after orchestrator walk completes', async () => { @@ -1075,4 +1087,160 @@ describe('FactoryScene', () => { }); }); }); + + describe('gate persistence', () => { + it('does not create new gate actors on second updateStatus call', () => { + const status = createMockRunStatus(); + const scene = new FactoryScene(status); + scene.onInitialize(); + + mockSceneAdd.mockClear(); + + const updatedStatus = createMockRunStatus({ + status: 'in_progress', + phases: { + ...emptyPhases(), + architecture: { status: 'in_progress', impactLevel: undefined, artifact: undefined }, + }, + }); + scene.updateStatus(updatedStatus); + + // Gate actors are re-added (6) but not newly constructed — they are the same instances + const gateCalls = mockSceneAdd.mock.calls.filter((call: unknown[]) => getActorFromCall(call).kind === 'gate'); + expect(gateCalls).toHaveLength(6); + }); + + it('calls setOpen when gate state changes between updates', () => { + const status = createMockRunStatus(); + const scene = new FactoryScene(status); + scene.onInitialize(); + + // Initial state: architecture is inferred as current, so only station 0 is active. + // Gates between inactive stations are closed. Now activate more stations. + const updatedStatus = createMockRunStatus({ + status: 'in_progress', + phases: { + ...emptyPhases(), + architecture: { status: 'completed', impactLevel: 'high', artifact: 'arch.md' }, + planning: { status: 'in_progress', stepCount: undefined, artifacts: undefined }, + }, + }); + scene.updateStatus(updatedStatus); + + // Gate between stations 0 and 1 should now be open (both active) + expect(mockGateSetOpen).toHaveBeenCalledWith(true); + }); + + it('does not call setOpen when gate state is unchanged', () => { + const status = createMockRunStatus(); + const scene = new FactoryScene(status); + scene.onInitialize(); + + mockGateSetOpen.mockClear(); + + // Re-apply the same status — no gate state changes + scene.updateStatus(createMockRunStatus()); + + expect(mockGateSetOpen).not.toHaveBeenCalled(); + }); + + it('orchestrator awaits waitForOpen before walking', () => { + // Initial: arch + planning completed → implementation inferred as current → orchestrator at station 2 + const status = createMockRunStatus({ + status: 'in_progress', + phases: { + ...emptyPhases(), + architecture: { status: 'completed', impactLevel: 'high', artifact: 'arch.md' }, + planning: { status: 'completed', stepCount: 3, artifacts: ['plan.md'] }, + }, + }); + const scene = new FactoryScene(status); + scene.onInitialize(); + mockGateWaitForOpen.mockClear(); + mockWalkPath.mockClear(); + + // Updated: implementation completed → review inferred as current → orchestrator moves to station 3 + const updatedStatus = createMockRunStatus({ + status: 'in_progress', + phases: { + ...emptyPhases(), + architecture: { status: 'completed', impactLevel: 'high', artifact: 'arch.md' }, + planning: { status: 'completed', stepCount: 3, artifacts: ['plan.md'] }, + implementation: { status: 'completed', qualityGates: 'passed', artifact: 'summary.md' }, + }, + }); + scene.updateStatus(updatedStatus); + + expect(mockGateWaitForOpen).toHaveBeenCalled(); + }); + + it('orchestrator awaits waitForOpen before walking backward', () => { + // Initial: all phases through simplifier completed, holistic inferred as current → orchestrator at station 5 + const status = createMockRunStatus({ + status: 'in_progress', + phases: { + ...emptyPhases(), + architecture: { status: 'completed', impactLevel: 'high', artifact: 'arch.md' }, + planning: { status: 'completed', stepCount: 3, artifacts: ['plan.md'] }, + implementation: { status: 'completed', artifact: 'code.md', qualityGates: undefined }, + parallelReview: { + aggregatedCriticality: 'low', + reviewRoundsUsed: 1, + reviewers: { + 'correctness-reviewer': { + ran: true, + status: 'completed', + criticality: 'low', + reason: undefined, + reReviewCriticality: undefined, + reReviewError: undefined, + }, + }, + coderFixCycleRan: false, + selectiveReReview: undefined, + }, + codeSimplifier: { ran: true, actionableFindings: true, coderFixCycleRan: false, artifact: undefined }, + }, + }); + const scene = new FactoryScene(status); + scene.onInitialize(); + mockGateWaitForOpen.mockClear(); + mockWalkPath.mockClear(); + + // Updated: holistic decided to skip → no inferred current phase → orchestrator falls back to station 4 (simplifier) + const updatedStatus = createMockRunStatus({ + status: 'in_progress', + phases: { + ...emptyPhases(), + architecture: { status: 'completed', impactLevel: 'high', artifact: 'arch.md' }, + planning: { status: 'completed', stepCount: 3, artifacts: ['plan.md'] }, + implementation: { status: 'completed', artifact: 'code.md', qualityGates: undefined }, + parallelReview: { + aggregatedCriticality: 'low', + reviewRoundsUsed: 1, + reviewers: { + 'correctness-reviewer': { + ran: true, + status: 'completed', + criticality: 'low', + reason: undefined, + reReviewCriticality: undefined, + reReviewError: undefined, + }, + }, + coderFixCycleRan: false, + selectiveReReview: undefined, + }, + codeSimplifier: { ran: true, actionableFindings: true, coderFixCycleRan: false, artifact: undefined }, + }, + phaseDecisions: { + holistic: { run: false, reason: 'skipped' }, + }, + }); + scene.updateStatus(updatedStatus); + + // Gate between stations 4 and 5 (index 4) should be awaited for backward movement + expect(mockGateWaitForOpen).toHaveBeenCalled(); + }); + }); });