From 875153ef63a12f2a569e0a3093d6558dfd0b1dce Mon Sep 17 00:00:00 2001 From: cottongin Date: Fri, 20 Mar 2026 13:08:52 -0400 Subject: [PATCH] fix: address code review issues from final review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- js/player-list.js | 2 +- js/room-code-display.js | 7 +++- js/state-manager.js | 88 ++++++++++++++++++++++++++--------------- optimized-controls.html | 64 ------------------------------ 4 files changed, 63 insertions(+), 98 deletions(-) diff --git a/js/player-list.js b/js/player-list.js index 0a5df99..b642fba 100644 --- a/js/player-list.js +++ b/js/player-list.js @@ -215,7 +215,7 @@ export class PlayerList { const emptyColor = inputs.emptyColor.value; const offsetY = `${parseInt(inputs.offset.value, 10) || 0}px`; - container.style.transform = `translateY(${offsetY})`; + container.style.transform = `translateY(calc(-50% + ${offsetY}))`; for (const slot of this._slots) { slot.nameEl.style.fontSize = fontPx; diff --git a/js/room-code-display.js b/js/room-code-display.js index 85c4061..63c37a7 100644 --- a/js/room-code-display.js +++ b/js/room-code-display.js @@ -83,6 +83,9 @@ export class RoomCodeDisplay { const mid = Math.floor(raw.length / 2); inputs.code1.value = raw.slice(0, mid); inputs.code2.value = raw.slice(mid); + } else { + inputs.code1.value = ''; + inputs.code2.value = ''; } this.#applySettings(); @@ -114,7 +117,7 @@ export class RoomCodeDisplay { * @param {{ roomCode?: string, [key: string]: unknown }} ctx */ update(ctx) { - if (!this.#inputs) { + if (!this.#active || !this.#inputs) { return; } @@ -122,7 +125,7 @@ export class RoomCodeDisplay { const current = this.#inputs.code1.value.toUpperCase() + this.#inputs.code2.value.toUpperCase(); - if (next !== current) { + if (next && next !== current) { this.activate(ctx); } } diff --git a/js/state-manager.js b/js/state-manager.js index 790802b..54a6271 100644 --- a/js/state-manager.js +++ b/js/state-manager.js @@ -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 */ } } } } diff --git a/optimized-controls.html b/optimized-controls.html index 47f8bf5..066c6e9 100644 --- a/optimized-controls.html +++ b/optimized-controls.html @@ -305,59 +305,6 @@ background-color: #c9302c; } - /* Auto/Manual mode toggle */ - .mode-toggle { - display: flex; - align-items: center; - gap: 8px; - } - - .mode-toggle-switch { - position: relative; - width: 36px; - height: 18px; - flex-shrink: 0; - } - - .mode-toggle-switch input { - opacity: 0; - width: 0; - height: 0; - } - - .mode-toggle-slider { - position: absolute; - cursor: pointer; - top: 0; left: 0; right: 0; bottom: 0; - background-color: #555; - border-radius: 18px; - transition: background-color 0.2s; - } - - .mode-toggle-slider:before { - content: ""; - position: absolute; - height: 14px; - width: 14px; - left: 2px; - bottom: 2px; - background-color: white; - border-radius: 50%; - transition: transform 0.2s; - } - - .mode-toggle-switch input:checked + .mode-toggle-slider { - background-color: #4CAF50; - } - - .mode-toggle-switch input:checked + .mode-toggle-slider:before { - transform: translateX(18px); - } - - .mode-label { - color: #ccc; - font-size: 12px; - } /* Player list */ #player-list-container { @@ -771,17 +718,6 @@ -
- -
- Manual - - Auto -
-