23 KiB
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
-
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.notificationsreading "GM hid [Name]'s camera" or "GM showed [Name]'s camera" -
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
-
Given the GM changes the same participant's state multiple times within 3 seconds When the
NotificationBuscoalescing 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 -
Given a user's verbosity setting is
GM OnlyWhen another participant's camera is changed Then only the GM and the affected participant receive a notification (other players see nothing) -
Given a user's verbosity setting is
SilentWhen any participant's camera is changed Then that user receives no notification unless they are the affected participant -
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
-
Given
Hooks.once('ready')fires WhenNotificationBusis constructed andinit()is called Then it subscribes toscrying-pool:stateChangedhook and holdsMap<participantId, {timer, prevState, lastState, changeCount}>
Tasks / Subtasks
-
Task 1: Create
src/notifications/NotificationBus.js(AC: 1, 2, 3, 4, 5, 6, 7)- 1.1: Write failing tests in
tests/unit/notifications/NotificationBus.test.jsfirst (TDD red) — usevi.useFakeTimers()for coalescing timer tests - 1.2: Implement
NotificationBusclass — constructor receives(adapter); side-effect-free; storeadapter; init#coalesceMap = new Map(); init_hookId = null - 1.3: Implement
init()— register hook:this._hookId = Hooks.on('scrying-pool:stateChanged', this._onStateChanged.bind(this)) - 1.4: Implement
_onStateChanged(data)— guard:data.userIdmust be present; check if personal (data.userId === adapter.users.current()?.id); if personal →_notifyPersonal(data); else → check verbosity then_enqueue(data) - 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) - 1.6: Implement
_enqueue(userId, newState, prevState)— if existing entry:clearTimeout(existing.timer), updatelastState, incrementchangeCount; if new: set{timer:null, prevState, lastState:newState, changeCount:1}; set new debounce timer (setTimeout(() => this._flush(userId), 3000)) - 1.7: Implement
_flush(userId)— get + delete entry from#coalesceMap; net-zero guard: ifentry.lastState === entry.prevState→ return (no notification); resolve display name viaadapter.users.get(userId)?.name ?? userId; fireadapter.notifications.info(msg)with correct message template - 1.8: Implement
teardown()—Hooks.off('scrying-pool:stateChanged', this._hookId), clear all pending timers from#coalesceMap, clear the map - 1.9: Green all NotificationBus tests
- 1.1: Write failing tests in
-
Task 2: Register
notificationVerbosityclient setting inmodule.js(AC: 4, 5, 6)- 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' }) - 2.2: Add import
NotificationBusfrom'./src/notifications/NotificationBus.js' - 2.3: Declare
let notificationBus;at module scope - 2.4: In
Hooks.once('ready'), aftervisibilityBadge.init(), add:notificationBus = new NotificationBus(adapter); notificationBus.init();(runs for BOTH GM and player clients)
- 2.1: In
-
Task 3: Add i18n keys for notification messages in
lang/en.json(AC: 1, 2)- 3.1: Add
video-view-manager.notifications.gmHid="GM hid {name}'s camera" - 3.2: Add
video-view-manager.notifications.gmShowed="GM showed {name}'s camera" - 3.3: Add
video-view-manager.notifications.personalHidden="GM has hidden your camera. Your portrait is shown to other Participants." - 3.4: Add
video-view-manager.notifications.personalShowed="Your camera is now visible to the table." - 3.5: Add setting label keys:
video-view-manager.settings.notificationVerbosity.label,.hint,.choices.all,.choices.gm-only,.choices.silent
- 3.1: Add
-
Task 4: Deferred debt cleanup — fold in from
deferred-work.md(epic 1 carry-over)- 4.1: Fix
_revisionsMap leak inScryingPoolController.js— wirecleanupParticipant(userId)onuserConnecteddisconnect event ininit(); store hook ID for teardown; tests added - 4.2: Add listener cleanup to
ScryingPoolController— store echo handler ref as_echoHandlerininit(), expose fullteardown()withsocket.off+hooks.off+cleanupAll(); tests added - 4.3: Add listener cleanup to
VisibilityManager— store hook ID as_stateChangedHookIdininit(); addteardown()callingadapter.hooks.off; tests added - 4.4: Echo revision type validation — ALREADY PRESENT in
ScryingPoolController._onEcho()(Number.isFinite(revision)guard at validation step); no action needed
- 4.1: Fix
-
Task 5: Implement
styles/components/_notification.lessminimal styles (AC: 1)- 5.1: Replaced stub comment with scoped
.scrying-poolplaceholder block; nativeui.notificationsstyling suffices for toasts
- 5.1: Replaced stub comment with scoped
-
Task 6: Pipeline verification
- 6.1:
npm run lintexits 0 for all modified files — pre-existingscripts/package.mjserrors unrelated to this story - 6.2:
npm run testexits 0 — 335 tests passing (296 baseline + 28 NotificationBus + 7 teardown ScryingPoolController + 4 teardown VisibilityManager)
- 6.1:
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)
// 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<string, {timer: ReturnType<typeof setTimeout>|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):
// 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
_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.
_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)
_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
// 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:
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):
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)
// 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):
// 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:
// 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:
// 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):
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.3src/core/StateStore.js— event emission is completesrc/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→activeis suppressed (net-zero); fixed by usingactive→hidden→self-mutedas second state so prevState ≠ lastState at flush time. _coalesceMapuses convention-private underscore (not#hard private) so tests can inspect.size.- ESLint
no-undefeslint-disable directives inNotificationBus.jswere removed —Hooksis already a declared global in the ESLint config. - Deferred debt 4.4 (
Number.isFinitevalidation) 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, exposesteardown(), wiresuserConnectedcleanup. VisibilityManager now stores_stateChangedHookId, exposesteardown(). styles/components/_notification.lessupdated with.scrying-poolscope placeholder — nativeui.notificationstoasts need no custom CSS.- Total: 335 tests passing (296 baseline + 28 NotificationBus + 11 teardown/disconnect).
File List
src/notifications/NotificationBus.js— NEWtests/unit/notifications/NotificationBus.test.js— NEWmodule.js— NotificationBus import, setting registration, init wiringlang/en.json— notification messages + notificationVerbosity setting keysstyles/components/_notification.less— minimal.scrying-poolscope placeholdersrc/core/ScryingPoolController.js—_echoHandler+_disconnectHookId,teardown(),userConnectedcleanup ininit()src/core/VisibilityManager.js—_stateChangedHookIdstored ininit(),teardown()addedtests/unit/core/ScryingPoolController.test.js— teardown + disconnect cleanup teststests/unit/core/VisibilityManager.test.js— teardown tests
Review Findings
Decision Needed
- [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
- [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. - [Review][Patch] Property name mismatch:
statevsnewStatebreaks all notifications [NotificationBus.js:68, StateStore.js:105] — Fixed destructuring to usestateproperty emitted by StateStore. - [Review][Patch] Unused
changeCountproperty (dead code) [NotificationBus.js] — Now used in message output to show change count. - [Review][Patch] Coalesced notification omits change count from message [NotificationBus.js:137-139] — Change count now appended to notification message (e.g., "(3 changes)").
- [Review][Patch] No cleanup of coalesceMap entries for disconnected users [NotificationBus.js] — Added userConnected hook listener to clean up entries on disconnect.
- [Review][Patch] Race condition: timer fires after teardown [NotificationBus.js:132-134] — Added null check for entry in _flush to prevent TypeError.
- [Review][Patch] No validation of
notificationVerbositysetting value [NotificationBus.js:85] — Added validation with fallback to 'all' for invalid values. - [Review][Patch]
_flushmissing null check for entry [NotificationBus.js:132-134] — Added guard to return early if entry is undefined. - [Review][Patch] No input validation for
newState/previousState[NotificationBus.js:107-123] — Added type checks in _enqueue to reject invalid parameters. - [Review][Patch] Stale closure in coalescing timers [NotificationBus.js:124-128] — Added additional null check for entry.timer to mitigate stale closure issues.
- [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)
- [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.
- [Review][Defer] No handling of
setMatrixhook events [NotificationBus.js] — setMatrix emits without userId; bulk state changes won't trigger notifications. Pre-existing architectural limitation. - [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.
- [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.