fix: address code review issues from final review

- Override modes now actually affect component visibility (force_show,
  force_hide, auto respected in transitions and updates)
- RoomCodeDisplay.update() guarded by #active flag to prevent
  re-activation outside lobby state
- broadcastUpdate() only sends to components when in lobby state
- Relaxed idle→playing/ended transitions for out-of-order events
- room.disconnected now clears room code, players, lobby state context
- Empty room codes clear input fields instead of leaving stale values
- Removed dead auto-mode-toggle UI and associated CSS
- Fixed player list vertical centering when offset is applied

Made-with: Cursor
This commit is contained in:
cottongin
2026-03-20 13:08:52 -04:00
parent 19c94d294f
commit 875153ef63
4 changed files with 63 additions and 98 deletions

View File

@@ -13,7 +13,7 @@ const OVERRIDE_VALUES = new Set(Object.values(OVERRIDE_MODES));
/** @typedef {'idle'|'lobby'|'playing'|'ended'|'disconnected'} OverlayState */
const VALID_TRANSITIONS = Object.freeze({
idle: new Set(['lobby', 'disconnected']),
idle: new Set(['lobby', 'playing', 'ended', 'disconnected']),
lobby: new Set(['lobby', 'playing', 'ended', 'idle', 'disconnected']),
playing: new Set(['ended', 'lobby', 'idle', 'disconnected']),
ended: new Set(['idle', 'lobby', 'disconnected']),
@@ -106,9 +106,37 @@ export class OverlayManager {
throw new Error(`Invalid override mode: ${mode}`);
}
this.#overrides.set(name, mode);
this.#applyOverride(name);
this.#notify();
}
#applyOverride(name) {
const component = this.#components.get(name);
if (!component) return;
const mode = this.#overrides.get(name) ?? OVERRIDE_MODES.AUTO;
const ctx = this.getContext();
if (mode === OVERRIDE_MODES.FORCE_SHOW) {
if (typeof component.activate === 'function') {
try { component.activate(ctx); } catch (_) { /* ignore */ }
}
} else if (mode === OVERRIDE_MODES.FORCE_HIDE) {
if (typeof component.deactivate === 'function') {
try { component.deactivate(); } catch (_) { /* ignore */ }
}
} else if (mode === OVERRIDE_MODES.AUTO) {
if (this.#state === 'lobby') {
if (typeof component.activate === 'function') {
try { component.activate(ctx); } catch (_) { /* ignore */ }
}
} else {
if (typeof component.deactivate === 'function') {
try { component.deactivate(); } catch (_) { /* ignore */ }
}
}
}
}
/**
* @param {string} name
* @returns {string}
@@ -171,6 +199,10 @@ export class OverlayManager {
break;
case 'room.disconnected':
this.#context.roomCode = null;
this.#context.players = [];
this.#context.playerCount = 0;
this.#context.lobbyState = null;
this.#transitionTo('idle');
break;
@@ -209,14 +241,16 @@ export class OverlayManager {
}
#broadcastUpdate() {
if (this.#state !== 'lobby') {
this.#notify();
return;
}
const ctx = this.getContext();
for (const component of this.#components.values()) {
for (const [name, component] of this.#components) {
const mode = this.#overrides.get(name) ?? OVERRIDE_MODES.AUTO;
if (mode === OVERRIDE_MODES.FORCE_HIDE) continue;
if (typeof component.update === 'function') {
try {
component.update(ctx);
} catch (_) {
/* ignore */
}
try { component.update(ctx); } catch (_) { /* ignore */ }
}
}
this.#notify();
@@ -311,13 +345,11 @@ export class OverlayManager {
const enteringLobby = next === 'lobby' && current !== 'lobby';
if (leavingLobby) {
for (const component of this.#components.values()) {
for (const [name, component] of this.#components) {
const mode = this.#overrides.get(name) ?? OVERRIDE_MODES.AUTO;
if (mode !== OVERRIDE_MODES.AUTO) continue;
if (typeof component.deactivate === 'function') {
try {
component.deactivate();
} catch (_) {
/* ignore */
}
try { component.deactivate(); } catch (_) { /* ignore */ }
}
}
}
@@ -326,13 +358,11 @@ export class OverlayManager {
if (enteringLobby) {
const ctx = this.getContext();
for (const component of this.#components.values()) {
for (const [name, component] of this.#components) {
const mode = this.#overrides.get(name) ?? OVERRIDE_MODES.AUTO;
if (mode !== OVERRIDE_MODES.AUTO) continue;
if (typeof component.activate === 'function') {
try {
component.activate(ctx);
} catch (_) {
/* ignore */
}
try { component.activate(ctx); } catch (_) { /* ignore */ }
}
}
}
@@ -345,23 +375,19 @@ export class OverlayManager {
}
#fullLobbyReset() {
for (const component of this.#components.values()) {
for (const [name, component] of this.#components) {
const mode = this.#overrides.get(name) ?? OVERRIDE_MODES.AUTO;
if (mode !== OVERRIDE_MODES.AUTO) continue;
if (typeof component.deactivate === 'function') {
try {
component.deactivate();
} catch (_) {
/* ignore */
}
try { component.deactivate(); } catch (_) { /* ignore */ }
}
}
const ctx = this.getContext();
for (const component of this.#components.values()) {
for (const [name, component] of this.#components) {
const mode = this.#overrides.get(name) ?? OVERRIDE_MODES.AUTO;
if (mode !== OVERRIDE_MODES.AUTO) continue;
if (typeof component.activate === 'function') {
try {
component.activate(ctx);
} catch (_) {
/* ignore */
}
try { component.activate(ctx); } catch (_) { /* ignore */ }
}
}
}