feat(venue): reactive crowd video layer behind the 3D highway (career mode 1/3) - #905
Conversation
… mode PR1) Two crossfading video backdrop planes in the highway_3d venue background style, driven by a new venue-crowd.js state machine that maps v3:live-performance-state to crowd states (bored/neutral/engaged/ecstatic) with 3s stability + 8s dwell hysteresis, plus one-shot reaction stingers on streak milestones and end-of-song accuracy. Inert without a venue pack manifest (career plugin, PR2) or the feedBack-venue-crowd-dev flag — the static bg plate behaves exactly as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…itions Codex preflight P2s: (1) videos created before highway_3d registered its globals never reached the backdrop planes — binding is now idempotent and retried from start/perf-event/re-activation paths; (2) a crowd-state switch committing while a stinger played was dropped because the machine had already advanced — it is now deferred and played when the stinger ends. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight round 2: (1) the global load token let a stinger cancel a committed loop load on the other layer — tokens are now per-element, and a stinger preempting an in-flight loop on its own layer requeues that loop for when the stinger ends; (2) setManifest(null)/deactivate left the last crowd frame bound and visible over the static plate — stop() now unbinds both layers from the renderer and zeroes the mix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…racy from perf events
Codex preflight round 3: (1) a failed/timed-out stinger left a deferred
loop switch queued forever; the failure path now flushes it. (2)
stats:recorded only carries {filename, arrangement}, so the end-of-song
reaction now uses the accuracyPct from the song's last
v3:live-performance-state event (a real percentage) instead of a field
that never existed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hard-stop on manifest swap Codex preflight round 4: (1) idleLayer() still points at the fading-in layer during a crossfade, so a stinger firing mid-fade overwrote the new loop with nothing requeued — the fading loop is now tracked and requeued like an in-flight load; (2) swapping venue packs while active now goes through stop() so _stopGen invalidates the old manifest's in-flight loads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ize change Codex preflight round 5: (1) an ended/timeout handler orphaned by stop() could fire into a later stinger's lifecycle on the reused element — handlers now detach unconditionally and carry a generation token; (2) the renderer only re-applied cover-crop on camera aspect changes, so a src swap with a different intrinsic size kept stale repeat/offset — it now recrops when videoWidth/Height change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…the layer Codex preflight round 6: the loop crossfade's completion callback could still run between a stinger's start and its canplaythrough, promoting the stinger's layer to active and pausing the real loop — it now bails when the fading loop was preempted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a two-layer crowd video system for Venue 3D, including state-driven playback, stingers, crossfades, renderer integration, activation lifecycle wiring, and automated tests. ChangesVenue crowd video experience
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant VenueScene3D
participant v3VenueCrowd
participant VideoElements
participant Highway3D
VenueScene3D->>v3VenueCrowd: setVenueActive(true)
v3VenueCrowd->>VideoElements: load and play crowd loop
v3VenueCrowd->>Highway3D: bind videos and update mix
VenueScene3D->>v3VenueCrowd: setVenueActive(false)
v3VenueCrowd->>Highway3D: unbind videos and reset mix
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/highway_3d/screen.js (1)
3408-3413: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCentralize the layer-offset multipliers.
1.04/1.03are inline scene-scale magic numbers controlling the two crowd planes' depth ordering. Per the plugin's constants convention, hoist these into the top constants block (e.g.VENUE_CROWD_LAYER_DIST_MUL = [1.04, 1.03]) so the stacking intent stays discoverable next to the other backdrop-distance constants.As per coding guidelines: "Keep the plugin's scene-scale constants, camera constants, and geometry constants centralized in the top constants block rather than introducing new magic numbers elsewhere."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/highway_3d/screen.js` around lines 3408 - 3413, Centralize the crowd layer distance multipliers by adding a named constant such as VENUE_CROWD_LAYER_DIST_MUL in the top constants block, preserving the existing 1.04 and 1.03 values and their layer ordering. Update the layer construction near the distance calculation to read the appropriate multiplier from that constant instead of using inline magic numbers.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/highway_3d/screen.js`:
- Around line 3464-3501: Reset layer.lastVidW and layer.lastVidH alongside
layer.lastAspect when rebinding a crowd layer’s video texture in the s.crowd.rev
update block, ensuring applyCoverCrop() runs after metadata becomes available
even when the replacement video has identical dimensions.
---
Nitpick comments:
In `@plugins/highway_3d/screen.js`:
- Around line 3408-3413: Centralize the crowd layer distance multipliers by
adding a named constant such as VENUE_CROWD_LAYER_DIST_MUL in the top constants
block, preserving the existing 1.04 and 1.03 values and their layer ordering.
Update the layer construction near the distance calculation to read the
appropriate multiplier from that constant instead of using inline magic numbers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: da03495d-9004-46e6-903c-965e5c81a66f
📒 Files selected for processing (5)
plugins/highway_3d/screen.jsstatic/v3/index.htmlstatic/v3/venue-crowd.jsstatic/v3/venue-scene-3d.jstests/js/venue_crowd.test.js
| if (s.crowd.rev !== _venueCrowdRev) { | ||
| s.crowd.rev = _venueCrowdRev; | ||
| s.crowd.layers.forEach((layer, i) => { | ||
| const el = _venueCrowdVideos[i]; | ||
| if (layer.videoEl === el) return; | ||
| if (layer.tex) { layer.mat.map = null; layer.tex.dispose(); layer.tex = null; } | ||
| layer.videoEl = el; | ||
| layer.lastAspect = 0; // force refit + recrop | ||
| if (el) { | ||
| const tex = new T.VideoTexture(el); | ||
| tex.colorSpace = T.SRGBColorSpace; | ||
| tex.wrapS = T.ClampToEdgeWrapping; | ||
| tex.wrapT = T.ClampToEdgeWrapping; | ||
| tex.minFilter = T.LinearFilter; | ||
| tex.magFilter = T.LinearFilter; | ||
| tex.generateMipmaps = false; | ||
| layer.tex = tex; | ||
| layer.mat.map = tex; | ||
| } | ||
| layer.mat.needsUpdate = true; | ||
| }); | ||
| } | ||
| const warm = coeffs.warmth; | ||
| s.crowd.layers.forEach((layer, i) => { | ||
| const el = layer.videoEl; | ||
| // videoWidth === 0 until metadata lands — showing the | ||
| // plane before that paints a black flash over the plate. | ||
| const ready = !!el && el.videoWidth > 0; | ||
| // venue-crowd.js swaps src on the same element (loop ↔ | ||
| // stinger); a new intrinsic size needs a fresh | ||
| // cover-crop, which _bgFitBackdropPlane only reapplies | ||
| // on camera aspect changes. | ||
| if (ready && (layer.lastVidW !== el.videoWidth || | ||
| layer.lastVidH !== el.videoHeight)) { | ||
| layer.lastVidW = el.videoWidth; | ||
| layer.lastVidH = el.videoHeight; | ||
| layer.applyCoverCrop(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline plugins/highway_3d/screen.js --match '_bgFitBackdropPlane|_bgCoverCrop' --items all
rg -nP -C4 'function\s+_bgFitBackdropPlane|function\s+_bgCoverCrop|applyCoverCrop' plugins/highway_3d/screen.jsRepository: got-feedBack/feedBack
Length of output: 5832
Reset the crowd crop trackers on texture rebind
layer.lastAspect = 0 only forces the plane fit; applyCoverCrop() is still skipped when the new <video> keeps the same videoWidth/videoHeight as the previous source. Reset lastVidW/lastVidH here so the fresh VideoTexture gets cover-cropped once metadata lands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/highway_3d/screen.js` around lines 3464 - 3501, Reset layer.lastVidW
and layer.lastVidH alongside layer.lastAspect when rebinding a crowd layer’s
video texture in the s.crowd.rev update block, ensuring applyCoverCrop() runs
after metadata becomes available even when the replacement video has identical
dimensions.
Two half-transparent layers let the static bg plate bleed through (~25% at mid-fade) — visible as a flash of the old still image on every state transition. The crossfade is now always the front layer fading over an opaque rear layer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: stop() zeroed the mix but left _activeLayer at 1, so a restart flashed layer 0's stale frame until the new loop loaded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: a song ending in ecstatic/bored left the next song's crowd stuck in that mood until the hysteresis window passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: the orphaned ramp kept pushing the mix toward the layer whose src the stinger had just replaced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… value Codex preflight: Number(null) is 0, so idle HUD resets overwrote _lastAccuracyPct before stats:recorded consumed it, suppressing the end-of-song stinger. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: a stinger straddling a song change could fade back into the previous song's layer or flush its pending loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: superseded loads left canplaythrough/error listeners attached to the persistent video elements — unbounded growth over a session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
On song:loaded, an optional pack intro plays once: a camera flyover video (idle layer, one-shot) with bar-crowd ambience audio that ducks out on song:play, near the flyover's landing, or at handoff — whichever first. Machine commits and stingers defer during the intro; stop()/song-change abort it. Packs without an intro behave as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex preflight: a failed/timed-out intro left the song with no crowd loop at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
First slice of the career-mode epic (bar → club → arena venues with an audience that reacts to your playing). This PR adds the runtime plumbing only — no backend changes, no assets:
static/v3/venue-crowd.js(new): crowd-state machine mappingv3:live-performance-state→bored | neutral | engaged | ecstaticwith 3 s stability + 8 s dwell hysteresis; dual-<video>crossfade (1.2 s) between seamless loop videos; one-shot reaction stingers on streak milestones (25/50/100) and end-of-song accuracy (≥90 % cheer, ≥75 % clap), rate-limited and race-guarded.plugins/highway_3d/screen.js: two crowd video planes in thevenuebackground style (in front of the static plate, which remains the fallback), bound via two new globalsh3dVenueBackdropSetVideo(layer, el)/h3dVenueBackdropSetMix(t), reusing the existing VideoTexture recipe from thevideobg style.static/v3/venue-scene-3d.js: activates/deactivates the crowd layer with the venue scene.Inert without a venue pack manifest — set by the upcoming career plugin (PR 2/3), or for testing via the
feedBack-venue-crowd-devlocalStorage flag. With no manifest the venue mode behaves exactly as today.Testing
tests/js/venue_crowd.test.js: 12 tests over the pure state machine (stability window, dwell, flapping, milestone/accuracy stingers, manifest normalization) + wiring assertions.Manual verification (dev flag + placeholder loops) planned as part of PR 3/3 on-device pass.
🤖 Generated with Claude Code
Summary by CodeRabbit