diff --git a/apps/notify/src/notifications-hub.ts b/apps/notify/src/notifications-hub.ts index 6059e72..17169c7 100644 --- a/apps/notify/src/notifications-hub.ts +++ b/apps/notify/src/notifications-hub.ts @@ -448,6 +448,28 @@ export class NotificationsHub extends DurableObject { return { sent: this.broadcastToConnected(payload) } } + /** + * The targeted form of {@link coachMessageAll}: the same `MessageReceived` frame from + * the Coach account (player 1), addressed to one player with a `ToPlayerId` — the shape + * `api`'s player-to-player send uses. + * + * Unlike the broadcast this one QUEUES when the recipient is offline (see + * {@link notifyPlayer}). The broadcast is online-only because it has no recipient to + * hold anything for; a message written to a named player is worth keeping until they + * next connect. + */ + async coachMessage( + playerId: number, + content: string + ): Promise<{ delivered: number; queued: boolean }> { + return this.notifyPlayer(playerId, NotificationType.MessageReceived, { + FromPlayerId: COACH_PLAYER_ID, + ToPlayerId: playerId, + Type: COACH_MESSAGE_TYPE, + Data: content, + }) + } + /** Broadcast a notification to every connected (handshaken) client. */ async broadcast( notificationType: string | number, diff --git a/apps/notify/src/notify.app.ts b/apps/notify/src/notify.app.ts index 6f43aa9..c5ab65d 100644 --- a/apps/notify/src/notify.app.ts +++ b/apps/notify/src/notify.app.ts @@ -176,6 +176,25 @@ const app = new Hono() return c.json({ success: true, ...result }) }) + // The targeted form of the broadcast above: one coach message to ONE player. Queued + // by the hub when they're offline (unlike coach-message-all, which reaches only + // whoever is connected), so this arrives either way. + .post('/internal/coach-message', async (c) => { + const body = await c.req + .json<{ playerId?: number; messageContent?: string }>() + .catch(() => null) + const content = typeof body?.messageContent === 'string' ? body.messageContent.trim() : '' + if (typeof body?.playerId !== 'number' || !Number.isInteger(body.playerId)) { + return c.json({ error: 'playerId is required' }, 400) + } + if (content === '') return c.json({ error: 'messageContent is required' }, 400) + const result = await c.env.RECFLARE_NOTIFICATIONS_HUB.getByName(HUB_INSTANCE).coachMessage( + body.playerId, + content + ) + return c.json({ success: true, ...result }) + }) + // Read-only view of the hub's routing state, for working out why a notification // didn't arrive: which connections are live, which players each one receives for, // and what's queued for a player who wasn't reachable. diff --git a/apps/notify/src/test/integration/api.test.ts b/apps/notify/src/test/integration/api.test.ts index 8410ec6..7688db4 100644 --- a/apps/notify/src/test/integration/api.test.ts +++ b/apps/notify/src/test/integration/api.test.ts @@ -65,7 +65,12 @@ interface HubRecord { async function connect( id: string, opts: { headers?: Record; query?: string } = {} -): Promise<{ ws: WebSocket; waitFor: (pred: (r: HubRecord) => boolean) => Promise }> { +): Promise<{ + ws: WebSocket + waitFor: (pred: (r: HubRecord) => boolean) => Promise + /** Everything this socket has received so far — for asserting something did NOT arrive. */ + records: HubRecord[] +}> { // The hub only accepts identified connections, so default to a token; pass explicit // `headers` (`{}` for none) where the test is about who is connecting. const auth = opts.headers ?? (await bearer(String(DEFAULT_CONNECT_PLAYER), ['gameClient'])) @@ -113,7 +118,7 @@ async function connect( ws.send(`{"protocol":"json","version":1}${RS}`) await waitFor((r) => r.type === 1 && r.target === 'OnConnect') - return { ws, waitFor } + return { ws, waitFor, records } } const send = (ws: WebSocket, msg: HubRecord) => ws.send(JSON.stringify(msg) + RS) @@ -337,6 +342,72 @@ describe('notification delivery', () => { b.ws.close() }) + test('coach-message reaches only the named player, addressed to them', async () => { + const playerId = 9010 + const target = await connect('coach-one', { + headers: await bearer(String(playerId), ['gameClient']), + }) + const bystander = await connect('coach-one-bystander') + send(target.ws, { + type: 1, + invocationId: 's', + target: 'SubscribeToPlayers', + arguments: [{ playerIds: [playerId] }], + }) + await target.waitFor((r) => r.type === 3 && r.invocationId === 's') + + const res = await post('/internal/coach-message', { playerId, messageContent: 'just you' }) + expect(res.status).toBe(200) + expect(await res.json()).toMatchObject({ delivered: 1, queued: false }) + + const note = await target.waitFor((r) => r.type === 1 && r.target === 'Notification') + const payload = JSON.parse((note.arguments as string[])[0]) as { + Id: string + Msg: Record + } + expect(payload.Id).toBe('2') // MessageReceived + // Same Coach frame as the broadcast, plus the recipient the broadcast can't name. + expect(payload.Msg).toMatchObject({ + FromPlayerId: 1, + ToPlayerId: playerId, + Type: 100, + Data: 'just you', + }) + + // The point of the targeted send: nobody else's socket sees it. The bystander is + // subscribed to nothing, and a broadcast would have reached it regardless. + expect(bystander.records.some((r) => r.target === 'Notification')).toBe(false) + + target.ws.close() + bystander.ws.close() + }) + + test('coach-message queues for a player who is offline', async () => { + const playerId = 9011 + const res = await post('/internal/coach-message', { + playerId, + messageContent: 'catch you later', + }) + expect(await res.json()).toMatchObject({ delivered: 0, queued: true }) + + // Unlike the broadcast, it survives until they connect. + const { ws, waitFor } = await connect('coach-one-late') + send(ws, { type: 1, target: 'SubscribeToPlayers', arguments: [{ playerIds: [playerId] }] }) + const note = await waitFor((r) => r.type === 1 && r.target === 'Notification') + expect( + (JSON.parse((note.arguments as string[])[0]) as { Msg: Record }).Msg + ).toMatchObject({ FromPlayerId: 1, ToPlayerId: playerId, Data: 'catch you later' }) + ws.close() + }) + + test('coach-message 400s without a player or a message', async () => { + expect((await post('/internal/coach-message', { messageContent: 'nobody' })).status).toBe(400) + expect((await post('/internal/coach-message', { playerId: 9012 })).status).toBe(400) + expect( + (await post('/internal/coach-message', { playerId: 9012, messageContent: ' ' })).status + ).toBe(400) + }) + test('coach-message-all 400s on an empty message', async () => { const res = await post('/internal/coach-message-all', { messageContent: ' ' }) expect(res.status).toBe(400) diff --git a/apps/www/src/client/App.tsx b/apps/www/src/client/App.tsx index 2755478..0b57a51 100644 --- a/apps/www/src/client/App.tsx +++ b/apps/www/src/client/App.tsx @@ -572,6 +572,42 @@ const coachMessageAll = (messageContent: string): Promise<{ sent?: number }> => authed: true, }) +/** + * The same coach message to ONE player. `notify` queues it when they're offline, so + * `queued` (rather than a 0 delivery) is what "they weren't online" looks like here — + * it still arrives on their next connect, unlike the broadcast. + */ +const coachMessage = ( + playerId: number, + messageContent: string +): Promise<{ delivered?: number; queued?: boolean }> => + call<{ delivered?: number; queued?: boolean }>(`${where().notify}/internal/coach-message`, { + json: { playerId, messageContent }, + authed: true, + }) + +/** + * Resolve an `@username` to the account id the workers address a player by. + * + * `accounts` serves no exact-name lookup, so this goes through the PREFIX search and + * keeps only an exact (case-insensitive) hit: a prefix match is a different player, and + * sending a message to whoever happened to sort first would be worse than refusing. The + * exact name always sorts first among its own prefixes, so it's inside the search limit + * whenever it exists. + */ +async function accountIdForUsername(input: string): Promise { + const name = input.trim().replace(/^@/, '') + if (name === '') throw new Error('Enter a username to send to.') + const matches = await call>( + `${where().accounts}/account/search?name=${encodeURIComponent(name)}` + ) + const found = Array.isArray(matches) + ? matches.find((m) => m.username?.toLowerCase() === name.toLowerCase()) + : undefined + if (typeof found?.accountId !== 'number') throw new Error(`There's no player called @${name}.`) + return found.accountId +} + /** Minimal history-based router: current pathname + a navigate() that pushes state. */ function useRouter() { const [path, setPath] = useState(() => window.location.pathname) @@ -2092,7 +2128,7 @@ function Dashboard({ ...(isAdmin() ? [ { id: 'maintenance', label: 'Server maintenance', render: () => }, - { id: 'coach', label: 'Broadcast message', render: () => }, + { id: 'coach', label: 'Coach message', render: () => }, ] : []), ] @@ -2227,28 +2263,61 @@ function RoomCard({ ) } -/** Admin-only: send a coach/system message to every online player. */ +/** + * Admin-only: send a coach/system message, either to one player by `@username` or to + * everyone online. + * + * The two go to different endpoints because they behave differently, not just in reach: + * the broadcast is online-only (nothing holds a message with no addressee), while a named + * recipient's message is queued by the hub and delivered whenever they next connect. The + * recipient box therefore says which of those the operator is about to do. + */ function CoachMessageForm() { + const [recipient, setRecipient] = useState('') const [message, setMessage] = useState('') const { pending, error, done, run } = useAction() + // The `@` is how the name is written, not part of it — accepted either way, shown back + // with it, and sent without it. + const handle = recipient.trim().replace(/^@/, '') + const toOne = handle !== '' return (
-

Broadcast message

+

Coach message

- Send a message from the Coach to every connected player. Players who aren't online - won't receive it. + Send a message from the Coach to one player, or leave the recipient blank to send it to + every connected player. A broadcast reaches only who is online right now; a message to one + player waits for them if they aren't.

{ e.preventDefault() void run(async () => { - const { sent } = await coachMessageAll(message.trim()) + const content = message.trim() + if (!toOne) { + const { sent } = await coachMessageAll(content) + setMessage('') + return `Sent to ${sent ?? 0} online player${sent === 1 ? '' : 's'}.` + } + // Resolved before sending: the workers address players by id, and a name that + // matches nobody should be a refusal rather than a message into the void. + const { queued } = await coachMessage(await accountIdForUsername(handle), content) setMessage('') - return `Sent to ${sent ?? 0} online player${sent === 1 ? '' : 's'}.` + return queued === true + ? `@${handle} is offline — it will arrive when they next connect.` + : `Sent to @${handle}.` }) }} > +