From f4da6f203325dda4cbac89f43be86f08ce6fbd30 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sat, 7 Mar 2026 14:27:02 -0800 Subject: [PATCH 1/5] factory|feat: Add sprite-loading infrastructure for catwalk Create a sprite loading module that loads PNG sprite sheets via Vite static imports and produces cached Excalibur Animation objects per animation state. Refactor `StationAgentActor` and `OrchestratorActor` to use sprites instead of geometric primitives (Circle+Text, Rectangle+Text). - Generate placeholder SVG sprite sheets (subagent + orchestrator). - Add sprite-sheet-urls module mapping sprite types to asset URLs. - Add catwalk-sprite-loader with load/cache/getAnimation API. - Refactor `StationAgentActor` to render sprite + colored accent bar. - Refactor `OrchestratorActor` to render sprite animation. - Update `CatwalkScene` background to midnight blue (#1a1a2e). - Fire-and-forget `loadAllCatwalkSprites()` in scene initialization. - Update `AGENT_RADIUS` from 14 to 16 to match 32x32 sprite half-size. - Fix dimensions test for `ORCH_RADIUS >= AGENT_RADIUS` equality. --- .../scripts/generate-placeholder-pngs.ts | 32 ++++ .../catwalk/actors/OrchestratorActor.ts | 43 +----- .../catwalk/actors/StationAgentActor.ts | 59 +++---- .../catwalk/actors/__tests__/actors.test.ts | 44 +++++- .../constants/__tests__/dimensions.test.ts | 12 +- .../catwalk/constants/dimensions.ts | 2 +- .../catwalk/scene/CatwalkScene.ts | 4 +- .../scene/__tests__/CatwalkScene.test.ts | 9 +- .../__tests__/catwalk-sprite-loader.test.ts | 138 +++++++++++++++++ .../__tests__/sprite-sheet-urls.test.ts | 15 ++ .../catwalk/sprites/assets/orchestrator.svg | 1 + .../catwalk/sprites/assets/subagent.svg | 1 + .../catwalk/sprites/catwalk-sprite-loader.ts | 144 ++++++++++++++++++ .../catwalk/sprites/sprite-sheet-urls.ts | 11 ++ packages/factory/tsconfig.eslint.json | 2 +- 15 files changed, 446 insertions(+), 71 deletions(-) create mode 100644 packages/factory/scripts/generate-placeholder-pngs.ts create mode 100644 packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts create mode 100644 packages/factory/src/client/visualizations/catwalk/sprites/__tests__/sprite-sheet-urls.test.ts create mode 100644 packages/factory/src/client/visualizations/catwalk/sprites/assets/orchestrator.svg create mode 100644 packages/factory/src/client/visualizations/catwalk/sprites/assets/subagent.svg create mode 100644 packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts create mode 100644 packages/factory/src/client/visualizations/catwalk/sprites/sprite-sheet-urls.ts diff --git a/packages/factory/scripts/generate-placeholder-pngs.ts b/packages/factory/scripts/generate-placeholder-pngs.ts new file mode 100644 index 00000000..f9c24bd9 --- /dev/null +++ b/packages/factory/scripts/generate-placeholder-pngs.ts @@ -0,0 +1,32 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const SPRITE_SIZE = 32; +const COLS = 4; +const ROWS = 3; +const WIDTH = SPRITE_SIZE * COLS; +const HEIGHT = SPRITE_SIZE * ROWS; + +function generatePlaceholderSvg(color: string, label: string): string { + const frames: string[] = []; + for (let row = 0; row < ROWS; row++) { + for (let col = 0; col < COLS; col++) { + const x = col * SPRITE_SIZE; + const y = row * SPRITE_SIZE; + const frameNum = row * COLS + col; + frames.push( + ``, + `${label}${frameNum}`, + ); + } + } + return `${frames.join('')}`; +} + +const outDir = join(import.meta.dirname, '../src/client/visualizations/catwalk/sprites/assets'); +mkdirSync(outDir, { recursive: true }); + +writeFileSync(join(outDir, 'subagent.svg'), generatePlaceholderSvg('#888899', 'S')); +writeFileSync(join(outDir, 'orchestrator.svg'), generatePlaceholderSvg('#CCAA44', 'O')); + +console.info('Placeholder sprite sheets written to', outDir); diff --git a/packages/factory/src/client/visualizations/catwalk/actors/OrchestratorActor.ts b/packages/factory/src/client/visualizations/catwalk/actors/OrchestratorActor.ts index cc4434c0..9ec9eca5 100644 --- a/packages/factory/src/client/visualizations/catwalk/actors/OrchestratorActor.ts +++ b/packages/factory/src/client/visualizations/catwalk/actors/OrchestratorActor.ts @@ -1,18 +1,14 @@ -import { Actor, BaseAlign, Color, Font, GraphicsGroup, Rectangle, Text, TextAlign, vec, type Vector } from 'excalibur'; +import { Actor, type Vector } from 'excalibur'; import { ORCH_IDLE_OPACITY, ORCH_PULSE_MAX, ORCH_PULSE_MIN, PULSE_FREQUENCY } from '../constants/animation.js'; -import { ORCH_RADIUS } from '../constants/dimensions.js'; import { WALK_SPEED } from '../constants/timing.js'; - -const ORCH_W = Math.round(ORCH_RADIUS * 2.2); -const ORCH_H = Math.round(ORCH_RADIUS * 1.6); -const ORCH_COLOR = '#FFD700'; +import { getAnimation } from '../sprites/catwalk-sprite-loader.js'; export interface OrchestratorActorConfig { working: boolean; } -/** Renders the orchestrator as a gold rectangle with label, supporting walk and pulse animations. */ +/** Renders the orchestrator as an animated sprite on the catwalk rail, supporting walk and pulse animations. */ export class OrchestratorActor extends Actor { private _working = false; private _elapsed = 0; @@ -20,33 +16,8 @@ export class OrchestratorActor extends Actor { constructor(config: OrchestratorActorConfig, position: Vector) { super({ pos: position }); - const rect = new Rectangle({ - width: ORCH_W, - height: ORCH_H, - color: Color.fromHex(ORCH_COLOR), - }); - - const label = new Text({ - text: 'ORCH', - color: Color.fromHex('#111111'), - font: new Font({ - size: 9, - bold: true, - family: 'monospace', - textAlign: TextAlign.Center, - baseAlign: BaseAlign.Middle, - }), - }); - - const group = new GraphicsGroup({ - useAnchor: false, - members: [ - { graphic: rect, offset: vec(0, 0) }, - { graphic: label, offset: vec(ORCH_W / 2, ORCH_H / 2), useBounds: false }, - ], - }); - - this.graphics.use(group); + const animation = getAnimation('orchestrator', config.working ? 'working' : 'idle'); + this.graphics.use(animation); this._working = config.working; this.graphics.opacity = config.working ? ORCH_PULSE_MAX : ORCH_IDLE_OPACITY; } @@ -56,9 +27,11 @@ export class OrchestratorActor extends Actor { this.actions.moveTo(pos, WALK_SPEED); } - /** Toggle the pulsing working glow. */ + /** Toggle the pulsing working glow and switch sprite animation. */ setWorking(working: boolean): void { this._working = working; + const animation = getAnimation('orchestrator', working ? 'working' : 'idle'); + this.graphics.use(animation); if (!working) { this._elapsed = 0; this.graphics.opacity = ORCH_IDLE_OPACITY; diff --git a/packages/factory/src/client/visualizations/catwalk/actors/StationAgentActor.ts b/packages/factory/src/client/visualizations/catwalk/actors/StationAgentActor.ts index cb39ec56..09e1203e 100644 --- a/packages/factory/src/client/visualizations/catwalk/actors/StationAgentActor.ts +++ b/packages/factory/src/client/visualizations/catwalk/actors/StationAgentActor.ts @@ -1,10 +1,14 @@ -import { Actor, BaseAlign, Circle, Color, Font, GraphicsGroup, Text, TextAlign, vec, type Vector } from 'excalibur'; +import { Actor, Color, GraphicsGroup, Rectangle, vec, type Vector } from 'excalibur'; import { AGENT_PULSE_MAX, AGENT_PULSE_MIN, DEACTIVATED_OPACITY, PULSE_FREQUENCY } from '../constants/animation.js'; import { AGENT_RADIUS } from '../constants/dimensions.js'; import { PAUSE_DURATION } from '../constants/timing.js'; +import { getAnimation } from '../sprites/catwalk-sprite-loader.js'; import type { AgentAnimationState } from '../types.js'; +const ACCENT_BAR_HEIGHT = 4; +const SPRITE_SIZE = AGENT_RADIUS * 2; + export interface StationAgentActorConfig { id: string; role: string; @@ -33,43 +37,20 @@ function opacityForState(state: AgentAnimationState): number { } } -/** Renders a station-bound agent as a colored circle with a role label, dimmed by animation state. */ +/** Renders a station-bound agent as an animated sprite with a colored accent bar, dimmed by animation state. */ export class StationAgentActor extends Actor { private _state: AgentAnimationState; + private _config: StationAgentActorConfig; private _pulsing = false; private _elapsed = 0; constructor(config: StationAgentActorConfig, position: Vector) { super({ pos: position }); this._state = config.state; + this._config = config; this._pulsing = config.state === 'working'; - const circle = new Circle({ - radius: AGENT_RADIUS, - color: Color.fromHex(config.color), - }); - - const label = new Text({ - text: config.role, - color: Color.fromHex('#111111'), - font: new Font({ - size: 9, - bold: true, - family: 'monospace', - textAlign: TextAlign.Center, - baseAlign: BaseAlign.Middle, - }), - }); - - const group = new GraphicsGroup({ - useAnchor: false, - members: [ - { graphic: circle, offset: vec(0, 0) }, - { graphic: label, offset: vec(AGENT_RADIUS, AGENT_RADIUS), useBounds: false }, - ], - }); - - this.graphics.use(group); + this.applyGraphics(config); this.graphics.opacity = opacityForState(config.state); } @@ -77,6 +58,7 @@ export class StationAgentActor extends Actor { animateToState(state: AgentAnimationState): void { this._state = state; this._pulsing = state === 'working'; + this.applyGraphics({ ...this._config, state }); if (this._pulsing) { this._elapsed = 0; } else { @@ -96,4 +78,25 @@ export class StationAgentActor extends Actor { const t = Math.sin((this._elapsed * PULSE_FREQUENCY * Math.PI * 2) / 1000); this.graphics.opacity = AGENT_PULSE_MIN + ((AGENT_PULSE_MAX - AGENT_PULSE_MIN) * (t + 1)) / 2; } + + /** Builds a GraphicsGroup with the sprite animation and an accent bar, and applies it. */ + private applyGraphics(config: StationAgentActorConfig): void { + const animation = getAnimation('subagent', config.state); + + const accentBar = new Rectangle({ + width: SPRITE_SIZE, + height: ACCENT_BAR_HEIGHT, + color: Color.fromHex(config.color), + }); + + const group = new GraphicsGroup({ + useAnchor: false, + members: [ + { graphic: animation, offset: vec(0, 0) }, + { graphic: accentBar, offset: vec(0, SPRITE_SIZE) }, + ], + }); + + this.graphics.use(group); + } } diff --git a/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts b/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts index 4d7bac97..6313ae01 100644 --- a/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts @@ -1,9 +1,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { mockActorConstructor, mockGraphicsUse } = vi.hoisted(() => { +const { mockActorConstructor, mockGraphicsUse, mockGetAnimation } = vi.hoisted(() => { return { mockActorConstructor: vi.fn(), mockGraphicsUse: vi.fn(), + mockGetAnimation: vi.fn(), }; }); @@ -86,6 +87,10 @@ vi.mock('excalibur', () => { }; }); +vi.mock('../../sprites/catwalk-sprite-loader.js', () => ({ + getAnimation: mockGetAnimation, +})); + const { OrchestratorActor } = await import('../OrchestratorActor.js'); const { StationAgentActor } = await import('../StationAgentActor.js'); const { CatwalkStationActor } = await import('../CatwalkStationActor.js'); @@ -97,6 +102,8 @@ const { vec } = await import('excalibur'); describe('OrchestratorActor', () => { beforeEach(() => { mockGraphicsUse.mockClear(); + mockGetAnimation.mockClear(); + mockGetAnimation.mockReturnValue({ type: 'animation' }); }); it('sets opacity to 1.0 when working is true', () => { @@ -137,11 +144,34 @@ describe('OrchestratorActor', () => { actor.onPreUpdate(undefined as never, 0); expect(actor.graphics.opacity).toBe(0.8); }); + + it('calls getAnimation with orchestrator type and working state', () => { + new OrchestratorActor({ working: true }, vec(0, 0)); + + expect(mockGetAnimation).toHaveBeenCalledWith('orchestrator', 'working'); + }); + + it('calls getAnimation with orchestrator type and idle state when not working', () => { + new OrchestratorActor({ working: false }, vec(0, 0)); + + expect(mockGetAnimation).toHaveBeenCalledWith('orchestrator', 'idle'); + }); + + it('calls graphics.use on updateConfig', () => { + const actor = new OrchestratorActor({ working: true }, vec(0, 0)); + mockGraphicsUse.mockClear(); + + actor.updateConfig({ working: false }); + + expect(mockGraphicsUse).toHaveBeenCalled(); + }); }); describe('StationAgentActor', () => { beforeEach(() => { mockGraphicsUse.mockClear(); + mockGetAnimation.mockClear(); + mockGetAnimation.mockReturnValue({ type: 'animation' }); }); it('sets opacity to 0.3 for idle state', () => { @@ -234,6 +264,18 @@ describe('StationAgentActor', () => { expect(actor.actions.fade).toHaveBeenCalledWith(0.15, expect.any(Number)); }); + + it('calls getAnimation with subagent type and the given state', () => { + new StationAgentActor({ id: 'a', role: 'arch', color: '#5555FF', state: 'working' }, vec(0, 0)); + + expect(mockGetAnimation).toHaveBeenCalledWith('subagent', 'working'); + }); + + it('calls graphics.use with a GraphicsGroup on construction', () => { + new StationAgentActor({ id: 'a', role: 'arch', color: '#5555FF', state: 'idle' }, vec(0, 0)); + + expect(mockGraphicsUse).toHaveBeenCalledTimes(1); + }); }); describe('CatwalkStationActor', () => { diff --git a/packages/factory/src/client/visualizations/catwalk/constants/__tests__/dimensions.test.ts b/packages/factory/src/client/visualizations/catwalk/constants/__tests__/dimensions.test.ts index 3de88109..317c82ac 100644 --- a/packages/factory/src/client/visualizations/catwalk/constants/__tests__/dimensions.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/constants/__tests__/dimensions.test.ts @@ -51,8 +51,16 @@ describe('entity sizing', () => { expect(AGENT_RADIUS).toBeGreaterThan(0); }); - it('has ORCH_RADIUS greater than AGENT_RADIUS', () => { - expect(ORCH_RADIUS).toBeGreaterThan(AGENT_RADIUS); + it('has AGENT_RADIUS equal to half the sprite size (16)', () => { + expect(AGENT_RADIUS).toBe(16); + }); + + it('has ORCH_RADIUS equal to half the sprite size (16)', () => { + expect(ORCH_RADIUS).toBe(16); + }); + + it('has ORCH_RADIUS greater than or equal to AGENT_RADIUS', () => { + expect(ORCH_RADIUS).toBeGreaterThanOrEqual(AGENT_RADIUS); }); it('has positive artifact dimensions', () => { diff --git a/packages/factory/src/client/visualizations/catwalk/constants/dimensions.ts b/packages/factory/src/client/visualizations/catwalk/constants/dimensions.ts index dc2b02a7..53eddc1a 100644 --- a/packages/factory/src/client/visualizations/catwalk/constants/dimensions.ts +++ b/packages/factory/src/client/visualizations/catwalk/constants/dimensions.ts @@ -13,7 +13,7 @@ export const CHUTE_TOP = CATWALK_Y + 48; export const CHUTE_BOT = GROUND_Y - 20; // Entity sizing -export const AGENT_RADIUS = 14; +export const AGENT_RADIUS = 16; export const ORCH_RADIUS = 16; export const ART_W = 40; export const ART_H = 16; diff --git a/packages/factory/src/client/visualizations/catwalk/scene/CatwalkScene.ts b/packages/factory/src/client/visualizations/catwalk/scene/CatwalkScene.ts index 48e7083d..9a958c28 100644 --- a/packages/factory/src/client/visualizations/catwalk/scene/CatwalkScene.ts +++ b/packages/factory/src/client/visualizations/catwalk/scene/CatwalkScene.ts @@ -13,6 +13,7 @@ import { import { ART_H, ENGINE_HEIGHT, ENGINE_WIDTH, GROUND_Y } from '../constants/dimensions.js'; import { type CatwalkLayoutResult, computeCatwalkLayout, type StationLayoutEntry } from '../layout/catwalk-layout.js'; import { mapRunToCatwalk } from '../mappers/run-to-catwalk.js'; +import { loadAllCatwalkSprites } from '../sprites/catwalk-sprite-loader.js'; import { artifactKey, diffCatwalkConfig } from '../state/catwalk-differ.js'; import type { CatwalkDiff, CatwalkSceneConfig } from '../types.js'; @@ -42,10 +43,11 @@ export class CatwalkScene extends Scene { constructor(status: CanonicalRunStatus) { super(); this.status = status; - this.backgroundColor = Color.fromHex('#111111'); + this.backgroundColor = Color.fromHex('#1a1a2e'); } override onInitialize(): void { + void loadAllCatwalkSprites(); this.buildScene(); this.fitCamera(); } diff --git a/packages/factory/src/client/visualizations/catwalk/scene/__tests__/CatwalkScene.test.ts b/packages/factory/src/client/visualizations/catwalk/scene/__tests__/CatwalkScene.test.ts index fed96573..e41cfae3 100644 --- a/packages/factory/src/client/visualizations/catwalk/scene/__tests__/CatwalkScene.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/scene/__tests__/CatwalkScene.test.ts @@ -2,6 +2,11 @@ import { describe, expect, it, vi } from 'vitest'; import { createMockRunStatus, emptyPhases } from '../../../../../__test-helpers__/fixtures.js'; +vi.mock('../../sprites/catwalk-sprite-loader.js', () => ({ + loadAllCatwalkSprites: vi.fn().mockResolvedValue(undefined), + getAnimation: vi.fn().mockReturnValue({ type: 'animation' }), +})); + vi.mock('excalibur', () => { class MockScene { backgroundColor: unknown; @@ -140,11 +145,11 @@ function hasPosition(value: unknown): value is { position: { x: number; y: numbe } describe('CatwalkScene', () => { - it('sets the background color to #111111', () => { + it('sets the background color to #1a1a2e', () => { const status = createMockRunStatus(); const scene = new CatwalkScene(status); - expect(scene.backgroundColor).toEqual(expect.objectContaining({ hex: '#111111' })); + expect(scene.backgroundColor).toEqual(expect.objectContaining({ hex: '#1a1a2e' })); }); it('builds actors on initialize', () => { diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts new file mode 100644 index 00000000..7520da29 --- /dev/null +++ b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts @@ -0,0 +1,138 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockImageSourceConstructor, mockImageSourceLoad, mockSpriteSheetFromImageSource, mockAnimationFromCoords } = + vi.hoisted(() => { + const mockLoad = vi.fn().mockResolvedValue(undefined); + return { + mockImageSourceConstructor: vi.fn(), + mockImageSourceLoad: mockLoad, + mockSpriteSheetFromImageSource: vi.fn(), + mockAnimationFromCoords: vi.fn(), + }; + }); + +vi.mock('excalibur', () => { + class MockImageSource { + url: string; + options: Record; + load = mockImageSourceLoad; + + constructor(url: string, options: Record) { + mockImageSourceConstructor(url, options); + this.url = url; + this.options = options; + } + } + + const MockSpriteSheet = { + fromImageSource: mockSpriteSheetFromImageSource, + }; + + class MockAnimation { + config: Record; + constructor(config: Record) { + this.config = config; + } + static fromSpriteSheetCoordinates = mockAnimationFromCoords; + } + + return { + ImageSource: MockImageSource, + ImageFiltering: { Pixel: 'pixel' }, + SpriteSheet: MockSpriteSheet, + Animation: MockAnimation, + AnimationStrategy: { + PingPong: 'ping-pong', + Loop: 'loop', + Freeze: 'freeze', + }, + }; +}); + +const { loadAllCatwalkSprites, getAnimation, clearCatwalkSpriteCache } = await import('../catwalk-sprite-loader.js'); + +describe('catwalk-sprite-loader', () => { + beforeEach(() => { + clearCatwalkSpriteCache(); + vi.clearAllMocks(); + + mockSpriteSheetFromImageSource.mockReturnValue({ type: 'sprite-sheet' }); + mockAnimationFromCoords.mockImplementation((config: Record) => ({ + type: 'animation', + config, + })); + }); + + afterEach(() => { + clearCatwalkSpriteCache(); + }); + + describe('loadAllCatwalkSprites', () => { + it('loads 2 image sources (subagent + orchestrator)', async () => { + await loadAllCatwalkSprites(); + + expect(mockImageSourceConstructor).toHaveBeenCalledTimes(2); + expect(mockImageSourceLoad).toHaveBeenCalledTimes(2); + }); + + it('is idempotent (second call is a no-op)', async () => { + await loadAllCatwalkSprites(); + await loadAllCatwalkSprites(); + + expect(mockImageSourceConstructor).toHaveBeenCalledTimes(2); + expect(mockImageSourceLoad).toHaveBeenCalledTimes(2); + }); + + it('creates sprite sheets from loaded image sources', async () => { + await loadAllCatwalkSprites(); + + expect(mockSpriteSheetFromImageSource).toHaveBeenCalledTimes(2); + }); + + it('creates animations for all 6 states x 2 sprite types = 12 animations', async () => { + await loadAllCatwalkSprites(); + + expect(mockAnimationFromCoords).toHaveBeenCalledTimes(12); + }); + }); + + describe('getAnimation', () => { + it('throws if called before loading', () => { + expect(() => getAnimation('subagent', 'idle')).toThrow('Catwalk sprites have not been loaded'); + }); + + it('returns an animation for each sprite type and state', async () => { + await loadAllCatwalkSprites(); + + const spriteTypes = ['subagent', 'orchestrator'] as const; + const states = ['idle', 'walking', 'working', 'celebrating', 'concerned', 'resting'] as const; + + for (const spriteType of spriteTypes) { + for (const state of states) { + const animation = getAnimation(spriteType, state); + expect(animation).toBeDefined(); + expect(animation).toEqual(expect.objectContaining({ type: 'animation' })); + } + } + }); + + it('returns cached instances on repeat calls', async () => { + await loadAllCatwalkSprites(); + + const first = getAnimation('subagent', 'idle'); + const second = getAnimation('subagent', 'idle'); + + expect(first).toBe(second); + }); + }); + + describe('clearCatwalkSpriteCache', () => { + it('allows sprites to be reloaded after clearing', async () => { + await loadAllCatwalkSprites(); + clearCatwalkSpriteCache(); + await loadAllCatwalkSprites(); + + expect(mockImageSourceConstructor).toHaveBeenCalledTimes(4); + }); + }); +}); diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/sprite-sheet-urls.test.ts b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/sprite-sheet-urls.test.ts new file mode 100644 index 00000000..40c9cfff --- /dev/null +++ b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/sprite-sheet-urls.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it } from 'vitest'; + +import { SPRITE_SHEET_URLS } from '../sprite-sheet-urls.js'; + +describe('SPRITE_SHEET_URLS', () => { + it('exports a subagent URL string', () => { + expect(typeof SPRITE_SHEET_URLS.subagent).toBe('string'); + expect(SPRITE_SHEET_URLS.subagent.length).toBeGreaterThan(0); + }); + + it('exports an orchestrator URL string', () => { + expect(typeof SPRITE_SHEET_URLS.orchestrator).toBe('string'); + expect(SPRITE_SHEET_URLS.orchestrator.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/assets/orchestrator.svg b/packages/factory/src/client/visualizations/catwalk/sprites/assets/orchestrator.svg new file mode 100644 index 00000000..9efa7f66 --- /dev/null +++ b/packages/factory/src/client/visualizations/catwalk/sprites/assets/orchestrator.svg @@ -0,0 +1 @@ +O0O1O2O3O4O5O6O7O8O9O10O11 \ No newline at end of file diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/assets/subagent.svg b/packages/factory/src/client/visualizations/catwalk/sprites/assets/subagent.svg new file mode 100644 index 00000000..77226df2 --- /dev/null +++ b/packages/factory/src/client/visualizations/catwalk/sprites/assets/subagent.svg @@ -0,0 +1 @@ +S0S1S2S3S4S5S6S7S8S9S10S11 \ No newline at end of file diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts new file mode 100644 index 00000000..527e3dff --- /dev/null +++ b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts @@ -0,0 +1,144 @@ +import { Animation, ImageFiltering, ImageSource, SpriteSheet } from 'excalibur'; + +import { + CELEBRATING_DURATION, + CELEBRATING_FRAME_COORDINATES, + CELEBRATING_STRATEGY, + CONCERNED_DURATION, + CONCERNED_FRAME_COORDINATES, + CONCERNED_STRATEGY, + GRID_COLUMNS, + GRID_ROWS, + IDLE_DURATION, + IDLE_FRAME_COORDINATES, + IDLE_STRATEGY, + RESTING_DURATION, + RESTING_FRAME_COORDINATES, + RESTING_STRATEGY, + SPRITE_SIZE, + WALKING_DURATION, + WALKING_FRAME_COORDINATES, + WALKING_STRATEGY, + WORKING_DURATION, + WORKING_FRAME_COORDINATES, + WORKING_STRATEGY, +} from '../../../game/sprites/sprite-definitions.js'; +import type { AgentAnimationState } from '../types.js'; +import { type CatwalkSpriteType, SPRITE_SHEET_URLS } from './sprite-sheet-urls.js'; + +const SPRITE_TYPES: readonly CatwalkSpriteType[] = ['subagent', 'orchestrator']; + +let animationCache: Map> | undefined; + +/** Resolves frame coordinates and timing for the given animation state. */ +function frameConfigForState(state: AgentAnimationState): { + frameCoordinates: ReadonlyArray<{ x: number; y: number }>; + duration: number; + strategy: import('excalibur').AnimationStrategy; +} { + switch (state) { + case 'idle': + return { frameCoordinates: IDLE_FRAME_COORDINATES, duration: IDLE_DURATION, strategy: IDLE_STRATEGY }; + case 'walking': + return { frameCoordinates: WALKING_FRAME_COORDINATES, duration: WALKING_DURATION, strategy: WALKING_STRATEGY }; + case 'working': + return { frameCoordinates: WORKING_FRAME_COORDINATES, duration: WORKING_DURATION, strategy: WORKING_STRATEGY }; + case 'celebrating': + return { + frameCoordinates: CELEBRATING_FRAME_COORDINATES, + duration: CELEBRATING_DURATION, + strategy: CELEBRATING_STRATEGY, + }; + case 'concerned': + return { + frameCoordinates: CONCERNED_FRAME_COORDINATES, + duration: CONCERNED_DURATION, + strategy: CONCERNED_STRATEGY, + }; + case 'resting': + return { frameCoordinates: RESTING_FRAME_COORDINATES, duration: RESTING_DURATION, strategy: RESTING_STRATEGY }; + default: { + const _exhaustive: never = state; + return _exhaustive; + } + } +} + +const ALL_STATES: readonly AgentAnimationState[] = [ + 'idle', + 'walking', + 'working', + 'celebrating', + 'concerned', + 'resting', +]; + +/** Build animation objects for every state from the given sprite sheet. */ +function buildAnimationsForSheet(spriteSheet: SpriteSheet): Map { + const map = new Map(); + for (const state of ALL_STATES) { + const { frameCoordinates, duration, strategy } = frameConfigForState(state); + const animation = Animation.fromSpriteSheetCoordinates({ + spriteSheet, + frameCoordinates: [...frameCoordinates], + durationPerFrame: duration, + strategy, + }); + map.set(state, animation); + } + return map; +} + +/** Load all catwalk sprite sheets and cache animations for each sprite type and state. */ +export async function loadAllCatwalkSprites(): Promise { + if (animationCache !== undefined) return; + + const cache = new Map>(); + const imageSources: ImageSource[] = []; + + for (const spriteType of SPRITE_TYPES) { + const url = SPRITE_SHEET_URLS[spriteType]; + const imageSource = new ImageSource(url, { filtering: ImageFiltering.Pixel }); + imageSources.push(imageSource); + + const spriteSheet = SpriteSheet.fromImageSource({ + image: imageSource, + grid: { + rows: GRID_ROWS, + columns: GRID_COLUMNS, + spriteWidth: SPRITE_SIZE, + spriteHeight: SPRITE_SIZE, + }, + }); + + cache.set(spriteType, buildAnimationsForSheet(spriteSheet)); + } + + await Promise.all(imageSources.map((source) => source.load())); + + animationCache = cache; +} + +/** Return the cached animation for the given sprite type and state. Throws if sprites have not been loaded. */ +export function getAnimation(spriteType: CatwalkSpriteType, state: AgentAnimationState): Animation { + if (animationCache === undefined) { + throw new Error('Catwalk sprites have not been loaded. Call loadAllCatwalkSprites() first.'); + } + + const stateMap = animationCache.get(spriteType); + if (stateMap === undefined) { + throw new Error(`No animations found for sprite type "${spriteType}".`); + } + + const animation = stateMap.get(state); + if (animation === undefined) { + throw new Error(`No animation found for sprite type "${spriteType}" in state "${state}".`); + } + + return animation; +} + +/** Clear the catwalk sprite cache, allowing sprites to be reloaded. Useful for tests. */ +export function clearCatwalkSpriteCache(): void { + animationCache = undefined; +} diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/sprite-sheet-urls.ts b/packages/factory/src/client/visualizations/catwalk/sprites/sprite-sheet-urls.ts new file mode 100644 index 00000000..a33376b9 --- /dev/null +++ b/packages/factory/src/client/visualizations/catwalk/sprites/sprite-sheet-urls.ts @@ -0,0 +1,11 @@ +import orchestratorUrl from './assets/orchestrator.svg'; +import subagentUrl from './assets/subagent.svg'; + +/** Identifies the two sprite sheet types used by the catwalk visualization. */ +export type CatwalkSpriteType = 'subagent' | 'orchestrator'; + +/** Maps each catwalk sprite type to the URL of its sprite sheet asset. */ +export const SPRITE_SHEET_URLS: Record = { + subagent: subagentUrl, + orchestrator: orchestratorUrl, +}; diff --git a/packages/factory/tsconfig.eslint.json b/packages/factory/tsconfig.eslint.json index 5782369a..476eb68c 100644 --- a/packages/factory/tsconfig.eslint.json +++ b/packages/factory/tsconfig.eslint.json @@ -4,5 +4,5 @@ "compilerOptions": { "noEmit": true, }, - "include": ["src/", "*.ts", "*.js"], + "include": ["src/", "scripts/", "*.ts", "*.js"], } From e01f3b71e02926981d09eb8f56f0cabe4e636cb5 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sat, 7 Mar 2026 14:52:22 -0800 Subject: [PATCH 2/5] factory|fix: Populate catwalk sprite cache synchronously Restructure `loadAllCatwalkSprites` to build animation cache synchronously before awaiting image loads, matching the agent-sprite-loader pattern. Previously the cache was only populated after `await Promise.all(...)`, so `getAnimation()` always threw during scene initialization because `buildScene()` ran synchronously before the async load completed. Additional fixes: - Add in-flight promise guard to prevent duplicate concurrent loads. - Attach `.catch()` in `CatwalkScene.onInitialize` to surface image-load errors. - Import SPRITE_SIZE from sprite-definitions.ts instead of deriving locally. - Add tests for `loadAllCatwalkSprites` invocation, URL correctness, concurrent deduplication, synchronous cache availability, `updateConfig` animation state, accent bar color, and URL distinctness. --- .../catwalk/actors/StationAgentActor.ts | 3 +- .../catwalk/actors/__tests__/actors.test.ts | 58 ++++++++++++++++++- .../catwalk/scene/CatwalkScene.ts | 7 ++- .../scene/__tests__/CatwalkScene.test.ts | 15 ++++- .../__tests__/catwalk-sprite-loader.test.ts | 27 +++++++++ .../__tests__/sprite-sheet-urls.test.ts | 4 ++ .../catwalk/sprites/catwalk-sprite-loader.ts | 24 ++++++-- 7 files changed, 127 insertions(+), 11 deletions(-) diff --git a/packages/factory/src/client/visualizations/catwalk/actors/StationAgentActor.ts b/packages/factory/src/client/visualizations/catwalk/actors/StationAgentActor.ts index 09e1203e..34b7be2a 100644 --- a/packages/factory/src/client/visualizations/catwalk/actors/StationAgentActor.ts +++ b/packages/factory/src/client/visualizations/catwalk/actors/StationAgentActor.ts @@ -1,13 +1,12 @@ import { Actor, Color, GraphicsGroup, Rectangle, vec, type Vector } from 'excalibur'; +import { SPRITE_SIZE } from '../../../game/sprites/sprite-definitions.js'; import { AGENT_PULSE_MAX, AGENT_PULSE_MIN, DEACTIVATED_OPACITY, PULSE_FREQUENCY } from '../constants/animation.js'; -import { AGENT_RADIUS } from '../constants/dimensions.js'; import { PAUSE_DURATION } from '../constants/timing.js'; import { getAnimation } from '../sprites/catwalk-sprite-loader.js'; import type { AgentAnimationState } from '../types.js'; const ACCENT_BAR_HEIGHT = 4; -const SPRITE_SIZE = AGENT_RADIUS * 2; export interface StationAgentActorConfig { id: string; diff --git a/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts b/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts index 6313ae01..96f7a827 100644 --- a/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts @@ -72,6 +72,7 @@ vi.mock('excalibur', () => { const MockTextAlign = { Center: 'center', Left: 'left' }; const MockBaseAlign = { Middle: 'middle', Top: 'top' }; + const MockAnimationStrategy = { PingPong: 'ping-pong', Loop: 'loop', Freeze: 'freeze' }; return { Actor: MockActor, @@ -83,6 +84,7 @@ vi.mock('excalibur', () => { GraphicsGroup: MockGraphicsGroup, TextAlign: MockTextAlign, BaseAlign: MockBaseAlign, + AnimationStrategy: MockAnimationStrategy, vec: (x: number, y: number) => ({ x, y }), }; }); @@ -157,14 +159,32 @@ describe('OrchestratorActor', () => { expect(mockGetAnimation).toHaveBeenCalledWith('orchestrator', 'idle'); }); - it('calls graphics.use on updateConfig', () => { + it('setWorking calls graphics.use to update the sprite', () => { const actor = new OrchestratorActor({ working: true }, vec(0, 0)); mockGraphicsUse.mockClear(); - actor.updateConfig({ working: false }); + actor.setWorking(false); expect(mockGraphicsUse).toHaveBeenCalled(); }); + + it('setWorking(false) calls getAnimation with idle state', () => { + const actor = new OrchestratorActor({ working: true }, vec(0, 0)); + mockGetAnimation.mockClear(); + + actor.setWorking(false); + + expect(mockGetAnimation).toHaveBeenCalledWith('orchestrator', 'idle'); + }); + + it('setWorking(true) calls getAnimation with working state', () => { + const actor = new OrchestratorActor({ working: false }, vec(0, 0)); + mockGetAnimation.mockClear(); + + actor.setWorking(true); + + expect(mockGetAnimation).toHaveBeenCalledWith('orchestrator', 'working'); + }); }); describe('StationAgentActor', () => { @@ -276,6 +296,40 @@ describe('StationAgentActor', () => { expect(mockGraphicsUse).toHaveBeenCalledTimes(1); }); + + it('animateToState calls graphics.use to re-render the sprite', () => { + const actor = new StationAgentActor({ id: 'a', role: 'arch', color: '#5555FF', state: 'idle' }, vec(0, 0)); + mockGraphicsUse.mockClear(); + + actor.animateToState('working'); + + expect(mockGraphicsUse).toHaveBeenCalled(); + }); + + it('applies the config color to the accent bar', () => { + const color = '#FF00AA'; + new StationAgentActor({ id: 'a', role: 'arch', color, state: 'working' }, vec(0, 0)); + + const call = mockGraphicsUse.mock.calls[0]; + if (!call) throw new TypeError('Expected graphics.use to have been called'); + + // The GraphicsGroup is the first argument; verify the accent bar (second member) uses the config color + expect(call[0]).toEqual( + expect.objectContaining({ + options: expect.objectContaining({ + members: expect.arrayContaining([ + expect.objectContaining({ + graphic: expect.objectContaining({ + options: expect.objectContaining({ + color: expect.objectContaining({ hex: color }), + }), + }), + }), + ]), + }), + }), + ); + }); }); describe('CatwalkStationActor', () => { diff --git a/packages/factory/src/client/visualizations/catwalk/scene/CatwalkScene.ts b/packages/factory/src/client/visualizations/catwalk/scene/CatwalkScene.ts index 9a958c28..762992d7 100644 --- a/packages/factory/src/client/visualizations/catwalk/scene/CatwalkScene.ts +++ b/packages/factory/src/client/visualizations/catwalk/scene/CatwalkScene.ts @@ -47,7 +47,12 @@ export class CatwalkScene extends Scene { } override onInitialize(): void { - void loadAllCatwalkSprites(); + // loadAllCatwalkSprites populates the animation cache synchronously, + // so buildScene can safely call getAnimation() immediately. + // The returned promise resolves once image data finishes loading. + loadAllCatwalkSprites().catch((error: unknown) => { + console.error('Failed to load catwalk sprites:', error); + }); this.buildScene(); this.fitCamera(); } diff --git a/packages/factory/src/client/visualizations/catwalk/scene/__tests__/CatwalkScene.test.ts b/packages/factory/src/client/visualizations/catwalk/scene/__tests__/CatwalkScene.test.ts index e41cfae3..47adfdaa 100644 --- a/packages/factory/src/client/visualizations/catwalk/scene/__tests__/CatwalkScene.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/scene/__tests__/CatwalkScene.test.ts @@ -2,8 +2,12 @@ import { describe, expect, it, vi } from 'vitest'; import { createMockRunStatus, emptyPhases } from '../../../../../__test-helpers__/fixtures.js'; +const { mockLoadAllCatwalkSprites } = vi.hoisted(() => ({ + mockLoadAllCatwalkSprites: vi.fn().mockResolvedValue(undefined), +})); + vi.mock('../../sprites/catwalk-sprite-loader.js', () => ({ - loadAllCatwalkSprites: vi.fn().mockResolvedValue(undefined), + loadAllCatwalkSprites: mockLoadAllCatwalkSprites, getAnimation: vi.fn().mockReturnValue({ type: 'animation' }), })); @@ -152,6 +156,15 @@ describe('CatwalkScene', () => { expect(scene.backgroundColor).toEqual(expect.objectContaining({ hex: '#1a1a2e' })); }); + it('calls loadAllCatwalkSprites on initialize', () => { + const status = createMockRunStatus({ status: 'in_progress' }); + const scene = new CatwalkScene(status); + + scene.onInitialize(); + + expect(mockLoadAllCatwalkSprites).toHaveBeenCalledOnce(); + }); + it('builds actors on initialize', () => { const status = createMockRunStatus({ status: 'in_progress' }); const scene = new CatwalkScene(status); diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts index 7520da29..0a4930b1 100644 --- a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts @@ -50,6 +50,7 @@ vi.mock('excalibur', () => { }); const { loadAllCatwalkSprites, getAnimation, clearCatwalkSpriteCache } = await import('../catwalk-sprite-loader.js'); +const { SPRITE_SHEET_URLS } = await import('../sprite-sheet-urls.js'); describe('catwalk-sprite-loader', () => { beforeEach(() => { @@ -75,6 +76,13 @@ describe('catwalk-sprite-loader', () => { expect(mockImageSourceLoad).toHaveBeenCalledTimes(2); }); + it('passes the correct URL for each sprite type', async () => { + await loadAllCatwalkSprites(); + + expect(mockImageSourceConstructor).toHaveBeenCalledWith(SPRITE_SHEET_URLS.subagent, expect.anything()); + expect(mockImageSourceConstructor).toHaveBeenCalledWith(SPRITE_SHEET_URLS.orchestrator, expect.anything()); + }); + it('is idempotent (second call is a no-op)', async () => { await loadAllCatwalkSprites(); await loadAllCatwalkSprites(); @@ -83,6 +91,16 @@ describe('catwalk-sprite-loader', () => { expect(mockImageSourceLoad).toHaveBeenCalledTimes(2); }); + it('deduplicates concurrent calls via in-flight promise guard', async () => { + const first = loadAllCatwalkSprites(); + const second = loadAllCatwalkSprites(); + + await Promise.all([first, second]); + + expect(mockImageSourceConstructor).toHaveBeenCalledTimes(2); + expect(mockImageSourceLoad).toHaveBeenCalledTimes(2); + }); + it('creates sprite sheets from loaded image sources', async () => { await loadAllCatwalkSprites(); @@ -101,6 +119,15 @@ describe('catwalk-sprite-loader', () => { expect(() => getAnimation('subagent', 'idle')).toThrow('Catwalk sprites have not been loaded'); }); + it('works synchronously after calling loadAllCatwalkSprites (before await)', () => { + // The cache is populated synchronously, so getAnimation is safe to call + // immediately without awaiting the load promise + loadAllCatwalkSprites().catch(() => undefined); + + expect(() => getAnimation('subagent', 'idle')).not.toThrow(); + expect(getAnimation('subagent', 'idle')).toBeDefined(); + }); + it('returns an animation for each sprite type and state', async () => { await loadAllCatwalkSprites(); diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/sprite-sheet-urls.test.ts b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/sprite-sheet-urls.test.ts index 40c9cfff..823d32c8 100644 --- a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/sprite-sheet-urls.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/sprite-sheet-urls.test.ts @@ -12,4 +12,8 @@ describe('SPRITE_SHEET_URLS', () => { expect(typeof SPRITE_SHEET_URLS.orchestrator).toBe('string'); expect(SPRITE_SHEET_URLS.orchestrator.length).toBeGreaterThan(0); }); + + it('exports distinct URLs for subagent and orchestrator', () => { + expect(SPRITE_SHEET_URLS.subagent).not.toBe(SPRITE_SHEET_URLS.orchestrator); + }); }); diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts index 527e3dff..0b66c605 100644 --- a/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts +++ b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts @@ -29,6 +29,7 @@ import { type CatwalkSpriteType, SPRITE_SHEET_URLS } from './sprite-sheet-urls.j const SPRITE_TYPES: readonly CatwalkSpriteType[] = ['subagent', 'orchestrator']; let animationCache: Map> | undefined; +let loadPromise: Promise | undefined; /** Resolves frame coordinates and timing for the given animation state. */ function frameConfigForState(state: AgentAnimationState): { @@ -89,9 +90,14 @@ function buildAnimationsForSheet(spriteSheet: SpriteSheet): Map { - if (animationCache !== undefined) return; +/** Build sprite sheets and animation objects synchronously, then load image data asynchronously. + * + * The animation cache is populated before `await`, so `getAnimation()` is safe to call + * immediately after invoking this function -- matching the `agent-sprite-loader` pattern + * where `SpriteSheet.fromImageSource` and animation creation are synchronous operations. */ +export function loadAllCatwalkSprites(): Promise { + if (animationCache !== undefined) return Promise.resolve(); + if (loadPromise !== undefined) return loadPromise; const cache = new Map>(); const imageSources: ImageSource[] = []; @@ -114,9 +120,11 @@ export async function loadAllCatwalkSprites(): Promise { cache.set(spriteType, buildAnimationsForSheet(spriteSheet)); } - await Promise.all(imageSources.map((source) => source.load())); - + // Populate cache synchronously so getAnimation() works immediately animationCache = cache; + + loadPromise = loadImageSources(imageSources); + return loadPromise; } /** Return the cached animation for the given sprite type and state. Throws if sprites have not been loaded. */ @@ -138,7 +146,13 @@ export function getAnimation(spriteType: CatwalkSpriteType, state: AgentAnimatio return animation; } +/** Load image data for all image sources. */ +async function loadImageSources(imageSources: ImageSource[]): Promise { + await Promise.all(imageSources.map((source) => source.load())); +} + /** Clear the catwalk sprite cache, allowing sprites to be reloaded. Useful for tests. */ export function clearCatwalkSpriteCache(): void { animationCache = undefined; + loadPromise = undefined; } From 0ff70d01ea9540a97b207e68448625c88aef1c73 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sat, 7 Mar 2026 15:00:05 -0800 Subject: [PATCH 3/5] factory|fix: Remove dead loadPromise guard and reset cache on load failure Remove the unreachable `loadPromise` guard from `loadAllCatwalkSprites` since `animationCache` is set synchronously in the same call frame, making it impossible for `loadPromise` to be defined while `animationCache` is undefined. On image-load failure, reset `animationCache` to undefined so subsequent calls can retry rather than silently returning a resolved promise with broken sprites. --- .../__tests__/catwalk-sprite-loader.test.ts | 17 ++++++++++++++++- .../catwalk/sprites/catwalk-sprite-loader.ts | 13 +++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts index 0a4930b1..ac94f877 100644 --- a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts @@ -91,7 +91,7 @@ describe('catwalk-sprite-loader', () => { expect(mockImageSourceLoad).toHaveBeenCalledTimes(2); }); - it('deduplicates concurrent calls via in-flight promise guard', async () => { + it('deduplicates concurrent calls (cache populated synchronously)', async () => { const first = loadAllCatwalkSprites(); const second = loadAllCatwalkSprites(); @@ -112,6 +112,21 @@ describe('catwalk-sprite-loader', () => { expect(mockAnimationFromCoords).toHaveBeenCalledTimes(12); }); + + it('resets cache on load failure so subsequent calls can retry', async () => { + mockImageSourceLoad.mockRejectedValueOnce(new Error('network error')); + + await expect(loadAllCatwalkSprites()).rejects.toThrow('network error'); + expect(() => getAnimation('subagent', 'idle')).toThrow('Catwalk sprites have not been loaded'); + + // Retry succeeds + mockImageSourceLoad.mockResolvedValue(undefined); + await loadAllCatwalkSprites(); + + expect(getAnimation('subagent', 'idle')).toBeDefined(); + // 2 calls for the failed attempt + 2 for the retry = 4 ImageSource constructions + expect(mockImageSourceConstructor).toHaveBeenCalledTimes(4); + }); }); describe('getAnimation', () => { diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts index 0b66c605..6eada556 100644 --- a/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts +++ b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts @@ -29,7 +29,6 @@ import { type CatwalkSpriteType, SPRITE_SHEET_URLS } from './sprite-sheet-urls.j const SPRITE_TYPES: readonly CatwalkSpriteType[] = ['subagent', 'orchestrator']; let animationCache: Map> | undefined; -let loadPromise: Promise | undefined; /** Resolves frame coordinates and timing for the given animation state. */ function frameConfigForState(state: AgentAnimationState): { @@ -97,7 +96,6 @@ function buildAnimationsForSheet(spriteSheet: SpriteSheet): Map { if (animationCache !== undefined) return Promise.resolve(); - if (loadPromise !== undefined) return loadPromise; const cache = new Map>(); const imageSources: ImageSource[] = []; @@ -120,11 +118,15 @@ export function loadAllCatwalkSprites(): Promise { cache.set(spriteType, buildAnimationsForSheet(spriteSheet)); } - // Populate cache synchronously so getAnimation() works immediately + // Populate cache synchronously so getAnimation() works immediately. + // Concurrent callers are deduplicated by the animationCache check above. animationCache = cache; - loadPromise = loadImageSources(imageSources); - return loadPromise; + return loadImageSources(imageSources).catch((error: unknown) => { + // Reset cache so a subsequent call can retry the load + animationCache = undefined; + throw error; + }); } /** Return the cached animation for the given sprite type and state. Throws if sprites have not been loaded. */ @@ -154,5 +156,4 @@ async function loadImageSources(imageSources: ImageSource[]): Promise { /** Clear the catwalk sprite cache, allowing sprites to be reloaded. Useful for tests. */ export function clearCatwalkSpriteCache(): void { animationCache = undefined; - loadPromise = undefined; } From 4d845fe3315fc57592aee1139b438fafd3ebf3f6 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sat, 7 Mar 2026 15:07:13 -0800 Subject: [PATCH 4/5] factory|refactor: Inline trivial helpers and remove dead mock class Inline `loadImageSources` (one-liner wrapper) directly into `loadAllCatwalkSprites`, converting it to async/await. Inline `stateForConfig` in `OrchestratorActor` since the ternary is self-explanatory at both call sites. Remove unused `MockCircle` from the actors test mock factory. --- .../catwalk/actors/__tests__/actors.test.ts | 8 -------- .../catwalk/sprites/catwalk-sprite-loader.ts | 15 ++++++--------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts b/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts index 96f7a827..3a143943 100644 --- a/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/actors/__tests__/actors.test.ts @@ -35,13 +35,6 @@ vi.mock('excalibur', () => { } } - class MockCircle { - options: Record; - constructor(options: Record) { - this.options = options; - } - } - class MockRectangle { options: Record; constructor(options: Record) { @@ -77,7 +70,6 @@ vi.mock('excalibur', () => { return { Actor: MockActor, Color: MockColor, - Circle: MockCircle, Rectangle: MockRectangle, Text: MockText, Font: MockFont, diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts index 6eada556..84fab8ba 100644 --- a/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts +++ b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts @@ -94,8 +94,8 @@ function buildAnimationsForSheet(spriteSheet: SpriteSheet): Map { - if (animationCache !== undefined) return Promise.resolve(); +export async function loadAllCatwalkSprites(): Promise { + if (animationCache !== undefined) return; const cache = new Map>(); const imageSources: ImageSource[] = []; @@ -122,11 +122,13 @@ export function loadAllCatwalkSprites(): Promise { // Concurrent callers are deduplicated by the animationCache check above. animationCache = cache; - return loadImageSources(imageSources).catch((error: unknown) => { + try { + await Promise.all(imageSources.map((source) => source.load())); + } catch (error: unknown) { // Reset cache so a subsequent call can retry the load animationCache = undefined; throw error; - }); + } } /** Return the cached animation for the given sprite type and state. Throws if sprites have not been loaded. */ @@ -148,11 +150,6 @@ export function getAnimation(spriteType: CatwalkSpriteType, state: AgentAnimatio return animation; } -/** Load image data for all image sources. */ -async function loadImageSources(imageSources: ImageSource[]): Promise { - await Promise.all(imageSources.map((source) => source.load())); -} - /** Clear the catwalk sprite cache, allowing sprites to be reloaded. Useful for tests. */ export function clearCatwalkSpriteCache(): void { animationCache = undefined; From 0a6a94aa95e4343af52155d31597610519d54d5c Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sat, 7 Mar 2026 15:54:49 -0800 Subject: [PATCH 5/5] factory|fix: Handle deactivated state in catwalk sprite loader The AgentAnimationState type includes 'deactivated' (added by the animation system), but frameConfigForState didn't handle it, causing a TypeScript error. Deactivated agents now reuse the idle animation; their visual dimming is handled by actor opacity. --- .../agents/scripts/extract-plugin-skills.sh | 140 ++++++++++++++++++ packages/agents/scripts/functions/colors.sh | 14 ++ .../__tests__/catwalk-sprite-loader.test.ts | 4 +- .../catwalk/sprites/catwalk-sprite-loader.ts | 4 + 4 files changed, 160 insertions(+), 2 deletions(-) create mode 100755 packages/agents/scripts/extract-plugin-skills.sh create mode 100644 packages/agents/scripts/functions/colors.sh diff --git a/packages/agents/scripts/extract-plugin-skills.sh b/packages/agents/scripts/extract-plugin-skills.sh new file mode 100755 index 00000000..7f5fa4aa --- /dev/null +++ b/packages/agents/scripts/extract-plugin-skills.sh @@ -0,0 +1,140 @@ +#!/usr/bin/env bash +set -euo pipefail + +# extract-plugin-skills.sh — Extract skills from Claude Code plugins for Rovo Dev. +# +# Copies SKILL.md files (and companion files) from the plugin cache to agents/rovodev/skills/. +# Finds the latest installed version of the plugin automatically. +# +# Usage: +# extract-plugin-skills.sh [skill...] +# extract-plugin-skills.sh --help + +readonly PROG="$(basename "$0")" + +source "$(git rev-parse --show-toplevel)/functions/colors.sh" + +repo_root="$(git rev-parse --show-toplevel)" +plugin_cache="$HOME/.claude/plugins/cache/claude-plugins-official" +output_base="$repo_root/agents/rovodev/skills" + +# -- Main flow -- + +main() { + # Show help (manual check -- getopts cannot parse long options) + if [[ "${1:-}" == "--help" ]]; then + show_usage 0 + fi + + # Parse options + while getopts ":h" opt; do + case $opt in + h) show_usage 0 ;; + *) + echo "$PROG: unknown option -$OPTARG" >&2 + show_usage + ;; + esac + done + shift $((OPTIND - 1)) + + # Validate arguments + if [[ $# -lt 1 ]]; then + echo "$PROG: plugin name is required" >&2 + show_usage + fi + + plugin="$1" + shift + requested_skills=("$@") + + # Find the latest version of the plugin + plugin_dir="$plugin_cache/$plugin" + if [[ ! -d "$plugin_dir" ]]; then + echo "${red}x${normal} $PROG: plugin not found: $plugin" >&2 + echo " Looked in: $plugin_dir" >&2 + exit 1 + fi + + latest_version=$(ls -1 "$plugin_dir" | sort -V | tail -1) + if [[ -z "$latest_version" ]]; then + echo "${red}x${normal} $PROG: no versions found for plugin: $plugin" >&2 + exit 1 + fi + + source_dir="$plugin_dir/$latest_version/skills" + if [[ ! -d "$source_dir" ]]; then + echo "${red}x${normal} $PROG: no skills directory in $plugin v$latest_version" >&2 + exit 1 + fi + + echo "Extracting from ${plugin} v${latest_version}" + echo "" + + # Determine which skills to extract + if [[ ${#requested_skills[@]} -eq 0 ]]; then + mapfile -t skill_dirs < <(find "$source_dir" -mindepth 1 -maxdepth 1 -type d | sort) + else + skill_dirs=() + for skill in "${requested_skills[@]}"; do + if [[ -d "$source_dir/$skill" ]]; then + skill_dirs+=("$source_dir/$skill") + else + echo "${yellow}!${normal} Skill not found: $skill (skipping)" >&2 + fi + done + fi + + if [[ ${#skill_dirs[@]} -eq 0 ]]; then + echo "${red}x${normal} $PROG: no skills to extract" >&2 + exit 1 + fi + + extracted=0 + for skill_path in "${skill_dirs[@]}"; do + skill_name=$(basename "$skill_path") + target_dir="$output_base/$skill_name" + + mkdir -p "$target_dir" + + for file in "$skill_path"/*; do + [[ -f "$file" ]] || continue + filename=$(basename "$file") + [[ "$filename" == "CREATION-LOG.md" ]] && continue + cp "$file" "$target_dir/$filename" + done + + echo "${green}ok${normal} $skill_name -> $target_dir" + ((extracted++)) || true + done + + echo "" + echo "Extracted $extracted skill(s) from $plugin v$latest_version" +} + +# region | Helper functions +show_usage() { + cat >&2 < [skill...] + $PROG --help + +Arguments: + Plugin name, e.g., superpowers (required) + [skill] Specific skill name(s) to extract (default: all) + +Options: + -h, --help Show this help + +Examples: + $PROG superpowers + $PROG superpowers brainstorming + $PROG superpowers brainstorming writing-plans +USAGE + exit "${1:-1}" +} +# endregion | Helper functions + +main "$@" diff --git a/packages/agents/scripts/functions/colors.sh b/packages/agents/scripts/functions/colors.sh new file mode 100644 index 00000000..9e10efe8 --- /dev/null +++ b/packages/agents/scripts/functions/colors.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash + +# Terminal color definitions for shell scripts. +# +# Source this file to use color variables in your scripts. +# Uses conditional assignment so callers can override colors before sourcing. +# +# Usage: +# source "$repo_dir/functions/colors.sh" + +: "${green:=$(tput setaf 2)}" +: "${yellow:=$(tput setaf 3)}" +: "${red:=$(tput setaf 1)}" +: "${normal:=$(tput sgr0)}" diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts index ac94f877..6d103e48 100644 --- a/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts +++ b/packages/factory/src/client/visualizations/catwalk/sprites/__tests__/catwalk-sprite-loader.test.ts @@ -107,10 +107,10 @@ describe('catwalk-sprite-loader', () => { expect(mockSpriteSheetFromImageSource).toHaveBeenCalledTimes(2); }); - it('creates animations for all 6 states x 2 sprite types = 12 animations', async () => { + it('creates animations for all 7 states x 2 sprite types = 14 animations', async () => { await loadAllCatwalkSprites(); - expect(mockAnimationFromCoords).toHaveBeenCalledTimes(12); + expect(mockAnimationFromCoords).toHaveBeenCalledTimes(14); }); it('resets cache on load failure so subsequent calls can retry', async () => { diff --git a/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts index 84fab8ba..f6b53823 100644 --- a/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts +++ b/packages/factory/src/client/visualizations/catwalk/sprites/catwalk-sprite-loader.ts @@ -57,6 +57,9 @@ function frameConfigForState(state: AgentAnimationState): { }; case 'resting': return { frameCoordinates: RESTING_FRAME_COORDINATES, duration: RESTING_DURATION, strategy: RESTING_STRATEGY }; + case 'deactivated': + // Deactivated agents reuse the idle animation; opacity is handled by the actor. + return { frameCoordinates: IDLE_FRAME_COORDINATES, duration: IDLE_DURATION, strategy: IDLE_STRATEGY }; default: { const _exhaustive: never = state; return _exhaustive; @@ -71,6 +74,7 @@ const ALL_STATES: readonly AgentAnimationState[] = [ 'celebrating', 'concerned', 'resting', + 'deactivated', ]; /** Build animation objects for every state from the given sprite sheet. */