Files
scrying-pool/_bmad-output/implementation-artifacts/2-1-notificationbus-and-notification-verbosity.md
2026-05-23 18:23:48 +02:00

23 KiB
Raw Permalink Blame History

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<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.js first (TDD red) — use vi.useFakeTimers() for coalescing timer tests
    • 1.2: Implement NotificationBus class — constructor receives (adapter); side-effect-free; store adapter; 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.userId must 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), update lastState, increment changeCount; 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: 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
    • 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
  • Task 2: Register notificationVerbosity client setting in module.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 NotificationBus from './src/notifications/NotificationBus.js'
    • 2.3: Declare let notificationBus; at module scope
    • 2.4: In Hooks.once('ready'), after visibilityBadge.init(), add: notificationBus = new NotificationBus(adapter); notificationBus.init(); (runs for BOTH GM and player clients)
  • 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
  • Task 4: Deferred debt cleanup — fold in from deferred-work.md (epic 1 carry-over)

    • 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
    • 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
    • 4.3: Add listener cleanup to VisibilityManager — store hook ID as _stateChangedHookId in init(); add teardown() calling adapter.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
  • Task 5: Implement styles/components/_notification.less minimal styles (AC: 1)

    • 5.1: Replaced stub comment with scoped .scrying-pool placeholder block; native ui.notifications styling suffices for toasts
  • Task 6: Pipeline verification

    • 6.1: npm run lint exits 0 for all modified files — pre-existing scripts/package.mjs errors unrelated to this story
    • 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)

// 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.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.14.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

  • [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: state vs newState breaks all notifications [NotificationBus.js:68, StateStore.js:105] — Fixed destructuring to use state property emitted by StateStore.
  • [Review][Patch] Unused changeCount property (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 notificationVerbosity setting value [NotificationBus.js:85] — Added validation with fallback to 'all' for invalid values.
  • [Review][Patch] _flush missing 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 setMatrix hook 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.