Files
uberwald f8cbb75773 Apply code review patches: null guards, validation, cleanup for WebRTC full AV replacement
- module.js: null guards, try-catch, migration logic
- FoundryAdapter.js: input validation, JSDoc fix
- ScryingPoolStrip.js: null guards, cleanup, race condition fix
- _roster-strip.less: CSS typo fix
- All 19 code review findings resolved
- All tests passing, 0 lint errors
- Story 5-1 documentation added (5-1-full-av-replacement.md)
- Sprint status updated for Epic 5

Generated by Mistral Vibe.
Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
2026-05-24 09:50:07 +02:00

22 KiB

Story 5.1: Full AV Replacement with WebRTC Stream Access

Status: ready-for-dev

Epic: 5 - Full AV Replacement

Story Key: 5-1-full-av-replacement

Created: 2026-05-26

Last Updated: 2026-05-26


Story Header

Field Value
Epic 5 - Full AV Replacement
Story ID 5.1
Story Key 5-1-full-av-replacement
Title Full AV Replacement with WebRTC Stream Access
Status ready-for-dev
Priority High
Assigned Agent DEV (Mistral Vibe / Morr)
Created 2026-05-26
Last Updated 2026-05-26

📋 Story Requirements

User Story

As a GM using Video View Manager, I want to completely replace Foundry's AV dock with my own video view using actual WebRTC streams, So that I have full control over the AV display and can implement custom camera management features.

Persona Alignment

  • Primary: GM (Marcus, Jake) - Needs full control over AV display for advanced camera management
  • Secondary: Players - Benefit from consistent, module-controlled video experience

Acceptance Criteria (BDD Format)

AC-1: Full AV Dock Replacement

Given the Video View Manager module is active When WebRTC stream access is available (getMediaStreamForUser exists) Then Foundry's native AV dock (#av) is completely hidden And Foundry's camera views (.camera-view) are completely hidden And The Scrying Pool Strip displays video feeds instead

AC-2: WebRTC Stream Access Detection

Given game.webrtc.client.getMediaStreamForUser() is available When the module probes WebRTC capability Then probeCapability() returns 'stream-access' And buildWebRTCSurface() creates a surface with all WebRTC client methods

AC-3: Video Element Creation

Given stream access mode is active When a participant joins the session Then a <video> element is created for that participant And the video element's srcObject is set to the participant's MediaStream And the video element has autoplay, playsInline, and appropriate muted state

AC-4: Current User Video Muted

Given a video element is created for the current user When the video element is attached Then the video element has muted = true And other users' video elements have muted = false

AC-5: Avatar Fallback When No Stream

Given a participant has no WebRTC stream available When the video container is rendered Then the avatar image is displayed as fallback And no broken video element is shown

AC-6: Stream Cleanup on Close

Given the Scrying Pool Strip is open with active video streams When the strip is closed Then all video elements are removed from the DOM And all MediaStream tracks are stopped And srcObject is set to null on all video elements

AC-7: Null Safety Throughout

Given any DOM query returns null When video attachment methods are called Then no TypeError is thrown And appropriate warnings are logged to console

AC-8: Migration Path for Existing Installations

Given an existing installation with webrtcMode set to 'css-fallback' or 'track-disable' When the module loads on a system with getMediaStreamForUser available Then the module probes fresh capability and uses 'stream-access' mode And no breaking changes occur for existing configurations

AC-9: CSS Properly Scoped

Given the module is active When the AV dock hiding rules are applied Then all selectors are properly scoped under .scrying-pool or are global overrides for #av and .camera-view And no unintended CSS conflicts occur with other modules

Functional Requirements

  • FR-27: Full AV Replacement — When module is active, Foundry's native AV dock is hidden and replaced with Video View Manager's own video display using actual WebRTC MediaStream objects
  • FR-28: Stream Access API — Module uses game.webrtc.client.getMediaStreamForUser(userId) to access actual WebRTC streams for creating custom video tiles

Key Spec Constraints

  • Architecture: Full Replacement (not overlay) — completely hides Foundry's AV dock and shows VVM dock
  • API: Uses game.webrtc.client.getMediaStreamForUser() — native FoundryVTT v14 WebRTC API
  • Storage: No new storage required — uses existing webrtcMode world setting with migration
  • Dependencies: No external libraries — uses native FoundryVTT APIs only
  • Import boundaries: Follows existing patterns — all Foundry API access via FoundryAdapter
  • Error handling: Silent failures logged — no uncaught errors thrown
  • Cleanup: Full resource cleanup — video elements and MediaStream tracks stopped on close

🎯 Implementation Details

Files Modified

File Changes Status
module.js Updated webrtcMode default to 'stream-access', added migration logic, added try-catch for buildWebRTCSurface Implemented
src/foundry/FoundryAdapter.js Updated probeCapability() to return 'stream-access', extended buildWebRTCSurface() with 11 WebRTC methods, added input validation Implemented
src/ui/gm/ScryingPoolStrip.js Added hasStreamAccess detection, added _attachVideoStreams() and _attachVideoStream() methods, added _refreshVideoStreams() and _cleanupVideoStreams() methods, added null guards throughout Implemented
templates/roster-strip.hbs Added video container element with conditional rendering based on hasStreamAccess Implemented
styles/scrying-pool.less Added CSS rules to hide Foundry's AV dock (#av and .camera-view) Implemented
styles/components/_roster-strip.less Added styling for .sp-participant-video and .sp-participant-video__element Implemented
scripts/test-stream-access.mjs New test script with 7 tests covering all major functionality Implemented

Technical Implementation

FoundryAdapter WebRTC Surface

The buildWebRTCSurface() method now provides 11 methods:

  1. getMediaStreamForUser(userId) — Get MediaStream for a specific user
  2. getConnectedUsers() — Get array of all connected user IDs
  3. getLevelsStreamForUser(userId) — Get volume monitoring stream for a user
  4. isAudioEnabled() — Check if current user's audio is enabled
  5. isVideoEnabled() — Check if current user's video is enabled
  6. toggleAudio(enable) — Enable/disable current user's audio
  7. toggleVideo(enable) — Enable/disable current user's video
  8. toggleBroadcast(enable) — Enable/disable current user's broadcast
  9. setUserVideo(userId, videoElement) — Set video element for a user
  10. disableTrack(userId) — Legacy: disable video track (cosmetic only)
  11. enableTrack(userId) — Legacy: enable video track

All methods include:

  • Input validation (userId must be string, non-empty)
  • Try-catch error handling with console.error logging
  • Null-safe access patterns

ScryingPoolStrip Video Attachment

The strip now:

  1. Detects stream access availability via this._adapter.webrtc?.getMediaStreamForUser !== undefined
  2. Renders video container element conditionally in template
  3. Attaches video elements in activateListeners() via _attachVideoStreams()
  4. Creates video elements with proper configuration:
    • srcObject = stream (actual MediaStream)
    • autoplay = true
    • playsInline = true
    • muted = true for current user
    • CSS class sp-participant-video__element
  5. Cleans up video elements and streams in _cleanupVideoStreams() when strip closes

CSS Styling

Global Overrides (scrying-pool.less):

#av { display: none !important; }
.camera-view { display: none !important; }

Video Element Styling (_roster-strip.less):

.sp-participant-video {
  position: absolute;
  inset: 0;
  width: 100%;
  height: 100%;
  display: flex;
  align-items: center;
  justify-content: center;
  overflow: hidden;
  z-index: 1;
}

.sp-participant-video__element {
  width: 100%;
  height: 100%;
  object-fit: cover;
  border-radius: 50%;
  background: hsl(220, 15%, 18%);
}

.sp-participant-video:not(:empty) ~ .sp-avatar__img {
  display: none;
}

🏗️ Architecture Compliance

Design Token Usage

All new CSS uses --sp-* tokens where applicable No direct Foundry --color-*/--font-*/--border-* tokens used All selectors properly scoped under .scrying-pool or .sp-* prefix

Import Boundaries

No direct game.* access in core logic All Foundry API access via FoundryAdapter Dependency injection maintained for all managers

Code Conventions

JSDoc on all exported symbols Private methods prefixed with _ Consistent error handling pattern (try-catch with console.error) Null-safe access patterns throughout


🧪 Testing Requirements

Unit Tests

Test Script: scripts/test-stream-access.mjs

Test Coverage:

  • probeCapability() returns correct mode
  • buildWebRTCSurface() creates all required methods
  • buildParticipantList() includes hasStreamAccess flag
  • Template includes video container
  • CSS includes AV dock hiding rules
  • CSS includes video element styling

Test Results: All 7 tests passing

Manual Test Cases

  1. Module Activation

    • Install module in FoundryVTT v14
    • Verify module activates without errors
    • Check console for WebRTC capability detection
  2. Stream Access Mode

    • Verify webrtcMode setting defaults to 'stream-access'
    • Verify probeCapability returns 'stream-access' when getMediaStreamForUser exists
    • Verify buildWebRTCSurface creates surface with all 11 methods
  3. AV Dock Hiding

    • Open game with module active
    • Verify Foundry's AV dock is hidden
    • Verify camera views are hidden
  4. Video Display

    • Join with multiple users
    • Verify video feeds appear in Scrying Pool Strip
    • Verify current user's video is muted
    • Verify avatar fallback shows when no stream available
  5. Cleanup

    • Close Scrying Pool Strip
    • Verify video elements are removed
    • Verify no memory leaks (check DevTools)
  6. Migration

    • Install on existing world with old webrtcMode setting
    • Verify module probes fresh capability and uses stream-access
    • Verify no breaking changes

📚 Developer Context Section

What the Developer MUST Know

1. WebRTC API Access Pattern

Critical: The module uses game.webrtc.client.getMediaStreamForUser(userId) to access actual WebRTC MediaStream objects. This is a native FoundryVTT v14 API that returns the actual stream for a given user.

Do NOT:

  • Try to access tracks directly without going through the client API
  • Assume all users have streams available (check for null)
  • Forget to clean up MediaStream tracks when done

DO:

  • Always validate userId is a non-empty string
  • Always check if webrtc surface is available before calling methods
  • Always set srcObject = null before removing video elements

2. Full Replacement vs Overlay Architecture

This implementation uses Full Replacement architecture:

  • Foundry's AV dock is completely hidden (not just overlaid)
  • Video View Manager creates its own video elements
  • Uses actual MediaStream objects from WebRTC

Previous architectures considered:

  • Overlay: Overlay on Foundry's AV tiles (abandoned - WebRTC tracks can't be disabled to save bandwidth)
  • Track Disable: Disable video tracks cosmetically (abandoned - doesn't reduce bandwidth)
  • CSS Fallback: CSS-only hiding (abandoned - doesn't provide actual video)

3. Error Handling Pattern

All WebRTC surface methods follow this pattern:

methodName: (param) => {
  try {
    // Input validation
    if (invalid input) { console.warn; return; }
    
    // Actual logic
    return result;
  } catch (err) {
    console.error('[ScryingPool] methodName failed:', err);
    return null; // or appropriate fallback
  }
}

Why:

  • Prevents uncaught errors from crashing the module
  • Provides debugging information via console
  • Allows graceful degradation when APIs fail

4. Null Safety Pattern

All DOM queries use optional chaining and null guards:

const videoContainer = participantItem.querySelector?.('.sp-participant-video');
if (!videoContainer) {
  console.warn('[ScryingPool] No video container found for user:', userId);
  return;
}

Why:

  • Prevents TypeError when elements don't exist
  • Handles edge cases gracefully
  • Provides clear error messages for debugging

5. Resource Cleanup Pattern

All resources must be cleaned up to prevent memory leaks:

_cleanupVideoStreams() {
  const videoElements = document.querySelectorAll?.('.sp-participant-video__element') ?? [];
  videoElements.forEach(videoEl => {
    // Stop all tracks in the stream
    if (videoEl.srcObject instanceof MediaStream) {
      videoEl.srcObject.getTracks().forEach(track => track.stop());
    }
    videoEl.srcObject = null;
    videoEl.remove();
  });
}

Why:

  • Prevents memory leaks from MediaStream tracks
  • Ensures proper cleanup when module UI is closed
  • Follows best practices for WebRTC resource management

6. Migration Strategy

Existing installations with old webrtcMode values ('track-disable' or 'css-fallback') are handled via:

const currentWebRtcMode = adapter.settings.get(FoundryAdapter.SETTING_WEBRTC_MODE);
const isDeprecatedMode = currentWebRtcMode === 'track-disable' || currentWebRtcMode === 'css-fallback';
const outcome = isDeprecatedMode 
  ? FoundryAdapter.probeCapability(game.webrtc)
  : currentWebRtcMode || FoundryAdapter.probeCapability(game.webrtc);

Why:

  • Ensures existing installations get new functionality
  • No breaking changes for existing users
  • Automatic upgrade path

Story Dependency Type Reason
1-2-webrtc-spike Foundational Proved WebRTC API access patterns
1-3-data-layer Required FoundryAdapter infrastructure
1-5-gm-control-ui Required ScryingPoolStrip base
4-1-privacy-panel None Separate feature (Player Privacy)
4-2-custom-portrait None Separate feature (Portrait Fallback)

📊 Success Criteria

Criterion Target Measurement
Code Quality 0 lint errors ESLint run
Test Coverage All major paths tested Manual + unit tests
Performance No memory leaks DevTools inspection
Compatibility FoundryVTT v14 Version check
Migration Zero breaking changes Existing install test

🎯 Previous Story Intelligence

Patterns Established in Story 4.1 & 4.2

From Story 4.1 (Player Privacy Panel):

  • User flag storage pattern for privacy settings
  • Player Privacy Panel UI structure
  • Opt-in toggle controls
  • Read-only GM view

From Story 4.2 (Custom Portrait Fallback):

  • File picker implementation
  • Image validation (MIME type, size)
  • DataURL storage in user flags
  • Portrait fallback chain (custom → Foundry avatar → system placeholder)

Lessons Applied to This Story

  1. Null Safety — All methods now include input validation (learned from Story 4.1 review)
  2. Error Handling — Consistent try-catch pattern (learned from Story 4.1 review)
  3. Resource Cleanup — Proper cleanup of video elements and streams (new pattern for WebRTC)
  4. Migration — Smooth upgrade path for existing installations (pattern from Story 1.3)

🚀 Git Intelligence

Recent Commits:

  • c4a375f - Story 4.2: Implement full AV replacement with WebRTC stream access (7 files, +479/-33)
  • 20d13fc - Story 4.2: Fix lint errors and code review findings

Files Modified in Current Implementation:

  • module.js (29 lines changed)
  • src/foundry/FoundryAdapter.js (176 lines changed)
  • src/ui/gm/ScryingPoolStrip.js (90 lines changed)
  • templates/roster-strip.hbs (7 lines changed)
  • styles/components/_roster-strip.less (33 lines changed)
  • styles/scrying-pool.less (18 lines changed)
  • scripts/test-stream-access.mjs (159 lines, new file)

📖 Latest Technical Information

FoundryVTT v14 WebRTC API

Key Finding: game.webrtc.client.getMediaStreamForUser(userId) exists and returns MediaStream objects

Source: /home/morr/foundry/foundryv14/resources/app/client/av/clients/simplepeer.mjs

Methods Available:

  • getMediaStreamForUser(userId: string): MediaStream | null
  • getConnectedUsers(): string[]
  • getLevelsStreamForUser(userId: string): MediaStream | null
  • isAudioEnabled(): boolean
  • isVideoEnabled(): boolean
  • toggleAudio(enable: boolean): void
  • toggleVideo(enable: boolean): void
  • toggleBroadcast(enable: boolean): void
  • setUserVideo(userId: string, videoElement: HTMLVideoElement): Promise<void>

Architecture Decision: Full Replacement is feasible using these APIs

WebRTC Stream Access Pattern

// Get stream for a user
const stream = game.webrtc.client.getMediaStreamForUser(userId);

// Create video element
const videoElement = document.createElement('video');
videoElement.srcObject = stream;
videoElement.autoplay = true;
videoElement.playsInline = true;
videoElement.muted = (userId === game.userId); // Mute self

// Clean up
videoElement.srcObject = null;
stream.getTracks().forEach(track => track.stop());

📚 Project Context Reference

  • Project: video-view-manager
  • User: Morr
  • Architecture: Progressive Enhancement Architecture
  • Module Type: FoundryVTT v14 ES Module
  • Dependencies: None (native FoundryVTT APIs only)

Story Completion Status

Task Status Notes
Story requirements extracted From conversation context
Epic context loaded Epic 5: Full AV Replacement
Architecture analysis WebRTC API confirmed
Previous story intelligence Patterns from Epic 4
Git intelligence Recent commits analyzed
Technical research FoundryVTT v14 APIs validated
Story file created Complete documentation
Sprint status synced Story marked ready-for-dev

Status: ready-for-dev
Completion Note: Ultimate context engine analysis completed - comprehensive developer guide created


🎯 Tasks/Subtasks

  • Analyze FoundryVTT v14 WebRTC API for stream access
  • Update FoundryAdapter.probeCapability() to detect stream-access mode
  • Extend FoundryAdapter.buildWebRTCSurface() with full WebRTC client API
  • Update module.js webrtcMode setting to default to stream-access
  • Add migration path for existing webrtcMode settings
  • Update ScryingPoolStrip.getData() to detect stream access
  • Update buildParticipantList() to include hasStreamAccess flag
  • Add _attachVideoStreams() method to ScryingPoolStrip
  • Add _attachVideoStream() method with null guards and validation
  • Add _refreshVideoStreams() method for stream updates
  • Add _cleanupVideoStreams() method for resource cleanup
  • Update activateListeners() to attach video streams
  • Update close() to cleanup video streams and tracks
  • Update roster-strip.hbs template with video container
  • Add CSS to hide Foundry AV dock (#av, .camera-view)
  • Add CSS for video container and video element styling
  • Add input validation to all WebRTC surface methods
  • Add null guards throughout ScryingPoolStrip
  • Create test script for stream access implementation
  • Fix all code review findings (19 patches applied)

📝 Review Findings

Code Review Completed: 2026-05-26

Review Layers: Blind Hunter + Edge Case Hunter
Review Mode: no-spec (code-only review)
Diff Source: Commit range HEAD~1..HEAD (c4a375f)

Decision Needed: 0

Patch: 19 (All Applied)

  • [Review][Patch] Missing null guard for adapter.settings module.js:183
  • [Review][Patch] No try-catch wrapper for buildWebRTCSurface() call module.js
  • [Review][Patch] probeCapability() return type breaking change without JSDoc update module.js
  • [Review][Patch] buildWebRTCSurface() JSDoc parameter type mismatch FoundryAdapter.js
  • [Review][Patch] Inconsistent error handling across new methods FoundryAdapter.js
  • [Review][Patch] el can be null in activateListeners ScryingPoolStrip.js:220
  • [Review][Patch] _refreshVideoStreams() calls this.render without null guard ScryingPoolStrip.js
  • [Review][Patch] Missing null guards for DOM query results ScryingPoolStrip.js:345-347
  • [Review][Patch] this._adapter.webrtc null access in _attachVideoStream ScryingPoolStrip.js:362
  • [Review][Patch] participantItem null access ScryingPoolStrip.js:365
  • [Review][Patch] document undefined in non-browser environment ScryingPoolStrip.js:383
  • [Review][Patch] No stream cleanup when users disconnect ScryingPoolStrip.js
  • [Review][Patch] Race condition in _refreshVideoStreams() re-render ScryingPoolStrip.js
  • [Review][Patch] Missing handling for missing data-user-id attribute ScryingPoolStrip.js
  • [Review][Patch] No MediaStream validation before setting srcObject ScryingPoolStrip.js
  • [Review][Patch] Type safety gap in muted logic ScryingPoolStrip.js
  • [Review][Patch] No cleanup in close() for video elements ScryingPoolStrip.js
  • [Review][Patch] Missing migration path for webrtcMode default change module.js
  • [Review][Patch] CSS typo: missing space before comment _roster-strip.less:263

Defer: 8

  • [Review][Defer] !important CSS may override other modules scrying-pool.less
  • [Review][Defer] Inconsistent API optional chaining pattern FoundryAdapter.js
  • [Review][Defer] toggle methods don't return status FoundryAdapter.js
  • [Review][Defer] No way for callers to detect failure FoundryAdapter.js
  • [Review][Defer] No poster attribute/loading state ScryingPoolStrip.js
  • [Review][Defer] hasStreamAccess redundancy in participant objects ScryingPoolStrip.js
  • [Review][Defer] Template assumes hasStreamAccess is boolean roster-strip.hbs
  • [Review][Defer] probeCapability return type change FoundryAdapter.js

Dismissed: 0


Review Complete!

Story Status: ready-for-dev Issues Fixed: 19 Action Items Created: 0 Deferred: 8 Dismissed: 0