Skip to content

Commit aeacf89

Browse files
committed
Narrow UIManager surface by exposing log and overlay subviews
Expose GameLogView as ui.log and OverlayView as ui.overlay, eliminating 15 thin pass-through methods from UIManager. Callers now use ui.log.logText() and ui.overlay.showToast() directly. Screen-mode and HUD methods remain on UIManager since they orchestrate multiple subviews.
1 parent 6888938 commit aeacf89

File tree

9 files changed

+139
-203
lines changed

9 files changed

+139
-203
lines changed

docs/BACKLOG.md

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -95,25 +95,6 @@ behavior for the covered flow.
9595
`src/client/game/session-controller.ts`,
9696
`src/client/main.ts`
9797

98-
### Narrow the `UIManager` surface
99-
100-
Reduce pass-through verbosity in `UIManager` so it remains a
101-
coordinator, not a large forwarding facade.
102-
103-
`UIManager` currently owns the right boundaries, but it still
104-
contains many thin methods that simply relay calls into
105-
subviews. The next step is to group or collapse those
106-
operations around clearer screen/view-model boundaries rather
107-
than one-method-per-button/per-label style forwarding.
108-
109-
Definition of done: `UIManager` exposes a smaller, more
110-
coherent API grouped around screen mode and major UI domains,
111-
with no regression in ownership clarity or testability.
112-
113-
**Files:** `src/client/ui/ui.ts`,
114-
`src/client/main.ts`,
115-
affected UI subviews and tests
116-
11798
### Reduce repetitive UI collection rendering
11899

119100
Introduce a clearer pattern for list-like UI rendering where

src/client/game/command-router.test.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,8 @@ const createDeps = (overrides?: {
152152
};
153153
const showAttackButton = vi.fn<CommandRouterDeps['ui']['showAttackButton']>();
154154
const showFireButton = vi.fn<CommandRouterDeps['ui']['showFireButton']>();
155-
const showToast = vi.fn<CommandRouterDeps['ui']['showToast']>();
156-
const toggleLog = vi.fn<CommandRouterDeps['ui']['toggleLog']>();
155+
const showToast = vi.fn<CommandRouterDeps['ui']['overlay']['showToast']>();
156+
const toggleLog = vi.fn<CommandRouterDeps['ui']['log']['toggle']>();
157157
const updateHUD = vi.fn<() => void>();
158158
const renderer = {
159159
centerOnHex: vi.fn<(position: { q: number; r: number }) => void>(),
@@ -196,8 +196,8 @@ const createDeps = (overrides?: {
196196
ui: {
197197
showAttackButton,
198198
showFireButton,
199-
showToast,
200-
toggleLog,
199+
overlay: { showToast },
200+
log: { toggle: toggleLog },
201201
},
202202
renderer,
203203
getCanvasCenter: () => ({ x: 400, y: 300 }),
@@ -256,7 +256,7 @@ describe('game-command-router', () => {
256256

257257
expect(deps.ctx.planningState.queuedAttacks).toHaveLength(1);
258258
expect(ui.showFireButton).toHaveBeenCalledWith(true, 1);
259-
expect(ui.showToast).toHaveBeenCalledWith(
259+
expect(ui.overlay.showToast).toHaveBeenCalledWith(
260260
'Undid last attack (1 queued)',
261261
'info',
262262
);
@@ -318,7 +318,10 @@ describe('game-command-router', () => {
318318
expect(deps.ctx.planningState.selectedShipId).toBe('ship-1');
319319
expect(deps.ctx.planningState.lastSelectedHex).toBe('0,0');
320320
expect(renderer.centerOnHex).toHaveBeenCalledWith({ q: 0, r: 0 });
321-
expect(ui.showToast).toHaveBeenCalledWith('Selected: Packet', 'info');
321+
expect(ui.overlay.showToast).toHaveBeenCalledWith(
322+
'Selected: Packet',
323+
'info',
324+
);
322325
expect(updateHUD).toHaveBeenCalledTimes(1);
323326
});
324327

src/client/game/command-router.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,10 @@ interface CommandRouterContext {
5252
interface CommandRouterUI {
5353
showAttackButton: (visible: boolean) => void;
5454
showFireButton: (visible: boolean, count: number) => void;
55-
showToast: (message: string, type: 'error' | 'info' | 'success') => void;
56-
toggleLog: () => void;
55+
overlay: {
56+
showToast: (message: string, type: 'error' | 'info' | 'success') => void;
57+
};
58+
log: { toggle: () => void };
5759
}
5860

5961
interface CommandRouterRenderer {
@@ -95,7 +97,7 @@ const undoQueuedAttack = (deps: CommandRouterDeps): void => {
9597
const count = popQueuedAttack(deps.ctx.planningState);
9698

9799
deps.ui.showFireButton(count > 0, count);
98-
deps.ui.showToast(
100+
deps.ui.overlay.showToast(
99101
count > 0 ? `Undid last attack (${count} queued)` : 'Attack queue cleared',
100102
'info',
101103
);
@@ -149,7 +151,7 @@ const selectShip = (
149151

150152
if (myAlive && myAlive.length > 1) {
151153
const name = SHIP_STATS[ship.type]?.name ?? ship.type;
152-
deps.ui.showToast(`Selected: ${name}`, 'info');
154+
deps.ui.overlay.showToast(`Selected: ${name}`, 'info');
153155
}
154156
} else {
155157
setSelectedShipId(deps.ctx.planningState, shipId);
@@ -257,7 +259,7 @@ export const dispatchGameCommand = (
257259
return;
258260
}
259261
case 'toggleLog':
260-
deps.ui.toggleLog();
262+
deps.ui.log.toggle();
261263
return;
262264
case 'toggleHelp':
263265
deps.toggleHelp();

src/client/game/integration.test.ts

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,18 @@ const createDeps = (
112112
clearTrails: track('renderer.clearTrails'),
113113
},
114114
ui: {
115-
showToast: track('ui.showToast'),
116-
logText: track('ui.logText'),
117-
setChatEnabled: track('ui.setChatEnabled'),
118-
hideReconnecting: track('ui.hideReconnecting'),
119115
setPlayerId: track('ui.setPlayerId'),
120-
clearLog: track('ui.clearLog'),
121-
showRematchPending: track('ui.showRematchPending'),
122-
showGameOver: track('ui.showGameOver'),
116+
log: {
117+
logText: track('ui.log.logText'),
118+
setChatEnabled: track('ui.log.setChatEnabled'),
119+
clear: track('ui.log.clear'),
120+
},
121+
overlay: {
122+
showToast: track('ui.overlay.showToast'),
123+
hideReconnecting: track('ui.overlay.hideReconnecting'),
124+
showRematchPending: track('ui.overlay.showRematchPending'),
125+
showGameOver: track('ui.overlay.showGameOver'),
126+
},
123127
updateLatency: track('ui.updateLatency'),
124128
},
125129
calls,
@@ -159,8 +163,10 @@ describe('client integration: connection flow', () => {
159163
});
160164

161165
expect(deps.ctx.reconnectAttempts).toBe(0);
162-
expect(deps.calls['ui.hideReconnecting']).toHaveLength(1);
163-
expect(deps.calls['ui.showToast']).toEqual([['Reconnected!', 'success']]);
166+
expect(deps.calls['ui.overlay.hideReconnecting']).toHaveLength(1);
167+
expect(deps.calls['ui.overlay.showToast']).toEqual([
168+
['Reconnected!', 'success'],
169+
]);
164170
// No state transition when already playing
165171
expect(deps.calls.setState).toBeUndefined();
166172
});
@@ -177,8 +183,8 @@ describe('client integration: connection flow', () => {
177183
expect(deps.calls.resetTurnTelemetry).toHaveLength(1);
178184
expect(deps.calls.applyGameState).toEqual([[state]]);
179185
expect(deps.calls['renderer.clearTrails']).toHaveLength(1);
180-
expect(deps.calls['ui.clearLog']).toHaveLength(1);
181-
expect(deps.calls['ui.setChatEnabled']).toEqual([[true]]);
186+
expect(deps.calls['ui.log.clear']).toHaveLength(1);
187+
expect(deps.calls['ui.log.setChatEnabled']).toEqual([[true]]);
182188
expect(deps.calls.logScenarioBriefing).toHaveLength(1);
183189
expect(deps.calls.setState).toEqual([['playing_astrogation']]);
184190
});
@@ -408,7 +414,7 @@ describe('client integration: chat and errors', () => {
408414
text: 'hello',
409415
});
410416

411-
expect(deps.calls['ui.logText']).toEqual([['You: hello', 'log-chat']]);
417+
expect(deps.calls['ui.log.logText']).toEqual([['You: hello', 'log-chat']]);
412418
});
413419

414420
it('opponent chat message logs with "Opponent" label', () => {
@@ -421,7 +427,7 @@ describe('client integration: chat and errors', () => {
421427
text: 'gg',
422428
});
423429

424-
expect(deps.calls['ui.logText']).toEqual([
430+
expect(deps.calls['ui.log.logText']).toEqual([
425431
['Opponent: gg', 'log-chat-opponent'],
426432
]);
427433
});
@@ -434,7 +440,9 @@ describe('client integration: chat and errors', () => {
434440
message: 'Invalid action',
435441
});
436442

437-
expect(deps.calls['ui.showToast']).toEqual([['Invalid action', 'error']]);
443+
expect(deps.calls['ui.overlay.showToast']).toEqual([
444+
['Invalid action', 'error'],
445+
]);
438446
});
439447

440448
it('rematch pending shows UI', () => {
@@ -444,7 +452,7 @@ describe('client integration: chat and errors', () => {
444452
type: 'rematchPending',
445453
} as S2C);
446454

447-
expect(deps.calls['ui.showRematchPending']).toHaveLength(1);
455+
expect(deps.calls['ui.overlay.showRematchPending']).toHaveLength(1);
448456
});
449457
});
450458

src/client/game/message-handler.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,18 @@ export interface MessageHandlerDeps {
4141
clearTrails: () => void;
4242
};
4343
ui: {
44-
showToast: (message: string, type: 'error' | 'info' | 'success') => void;
45-
logText: (text: string, cssClass?: string) => void;
46-
setChatEnabled: (enabled: boolean) => void;
47-
hideReconnecting: () => void;
4844
setPlayerId: (id: number) => void;
49-
clearLog: () => void;
50-
showRematchPending: () => void;
51-
showGameOver: (won: boolean, reason: string) => void;
45+
log: {
46+
logText: (text: string, cssClass?: string) => void;
47+
setChatEnabled: (enabled: boolean) => void;
48+
clear: () => void;
49+
};
50+
overlay: {
51+
showToast: (message: string, type: 'error' | 'info' | 'success') => void;
52+
hideReconnecting: () => void;
53+
showRematchPending: () => void;
54+
showGameOver: (won: boolean, reason: string) => void;
55+
};
5256
updateLatency: (ms: number) => void;
5357
};
5458
}
@@ -68,8 +72,8 @@ export const handleServerMessage = (
6872
applyWelcomeSession(deps.ctx, plan.playerId, plan.code);
6973
deps.storePlayerToken(plan.code, plan.playerToken);
7074
if (plan.showReconnectToast) {
71-
deps.ui.hideReconnecting();
72-
deps.ui.showToast('Reconnected!', 'success');
75+
deps.ui.overlay.hideReconnecting();
76+
deps.ui.overlay.showToast('Reconnected!', 'success');
7377
}
7478
deps.renderer.setPlayerId(plan.playerId);
7579
deps.ui.setPlayerId(plan.playerId);
@@ -85,8 +89,8 @@ export const handleServerMessage = (
8589
deps.resetTurnTelemetry();
8690
deps.applyGameState(deps.deserializeState(plan.state));
8791
deps.renderer.clearTrails();
88-
deps.ui.clearLog();
89-
deps.ui.setChatEnabled(true);
92+
deps.ui.log.clear();
93+
deps.ui.log.setChatEnabled(true);
9094
deps.logScenarioBriefing();
9195
deps.setState(plan.nextState);
9296
break;
@@ -123,16 +127,16 @@ export const handleServerMessage = (
123127
deps.showGameOverOutcome(plan.won, plan.reason);
124128
break;
125129
case 'rematchPending':
126-
deps.ui.showRematchPending();
130+
deps.ui.overlay.showRematchPending();
127131
break;
128132
case 'error':
129133
console.error('Server error:', plan.message);
130-
deps.ui.showToast(plan.message, 'error');
134+
deps.ui.overlay.showToast(plan.message, 'error');
131135
break;
132136
case 'chat': {
133137
const isOwn = plan.playerId === deps.ctx.playerId;
134138
const label = isOwn ? 'You' : 'Opponent';
135-
deps.ui.logText(
139+
deps.ui.log.logText(
136140
`${label}: ${plan.text}`,
137141
isOwn ? 'log-chat' : 'log-chat-opponent',
138142
);

src/client/game/presentation.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,23 +41,31 @@ export interface PresentationDeps {
4141
showLandingEffect: (hex: HexCoord) => void;
4242
};
4343
ui: {
44-
logMovementEvents: (events: MovementEvent[], ships: Ship[]) => void;
45-
logCombatResults: (results: CombatResult[], ships: Ship[]) => void;
46-
showToast: (message: string, type: 'error' | 'info' | 'success') => void;
47-
logText: (text: string, cssClass?: string) => void;
48-
logLanding: (shipName: string, bodyName: string) => void;
49-
showGameOver: (won: boolean, reason: string, stats?: GameOverStats) => void;
44+
log: {
45+
logMovementEvents: (events: MovementEvent[], ships: Ship[]) => void;
46+
logCombatResults: (results: CombatResult[], ships: Ship[]) => void;
47+
logText: (text: string, cssClass?: string) => void;
48+
logLanding: (shipName: string, bodyName: string) => void;
49+
};
50+
overlay: {
51+
showToast: (message: string, type: 'error' | 'info' | 'success') => void;
52+
showGameOver: (
53+
won: boolean,
54+
reason: string,
55+
stats?: GameOverStats,
56+
) => void;
57+
};
5058
};
5159
}
5260

5361
const logLandings = (deps: PresentationDeps, movements: ShipMovement[]) => {
5462
const gameState = deps.getGameState();
5563
if (!gameState) return;
5664
for (const entry of deriveLandingLogEntries(gameState, movements)) {
57-
deps.ui.logLanding(entry.shipName, entry.bodyName);
65+
deps.ui.log.logLanding(entry.shipName, entry.bodyName);
5866
deps.renderer.showLandingEffect(entry.destination);
5967
if (entry.resupplyText) {
60-
deps.ui.logText(entry.resupplyText);
68+
deps.ui.log.logText(entry.resupplyText);
6169
}
6270
}
6371
};
@@ -75,7 +83,7 @@ export const presentMovementResult = (
7583
playThrust();
7684
if (events.length > 0) {
7785
deps.renderer.showMovementEvents(events);
78-
deps.ui.logMovementEvents(events, state.ships);
86+
deps.ui.log.logMovementEvents(events, state.ships);
7987
if (
8088
events.some(
8189
(event) => event.damageType === 'eliminated' || event.type === 'crash',
@@ -97,7 +105,7 @@ export const presentCombatResults = (
97105
) => {
98106
deps.applyGameState(state);
99107
deps.renderer.showCombatResults(results, previousState);
100-
deps.ui.logCombatResults(results, state.ships);
108+
deps.ui.log.logCombatResults(results, state.ships);
101109
for (const [i, result] of results.entries()) {
102110
const target =
103111
result.targetType === 'ship'
@@ -119,7 +127,7 @@ export const presentCombatResults = (
119127
? 'info'
120128
: 'info';
121129
setTimeout(
122-
() => deps.ui.showToast(`${targetName}: ${outcome}`, toastType),
130+
() => deps.ui.overlay.showToast(`${targetName}: ${outcome}`, toastType),
123131
i * 400,
124132
);
125133
}
@@ -141,13 +149,13 @@ export const showGameOverOutcome = (
141149
const gameState = deps.getGameState();
142150
const playerId = deps.getPlayerId();
143151
const plan = deriveGameOverPlan(gameState, playerId, won, reason);
144-
deps.ui.logText(plan.logText, plan.logClass);
152+
deps.ui.log.logText(plan.logText, plan.logClass);
145153
const loserShips =
146154
gameState?.ships.filter((ship: Ship) =>
147155
plan.loserShipIds.includes(ship.id),
148156
) ?? [];
149157
if (loserShips.length === 0) {
150-
deps.ui.showGameOver(won, reason, plan.stats);
158+
deps.ui.overlay.showGameOver(won, reason, plan.stats);
151159
if (plan.resultSound === 'victory') {
152160
playVictory();
153161
} else {
@@ -158,7 +166,7 @@ export const showGameOverOutcome = (
158166
playExplosion();
159167
const animDuration = deps.renderer.triggerGameOverExplosions(loserShips);
160168
setTimeout(() => {
161-
deps.ui.showGameOver(won, reason, plan.stats);
169+
deps.ui.overlay.showGameOver(won, reason, plan.stats);
162170
if (plan.resultSound === 'victory') {
163171
playVictory();
164172
} else {

0 commit comments

Comments
 (0)