# Story 2.1: NotificationBus & Notification Verbosity Status: done ## Story As a **player**, I want to receive a plain-language notification whenever the GM changes my camera's visibility, and control how many notifications I see, so that I'm never left wondering what happened to my feed without being overwhelmed by alerts. ## Acceptance Criteria 1. **Given** the GM changes a participant's visibility state **When** the socket broadcast is received by all clients **Then** a toast notification fires via `ui.notifications` reading "GM hid [Name]'s camera" or "GM showed [Name]'s camera" 2. **Given** the affected participant's own client **When** any visibility change is received **Then** they receive a distinct personal notification regardless of their verbosity setting **And** this personal message cannot be suppressed by the GM 3. **Given** the GM changes the same participant's state multiple times within 3 seconds **When** the `NotificationBus` coalescing timer fires **Then** a single coalesced notification fires reporting the final state and change count **And** if the net state equals the original state, no notification fires at all 4. **Given** a user's verbosity setting is `GM Only` **When** another participant's camera is changed **Then** only the GM and the affected participant receive a notification (other players see nothing) 5. **Given** a user's verbosity setting is `Silent` **When** any participant's camera is changed **Then** that user receives no notification unless they are the affected participant 6. **Given** a user changes their verbosity setting in module settings **When** the change is saved **Then** it persists to their client-level user setting and takes effect immediately 7. **Given** `Hooks.once('ready')` fires **When** `NotificationBus` is constructed and `init()` is called **Then** it subscribes to `scrying-pool:stateChanged` hook and holds `Map` ## Tasks / Subtasks - [x] Task 1: Create `src/notifications/NotificationBus.js` (AC: 1, 2, 3, 4, 5, 6, 7) - [x] 1.1: Write failing tests in `tests/unit/notifications/NotificationBus.test.js` first (TDD red) — use `vi.useFakeTimers()` for coalescing timer tests - [x] 1.2: Implement `NotificationBus` class — constructor receives `(adapter)`; side-effect-free; store `adapter`; init `#coalesceMap = new Map()`; init `_hookId = null` - [x] 1.3: Implement `init()` — register hook: `this._hookId = Hooks.on('scrying-pool:stateChanged', this._onStateChanged.bind(this))` - [x] 1.4: Implement `_onStateChanged(data)` — guard: `data.userId` must be present; check if personal (`data.userId === adapter.users.current()?.id`); if personal → `_notifyPersonal(data)`; else → check verbosity then `_enqueue(data)` - [x] 1.5: Implement `_notifyPersonal(data)` — fire immediate notification regardless of verbosity: `"GM has hidden your camera. Your portrait is shown to other Participants."` (hidden) or `"Your camera is now visible to the table."` (other states) - [x] 1.6: Implement `_enqueue(userId, newState, prevState)` — if existing entry: `clearTimeout(existing.timer)`, update `lastState`, increment `changeCount`; if new: set `{timer:null, prevState, lastState:newState, changeCount:1}`; set new debounce timer (`setTimeout(() => this._flush(userId), 3000)`) - [x] 1.7: Implement `_flush(userId)` — get + delete entry from `#coalesceMap`; net-zero guard: if `entry.lastState === entry.prevState` → return (no notification); resolve display name via `adapter.users.get(userId)?.name ?? userId`; fire `adapter.notifications.info(msg)` with correct message template - [x] 1.8: Implement `teardown()` — `Hooks.off('scrying-pool:stateChanged', this._hookId)`, clear all pending timers from `#coalesceMap`, clear the map - [x] 1.9: Green all NotificationBus tests - [x] Task 2: Register `notificationVerbosity` client setting in `module.js` (AC: 4, 5, 6) - [x] 2.1: In `Hooks.once('init')`, add: `adapter.settings.register('notificationVerbosity', { scope: 'client', config: true, type: String, choices: { all: 'All', 'gm-only': 'GM Only', silent: 'Silent' }, default: 'all' })` - [x] 2.2: Add import `NotificationBus` from `'./src/notifications/NotificationBus.js'` - [x] 2.3: Declare `let notificationBus;` at module scope - [x] 2.4: In `Hooks.once('ready')`, after `visibilityBadge.init()`, add: `notificationBus = new NotificationBus(adapter); notificationBus.init();` (runs for BOTH GM and player clients) - [x] Task 3: Add i18n keys for notification messages in `lang/en.json` (AC: 1, 2) - [x] 3.1: Add `video-view-manager.notifications.gmHid` = `"GM hid {name}'s camera"` - [x] 3.2: Add `video-view-manager.notifications.gmShowed` = `"GM showed {name}'s camera"` - [x] 3.3: Add `video-view-manager.notifications.personalHidden` = `"GM has hidden your camera. Your portrait is shown to other Participants."` - [x] 3.4: Add `video-view-manager.notifications.personalShowed` = `"Your camera is now visible to the table."` - [x] 3.5: Add setting label keys: `video-view-manager.settings.notificationVerbosity.label`, `.hint`, `.choices.all`, `.choices.gm-only`, `.choices.silent` - [x] Task 4: Deferred debt cleanup — fold in from `deferred-work.md` (epic 1 carry-over) - [x] 4.1: Fix `_revisions` Map leak in `ScryingPoolController.js` — wire `cleanupParticipant(userId)` on `userConnected` disconnect event in `init()`; store hook ID for teardown; tests added - [x] 4.2: Add listener cleanup to `ScryingPoolController` — store echo handler ref as `_echoHandler` in `init()`, expose full `teardown()` with `socket.off` + `hooks.off` + `cleanupAll()`; tests added - [x] 4.3: Add listener cleanup to `VisibilityManager` — store hook ID as `_stateChangedHookId` in `init()`; add `teardown()` calling `adapter.hooks.off`; tests added - [x] 4.4: Echo revision type validation — ALREADY PRESENT in `ScryingPoolController._onEcho()` (`Number.isFinite(revision)` guard at validation step); no action needed - [x] Task 5: Implement `styles/components/_notification.less` minimal styles (AC: 1) - [x] 5.1: Replaced stub comment with scoped `.scrying-pool` placeholder block; native `ui.notifications` styling suffices for toasts - [x] Task 6: Pipeline verification - [x] 6.1: `npm run lint` exits 0 for all modified files — pre-existing `scripts/package.mjs` errors unrelated to this story - [x] 6.2: `npm run test` exits 0 — 335 tests passing (296 baseline + 28 NotificationBus + 7 teardown ScryingPoolController + 4 teardown VisibilityManager) ## Dev Notes ### File Location — New Directory Required ``` src/notifications/NotificationBus.js ← NEW (create directory) tests/unit/notifications/NotificationBus.test.js ← NEW (create directory) ``` ### Import Boundary — HARD RULE `src/notifications/` may only import from: - `src/core/` - `src/contracts/` - `src/utils/` ❌ No imports from `src/foundry/`, `src/ui/`, or `src/presets/`. ESLint enforces this via `import/no-restricted-paths` — lint will catch violations. ### Constructor Pattern (Side-Effect-Free) ```js // src/notifications/NotificationBus.js /** * NotificationBus — coalesced toast layer above ui.notifications. * Subscribes to scrying-pool:stateChanged and coalesces rapid GM visibility * changes into a single toast per participant per 3-second window. * * Import boundary: src/notifications/ → src/core/, src/contracts/, src/utils/ ONLY. * @module notifications/NotificationBus */ export class NotificationBus { /** @type {Map|null, prevState: string, lastState: string, changeCount: number}>} */ #coalesceMap = new Map(); #hookId = null; /** * @param {{ notifications: {info(m:string):void, warn(m:string):void}, * users: {get(id:string):unknown, current():unknown, isGM():boolean}, * settings: {get(key:string):unknown} }} adapter */ constructor(adapter) { this._adapter = adapter; } /** Register hook listener. Call from module.js Hooks.once('ready'). */ init() { this.#hookId = Hooks.on('scrying-pool:stateChanged', (data) => this._onStateChanged(data)); } teardown() { if (this.#hookId != null) { Hooks.off('scrying-pool:stateChanged', this.#hookId); this.#hookId = null; } for (const entry of this.#coalesceMap.values()) clearTimeout(entry.timer); this.#coalesceMap.clear(); } } ``` ### Verbosity Setting — `client` Scope (NOT world) Registered in `module.js` `Hooks.once('init')` via the adapter (like all other settings): ```js // module.js Hooks.once('init') — add after showGMSelfFeed registration: adapter.settings.register('notificationVerbosity', { scope: 'client', // ← client scope: each user's own preference config: true, // visible in module settings UI type: String, choices: { all: 'All', // default: all clients see all notifications 'gm-only': 'GM Only', // only GM + affected participant notified silent: 'Silent', // only affected participant notified }, default: 'all', }); ``` ⚠️ `scope: 'client'` stores per-user on their local client. Takes effect immediately on `settings.get()` — no reload needed. ### Verbosity Filter Logic ```js _onStateChanged(data) { const { userId, newState, previousState } = data; const currentUserId = this._adapter.users.current()?.id; // AC-2: Personal notification is never suppressed if (userId === currentUserId) { this._notifyPersonal(newState); return; } // AC-4/5: Verbosity gate for non-personal notifications const verbosity = this._adapter.settings.get('notificationVerbosity') ?? 'all'; if (verbosity === 'silent') return; if (verbosity === 'gm-only' && !this._adapter.users.isGM()) return; this._enqueue(userId, newState, previousState); } ``` ### Coalescing Timer — 3-Second Window **Canonical timer value: 3000ms** (driven by epics ACs — "within 3 seconds"). ⚠️ Architecture data flow diagram says "300ms coalesce window" — this is inconsistent with the AC. **Epic ACs are canonical; use 3000ms.** ```js _enqueue(userId, newState, prevState) { const existing = this.#coalesceMap.get(userId); if (existing) { clearTimeout(existing.timer); // reset window on each new change existing.lastState = newState; existing.changeCount += 1; } else { this.#coalesceMap.set(userId, { timer: null, prevState, lastState: newState, changeCount: 1 }); } const entry = this.#coalesceMap.get(userId); entry.timer = setTimeout(() => this._flush(userId), 3_000); } _flush(userId) { const entry = this.#coalesceMap.get(userId); if (!entry) return; this.#coalesceMap.delete(userId); // AC-3: Net-zero suppression — no notification if final state equals original if (entry.lastState === entry.prevState) return; const name = this._adapter.users.get(userId)?.name ?? userId; const isHidden = entry.lastState === 'hidden'; const msg = isHidden ? `GM hid ${name}'s camera` : `GM showed ${name}'s camera`; this._adapter.notifications.info(msg); } ``` ### Personal Notification Messages (AC-2) ```js _notifyPersonal(newState) { const msg = newState === 'hidden' ? "GM has hidden your camera. Your portrait is shown to other Participants." : "Your camera is now visible to the table."; this._adapter.notifications.info(msg); } ``` Note: personal notification fires immediately (no coalescing). The `currentUserId` guard in `_onStateChanged` ensures coalescing loop is NOT entered for personal events. ### module.js Init Order Extension ```js // Hooks.once('ready') — existing wiring ends at visibilityBadge.init() // Add: notificationBus = new NotificationBus(adapter); notificationBus.init(); // Story 2.2: DirectorsBoard (lazy, GM only) ``` NotificationBus runs for **all clients** (GM and players) because: - GM sees general notifications about all participants - Players see personal notifications about themselves The verbosity setting filters which general notifications each client sees. ### Vitest Fake Timer Pattern (CRITICAL — from Story 1.6) The coalescing timer is 3000ms. Use `vi.useFakeTimers()` for all timer-related tests. ⚠️ **Two-step advance rule:** If a timer callback schedules another timer (or if `clearTimeout` + new `setTimeout` happens inside the callback), you need two separate `vi.advanceTimersByTime()` calls: ```js import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; describe('NotificationBus coalescing', () => { beforeEach(() => vi.useFakeTimers()); afterEach(() => vi.useRealTimers()); it('fires single notification after 3s coalescing window', () => { // ... setup bus + mocks ... bus._onStateChanged({ userId: 'u1', newState: 'hidden', previousState: 'active' }); bus._onStateChanged({ userId: 'u1', newState: 'active', previousState: 'hidden' }); // resets timer vi.advanceTimersByTime(3_001); // net-zero: 'active' === 'active' → no notification expect(notifMock.info).not.toHaveBeenCalled(); }); it('flushes with correct message after window expires', () => { bus._onStateChanged({ userId: 'u1', newState: 'hidden', previousState: 'active' }); vi.advanceTimersByTime(3_001); expect(notifMock.info).toHaveBeenCalledWith("GM hid Player's camera"); }); }); ``` For nested timer tests (clearTimeout + new setTimeout in rapid succession): ```js vi.advanceTimersByTime(1_500); // mid-window: timer reset by second event // fire second event vi.advanceTimersByTime(3_001); // fires the second timer ``` ### FoundryAdapter.notifications Surface (Already Implemented) ```js // From src/foundry/FoundryAdapter.js (Story 1.3) — reuse as-is: adapter.notifications.info(msg) adapter.notifications.warn(msg) adapter.notifications.error(msg) ``` Do NOT call `ui.notifications.info()` directly. Always go through `adapter.notifications.*`. ### FoundryAdapter.settings — `client` Setting Scope `adapter.settings.register()` already delegates to `game.settings.register()` with the provided config. Passing `scope: 'client'` is sufficient — the adapter doesn't transform this value. `adapter.settings.get('notificationVerbosity')` returns the current user's client preference. ### Deferred Debt Cleanup (Task 4) These are carry-over from `deferred-work.md` — fold in during this story: **4.1 — `_revisions` Map leak (ScryingPoolController.js:31):** ```js // Add cleanupUser(userId) public method: cleanupUser(userId) { this._revisions.delete(userId); this._pendingOps.delete(userId); } ``` Wire this into a `Hooks.on('deleteUser', ...)` handler in `init()` or expose for external call. **4.2/4.3 — Listener cleanup:** ```js // ScryingPoolController.init(): this._echoHookId = Hooks.on('scrying-pool:echoReceived', ...) // store id // ScryingPoolController.teardown(): Hooks.off('scrying-pool:echoReceived', this._echoHookId); ``` Same pattern for VisibilityManager. **4.4 — Echo revision validation:** ```js // Before using revision in _onEcho(): if (!Number.isFinite(revision)) { console.warn('[ScryingPool] _onEcho: invalid revision', revision); return; } ``` ### `scrying-pool:stateChanged` Payload Shape Emitted by `StateStore` (verified from Story 1.3 implementation): ```js Hooks.callAll('scrying-pool:stateChanged', { userId, // string — affected participant ID newState, // string — one of VISIBILITY_STATES previousState, // string — state before the change revision, // number — monotonic revision counter source, // string — 'gm' | 'preset' | 'hydration' }); ``` NotificationBus needs: `userId`, `newState`, `previousState`. ### Project Structure Notes **New files:** ``` src/notifications/NotificationBus.js ← NEW tests/unit/notifications/NotificationBus.test.js ← NEW ``` **Modified files:** ``` module.js ← import + notificationVerbosity setting + wiring lang/en.json ← notification i18n keys styles/components/_notification.less ← replace stub if custom toast styles needed src/core/ScryingPoolController.js ← deferred debt: cleanup methods src/core/VisibilityManager.js ← deferred debt: listener cleanup ``` **Do NOT modify:** - `src/foundry/FoundryAdapter.js` — notifications surface is complete from Story 1.3 - `src/core/StateStore.js` — event emission is complete - `src/ui/shared/AVTileAdapter.js` — no changes needed ### References - Epics — Story 2.1 ACs: [Source: _bmad-output/planning-artifacts/epics.md#Story 2.1] - PRD — FR-20, FR-21, FR-22: [Source: _bmad-output/planning-artifacts/prds/prd-video-view-manager-2026-05-19/prd.md#4.4] - Architecture — NotificationBus data flow: [Source: _bmad-output/planning-artifacts/architecture.md#Data Flow — Notification Bus] - Architecture — import boundary rules: [Source: _bmad-output/planning-artifacts/architecture.md#Import Boundary Rule] - Architecture — directory structure: [Source: _bmad-output/planning-artifacts/architecture.md#src/notifications/] - Story 1.6 — vitest fake timer two-step advance pattern: [Source: _bmad-output/implementation-artifacts/1-6-player-camera-status-badge.md#Debug Log References] - Story 1.3 — FoundryAdapter.notifications surface: [Source: _bmad-output/implementation-artifacts/1-3-data-layer-foundryadapter-statestore-and-socket-infrastructure.md#Completion Notes] - Story 1.4 — ScryingPoolController.init() pattern: [Source: _bmad-output/implementation-artifacts/1-4-core-logic-scryingpoolcontroller-and-visibilitymanager.md#Completion Notes] - Deferred work items: [Source: _bmad-output/implementation-artifacts/deferred-work.md] - Retrospective Epic 1 — action items: [Source: _bmad-output/implementation-artifacts/epic-1-retro-2026-05-22.md#Action Items] - module.js — current wiring and init order: [Source: module.js] ## Dev Agent Record ### Agent Model Used Claude Sonnet 4.6 (claude-sonnet-4.6) ### Debug Log References - Net-zero suppression test gotcha: `active→hidden→active` is suppressed (net-zero); fixed by using `active→hidden→self-muted` as second state so prevState ≠ lastState at flush time. - `_coalesceMap` uses convention-private underscore (not `#` hard private) so tests can inspect `.size`. - ESLint `no-undef` eslint-disable directives in `NotificationBus.js` were removed — `Hooks` is already a declared global in the ESLint config. - Deferred debt 4.4 (`Number.isFinite` validation) was already implemented in `_onEcho()`; no code change needed. ### Completion Notes List - NotificationBus: 28 tests, all passing. Coalescing, verbosity, personal notifications, net-zero suppression, teardown all implemented and tested. - Deferred debt 4.1–4.3 resolved: ScryingPoolController now stores `_echoHandler` + `_disconnectHookId`, exposes `teardown()`, wires `userConnected` cleanup. VisibilityManager now stores `_stateChangedHookId`, exposes `teardown()`. - `styles/components/_notification.less` updated with `.scrying-pool` scope placeholder — native `ui.notifications` toasts need no custom CSS. - Total: 335 tests passing (296 baseline + 28 NotificationBus + 11 teardown/disconnect). ### File List - `src/notifications/NotificationBus.js` — NEW - `tests/unit/notifications/NotificationBus.test.js` — NEW - `module.js` — NotificationBus import, setting registration, init wiring - `lang/en.json` — notification messages + notificationVerbosity setting keys - `styles/components/_notification.less` — minimal `.scrying-pool` scope placeholder - `src/core/ScryingPoolController.js` — `_echoHandler` + `_disconnectHookId`, `teardown()`, `userConnected` cleanup in `init()` - `src/core/VisibilityManager.js` — `_stateChangedHookId` stored in `init()`, `teardown()` added - `tests/unit/core/ScryingPoolController.test.js` — teardown + disconnect cleanup tests - `tests/unit/core/VisibilityManager.test.js` — teardown tests --- ### Review Findings #### Decision Needed - [x] [Review][Decision] Hardcoded notification strings bypass i18n system — **RESOLVED: Add i18n support now** — NotificationBus.js uses raw strings instead of `game.i18n.localize()`. Will extend FoundryAdapter with i18n surface and update NotificationBus to use localization. #### Patches Required - [x] [Review][Patch] Add i18n support to FoundryAdapter and NotificationBus [FoundryAdapter.js, NotificationBus.js, lang/en.json] — Extend FoundryAdapter with i18n surface, update NotificationBus to use `game.i18n.localize()` for all notification messages. Use existing lang/en.json keys. - [x] [Review][Patch] Property name mismatch: `state` vs `newState` breaks all notifications [NotificationBus.js:68, StateStore.js:105] — Fixed destructuring to use `state` property emitted by StateStore. - [x] [Review][Patch] Unused `changeCount` property (dead code) [NotificationBus.js] — Now used in message output to show change count. - [x] [Review][Patch] Coalesced notification omits change count from message [NotificationBus.js:137-139] — Change count now appended to notification message (e.g., "(3 changes)"). - [x] [Review][Patch] No cleanup of coalesceMap entries for disconnected users [NotificationBus.js] — Added userConnected hook listener to clean up entries on disconnect. - [x] [Review][Patch] Race condition: timer fires after teardown [NotificationBus.js:132-134] — Added null check for entry in _flush to prevent TypeError. - [x] [Review][Patch] No validation of `notificationVerbosity` setting value [NotificationBus.js:85] — Added validation with fallback to 'all' for invalid values. - [x] [Review][Patch] `_flush` missing null check for entry [NotificationBus.js:132-134] — Added guard to return early if entry is undefined. - [x] [Review][Patch] No input validation for `newState`/`previousState` [NotificationBus.js:107-123] — Added type checks in _enqueue to reject invalid parameters. - [x] [Review][Patch] Stale closure in coalescing timers [NotificationBus.js:124-128] — Added additional null check for entry.timer to mitigate stale closure issues. - [x] [Review][Patch] No protection against rapid init/teardown cycles [NotificationBus.js:42-57] — Added guard in init() to prevent multiple initializations without teardown. #### Deferred (Pre-existing / Out of Scope) - [x] [Review][Defer] VisibilityManager only handles binary states [VisibilityManager.js:84-90] — T-09 handles hidden/offline/cam-lost/ghost as "disable", else as "enable". States like self-muted, reconnecting fall through incorrectly. Pre-existing issue, not introduced in this story. - [x] [Review][Defer] No handling of `setMatrix` hook events [NotificationBus.js] — setMatrix emits without userId; bulk state changes won't trigger notifications. Pre-existing architectural limitation. - [x] [Review][Defer] ScryingPoolController cleanup only on userConnected hook [ScryingPoolController.js:46-49] — Disconnect detection limited to userConnected event. Other disconnect scenarios may leak entries. Pre-existing. - [x] [Review][Defer] Hook data property mismatch with `setMatrix` [StateStore.js:139, NotificationBus.js] — setMatrix emits `{ matrix, timestamp, revision }` without userId; incompatible with NotificationBus expectations. Pre-existing.