From fe07fa3e8f726d7c8281a2589c9f6575d0d75374 Mon Sep 17 00:00:00 2001 From: Jage9 Date: Tue, 24 Feb 2026 19:56:44 -0500 Subject: [PATCH] Use structured piano status packets instead of message text matching --- client/public/version.js | 2 +- client/src/items/types/behaviorRegistry.ts | 7 +++ client/src/items/types/piano/behavior.ts | 35 ++--------- client/src/items/types/piano/runtime.ts | 45 +++++++++----- client/src/items/types/runtimeShared.ts | 1 + client/src/main.ts | 1 + client/src/network/messageHandlers.ts | 6 ++ client/src/network/protocol.ts | 16 +++++ docs/protocol-notes.md | 3 + docs/runtime-flow.md | 1 + server/app/models.py | 15 +++++ server/app/server.py | 72 ++++++++++++++++++---- server/tests/test_item_use_cooldown.py | 12 +++- 13 files changed, 155 insertions(+), 61 deletions(-) diff --git a/client/public/version.js b/client/public/version.js index b3f58fa..e121865 100644 --- a/client/public/version.js +++ b/client/public/version.js @@ -1,5 +1,5 @@ // Maintainer-controlled web client version. // Format: YYYY.MM.DD Rn (example: 2026.02.20 R2) -window.CHGRID_WEB_VERSION = "2026.02.25 R231"; +window.CHGRID_WEB_VERSION = "2026.02.25 R232"; // Optional display timezone for timestamps. Falls back to America/Detroit if unset/invalid. window.CHGRID_TIME_ZONE = "America/Detroit"; diff --git a/client/src/items/types/behaviorRegistry.ts b/client/src/items/types/behaviorRegistry.ts index 037bc0e..b52af09 100644 --- a/client/src/items/types/behaviorRegistry.ts +++ b/client/src/items/types/behaviorRegistry.ts @@ -83,6 +83,13 @@ export class ItemBehaviorRegistry { } } + /** Routes incoming item-piano-status packets to behavior modules that track piano runtime state. */ + onPianoStatus(message: Extract): void { + for (const behavior of this.behaviors) { + behavior.onPianoStatus?.(message); + } + } + /** Stops all remote notes for one sender across behavior modules that own remote note runtimes. */ stopAllRemoteNotesForSender(senderId: string): void { for (const behavior of this.behaviors) { diff --git a/client/src/items/types/piano/behavior.ts b/client/src/items/types/piano/behavior.ts index 0015b9f..cb0c357 100644 --- a/client/src/items/types/piano/behavior.ts +++ b/client/src/items/types/piano/behavior.ts @@ -11,17 +11,6 @@ export function createPianoBehavior(deps: ItemBehaviorDeps): ItemBehavior { openHelpViewer: deps.openHelpViewer, }); - const statusMessages = new Set([ - 'record', - 'pause', - 'resume', - 'play', - 'stop', - 'No recording saved on this piano.', - 'Stop recording before playback.', - 'This piano is already recording.', - ]); - return { onInit: async () => { await controller.loadHelpFromUrl(deps.withBase('piano.json')); @@ -30,25 +19,10 @@ export function createPianoBehavior(deps: ItemBehaviorDeps): ItemBehavior { onCleanup: () => { controller.cleanup(); }, - onUseResultMessage: (message) => { - controller.onUseResultMessage(message); - if ( - message.type === 'item_action_result' && - message.ok && - message.action === 'use' && - typeof message.itemId === 'string' && - typeof message.message === 'string' && - message.message.toLowerCase().includes('begin playing') - ) { - const item = deps.state.items.get(message.itemId); - if (item?.type === 'piano') { - void controller.startUseMode(item.id); - } - } - }, onActionResultStatus: (message) => { - if (message.action !== 'use') return false; - if (!statusMessages.has(message.message)) return false; + if (message.action !== 'use' || typeof message.itemId !== 'string') return false; + const item = deps.state.items.get(message.itemId); + if (item?.type !== 'piano') return false; deps.updateStatus(message.message); if (message.ok) { deps.audio.sfxUiBlip(); @@ -57,6 +31,9 @@ export function createPianoBehavior(deps: ItemBehaviorDeps): ItemBehavior { } return true; }, + onPianoStatus: (message) => { + controller.onPianoStatus(message); + }, onPropertyPreviewChange: (item, key, value) => { controller.onPreviewPropertyChange(item, key, value); }, diff --git a/client/src/items/types/piano/runtime.ts b/client/src/items/types/piano/runtime.ts index deafb62..706dde4 100644 --- a/client/src/items/types/piano/runtime.ts +++ b/client/src/items/types/piano/runtime.ts @@ -4,7 +4,7 @@ import { isPianoInstrumentId, type PianoInstrumentId, } from '../../../audio/pianoSynth'; -import { type IncomingMessage, type OutgoingMessage } from '../../../network/protocol'; +import { type OutgoingMessage } from '../../../network/protocol'; import { type GameMode, type WorldItem } from '../../../state/gameState'; import { getItemPropertyOptionValues } from '../../itemRegistry'; @@ -494,25 +494,36 @@ export class PianoController { } } - /** Applies recording-state transitions from successful piano use result messages. */ - onUseResultMessage(message: IncomingMessage): void { - if ( - message.type !== 'item_action_result' || - !message.ok || - message.action !== 'use' || - typeof message.itemId !== 'string' || - !this.activePianoItemId || - message.itemId !== this.activePianoItemId - ) { + /** Applies server-reported piano mode/recording/playback state transitions. */ + onPianoStatus(message: { + itemId: string; + event: + | 'use_mode_entered' + | 'record_started' + | 'record_paused' + | 'record_resumed' + | 'record_stopped' + | 'playback_started' + | 'playback_stopped'; + recordingState?: 'idle' | 'recording' | 'paused' | 'playback'; + }): void { + if (message.event === 'use_mode_entered') { + void this.startUseMode(message.itemId); return; } - if (message.message === 'record' || message.message === 'resume') { - this.activePianoRecordingState = 'recording'; - } else if (message.message === 'pause') { - this.activePianoRecordingState = 'paused'; - } else if (message.message === 'stop') { - this.activePianoRecordingState = 'idle'; + if (!this.activePianoItemId || message.itemId !== this.activePianoItemId) { + return; } + const state = message.recordingState; + if (state === 'recording') { + this.activePianoRecordingState = 'recording'; + return; + } + if (state === 'paused') { + this.activePianoRecordingState = 'paused'; + return; + } + this.activePianoRecordingState = 'idle'; } /** Exits piano mode if the active piano item disappears from local world state. */ diff --git a/client/src/items/types/runtimeShared.ts b/client/src/items/types/runtimeShared.ts index cf9660e..2a04c2e 100644 --- a/client/src/items/types/runtimeShared.ts +++ b/client/src/items/types/runtimeShared.ts @@ -32,5 +32,6 @@ export type ItemBehavior = { handleModeInput?: (mode: GameMode, code: string) => boolean; handleModeKeyUp?: (mode: GameMode, code: string) => boolean; onRemotePianoNote?: (message: Extract) => void; + onPianoStatus?: (message: Extract) => void; onStopAllRemoteNotesForSender?: (senderId: string) => void; }; diff --git a/client/src/main.ts b/client/src/main.ts index ebe5ecf..5808da4 100644 --- a/client/src/main.ts +++ b/client/src/main.ts @@ -1379,6 +1379,7 @@ const onAppMessage = createOnMessageHandler({ }, handleItemActionResultStatus: (message) => itemBehaviorRegistry.onActionResultStatus(message), handleRemotePianoNote: (message) => itemBehaviorRegistry.onRemotePianoNote(message), + handlePianoStatus: (message) => itemBehaviorRegistry.onPianoStatus(message), stopAllRemoteNotesForSender: (senderId) => itemBehaviorRegistry.stopAllRemoteNotesForSender(senderId), TELEPORT_SOUND_URL, TELEPORT_START_SOUND_URL, diff --git a/client/src/network/messageHandlers.ts b/client/src/network/messageHandlers.ts index 7df4261..e8f382a 100644 --- a/client/src/network/messageHandlers.ts +++ b/client/src/network/messageHandlers.ts @@ -49,6 +49,7 @@ type MessageHandlerDeps = { playRemoteSpatialStepOrTeleport: (url: string, peerX: number, peerY: number) => void; handleItemActionResultStatus: (message: Extract) => boolean; handleRemotePianoNote: (message: Extract) => void; + handlePianoStatus: (message: Extract) => void; stopAllRemoteNotesForSender: (senderId: string) => void; TELEPORT_SOUND_URL: string; TELEPORT_START_SOUND_URL: string; @@ -272,6 +273,11 @@ export function createOnMessageHandler(deps: MessageHandlerDeps): (message: Inco deps.handleRemotePianoNote(message); break; } + + case 'item_piano_status': { + deps.handlePianoStatus(message); + break; + } } }; } diff --git a/client/src/network/protocol.ts b/client/src/network/protocol.ts index 2e10186..f688fa8 100644 --- a/client/src/network/protocol.ts +++ b/client/src/network/protocol.ts @@ -180,6 +180,21 @@ export const itemPianoNoteSchema = z.object({ emitRange: z.number().int().min(1), }); +export const itemPianoStatusSchema = z.object({ + type: z.literal('item_piano_status'), + itemId: z.string(), + event: z.enum([ + 'use_mode_entered', + 'record_started', + 'record_paused', + 'record_resumed', + 'record_stopped', + 'playback_started', + 'playback_stopped', + ]), + recordingState: z.enum(['idle', 'recording', 'paused', 'playback']).optional(), +}); + export const incomingMessageSchema = z.discriminatedUnion('type', [ welcomeMessageSchema, signalMessageSchema, @@ -194,6 +209,7 @@ export const incomingMessageSchema = z.discriminatedUnion('type', [ itemActionResultSchema, itemUseSoundSchema, itemPianoNoteSchema, + itemPianoStatusSchema, ]); export type IncomingMessage = z.infer; diff --git a/docs/protocol-notes.md b/docs/protocol-notes.md index f1ec0a1..e58b0fd 100644 --- a/docs/protocol-notes.md +++ b/docs/protocol-notes.md @@ -31,11 +31,14 @@ This is a behavior guide for packet semantics beyond raw schemas. - `item_action_result`: action success/failure and user-facing message. - `item_use_sound`: spatial one-shot sound on successful item use (if `useSound` configured). - `item_piano_note`: broadcast piano note on/off with resolved instrument/envelope/spatial params. +- `item_piano_status`: structured piano mode/record/playback state events for client runtime control. ## Item Packet Behavior - `item_upsert` is full-state replacement for one item, not partial patch. - `item_action_result` messages are intended for direct screen-reader/user status feedback. +- Piano runtime control no longer depends on parsing `item_action_result.message` text. +- `item_piano_status` carries machine-readable piano events (`use_mode_entered`, record/playback transitions). - `item_use_sound` contains absolute item world coordinates (`x`, `y`) and sound path. - `item_piano_note` contains: - `itemId`, `senderId`, `keyId`, `midi`, `on` diff --git a/docs/runtime-flow.md b/docs/runtime-flow.md index ffbd2a0..32d0d48 100644 --- a/docs/runtime-flow.md +++ b/docs/runtime-flow.md @@ -46,6 +46,7 @@ Core incoming message effects: - `item_action_result`: success/error status for actions. - `item_use_sound`: play one-shot spatial sample (world layer gated). - `item_piano_note`: start/stop synthesized piano notes from remote users (item layer gated). +- `item_piano_status`: structured piano mode/record/playback transitions (client runtime state). - `pong`: - positive `clientSentAt`: user ping response (`P` command) - negative `clientSentAt`: internal heartbeat response diff --git a/server/app/models.py b/server/app/models.py index 57b08fb..2c15958 100644 --- a/server/app/models.py +++ b/server/app/models.py @@ -247,3 +247,18 @@ class ItemPianoNoteBroadcastPacket(BasePacket): x: int y: int emitRange: int + + +class ItemPianoStatusPacket(BasePacket): + type: Literal["item_piano_status"] + itemId: str + event: Literal[ + "use_mode_entered", + "record_started", + "record_paused", + "record_resumed", + "record_stopped", + "playback_started", + "playback_stopped", + ] + recordingState: Literal["idle", "recording", "paused", "playback"] | None = None diff --git a/server/app/server.py b/server/app/server.py index 328bfa0..5450510 100644 --- a/server/app/server.py +++ b/server/app/server.py @@ -52,6 +52,7 @@ from .models import ( ItemPianoNoteBroadcastPacket, ItemPianoNotePacket, ItemPianoRecordingPacket, + ItemPianoStatusPacket, ItemPickupPacket, ItemRemovePacket, ItemUpdatePacket, @@ -328,7 +329,7 @@ class SignalingServer: elapsed_ms += max(0, int((now_value - float(last_resume)) * 1000)) return max(0, elapsed_ms) - async def _finalize_piano_recording(self, item_id: str, *, status_message: str | None = None) -> None: + async def _finalize_piano_recording(self, item_id: str, *, notify_owner: bool = False) -> None: """Persist and broadcast one active recording session, then clear runtime state.""" session = self.piano_recording_state_by_item.pop(item_id, None) @@ -414,8 +415,14 @@ class SignalingServer: await self._broadcast_item(item) owner_id = str(session.get("ownerClientId", "")) owner = self._get_client_by_id(owner_id) if owner_id else None - if owner and status_message: - await self._send_item_result(owner, True, "use", status_message, item.id) + if owner and notify_owner: + await self._send_piano_status( + owner, + item_id=item.id, + event="record_stopped", + recording_state="idle", + ) + await self._send_item_result(owner, True, "use", "Recording stopped.", item.id) async def _auto_stop_piano_recording(self, item_id: str) -> None: """Stop a recording automatically at the max recording duration.""" @@ -426,7 +433,7 @@ class SignalingServer: if not isinstance(session, dict): return if self._recording_elapsed_ms(session) >= PIANO_RECORDING_MAX_MS: - await self._finalize_piano_recording(item_id, status_message="stop") + await self._finalize_piano_recording(item_id, notify_owner=True) return await asyncio.sleep(0.25) except asyncio.CancelledError: @@ -636,6 +643,34 @@ class SignalingServer: ), ) + async def _send_piano_status( + self, + client: ClientConnection, + *, + item_id: str, + event: Literal[ + "use_mode_entered", + "record_started", + "record_paused", + "record_resumed", + "record_stopped", + "playback_started", + "playback_stopped", + ], + recording_state: Literal["idle", "recording", "paused", "playback"] | None = None, + ) -> None: + """Send structured piano state transitions without relying on status-message text.""" + + await self._send( + client.websocket, + ItemPianoStatusPacket( + type="item_piano_status", + itemId=item_id, + event=event, + recordingState=recording_state, + ), + ) + async def _broadcast_item(self, item: WorldItem) -> None: """Broadcast a full item snapshot update to all connected clients.""" @@ -1100,6 +1135,13 @@ class SignalingServer: y=item.y, ) ) + if item.type == "piano": + await self._send_piano_status( + client, + item_id=item.id, + event="use_mode_entered", + recording_state="idle", + ) await self._send_item_result(client, True, "use", use_result.self_message, item.id) if use_result.delayed_self_message is not None and use_result.delayed_others_message is not None: asyncio.create_task( @@ -1156,7 +1198,7 @@ class SignalingServer: } ) if elapsed_ms >= PIANO_RECORDING_MAX_MS: - await self._finalize_piano_recording(item.id, status_message="stop") + await self._finalize_piano_recording(item.id, notify_owner=True) await self._broadcast_item_piano_note( item, sender_id=client.id, @@ -1188,12 +1230,14 @@ class SignalingServer: if existing.get("paused") is True: existing["paused"] = False existing["lastResumeMonotonic"] = time.monotonic() - await self._send_item_result(client, True, "use", "resume", item.id) + await self._send_piano_status(client, item_id=item.id, event="record_resumed", recording_state="recording") + await self._send_item_result(client, True, "use", "Recording resumed.", item.id) else: existing["elapsedMs"] = self._recording_elapsed_ms(existing) existing["paused"] = True existing.pop("lastResumeMonotonic", None) - await self._send_item_result(client, True, "use", "pause", item.id) + await self._send_piano_status(client, item_id=item.id, event="record_paused", recording_state="paused") + await self._send_item_result(client, True, "use", "Recording paused.", item.id) return self._cancel_piano_playback(item.id) recording_state = { @@ -1206,7 +1250,8 @@ class SignalingServer: self.piano_recording_state_by_item[item.id] = recording_state auto_stop_task = asyncio.create_task(self._auto_stop_piano_recording(item.id)) recording_state["autoStopTask"] = auto_stop_task - await self._send_item_result(client, True, "use", "record", item.id) + await self._send_piano_status(client, item_id=item.id, event="record_started", recording_state="recording") + await self._send_item_result(client, True, "use", "Recording started.", item.id) return if packet.action == "stop_record": @@ -1215,9 +1260,10 @@ class SignalingServer: await self._send_item_result(client, False, "use", "This piano is already recording.", item.id) return if existing and existing.get("ownerClientId") == client.id: - await self._finalize_piano_recording(item.id, status_message="stop") + await self._finalize_piano_recording(item.id, notify_owner=True) return - await self._send_item_result(client, True, "use", "stop", item.id) + await self._send_piano_status(client, item_id=item.id, event="record_stopped", recording_state="idle") + await self._send_item_result(client, True, "use", "Recording stopped.", item.id) return if packet.action == "playback": @@ -1232,12 +1278,14 @@ class SignalingServer: self._cancel_piano_playback(item.id) playback_task = asyncio.create_task(self._start_piano_playback(item)) self.piano_playback_tasks_by_item[item.id] = playback_task - await self._send_item_result(client, True, "use", "play", item.id) + await self._send_piano_status(client, item_id=item.id, event="playback_started", recording_state="playback") + await self._send_item_result(client, True, "use", "Playback started.", item.id) return if packet.action == "stop_playback": self._cancel_piano_playback(item.id) - await self._send_item_result(client, True, "use", "stop", item.id) + await self._send_piano_status(client, item_id=item.id, event="playback_stopped", recording_state="idle") + await self._send_item_result(client, True, "use", "Playback stopped.", item.id) return return diff --git a/server/tests/test_item_use_cooldown.py b/server/tests/test_item_use_cooldown.py index 87df17d..eb48ca0 100644 --- a/server/tests/test_item_use_cooldown.py +++ b/server/tests/test_item_use_cooldown.py @@ -591,6 +591,8 @@ async def test_piano_recording_toggle_and_save(monkeypatch: pytest.MonkeyPatch) client, json.dumps({"type": "item_piano_recording", "itemId": item.id, "action": "toggle_record"}), ) + assert send_payloads[-2].type == "item_piano_status" + assert send_payloads[-2].event == "record_started" assert send_payloads[-1].ok is True assert item.id in server.piano_recording_state_by_item @@ -606,16 +608,20 @@ async def test_piano_recording_toggle_and_save(monkeypatch: pytest.MonkeyPatch) client, json.dumps({"type": "item_piano_recording", "itemId": item.id, "action": "toggle_record"}), ) + assert send_payloads[-2].type == "item_piano_status" + assert send_payloads[-2].event == "record_paused" assert send_payloads[-1].ok is True - assert send_payloads[-1].message == "pause" + assert send_payloads[-1].message == "Recording paused." assert item.id in server.piano_recording_state_by_item await server._handle_message( client, json.dumps({"type": "item_piano_recording", "itemId": item.id, "action": "stop_record"}), ) + assert send_payloads[-2].type == "item_piano_status" + assert send_payloads[-2].event == "record_stopped" assert send_payloads[-1].ok is True - assert send_payloads[-1].message == "stop" + assert send_payloads[-1].message == "Recording stopped." assert item.id not in server.piano_recording_state_by_item song_id = item.params.get("songId") assert isinstance(song_id, str) @@ -665,6 +671,8 @@ async def test_piano_playback_starts_task(monkeypatch: pytest.MonkeyPatch) -> No client, json.dumps({"type": "item_piano_recording", "itemId": item.id, "action": "playback"}), ) + assert send_payloads[-2].type == "item_piano_status" + assert send_payloads[-2].event == "playback_started" assert send_payloads[-1].ok is True task = server.piano_playback_tasks_by_item.get(item.id) assert task is not None