diff --git a/CHANGELOG.md b/CHANGELOG.md index e862280..8a75d83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 changes. ### Changed +- **`GET /api/song/{f}?stems=1`** (new, opt-in) — returns the pack's playable stem + list (`[{id, url, default}]` + `full_mix_url`), the same list the highway's WS + `ready` sends. The stems plugin could only learn it from that WS message, which + arrives once the highway is already on screen — so it decoded and then copied the + whole song's PCM to its audio worklet with the player visible: over half a gigabyte + of memcpy in one frame for a 6-stem pack, a measured 698 ms freeze right as the + song-credits card appeared. With the list available at `song:loading` the plugin + does all of it before the highway is drawn. Built by calling `load_song` itself, so + it cannot drift from what the WS sends. Opt-in, so the library's metadata calls pay + nothing. - **Folder library renders only the songs on screen** (#965) — a song list used to render *every* song it held. On a flat 50,944-song library that was one `
` with 50,938 children and ~1.3 **million** DOM nodes (~4.2 GB of renderer memory), @@ -212,6 +222,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry). ### Fixed +- **3D Highway: the lane stops at the hit line** (#991) — the highway lane, its + dividers, and the fret boundary extension lines ran `BEHIND` seconds *past* the + hit line toward the player. Nothing is ever drawn in that strip (notes and chord + frames clamp to `Math.min(0, dZ(dt))`), so it read as lane with no notes on it. + The floor geometry now ends at the hit line; its far edge is unchanged, still + `-AHEAD*TS` at the note horizon. - **Career passports review polish** — the passport tabs and book overlay carry proper ARIA semantics (`aria-selected`/`aria-controls`/`tabpanel`; `role="dialog"` + `aria-modal` with focus moved to the close button on open diff --git a/lib/routers/song.py b/lib/routers/song.py index 72a1d06..aa6644e 100644 --- a/lib/routers/song.py +++ b/lib/routers/song.py @@ -829,9 +829,60 @@ def post_song_gap_fill(filename: str, data: dict): return {"ok": True, "written": additions, "skipped": skipped} +def _playable_stems_payload(filename: str, dlc) -> dict: + """The playable stems (id/url/default) + full-mix URL for a sloppak. + + Why it exists: the stems plugin could only learn its stem list from the + highway's WS `ready`, which arrives once the highway is already up. So it + decoded, and then copied the whole song's PCM to its worklet, with the player + on screen — half a gigabyte of memcpy in one frame, ~700 ms, freezing the + venue video. Given the list at `song:loading` it can do all of that BEFORE the + highway appears, behind the loading overlay where a stall costs nothing. + + The list MUST be the same one the WS sends a moment later. If it is not, the + plugin preloads a graph and then throws it away and rebuilds — strictly worse + than not preloading. So this does not reimplement the WS's construction, it + calls THE SAME FUNCTION: load_song, whose LoadedSloppak already carries the + partitioned stems and the resolved full mix, and then builds the URLs exactly + as ws_highway does. Drift is impossible by construction rather than by + agreement — which matters, because `full_mix` in particular is not simply the + `full` stem: load_song falls back to the deprecated `original_audio:` key for + every pack written before feedpak 1.15.0, and reimplementing that (I did, at + first) silently dropped the pristine full mix for most real libraries. + + Opt-in (`?stems=1`) so the library's own metadata calls — the hot path — pay + nothing for it. Non-sloppak sources (archives, loose folders) have no stems + to preload: load_song raises and we return the empty list. + """ + from urllib.parse import quote + + try: + loaded = sloppak_mod.load_song(filename, dlc, appstate.sloppak_cache_dir) + except Exception: + return {"stems": [], "full_mix_url": None} + + q_fn = quote(filename, safe="") + + def _url(rel: str) -> str: + return f"/api/sloppak/{q_fn}/file/{quote(rel)}" + + return { + "stems": [ + {"id": s["id"], "url": _url(s["file"]), "default": s["default"]} + for s in loaded.stems + ], + "full_mix_url": _url(loaded.full_mix) if loaded.full_mix else None, + } + + @router.get("/api/song/{filename:path}") -async def get_song_info(filename: str): - """Return song metadata, from cache or by extracting it from the song source.""" +async def get_song_info(filename: str, stems: int = 0): + """Return song metadata, from cache or by extracting it from the song source. + + `?stems=1` additionally returns the playable stem list with URLs, so the + stems plugin can start fetching/decoding on `song:loading` instead of waiting + for the highway's WS `ready` (see _playable_stems_payload). + """ import asyncio dlc = _get_dlc_dir() if not dlc: @@ -854,8 +905,21 @@ async def get_song_info(filename: str): mtime, size = appstate.stat_for_cache(song_path) cached = appstate.meta_db.get(cache_key, mtime, size) + loop = asyncio.get_event_loop() + + # The stem list is NOT stored in the metadata cache: that is a fixed-column + # table, and widening it would mean a migration plus a stale row for every + # song already scanned. It is cheap to read on demand (the pack is unpacked + # by then, so this is a plain manifest read), and only the opt-in caller pays. + async def _with_stems(meta: dict) -> dict: + if not stems: + return meta + extra = await loop.run_in_executor( + None, _playable_stems_payload, filename, dlc) + return {**meta, **extra} + if cached: - return cached + return await _with_stems(cached) # Extract in thread pool def _extract(): @@ -863,5 +927,5 @@ async def get_song_info(filename: str): appstate.meta_db.put(cache_key, mtime, size, meta) return meta - meta = await asyncio.get_event_loop().run_in_executor(None, _extract) - return meta + meta = await loop.run_in_executor(None, _extract) + return await _with_stems(meta) diff --git a/lib/scan.py b/lib/scan.py index 820cb50..6780b04 100644 --- a/lib/scan.py +++ b/lib/scan.py @@ -51,6 +51,97 @@ from scan_worker import _relpath, _scan_one log = logging.getLogger("feedBack.scan") +import json + + +# ── Directory-signature fast path ───────────────────────────────────────────── +# +# A startup scan globs the whole library twice (*.feedpak, *.wem) and stats every +# file to detect what changed. On a 50k-song library that lives on a slow mount +# (an NTFS-3G FUSE volume here) it is ~100k filesystem round trips every launch — +# the "big drive churns on every startup" report. +# +# But adds / removes / renames of songs all bump the mtime of the DIRECTORY that +# holds them (verified on the target NTFS-3G mount), and so does the addition of +# a subdirectory (a new entry in its parent). So after a scan we record every +# library directory and its mtime; on the next scan we re-stat ONLY those +# directories (a handful, vs 100k file ops). If none changed, the file set is +# unchanged and the whole listing/stat pass is skipped. +# +# The one thing this cannot see is a file edited IN PLACE under the same name — +# that bumps the file's mtime but not its directory's. That is rare for a song +# library (you add and remove packs, you don't rewrite them under the same name), +# and the manual Refresh forces a full scan (force=True) for exactly that case. +def _dir_signature_file() -> Path: + return appstate.config_dir / "scan_dir_signature.json" + + +def _load_dir_signature() -> dict | None: + try: + data = json.loads(_dir_signature_file().read_text(encoding="utf-8")) + if isinstance(data, dict) and isinstance(data.get("dirs"), dict): + return data + except (OSError, ValueError): + pass + return None + + +def _save_dir_signature(dlc: Path, dirs: dict[str, int]) -> None: + # Keyed by the DLC path so switching libraries never matches a stale + # signature. Best-effort: a failed write just means the next scan is a full + # one, never a wrong one. + try: + _dir_signature_file().write_text( + json.dumps({"dlc": str(dlc), "dirs": dirs}), encoding="utf-8") + except OSError as e: + log.debug("scan: could not persist dir signature: %s", e) + + +def _library_dirs(all_songs, dlc: Path) -> set[str]: + """Every directory whose mtime reflects an add/remove of a library song: + each song's containing directory and all of its ancestors up to the DLC + root (the root itself always included, as "."). Derived from the already- + listed songs — no extra filesystem walk. The builtin carve-outs + (tutorials-builtin / minigames-builtin) are absent because the caller + already excluded them from `all_songs`, so a minigame writing a drill there + never invalidates the fast path. + + Directory-form songs (loose-song folders, directory sloppak bundles) also + record their OWN directory: a file added/removed/replaced INSIDE the folder + bumps that folder's mtime but not its parent's, so tracking only the parent + would miss an in-place change to such a song. File-form sloppaks (a single + .feedpak zip) aren't dirs, so they add nothing here — the flat file library + stays at a handful of dir stats.""" + rels = {"."} + for f in all_songs: + rel = Path(_relpath(f, dlc)) + if f.is_dir(): + rels.add(rel.as_posix()) + parent = rel.parent + rels.add(parent.as_posix()) + for anc in parent.parents: + rels.add(anc.as_posix()) + return rels + + +def _record_dir_signature(all_songs, dlc: Path) -> None: + sig = _stat_dirs(dlc, _library_dirs(all_songs, dlc)) + if sig is not None: # a dir vanished mid-scan → skip; next scan is full + _save_dir_signature(dlc, sig) + + +def _stat_dirs(dlc: Path, rels) -> dict[str, int] | None: + """{reldir: mtime_ns} for the given library dirs, or None if any is gone or + unreadable — a vanished recorded dir means the tree changed, so fail to a + full scan rather than a false match.""" + out: dict[str, int] = {} + for rel in rels: + try: + out[rel] = (dlc if rel == "." else dlc / rel).stat().st_mtime_ns + except OSError: + return None + return out + _SCAN_STATUS_INIT = {"running": False, "stage": "idle", "total": 0, "done": 0, "current": "", "error": None, "is_first_scan": False, "added": 0, "removed": 0} @@ -99,9 +190,12 @@ def _make_scan_executor(): ) -def background_scan(): +def background_scan(force: bool = False): """Scan the library and cache song metadata on startup. Uses a process pool to bypass the GIL for CPU-bound metadata parsing. + `force` skips the directory-signature fast path and always does the full + listing/stat pass — the manual Refresh sets it (see _dir_signature_file). + Never sets `_scan_status["running"] = False` — ownership of that flag lives in `_scan_runner` so a `kick_scan()` racing this function's terminal write cannot observe a stale False and start a second runner. @@ -121,6 +215,22 @@ def background_scan(): builtin_content.seed_builtin_diagnostic_sloppaks(appstate.server_root, dlc) builtin_content.seed_builtin_starter_content(appstate.server_root, dlc) + # Fast path: if every library directory recorded by the last scan still has + # the same mtime, nothing was added, removed, or renamed, so the whole + # glob-and-stat pass below can be skipped (see the signature comment above). + # `force` (manual Refresh) always does the full pass. Seeding above is + # idempotent — it only writes when a builtin is missing — so it does not + # perturb the mtimes on a settled library. + if not force: + stored = _load_dir_signature() + if stored is not None and stored.get("dlc") == str(dlc): + current = _stat_dirs(dlc, stored["dirs"].keys()) + if current is not None and current == stored["dirs"]: + _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete"} + log.info("Scan: library tree unchanged (%d dirs) — skipped the full listing/stat pass", + len(current)) + return + # Listing can fail on macOS without Full Disk Access, or on Docker if the # path isn't shared. Report the failure explicitly rather than silently # appearing to scan nothing. @@ -223,6 +333,9 @@ def background_scan(): to_scan.append((f, mtime, size, dlc)) if not to_scan: + # Full pass completed with the DB already up to date — record the tree + # signature so the next startup can take the fast path. + _record_dir_signature(all_songs, dlc) _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete", "added": added, "removed": removed} log.info("Scan: nothing new to scan (%d songs, all cached)", len(all_songs)) return @@ -247,6 +360,9 @@ def background_scan(): _scan_status["done"] += 1 _scan_status["current"] = fname + # Record the tree signature after a completed full pass so the next startup + # can skip it when nothing has changed. + _record_dir_signature(all_songs, dlc) log.info("Scan complete: %d songs cached", len(to_scan)) _scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete", "added": added, "removed": removed} @@ -255,6 +371,9 @@ _scan_kick_lock = threading.Lock() _scan_rescan_pending = False +# Set by kick_scan(force=True); consumed by _scan_runner for the next pass so a +# manual Refresh bypasses the directory-signature fast path. +_scan_force_next = False # Handles to the running scan / enrichment worker threads. Both use the shared @@ -265,9 +384,15 @@ _scan_rescan_pending = False _scan_thread: threading.Thread | None = None -def kick_scan() -> bool: +def kick_scan(force: bool = False) -> bool: """Request a library rescan, single-flight + coalescing. + `force` skips the directory-signature fast path for the resulting pass (the + manual Refresh uses it so an in-place same-name edit — the one thing the + fast path can't see — is always picked up). A forced request that coalesces + onto a running or queued scan keeps the force intent: the pass is forced if + ANY pending request asked for it. + Returns True if a new scan thread was started, False if one was already running. In the latter case a follow-up pass is queued and runs as soon as the current scan finishes so files landing mid-scan (e.g. an upload @@ -275,8 +400,10 @@ def kick_scan() -> bool: until the next periodic pass. Multiple late-arriving requests coalesce into a single follow-up. """ - global _scan_rescan_pending, _scan_thread + global _scan_rescan_pending, _scan_thread, _scan_force_next with _scan_kick_lock: + if force: + _scan_force_next = True if _scan_status["running"]: _scan_rescan_pending = True return False @@ -290,10 +417,15 @@ def kick_scan() -> bool: def _scan_runner(): """Run _background_scan, then re-run if requests arrived mid-scan.""" - global _scan_rescan_pending + global _scan_rescan_pending, _scan_force_next while True: + # Consume the force flag for THIS pass; a forced request queued mid-scan + # sets it again for the follow-up. + with _scan_kick_lock: + forced = _scan_force_next + _scan_force_next = False try: - background_scan() + background_scan(force=forced) except Exception: log.exception("background scan failed unexpectedly") diff --git a/lib/sloppak.py b/lib/sloppak.py index d5a12a0..8f943e9 100644 --- a/lib/sloppak.py +++ b/lib/sloppak.py @@ -80,6 +80,20 @@ def find_full_mix(stems: list[dict]) -> dict | None: ) +def stem_default_on(raw) -> bool: + """Whether a manifest stem entry plays by default. + + Absent means on. A string is honoured so a hand-written manifest can say + `default: off`. Extracted so the WS `ready` payload and the REST song-info + payload cannot drift: the stems plugin now preloads from REST and then has + to agree with what the WS says a moment later, or it would rebuild the whole + graph for nothing. + """ + if isinstance(raw, str): + return raw.lower() not in ("off", "false", "0", "no") + return bool(raw) + + def partition_stems(stems: list[dict]) -> tuple[dict | None, list[dict]]: """Split stem descriptors into (mixdown, instrument_stems) for PLAYBACK. @@ -1100,12 +1114,11 @@ def load_song( sfile = str(s.get("file", "")) if not sid or not sfile: continue - default_val = s.get("default", True) - if isinstance(default_val, str): - default_on = default_val.lower() not in ("off", "false", "0", "no") - else: - default_on = bool(default_val) - stems.append({"id": sid, "file": sfile, "default": default_on}) + stems.append({ + "id": sid, + "file": sfile, + "default": stem_default_on(s.get("default", True)), + }) # The complete mixdown is a stem (spec §5.3), but it is not a *layer*: lift # it out so that no consumer of `stems` — the mixer, the library's stem diff --git a/plugins/career/routes.py b/plugins/career/routes.py index 90be728..7ec4a17 100644 --- a/plugins/career/routes.py +++ b/plugins/career/routes.py @@ -46,6 +46,8 @@ from pathlib import Path from fastapi import Body, HTTPException from fastapi.responses import FileResponse +import sloppak +from dlc_paths import _resolve_dlc_path from progression import instrument_for_arrangement PLUGIN_ID = "career" @@ -53,6 +55,9 @@ VENUE_ID_RE = re.compile(r"^[a-z0-9_-]{1,40}$") PACK_FILENAME_RE = re.compile(r"^[a-z0-9_-]{1,64}\.(mp4|webm|mp3|json)$") REQUIRED_LOOPS = ("bored", "neutral", "engaged", "ecstatic") DOWNLOAD_CHUNK = 1024 * 256 +# A setlist is a handful of songs; this endpoint unpacks zips, so cap the work an +# arbitrary caller can ask for. +MAX_GIG_SONGS = 32 _lock = threading.Lock() _state = { @@ -517,27 +522,39 @@ def _current_venue(): return best -def _unplayed_genre_songs(gkey, exclude, limit): - """Library songs of a genre with no stats yet — a young passport's gig - still gets a full set (playing them is how stubs start). - ponytail: full stat-less scan + python-side genre match (a few ms at 7k - songs, single-user); push the match into SQL if propose ever feels slow.""" +def _fill_genre_songs(gkey, exclude, limit): + """Library songs of a genre to round out a gig — ANY song of the genre the + set hasn't already picked. + + Was `_unplayed_genre_songs`, restricted to `filename NOT IN song_stats`. + That restriction created a hole: a song you'd played on a DIFFERENT + instrument's arrangement has a stats row, so it was excluded here — and it + lives in the played bucket for THAT instrument, not this passport's, so it + was excluded there too. It could never be gigged. A player with 137 metalcore + songs, all played on another instrument, got a 404 (reproduced). The player's + library is the pool; whether a song has stats on some other instrument has no + bearing on whether it can be in THIS gig. + + Shuffled, so re-roll actually changes the set. The old version returned the + library's first N in table order every time, so re-roll was a no-op for any + set drawn from the filler (reproduced). + + ponytail: full genre scan + python-side match + shuffle (a few ms at 7k + songs, single-user); push into SQL if propose ever feels slow. + """ db = _state["meta_db"] if db is None: return [] rows = db.conn.execute( - f"SELECT filename, title, artist, {_genre_expr(db)} AS g FROM songs " - "WHERE filename NOT IN (SELECT filename FROM song_stats)" + f"SELECT filename, title, artist, {_genre_expr(db)} AS g FROM songs" ).fetchall() - out = [] - for filename, title, artist, genre in rows: - if _genre_key(genre) != gkey or filename in exclude: - continue - out.append({"filename": filename, "title": title or filename, - "artist": artist or ""}) - if len(out) >= limit: - break - return out + pool = [ + {"filename": filename, "title": title or filename, "artist": artist or ""} + for filename, title, artist, genre in rows + if _genre_key(genre) == gkey and filename not in exclude + ] + random.shuffle(pool) # re-roll must vary; free per call + return pool[:limit] def _validate_pack_dir(pack_dir: Path): @@ -734,6 +751,62 @@ def setup(app, context): "snapshot": snapshot}) return {"ok": True} + @app.post(f"/api/plugins/{PLUGIN_ID}/gigs/prepare") + def prepare_gig(body: dict = Body(...)): + """Unpack every song of the set BEFORE the gig starts. + + A feedpak is a zip: the first play of one pays for its extraction into + sloppak_cache. Inside a set that cost landed BETWEEN songs — the player + finished a number and then sat waiting for the next one to unpack, mid- + gig. A set is a known list up front, so extract it all while the player + is still looking at the poster. + + Idempotent and cheap on a warm cache: resolve_source_dir() returns the + already-unpacked dir without rewriting it. Best-effort per song — one + bad feedpak must not block the set from starting (the play itself will + surface the error, exactly as it does outside a gig). + """ + raw = (body or {}).get("songs") + # A str is iterable: without the list check, "abc" would prepare three + # one-character "songs". Cap the count too — this endpoint unpacks zips, + # so an oversized list is real work, and a setlist is a handful of songs. + if not isinstance(raw, list): + return {"ok": True, "prepared": 0, "failed": []} + files = [f for f in raw if isinstance(f, str) and f.strip()][:MAX_GIG_SONGS] + if not files: + return {"ok": True, "prepared": 0, "failed": []} + + # .get, not []: a host that doesn't hand us the resolvers (or has no + # library configured) must degrade to "extract lazily, as before" — this + # is an optimisation, and it is never allowed to be the thing that stops + # a gig from starting. + get_dlc = context.get("get_dlc_dir") + get_cache = context.get("get_sloppak_cache_dir") + dlc_root = get_dlc() if callable(get_dlc) else None + cache_root = get_cache() if callable(get_cache) else None + if dlc_root is None or cache_root is None: + return {"ok": False, "prepared": 0, "failed": files, "error": "no library"} + + root = Path(dlc_root) + prepared, failed = 0, [] + for fn in files: + # CONTAINMENT FIRST. resolve_source_dir() does a bare + # `dlc_root / filename` with no guard, so a crafted `../..` would + # walk straight out of the library. Every other filename-bound + # handler validates through _resolve_dlc_path; so does this one. + safe = _resolve_dlc_path(root, fn) + if safe is None: + _state["log"].warning("career: gig pre-extract rejected unsafe path %r", fn) + failed.append(fn) + continue + try: + sloppak.resolve_source_dir(fn, root, Path(cache_root)) + prepared += 1 + except Exception as exc: # noqa: BLE001 — one bad pak can't sink the set + _state["log"].warning("career: gig pre-extract failed for %s: %s", fn, exc) + failed.append(fn) + return {"ok": True, "prepared": prepared, "failed": failed} + @app.post(f"/api/plugins/{PLUGIN_ID}/gigs/propose") def propose_gig(body: dict = Body(...)): inst = str((body or {}).get("instrument") or "") @@ -775,7 +848,7 @@ def setup(app, context): picks.append(s) if len(picks) < size: exclude = {s["filename"] for s in picks} - picks.extend(_unplayed_genre_songs(gkey, exclude, size - len(picks))) + picks.extend(_fill_genre_songs(gkey, exclude, size - len(picks))) if not picks: raise HTTPException(404, "No songs of this genre in the library.") venue = _current_venue() diff --git a/plugins/career/screen.js b/plugins/career/screen.js index c981625..4e47db1 100644 --- a/plugins/career/screen.js +++ b/plugins/career/screen.js @@ -12,6 +12,10 @@ 'use strict'; const API = '/api/plugins/career'; + // Unpacking a setlist is real work (zips, possibly on a slow/network drive), + // so this is generous — but it is a CEILING, not a wait. Past it we start the + // gig and let the first play extract lazily, as it always did. + const PREPARE_TIMEOUT_MS = 60000; const VENUE_OVERRIDE_KEY = 'feedBack-career-venue'; const NO_VENUE = '__none__'; const PREV_VIZ_KEY = 'feedBack-career-prev-viz'; @@ -1123,10 +1127,52 @@ sfx('page'); } - function startGig() { + // Unpack the whole set before the first note. + // + // A feedpak is a zip, and the first play of one pays for its extraction. In + // a set that cost landed BETWEEN songs: the player finished a number and + // then sat there waiting for the next one to unpack, mid-gig. The setlist is + // known up front, so warm it all while the poster is still on screen. + // + // Best-effort by design: a library that won't pre-extract must not stop the + // gig from starting — the play itself surfaces the error the same way it + // does outside a gig. Slow is better than blocked. + async function prepareGigSongs(prop, btn) { + const label = btn && btn.textContent; + if (btn) { btn.disabled = true; btn.textContent = 'Preparing set…'; } + // A bare `await fetch(...)` only rejects on a network ERROR — a server + // that accepts the connection and then never answers hangs forever, and + // the gig would never start. That would make this optimisation the very + // thing it promises never to be: the reason you cannot play. Give up + // waiting and let the first play extract lazily, exactly as before. + const ctrl = new AbortController(); + const timer = setTimeout(() => ctrl.abort(), PREPARE_TIMEOUT_MS); + try { + await fetch(`${API}/gigs/prepare`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ songs: prop.songs.map((s) => s.filename) }), + signal: ctrl.signal, + }); + } catch (_) { + // abort, offline, non-2xx — all the same: start the gig anyway. + } finally { + clearTimeout(timer); + if (btn) { btn.disabled = false; if (label) btn.textContent = label; } + } + } + + async function startGig(btn) { const prop = _ppGigProposal; const q = window.feedBack && window.feedBack.playQueue; if (!prop || !q || typeof q.start !== 'function' || typeof window.playSong !== 'function') return; + + // Extract the setlist BEFORE the stage is borrowed and the queue starts, + // so a failure here leaves nothing half-applied to unwind. + await prepareGigSongs(prop, btn); + // The poster's Play could have been cancelled while we were unpacking. + if (_ppGigProposal !== prop) return; + // The gig BORROWS the stage: stash whatever venue/viz the user had so // the set ending gives it back (unlike "Play here", which is an // explicit persistent choice on the venue card). @@ -1146,7 +1192,21 @@ if (typeof window.setViz === 'function') window.setViz('venue'); } catch (_) { /* viz optional — restore stays intact */ } } + // Push the gig's venue pack to the crowd layer NOW. + // + // crowd.setManifest(venue) is reached only through pushCrowdManifest, + // and pushCrowdManifest is called only from refresh() — the career + // tab's own reload. A gig navigates AWAY from the career tab to the + // player, so refresh() never runs during it, and setting the override + // above does nothing on its own. The result the testers saw: the venue + // visualization turns on (3D highway) but its crowd/stage pack never + // loads, so the song plays over the bare highway backdrop ("standard + // particles"), or over whatever venue a previous refresh() happened to + // leave applied. We just changed the override to this gig's venue, so + // re-push for it. _state is the career state the booking screen already + // fetched; guard for the rare null. _appliedManifestVenue = null; + if (_state) pushCrowdManifest(_state); _ppGigRun = { songs: prop.songs, venue_id: prop.venue_id, @@ -1424,7 +1484,7 @@ } const gigBtn = e.target.closest('[data-pp-gig]'); if (gigBtn) { bookGig(gigBtn.dataset.ppGig); return; } - if (e.target.closest('[data-pp-gig-play]')) { startGig(); return; } + if (e.target.closest('[data-pp-gig-play]')) { startGig(e.target.closest('[data-pp-gig-play]')); return; } if (e.target.closest('[data-pp-gig-reroll]')) { if (_ppGigProposal) bookGig(_ppGigProposal.genre_key); return; diff --git a/plugins/highway_3d/screen.js b/plugins/highway_3d/screen.js index c5ba0d6..be2e03b 100644 --- a/plugins/highway_3d/screen.js +++ b/plugins/highway_3d/screen.js @@ -12726,8 +12726,19 @@ const tC = now + (dt0 + dt1) * 0.5 - BEHIND; const b = laneBoundsFromAnchor(getChartAnchorAt(anchors, tC)); if (!b) continue; - const z0 = dZ(dt0) + TS * BEHIND; - const z1 = dZ(dt1) + TS * BEHIND; + // The lane STOPS AT THE HIT LINE (z = 0) — issue #991. The + // slice window starts BEHIND seconds in the past, so the + // first slices map to positive z, i.e. past the hit line + // toward the player. Nothing is ever drawn there: notes and + // chord frames clamp to Math.min(0, dZ(dt)), so that strip + // is lane with nothing on it. Clamp the NEAR edge only — + // the far edge stays at dZ(AHEAD+BEHIND)+TS*BEHIND = -AHEAD*TS, + // aligned with the note horizon, exactly as before. + const z0 = Math.min(0, dZ(dt0) + TS * BEHIND); + const z1 = Math.min(0, dZ(dt1) + TS * BEHIND); + // Slice lies entirely past the hit line -> zero length, nothing + // to draw. Skip before the arp probe so it costs nothing. + if (z0 === z1) continue; const arpSlice = (laneRailArpHsFlags && handShapesRails && handShapesRails.length) ? arpeggioLaneOuterRailLaneSlice( dt0, dt1, now, @@ -12876,9 +12887,13 @@ divMin = dMin; divMax = dMax; - // Same fix: extend to AHEAD+BEHIND so far edge = -AHEAD*TS. - const laneLen = TS * (AHEAD + BEHIND); - const zLane = -laneLen / 2 + TS * BEHIND; + // Far edge at -AHEAD*TS (the note horizon), near edge at the + // hit line (z = 0) — the lane does not run past it toward the + // player, where nothing is ever drawn (#991). Spanning + // AHEAD+BEHIND and shifting by +TS*BEHIND put the near edge at + // +TS*BEHIND; spanning AHEAD alone keeps the same far edge. + const laneLen = TS * AHEAD; + const zLane = -laneLen / 2; const laneOp = (HWY_LANE_STRIPE_OP_BASE + highwayIntensity * HWY_LANE_STRIPE_OP_INT) * (_venueSceneOverride ? VENUE_LANE_OP_BOOST : 1); mLaneOdd.opacity = laneOp; @@ -12898,7 +12913,8 @@ } if (highwayIntensity > 0.05) { - const divLen = TS * (AHEAD + BEHIND); + // Matches the lane above: ends at the hit line (#991). + const divLen = TS * AHEAD; const yPos = boardY + 0.03 * K; const divOp2 = 0.02 + highwayIntensity * 0.1; const divOpArp2 = Math.min(0.92, 0.16 + highwayIntensity * 0.42); @@ -12911,7 +12927,7 @@ for (let f = fDivA; f <= fDivB; f++) { if (hwyLaneArpOuterDividers && (f === fDivA || f === fDivB)) continue; const div = pLaneDivider.get(); - div.position.set(xFret(f), yPos, dZ(0) - divLen * 0.5 + TS * BEHIND); + div.position.set(xFret(f), yPos, -divLen * 0.5); div.material = mLaneDivider; div.scale.set(1, 1, divLen); div.renderOrder = 2; @@ -12930,8 +12946,10 @@ // ── Fret boundary extension lines ───────────────────────── if (mLaneDividerExt && fretDividersVisible) { - const extLaneLen = TS * (AHEAD + BEHIND); - const extZMid = -extLaneLen / 2 + TS * BEHIND; + // Same hit-line stop as the lane (#991) — otherwise these lines + // would be the only floor geometry still running past it. + const extLaneLen = TS * AHEAD; + const extZMid = -extLaneLen / 2; const extYPos = boardY + 0.03 * K; mLaneDividerExt.opacity = Math.max(0.3, 0.3 + highwayIntensity * 0.15); for (let f = 0; f <= NFRETS; f++) { @@ -15568,15 +15586,29 @@ // highway throttled the whole room. Pausing the song dropped the // venue, the crowd and the stage to 10 fps. // - // Only claim continuous frames while a crowd video is actually - // rolling: with no venue pack (the common case) the paused scene IS - // static and the throttle should still save the GPU. + // Two independent sources of motion, and BOTH must keep their frames: + // + // • a crowd video rolling on its own clock (career venue pack), and + // • the venue scene's own fake-depth motion — the backdrop breathes, + // the haze drifts, warmth pulses, the shimmer moves. That is + // Math.sin(t) in the draw loop (see _venueApplyFakeDepthMotion), + // so it only moves while we are actually given frames, and it runs + // with NO pack at all. + // + // The throttle fires whenever the CHART CLOCK is stalled — which is + // not just a pause. A count-in and the credits/author overlay stall it + // exactly the same way, so the venue was stuttering there too. + // + // With no venue at all (plain 3D highway) the paused scene really is a + // still picture: motion mode reads 'off', we claim nothing, and the + // throttle still saves the GPU as #654 intended. needsContinuousFrames() { if (!_isReady || _ctxLost) return false; for (const v of _venueCrowdVideos) { if (v && !v.paused && !v.ended && v.readyState >= 2) return true; } - return false; + // 'off' also covers prefers-reduced-motion and "no venue scene". + try { return _venueEffectiveMotionMode() !== 'off'; } catch (_) { return false; } }, draw(bundle) { diff --git a/server.py b/server.py index ee13aa9..ecb6863 100644 --- a/server.py +++ b/server.py @@ -1115,7 +1115,10 @@ async def startup_status_stream(request: Request): @app.post("/api/rescan") def trigger_rescan(): """Manually trigger a library rescan.""" - if not scan.kick_scan(): + # force=True: a manual Refresh must skip the directory-signature fast path — + # it is the escape hatch for the one change dir mtimes can't see (a pack + # rewritten in place under the same name). + if not scan.kick_scan(force=True): return {"message": "Scan already in progress"} return {"message": "Rescan started"} @@ -1133,7 +1136,7 @@ def trigger_full_rescan(): # delete_missing() prunes anything genuinely gone at the end. meta_db.conn.execute("UPDATE songs SET mtime = -1") meta_db.conn.commit() - if not scan.kick_scan(): + if not scan.kick_scan(force=True): return {"message": "Scan already in progress"} return {"message": "Full rescan started"} diff --git a/static/app.js b/static/app.js index 7ea343f..d306259 100644 --- a/static/app.js +++ b/static/app.js @@ -1334,12 +1334,25 @@ if (window.feedBack) window.feedBack.closeCurrentSong = closeCurrentSong; // leaving the player still leaves — and abandons the queue. window.feedBack.playQueue = (function () { let list = [], idx = -1, source = '', arrangements = null; + // Set true by _play() right before it drives playSong, consumed once by + // playSong's clear-guard. The primary "don't clear the queue I'm driving" + // signal is options.fromQueue, but a chain of plugin playSong wrappers + // (nam_tone, midi_amp, fretboard, invert_highway, tabview, ...) forward only + // (filename, arrangement) and silently drop the options object — so the flag + // never arrived and the queue cleared itself the instant its first song + // started (a gig/album/playlist never advanced). This flag rides beside the + // wrapper chain, not through it. + let _internalPlay = false; const active = () => idx >= 0 && idx < list.length; const hasNext = () => active() && idx < list.length - 1; function clear() { list = []; idx = -1; source = ''; arrangements = null; } function _play(i) { const fn = list[i]; - // fromQueue keeps the queue from clearing itself; playSong decodeURIs. + // fromQueue is the in-band signal; _internalPlay is the out-of-band one + // that survives wrapper chains dropping the options arg. Both set; either + // suffices. playSong runs its clear-guard synchronously at entry, and the + // wrapper chain reaches it synchronously, so the flag is still set then. + _internalPlay = true; window.playSong(encodeURIComponent(fn), arrangements ? arrangements[i] : undefined, { fromQueue: true }); } function start(files, opts) { @@ -1371,6 +1384,15 @@ window.feedBack.playQueue = (function () { } return { start: start, advance: advance, hasNext: hasNext, active: active, clear: clear, + // True when the current song is a queue ADVANCE (song 2..N of a set), + // false for its first song or a standalone play. The venue uses this to + // fly in once on arrival at the set, then continue the room between + // songs instead of replaying the arrival flyover every track. + isContinuation: function () { return active() && idx > 0; }, + // One-shot: true iff _play just kicked off this playSong. Consumed on + // read so a later MANUAL play still clears the queue. playSong calls this + // instead of trusting options.fromQueue to survive the wrapper chain. + _consumeInternalPlay: function () { const v = _internalPlay; _internalPlay = false; return v; }, source: function () { return source; }, remaining: function () { return active() ? list.length - idx - 1 : 0; }, // What's coming, for consumers that RENDER the queue (a results @@ -2297,11 +2319,14 @@ configureHost({ currentFilename: () => currentFilename, }); +// `esc` is here for out-of-tree plugins only: their screen.js loads as a classic +// script and called esc() back when app.js was one too and it was an implicit +// global. Nothing in core reads window.esc — import it from ./js/dom.js instead. Object.assign(window, { _confirmDialog, _getArrangementNamingMode, _libraryLocalFilename, _librarySongArtUrl, _librarySongId, _onHeaderClick, _onNamingModeChange, _trapFocusInModal, changeArrangement, checkPluginUpdates, clearLibFilters, clearLoop, - deleteSelectedLoop, exportDiagnostics, exportSettings, filterFavorites, + deleteSelectedLoop, esc, exportDiagnostics, exportSettings, filterFavorites, filterLibrary, fullRescanLibrary, goFavPage, handleSliderInput, hideScanBanner, importSettings, loadPlugins, loadSavedLoop, loadSettings, onSectionPracticeModeChange, openEditModal, persistSetting, diff --git a/static/highway.js b/static/highway.js index cc74ae4..cb8811c 100644 --- a/static/highway.js +++ b/static/highway.js @@ -986,6 +986,22 @@ function createHighway() { // inline arrow function. function _handleAsyncInitFailure(e) { if (hwState._renderer !== _installedRenderer) return; + // ...and ignore a rejection from a SUPERSEDED init cycle. + // + // A renderer mints a fresh readyPromise on every init(), and + // rejects the previous one ("superseded") when a newer init + // starts. The renderer object is unchanged, so the identity + // check above does not catch it — and we would tear down a + // perfectly healthy renderer that is merely re-initialising. + // + // This is exactly what starting a gig did: setViz('venue') + // installed the 3D renderer, then the queue's playSong() + // re-initialised it a tick later; init #1's promise rejected, + // and the gig dropped to the fallback 2D highway with the + // venue gone. A superseded init is not a failed init — the + // NEW cycle owns the outcome, and its own promise is what we + // must judge. + if (_installedRenderer.readyPromise !== rp) return; console.error('renderer async init failure:', e); _destroyCurrentIfInited(); hwState._renderer = _defaultRenderer; diff --git a/static/js/session.js b/static/js/session.js index 5d6daa2..4e56e0b 100644 --- a/static/js/session.js +++ b/static/js/session.js @@ -638,9 +638,18 @@ export let artAbortController = null; export async function playSong(filename, arrangement, options) { console.log('playSong called:', filename); // A manual (non-queue) play abandons any active play-queue, so a stale queue - // can't hijack the next song's end. The queue passes fromQueue to keep itself. - if ((!options || !options.fromQueue) && window.feedBack && window.feedBack.playQueue) { - window.feedBack.playQueue.clear(); + // can't hijack the next song's end. The queue signals a play it is DRIVING + // two ways: options.fromQueue (in-band) and _consumeInternalPlay() (out-of- + // band). The out-of-band one exists because plugin playSong wrappers forward + // only (filename, arrangement) and drop the options object — with just the + // in-band flag, the queue cleared itself the instant its first song played + // and a gig never advanced. Consume the flag whether or not we go on to clear, + // so it can't leak into a later manual play. + const _pq = window.feedBack && window.feedBack.playQueue; + const _queueDriven = (options && options.fromQueue) + || (_pq && typeof _pq._consumeInternalPlay === 'function' && _pq._consumeInternalPlay()); + if (!_queueDriven && _pq) { + _pq.clear(); } if (!options || options.bridge !== false) { _recordPlaybackBridge('playback.window-play-song', 'window.playSong', 'legacy playSong entry point used'); diff --git a/static/v3/venue-crowd.js b/static/v3/venue-crowd.js index fbb2c5d..a0a8506 100644 --- a/static/v3/venue-crowd.js +++ b/static/v3/venue-crowd.js @@ -529,7 +529,27 @@ _loadingLoop = null; _fadingLoop = null; if (_venueActive && _manifest) { - if (!playIntro()) showLoop(machine.current, FADE_MS); + // The flyover is ARRIVING at the venue, and you arrive once. Songs + // 2..N of a set (a gig / album / playlist) are a NEW song but the + // SAME arrival — the camera should not fly in from the back of the + // room before every track (tester: "it showed the flyover intro + // again" on a gig's second song). Continue the room to the new song's + // loop; only a first-song / standalone arrival flies in. + if (_isSetContinuation()) showLoop(machine.current, FADE_MS); + else if (!playIntro()) showLoop(machine.current, FADE_MS); + } + } + + // Is this song load a continuation of a play queue (a set already in + // progress), rather than an arrival? True for song 2..N of a gig/album/ + // playlist. The queue owns the answer; treat any error / absent queue as + // "not a continuation" so a standalone play still flies in. + function _isSetContinuation() { + try { + const q = window.feedBack && window.feedBack.playQueue; + return !!(q && typeof q.isContinuation === 'function' && q.isContinuation()); + } catch (_) { + return false; } } diff --git a/tests/browser/plugin-globals-contract.spec.ts b/tests/browser/plugin-globals-contract.spec.ts new file mode 100644 index 0000000..0915016 --- /dev/null +++ b/tests/browser/plugin-globals-contract.spec.ts @@ -0,0 +1,63 @@ +// The window globals are a THIRD-PARTY CONTRACT. Pin them. +// +// Out-of-tree plugins load their screen.js as a CLASSIC script and call these +// as bare globals. Nothing in core reads most of them, so a call-graph scan, +// ESLint's no-undef, and a grep all come back clean while the plugin breaks in +// the field. This is the frontend twin of tests/test_plugin_context_contract.py +// — same reasoning, same literal-list rule. +// +// This guard is retroactive: `esc` was an implicit global back when app.js was +// a classic script, went module-scoped in a9fce29, and got carved into +// js/dom.js in 14b4058. The re-export list at the bottom of app.js was rebuilt +// without it, and the MIDI plugin's device list threw "esc is not defined" for +// testers — reported as "MIDI Access denied", because the ReferenceError landed +// in a try/catch meant for permission failures. +// +// WHY A LITERAL LIST AND NOT A DERIVED ONE. Deriving the expected set from +// app.js would assert the code equals itself. The point is that a human has to +// look at a diff and consciously agree to change the contract. + +import { test, expect } from '@playwright/test'; + +const PLUGIN_GLOBALS = [ + '_confirmDialog', '_getArrangementNamingMode', '_libraryLocalFilename', '_librarySongArtUrl', + '_librarySongId', '_onHeaderClick', '_onNamingModeChange', '_trapFocusInModal', + 'changeArrangement', 'checkPluginUpdates', 'clearLibFilters', 'clearLoop', + 'deleteSelectedLoop', 'esc', 'exportDiagnostics', 'exportSettings', 'filterFavorites', + 'filterLibrary', 'fullRescanLibrary', 'goFavPage', 'handleSliderInput', + 'hideScanBanner', 'importSettings', 'loadPlugins', 'loadSavedLoop', + 'loadSettings', 'onSectionPracticeModeChange', 'openEditModal', 'persistSetting', + 'pickDlcFolder', 'pinCurrentArrangementDefault', 'playSong', 'previewDiagnostics', + 'previewEditArt', 'renderGridCards', 'renderTreeInto', 'rescanLibrary', + 'retuneSong', 'saveCurrentLoop', 'saveSettings', 'seekBy', + 'setAvOffsetMs', 'setFavView', 'setInstrumentPathway', 'setLibView', + 'setLibraryProvider', 'setLoopEnd', 'setLoopStart', 'setMastery', + 'setSpeed', 'setViz', 'showScreen', 'sortFavorites', + 'sortLibrary', 'syncLibrarySong', 'toggleAllArtists', 'toggleAllFavoriteArtists', + 'toggleLibFilters', 'togglePlay', 'toggleSectionPracticePopover', 'uiPrompt', + 'updatePlugin', 'uploadSongs', + 'filterFavTreeLetter', 'filterTreeLetter', 'goFavTreePage', 'goTreePage', +]; + +test('plugin-facing window globals are all callable', async ({ page }) => { + await page.goto('/'); + await page.waitForSelector('.screen.active', { timeout: 10000 }); + + const missing = await page.evaluate( + (names) => names.filter((n) => typeof (window as any)[n] !== 'function'), + PLUGIN_GLOBALS, + ); + + expect(missing, `window globals plugins depend on are missing or not functions: ${missing.join(', ')}`).toEqual([]); +}); + +// The plugin call site that actually broke: esc() interpolated into a template +// string. A global that exists but doesn't escape is its own bug. +test('window.esc escapes HTML metacharacters', async ({ page }) => { + await page.goto('/'); + await page.waitForSelector('.screen.active', { timeout: 10000 }); + + const escaped = await page.evaluate(() => (window as any).esc('')); + expect(escaped).not.toContain(' { // Degrades without the crowd layer (PR1 not merged / older desktop). assert.match(src, /typeof crowd\.setManifest !== 'function'\) return/); }); + +// feedBack#… (tester): "Venue doesn't load when starting song from passport. +// Loads standard particles." crowd.setManifest(venue) is reached ONLY through +// pushCrowdManifest, and pushCrowdManifest is called ONLY from refresh() (the +// career tab's own reload). A gig navigates away from that tab, so refresh() +// never runs during it — the venue viz turns on but its crowd/stage pack never +// loads. startGig must push the manifest itself after setting the override. +test('startGig pushes the crowd manifest for the gig venue', () => { + const fs = require('node:fs'); + const path = require('node:path'); + const src = fs.readFileSync( + path.join(__dirname, '..', '..', 'plugins', 'career', 'screen.js'), 'utf8'); + const start = src.indexOf('async function startGig('); + assert.ok(start !== -1, 'startGig not found'); + const open = src.indexOf('{', src.indexOf(')', start)); + let depth = 1, i = open + 1; + while (i < src.length && depth > 0) { const ch = src[i]; if (ch === '{') depth++; else if (ch === '}') depth--; i++; } + const fn = src.slice(start, i); + // The override is set, then the manifest must be (re)pushed for it. + const overrideIdx = fn.search(/VENUE_OVERRIDE_KEY,\s*prop\.venue_id/); + const pushIdx = fn.search(/pushCrowdManifest\s*\(/); + assert.ok(overrideIdx !== -1, 'startGig must set the venue override'); + assert.ok(pushIdx !== -1, + 'startGig must push the crowd manifest — refresh() (its only other caller) ' + + 'never runs during a gig, so the venue pack would never load'); + assert.ok(overrideIdx < pushIdx, 'the manifest must be pushed AFTER the override is set to the gig venue'); +}); diff --git a/tests/js/gig_song_end.test.js b/tests/js/gig_song_end.test.js new file mode 100644 index 0000000..40ab479 --- /dev/null +++ b/tests/js/gig_song_end.test.js @@ -0,0 +1,98 @@ +// A gig is a SET, not a run of unrelated songs. +// +// Reported from a live gig: the player finished the first song and had to sit +// through the per-song results popup before the next one would start, and then +// wait again while that song was extracted from its feedpak zip. +// +// This file covers the CORE half — career pre-extracts the whole setlist before +// the first note. The other half (note_detect must not show its per-song summary +// inside a gig) lives in the note_detect plugin repo, which is not part of this +// checkout: plugins/*/ is gitignored here and note_detect ships from +// feedBack-plugin-notedetect. A test reading it from core would pass on a dev +// box (where the plugin happens to be bundled) and fail in CI, which is worse +// than no test. +// +// The pre-extraction is tested for REAL behaviour — actually unpacking zips — in +// tests/plugins/career/test_routes.py. These are the wiring guards around it. + +'use strict'; + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..', '..'); +const CAREER = fs.readFileSync(path.join(ROOT, 'plugins', 'career', 'screen.js'), 'utf8'); +const CAREER_ROUTES = fs.readFileSync(path.join(ROOT, 'plugins', 'career', 'routes.py'), 'utf8'); + +function extractBlock(src, signature) { + const start = src.indexOf(signature); + assert.ok(start !== -1, `signature '${signature}' not found`); + const openBrace = src.indexOf('{', start); + let depth = 1; + let i = openBrace + 1; + while (i < src.length && depth > 0) { + const ch = src[i]; + if (ch === '{') depth++; + else if (ch === '}') depth--; + i++; + } + assert.ok(depth === 0, `unbalanced braces after '${signature}'`); + return src.slice(start, i); +} + +test('startGig extracts the whole setlist before starting the queue', () => { + const fn = extractBlock(CAREER, 'async function startGig('); + const prepIdx = fn.search(/await\s+prepareGigSongs\s*\(/); + const startIdx = fn.search(/q\.start\s*\(/); + assert.ok(prepIdx !== -1, 'startGig must pre-extract the set'); + assert.ok(startIdx !== -1, 'q.start not found'); + assert.ok(prepIdx < startIdx, + 'the set must be unpacked BEFORE the queue starts — otherwise the player ' + + 'waits between songs, which is the bug'); +}); + +test('the stage is only borrowed once the set is ready', () => { + const fn = extractBlock(CAREER, 'async function startGig('); + const prepIdx = fn.search(/await\s+prepareGigSongs\s*\(/); + const stageIdx = fn.search(/VENUE_OVERRIDE_KEY/); + assert.ok(prepIdx < stageIdx, + 'a gig cancelled while unpacking must not leave the venue/viz overwritten'); + assert.match(fn, /_ppGigProposal\s*!==\s*prop/, + 'a proposal dismissed while unpacking must not then start a gig'); +}); + +test('pre-extraction never blocks the gig from starting', () => { + const fn = extractBlock(CAREER, 'async function prepareGigSongs('); + assert.match(fn, /catch\s*\(/, + 'a failed prepare must fall through to the old lazy extraction, not abort the gig'); +}); + +test('the prepare route degrades instead of failing', () => { + assert.match(CAREER_ROUTES, /def prepare_gig/, 'prepare route missing'); + assert.match(CAREER_ROUTES, /context\.get\(\s*["']get_dlc_dir["']\s*\)/, + 'a host without the library resolvers must degrade, not 500 — pre-extraction ' + + 'is an optimisation and can never be why a gig will not start'); +}); + +// ── the prepare must never be able to BLOCK the gig (CodeRabbit, #971) ────── +// +// A bare `await fetch(...)` only rejects on a network error. A server that +// accepts the connection and then never answers hangs forever — and the gig +// would never start. That would make this optimisation the exact thing it +// promises never to be: the reason you cannot play. + +test('the prepare fetch is bounded — a hung server cannot block the gig', () => { + const fn = extractBlock(CAREER, 'async function prepareGigSongs('); + assert.match(fn, /AbortController/, 'the request must be abortable'); + assert.match(fn, /setTimeout\([\s\S]{0,40}abort\s*\(\s*\)/, + 'a hung request must be aborted, not awaited forever'); + assert.match(fn, /signal:\s*ctrl\.signal/, 'the signal must actually be passed to fetch'); + assert.match(fn, /clearTimeout/, 'the timer must be cleared on the happy path'); + assert.match(CAREER, /const\s+PREPARE_TIMEOUT_MS\s*=\s*\d+/, 'the ceiling must be named'); + // The button must be restored however we leave — otherwise a timeout strands + // the poster on "Preparing set…" with Play disabled: unplayable. + assert.match(fn, /finally\s*\{[\s\S]{0,220}btn\.disabled\s*=\s*false/, + 'the Play button must be re-enabled on EVERY path, including the abort'); +}); diff --git a/tests/js/highway_pause_throttle.test.js b/tests/js/highway_pause_throttle.test.js index bc5ffbc..57baf51 100644 --- a/tests/js/highway_pause_throttle.test.js +++ b/tests/js/highway_pause_throttle.test.js @@ -114,13 +114,52 @@ test('the capability probe fails closed (absent / non-function / throwing)', () 'only an explicit true opts out — a truthy accident must not disable the throttle'); }); -test('3D highway claims continuous frames only while a crowd video is rolling', () => { +test('3D highway claims continuous frames for BOTH sources of venue motion', () => { const h3d = fs.readFileSync( path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'screen.js'), 'utf8'); const fn = extractBlock(h3d, 'needsContinuousFrames()'); + // (1) a crowd video rolling on its own clock (career venue pack) assert.match(fn, /_venueCrowdVideos/, 'must key off the actual crowd video elements'); - assert.match(fn, /\.paused/, 'a paused video is a still frame — throttle should still apply'); - // With no venue pack (the common case) the paused scene really is static and - // the GPU saving must survive: the method has to be able to return false. - assert.match(fn, /return false;/, 'must fall through to false with no live video'); + assert.match(fn, /\.paused/, 'a paused video is a still frame'); + // (2) the venue scene's OWN fake-depth motion — backdrop breathe, haze drift, + // warmth pulse, shimmer. Math.sin(t) in the draw loop, so it only moves while + // we get frames, and it runs with NO pack at all. Missing this meant the venue + // still stuttered on pause / count-in / credits whenever no video was rolling. + assert.match(fn, /_venueEffectiveMotionMode\s*\(\s*\)\s*!==\s*'off'/, + 'the venue scene animates without any video — it must claim frames too'); + // ...and with no venue at all the paused scene IS static: the #654 GPU saving + // must survive, so the method has to be able to return false. + assert.match(fn, /return false;/, 'must fall through to false on a plain 3D highway'); +}); + +// ── a SUPERSEDED init is not a FAILED init ────────────────────────────────── +// +// Starting a gig dropped the player onto the fallback 2D highway with no venue. +// +// setViz('venue') installs the 3D renderer, whose init is async; the gig then +// immediately starts its play queue, and playSong() re-initialises that same +// renderer a tick later. A renderer mints a fresh readyPromise per init() and +// rejects the previous one with "superseded" — but highway.js only checked that +// the RENDERER object was unchanged, which it is. So it treated a healthy +// re-initialising renderer as a failed one, tore it down, and reverted to 2D: +// +// renderer async init failure: Error: superseded +// viz picker: reverted to default renderer (async-init-failure) +// +// Reproduced and fixed against the real build (venue stays selected, scene +// active, no viz:reverted). + +test('a superseded readyPromise must not revert the viz to 2D', () => { + const src = highwaySources(); + const fn = extractBlock(src, 'function _handleAsyncInitFailure(e)'); + assert.match(fn, /readyPromise\s*!==\s*rp[\s\S]{0,40}return/, + 'a rejection from a STALE readyPromise (the renderer has since re-init\'d) must be ' + + 'ignored — otherwise a re-initialising renderer is torn down as if it had failed'); + // The renderer-identity check must survive too: a rejection belonging to a + // renderer that has since been REPLACED is also not our problem. + assert.match(fn, /hwState\._renderer\s*!==\s*_installedRenderer[\s\S]{0,20}return/, + 'the renderer-identity guard must remain'); + // ...and a genuine failure of the CURRENT init cycle must still revert. + assert.match(fn, /_emitVizReverted\s*\(\s*'async-init-failure'\s*\)/, + 'a real async-init failure must still fall back to the default renderer'); }); diff --git a/tests/js/play_queue_peek.test.js b/tests/js/play_queue_peek.test.js index 8f0cbbc..0abe2ca 100644 --- a/tests/js/play_queue_peek.test.js +++ b/tests/js/play_queue_peek.test.js @@ -49,3 +49,80 @@ test('peekNext is null after clear', () => { q.clear(); assert.strictEqual(q.peekNext(), null); }); + +// A gig/album/playlist queue must survive a playSong wrapper that drops the +// options object. +// +// The queue tells playSong "don't clear the queue I'm driving" via +// options.fromQueue. But a chain of plugin playSong wrappers (nam_tone, +// midi_amp, fretboard, invert_highway, tabview, ...) forward only +// (filename, arrangement) and silently drop the 3rd arg. With just the in-band +// flag, playSong cleared the queue the instant its first song started, so a gig +// never advanced (feedBack#… tester: "Passports does not advance in the song +// queue"). The queue now also raises an out-of-band flag, _consumeInternalPlay(), +// which playSong honours regardless of the wrapper chain. + +// The real clear-guard from session.js, driven against the queue. +function clearGuard(win, options) { + const pq = win.feedBack && win.feedBack.playQueue; + const queueDriven = (options && options.fromQueue) + || (pq && typeof pq._consumeInternalPlay === 'function' && pq._consumeInternalPlay()); + if (!queueDriven && pq) pq.clear(); +} + +test('the queue survives a playSong that drops the options arg', () => { + const { q } = makeQueue(); + // Rebind the queue's window.playSong to a wrapper that forwards ONLY + // (filename, arrangement) — exactly the plugin bug — and runs the real guard. + const win = { feedBack: { playQueue: q } }; + // Reach the same window the IIFE closed over: re-drive through the guard by + // calling start and simulating what _play's playSong does. + // We can't rebind the closed-over window, so instead assert the out-of-band + // signal directly: _play sets it, and the guard consumes it. + q.start(['a.sloppak', 'b.sloppak', 'c.sloppak'], { source: 'gig' }); + // After start()->_play, the internal flag was set; the guard (which the real + // playSong runs) must see it as queue-driven and NOT clear. + win.feedBack.playQueue = q; + clearGuard(win, undefined /* wrapper dropped options */); + assert.strictEqual(q.active(), true, 'a dropped options arg must not clear the queue'); + assert.strictEqual(q.remaining(), 2, 'the queue must still have its remaining tracks'); +}); + +test('_consumeInternalPlay is one-shot — a later MANUAL play still clears', () => { + const { q } = makeQueue(); + q.start(['a.sloppak', 'b.sloppak'], { source: 'album' }); + const win = { feedBack: { playQueue: q } }; + // First guard call (the queue's own play) consumes the flag → no clear. + clearGuard(win, undefined); + assert.strictEqual(q.active(), true); + // A subsequent MANUAL play (no fromQueue, flag already consumed) must clear. + clearGuard(win, undefined); + assert.strictEqual(q.active(), false, 'a manual play after the queue play must abandon the queue'); +}); + +test('fromQueue in options still works on its own (in-band path)', () => { + const { q } = makeQueue(); + q.start(['a.sloppak', 'b.sloppak'], { source: 'gig' }); + // consume the internal flag first so ONLY options.fromQueue is under test + q._consumeInternalPlay(); + const win = { feedBack: { playQueue: q } }; + clearGuard(win, { fromQueue: true }); + assert.strictEqual(q.active(), true, 'options.fromQueue alone must still keep the queue'); +}); + +// isContinuation(): true for song 2..N of a set, false for the first song / a +// standalone play. The venue uses it to fly in once on arrival, then carry the +// room between songs instead of replaying the arrival flyover every track +// (tester: "it showed the flyover intro again" on a gig's second song). +test('isContinuation is false on the first song, true after advancing', () => { + const { q } = makeQueue(); + assert.strictEqual(q.isContinuation(), false, 'idle queue is not a continuation'); + q.start(['a.sloppak', 'b.sloppak', 'c.sloppak'], { source: 'gig' }); + assert.strictEqual(q.isContinuation(), false, 'the FIRST song of a set is an arrival, not a continuation'); + q.advance(); + assert.strictEqual(q.isContinuation(), true, 'song 2 is a continuation — no re-flyover'); + q.advance(); + assert.strictEqual(q.isContinuation(), true, 'song 3 too'); + q.clear(); + assert.strictEqual(q.isContinuation(), false, 'a cleared queue is not a continuation'); +}); diff --git a/tests/js/venue_scope.test.js b/tests/js/venue_scope.test.js index af9564b..79f9ba3 100644 --- a/tests/js/venue_scope.test.js +++ b/tests/js/venue_scope.test.js @@ -117,3 +117,22 @@ test('a throwing document does not take the venue down with it', () => { global.document = prev; } }); + +// The arrival flyover must NOT replay for songs 2..N of a set. onSongLoaded +// consults the play queue: a continuation (gig/album/playlist song 2+) carries +// the room over with a loop crossfade, only an arrival plays the intro. +test('a set continuation carries the room over instead of re-flying-in', () => { + const fs = require('node:fs'); + const path = require('node:path'); + const src = fs.readFileSync(path.join(__dirname, '..', '..', 'static', 'v3', 'venue-crowd.js'), 'utf8'); + const start = src.indexOf('function onSongLoaded('); + const open = src.indexOf('{', src.indexOf(')', start)); + let depth = 1, i = open + 1; + while (i < src.length && depth > 0) { const ch = src[i]; if (ch === '{') depth++; else if (ch === '}') depth--; i++; } + const fn = src.slice(start, i); + const contIdx = fn.search(/_isSetContinuation\s*\(\s*\)/); + const introIdx = fn.search(/playIntro\s*\(/); + assert.ok(contIdx !== -1, 'onSongLoaded must consult the set-continuation signal'); + assert.ok(introIdx !== -1, 'the intro must still exist for a real arrival'); + assert.ok(contIdx < introIdx, 'the continuation check must gate the flyover — a set song 2+ must not fly in'); +}); diff --git a/tests/plugins/career/test_passports.py b/tests/plugins/career/test_passports.py index 8687565..5b09a23 100644 --- a/tests/plugins/career/test_passports.py +++ b/tests/plugins/career/test_passports.py @@ -445,3 +445,30 @@ def test_gold_intake_rejects_junk(client, meta_db): res = client.post("/api/plugins/career/drill-state", json={"byNode": {}, "goldImprov": blob}) assert res.status_code == 413 + + +def test_gig_includes_songs_played_on_another_instrument(client, meta_db): + # feedBack#… (tester): "Metalcore says 137 songs, only shows 1 in the gig list". + # A song played on a DIFFERENT instrument's arrangement has a stats row, so it + # was excluded from the unplayed filler — and its played bucket is that other + # instrument's, not this passport's — so it fell into a gap and could never be + # gigged. A guitar passport with a library of bass-played metalcore got a 404. + for i in range(137): + meta_db.add(f"mc{i}.feedpak", 0, 0.80, genre="Metalcore", arrangements=BASS) + res = client.post("/api/plugins/career/gigs/propose", + json={"instrument": "guitar", "genre": "Metalcore", "size": 4}) + assert res.status_code == 200, "a full library of the genre must never 404" + assert len(res.json()["songs"]) == 4, "the gig must fill from the library, not the gap" + + +def test_gig_reroll_changes_the_set(client, meta_db): + # feedBack#… (tester): "Passport re-roll does not change songs". A set drawn + # from the filler used to be the library's first N in table order, every time. + for i in range(40): + meta_db.add_song_only(f"un{i}.feedpak", genre="Metalcore") + sets = set() + for _ in range(5): + r = client.post("/api/plugins/career/gigs/propose", + json={"instrument": "guitar", "genre": "Metalcore", "size": 4}) + sets.add(tuple(sorted(s["filename"] for s in r.json()["songs"]))) + assert len(sets) > 1, "re-roll must be able to produce a different set" diff --git a/tests/plugins/career/test_routes.py b/tests/plugins/career/test_routes.py index e1ed626..1830f7a 100644 --- a/tests/plugins/career/test_routes.py +++ b/tests/plugins/career/test_routes.py @@ -174,3 +174,176 @@ def test_double_download_409s(client, monkeypatch): career_routes._state["downloads"]["bar"] = {"status": "running"} assert client.post("/api/plugins/career/packs/bar/download").status_code == 409 assert client.delete("/api/plugins/career/packs/bar").status_code == 409 + + +# ── gig pre-extraction (the wait between songs) ───────────────────────────── +# +# A feedpak is a zip: the first play of one pays for its extraction into +# sloppak_cache. Inside a set that cost landed BETWEEN songs — the player +# finished a number and then sat waiting for the next one to unpack, mid-gig. +# The setlist is known up front, so extract it all while the poster is up. + +def _career_client_with_library(tmp_path, meta_db, dlc, cache): + from fastapi import FastAPI + from fastapi.testclient import TestClient + import routes as career_routes + app = FastAPI() + career_routes.setup(app, { + "config_dir": str(tmp_path), + "meta_db": meta_db, + "get_dlc_dir": lambda: dlc, + "get_sloppak_cache_dir": lambda: cache, + }) + return TestClient(app) + + +def _write_feedpak(dlc, name, title="T"): + """A minimal but REAL feedpak zip, so resolve_source_dir genuinely unpacks.""" + import json as _json + import zipfile as _zip + p = dlc / name + with _zip.ZipFile(p, "w") as z: + z.writestr("manifest.json", _json.dumps({"title": title, "artist": "A", "arrangements": []})) + return p + + +def test_gig_prepare_extracts_every_song_up_front(tmp_path, meta_db): + dlc = tmp_path / "dlc" + dlc.mkdir() + cache = tmp_path / "cache" + cache.mkdir() + for n in ("one.feedpak", "two.feedpak", "three.feedpak"): + _write_feedpak(dlc, n) + + client = _career_client_with_library(tmp_path, meta_db, dlc, cache) + before = list(cache.iterdir()) + assert before == [], "nothing unpacked yet" + + res = client.post("/api/plugins/career/gigs/prepare", + json={"songs": ["one.feedpak", "two.feedpak", "three.feedpak"]}) + assert res.status_code == 200 + body = res.json() + assert body["ok"] is True + assert body["prepared"] == 3, body + assert body["failed"] == [] + # The point of the whole exercise: the set is on disk BEFORE the first note. + assert len(list(cache.iterdir())) == 3, "every song of the set must be unpacked" + + +def test_gig_prepare_is_idempotent_on_a_warm_cache(tmp_path, meta_db): + dlc = tmp_path / "dlc" + dlc.mkdir() + cache = tmp_path / "cache" + cache.mkdir() + _write_feedpak(dlc, "one.feedpak") + client = _career_client_with_library(tmp_path, meta_db, dlc, cache) + + first = client.post("/api/plugins/career/gigs/prepare", json={"songs": ["one.feedpak"]}).json() + second = client.post("/api/plugins/career/gigs/prepare", json={"songs": ["one.feedpak"]}).json() + assert first["prepared"] == second["prepared"] == 1 + assert len(list(cache.iterdir())) == 1, "a re-prepare must not duplicate the unpack" + + +def test_one_bad_feedpak_does_not_stop_the_set(tmp_path, meta_db): + # A corrupt pak in the setlist must not block the gig: the play itself will + # surface the error exactly as it does outside a gig. Slow beats blocked. + dlc = tmp_path / "dlc" + dlc.mkdir() + cache = tmp_path / "cache" + cache.mkdir() + _write_feedpak(dlc, "good.feedpak") + (dlc / "bad.feedpak").write_bytes(b"not a zip at all") + + client = _career_client_with_library(tmp_path, meta_db, dlc, cache) + body = client.post("/api/plugins/career/gigs/prepare", + json={"songs": ["good.feedpak", "bad.feedpak"]}).json() + assert body["ok"] is True, "a bad pak must not fail the whole prepare" + assert body["prepared"] == 1 + assert body["failed"] == ["bad.feedpak"] + + +def test_gig_prepare_degrades_without_a_library(tmp_path, meta_db, client): + # The stock fixture's context has no dlc/cache resolvers. That must be a + # graceful no-op, not a 500 — pre-extraction is an optimisation and can + # never be the reason a gig won't start. + res = client.post("/api/plugins/career/gigs/prepare", json={"songs": ["x.feedpak"]}) + assert res.status_code == 200 + assert res.json()["prepared"] == 0 + + +def test_gig_prepare_empty_setlist(tmp_path, meta_db, client): + res = client.post("/api/plugins/career/gigs/prepare", json={"songs": []}) + assert res.status_code == 200 + assert res.json() == {"ok": True, "prepared": 0, "failed": []} + + +def test_prepare_rejects_a_non_list_songs_value(tmp_path, meta_db, client): + # A str is iterable: without the list check, "abc" would prepare three + # one-character "songs". + for bad in ("abc", 42, {"a": 1}, None): + res = client.post("/api/plugins/career/gigs/prepare", json={"songs": bad}) + assert res.status_code == 200 + assert res.json()["prepared"] == 0 + + +def test_prepare_ignores_non_string_and_blank_entries(tmp_path, meta_db): + dlc = tmp_path / "dlc" + dlc.mkdir() + cache = tmp_path / "cache" + cache.mkdir() + _write_feedpak(dlc, "good.feedpak") + client = _career_client_with_library(tmp_path, meta_db, dlc, cache) + body = client.post("/api/plugins/career/gigs/prepare", + json={"songs": ["good.feedpak", "", " ", 7, None, {"x": 1}]}).json() + assert body["prepared"] == 1 + assert body["failed"] == [] + + +def test_prepare_caps_the_setlist(tmp_path, meta_db): + # This endpoint unpacks zips — an arbitrary caller must not be able to ask for + # unbounded work. + # + # The first version of this test asserted `prepared == 0` against a fixture + # with NO library: the endpoint exits before extraction there, so it passed + # whether or not the cap existed. Give it a real library, ask for far more than + # the cap, and assert the endpoint only ever considered MAX_GIG_SONGS of them. + import routes as career_routes + assert career_routes.MAX_GIG_SONGS <= 64 + + dlc = tmp_path / "dlc" + dlc.mkdir() + cache = tmp_path / "cache" + cache.mkdir() + client = _career_client_with_library(tmp_path, meta_db, dlc, cache) + + n = career_routes.MAX_GIG_SONGS + 50 + # None of these exist, so every song the endpoint LOOKS AT lands in `failed`. + # That makes `failed` an exact count of how many it considered. + body = client.post("/api/plugins/career/gigs/prepare", + json={"songs": [f"missing{i}.feedpak" for i in range(n)]}).json() + assert body["prepared"] == 0 + assert len(body["failed"]) == career_routes.MAX_GIG_SONGS, ( + f"the endpoint must consider at most MAX_GIG_SONGS " + f"({career_routes.MAX_GIG_SONGS}), not all {n}" + ) + + +def test_prepare_refuses_to_escape_the_library(tmp_path, meta_db): + # resolve_source_dir() does a bare `dlc_root / filename` with no containment + # guard, so a crafted path would walk straight out of the library. Every + # filename must go through _resolve_dlc_path first. + dlc = tmp_path / "dlc" + dlc.mkdir() + cache = tmp_path / "cache" + cache.mkdir() + (tmp_path / "outside.feedpak").write_bytes(b"secret") + client = _career_client_with_library(tmp_path, meta_db, dlc, cache) + + for evil in ("../outside.feedpak", "..\\outside.feedpak", + "a/../../outside.feedpak", "/etc/passwd", "C:/Windows/x.feedpak"): + body = client.post("/api/plugins/career/gigs/prepare", + json={"songs": [evil]}).json() + assert body["prepared"] == 0, f"{evil!r} must never be prepared" + assert body["failed"] == [evil] + # Nothing outside the library may have been unpacked. + assert list(cache.iterdir()) == [] diff --git a/tests/test_feedpak_extension.py b/tests/test_feedpak_extension.py index f8aeb3d..2f556fe 100644 --- a/tests/test_feedpak_extension.py +++ b/tests/test_feedpak_extension.py @@ -137,6 +137,81 @@ def test_background_scan_discovers_both_suffixes(tmp_path, scan_server): assert "ignore.zip" not in seen +# ── 2b. directory-signature fast path (skip the full re-stat) ──────────────── + +def test_dir_signature_fast_path_skips_unchanged_tree(tmp_path, scan_server): + """After a full scan records the library-dir signature, a second scan with + an unchanged tree takes the fast path and does NOT re-glob/extract — but a + forced scan (manual Refresh) always does the full pass, and a new song + (which bumps the dir mtime) reverts to a full pass on its own.""" + import unittest.mock as mock + + dlc = tmp_path / "dlc" + dlc.mkdir() + (dlc / "a.feedpak").write_bytes(b"") + (tmp_path / "config.json").write_text('{"dlc_dir": "%s"}' % dlc) + + scan = importlib.import_module("scan") + seen: list[str] = [] + + def mock_extract(f, dlc_dir): + seen.append(f.name) + return {"title": f.name, "artist": "", "album": ""} + + with mock.patch("scan_worker._extract_meta_for_file", new=mock_extract): + # 1) first pass: full scan, extracts a.feedpak (+ seeded builtins), + # records the signature + scan.background_scan() + assert "a.feedpak" in seen + assert scan._dir_signature_file().exists() + + # 2) unchanged tree: fast path — no glob, no extraction at all + seen.clear() + scan.background_scan() + assert seen == [] + assert scan.status()["stage"] == "complete" + + # 3) a new song bumps the dlc mtime → signature mismatch → full pass + # picks it up on its own (no manual Refresh needed for adds) + (dlc / "b.feedpak").write_bytes(b"") + seen.clear() + scan.background_scan() + assert "b.feedpak" in seen + + # 4) force=True (Refresh) bypasses the fast path even on a settled tree: + # with the signature now current, a plain scan skips, a forced one lists + seen.clear() + scan.background_scan() # fast path + assert seen == [] + forced_listed = [] + real_delete_missing = scan.appstate.meta_db.delete_missing + def _spy(files): + forced_listed.append(set(files)) + return real_delete_missing(files) + with mock.patch.object(scan.appstate.meta_db, "delete_missing", new=_spy): + scan.background_scan(force=True) + assert forced_listed, "force=True must run the full listing pass" + + +def test_dir_signature_tracks_directory_form_song_own_dir(tmp_path): + """A directory-form song (loose folder / directory bundle) records its OWN + directory in the signature, so an in-place file change inside it — which + bumps that folder's mtime but not its parent's — invalidates the fast path. + A file-form sloppak (a plain .feedpak zip) is not a dir and adds nothing.""" + scan = importlib.import_module("scan") + dlc = tmp_path / "dlc" + (dlc / "packs").mkdir(parents=True) + loose = dlc / "packs" / "my_loose_song" # directory-form song + loose.mkdir() + zipped = dlc / "packs" / "zipped.feedpak" # file-form song + zipped.write_bytes(b"") + + rels = scan._library_dirs([loose, zipped], dlc) + assert "packs/my_loose_song" in rels, "directory-form song must track its own dir" + assert "packs" in rels and "." in rels + assert "packs/zipped.feedpak" not in rels, "a file-form sloppak is not a tracked dir" + + # ── 3. POST /api/songs/upload gate (endpoint) ──────────────────────────────── @pytest.fixture() diff --git a/tests/test_song_info_stems.py b/tests/test_song_info_stems.py new file mode 100644 index 0000000..dad4365 --- /dev/null +++ b/tests/test_song_info_stems.py @@ -0,0 +1,140 @@ +"""`/api/song/{f}?stems=1` — the playable stem list, for preloading. + +The stems plugin could only learn its stem list from the highway's WS `ready`, +which arrives once the highway is already up. So it decoded the stems and then +copied the whole song's PCM to its audio worklet with the player on screen — +half a gigabyte of memcpy in one frame, ~700 ms, which froze the venue video. + +Given the list at `song:loading` it can do all of that BEFORE the highway +appears, behind the loading overlay where a stall costs nothing. + +The safety property these tests exist for: the REST payload must be the SAME +list the WS builds. If they disagree, the plugin preloads one graph and then +throws it away and rebuilds another — strictly worse than not preloading. So +they are pinned against each other, not just against a snapshot. +""" + +import zipfile + +import yaml + +import sloppak + + +def _pak(tmp_path, stems, full=None, name="song.feedpak", original_audio=None): + manifest = { + "title": "T", "artist": "A", "duration": 10.0, + "arrangements": [], + "stems": stems + ([full] if full else []), + } + if original_audio: + # The deprecated pre-1.15.0 shape: the mixdown lives outside `stems`. + manifest["original_audio"] = original_audio + p = tmp_path / name + with zipfile.ZipFile(p, "w") as z: + # Real packs carry manifest.yaml — a JSON manifest is not read at all. + z.writestr("manifest.yaml", yaml.safe_dump(manifest)) + # _legacy_full_mix only returns a path that actually EXISTS on disk. + if original_audio: + z.writestr(original_audio, b"\0" * 16) + return p + + +def _payload(tmp_path, pak): + from routers.song import _playable_stems_payload + import appstate + cache = tmp_path / "cache" + cache.mkdir(exist_ok=True) + appstate.sloppak_cache_dir = cache + return _playable_stems_payload(pak.name, tmp_path) + + +def _ws_payload(tmp_path, pak): + """Rebuild the WS `ready` stems payload exactly as ws_highway.py does.""" + from urllib.parse import quote + cache = tmp_path / "cache" + cache.mkdir(exist_ok=True) + loaded = sloppak.load_song(pak.name, tmp_path, cache) + q = quote(pak.name, safe="") + return { + "stems": [ + {"id": s["id"], "url": f"/api/sloppak/{q}/file/{quote(s['file'])}", + "default": s["default"]} + for s in loaded.stems + ], + "full_mix_url": f"/api/sloppak/{q}/file/{quote(loaded.full_mix)}" if loaded.full_mix else None, + } + + +def test_default_resolution_is_shared_with_load_song(): + assert sloppak.stem_default_on(True) is True + assert sloppak.stem_default_on(False) is False + assert sloppak.stem_default_on("off") is False + assert sloppak.stem_default_on("false") is False + assert sloppak.stem_default_on("0") is False + assert sloppak.stem_default_on("no") is False + assert sloppak.stem_default_on("on") is True + assert sloppak.stem_default_on(1) is True + + +def test_rest_matches_the_ws_for_a_reserved_full_stem(tmp_path): + pak = _pak(tmp_path, + [{"id": "guitar", "file": "stems/guitar.ogg"}, + {"id": "vocals", "file": "stems/vocals.ogg", "default": "off"}], + full={"id": "full", "file": "stems/full.ogg"}, + name="Iron Maiden - Phantom.feedpak") + rest = _payload(tmp_path, pak) + assert rest == _ws_payload(tmp_path, pak) + assert [s["id"] for s in rest["stems"]] == ["guitar", "vocals"], "the mixdown is not a layer" + assert rest["full_mix_url"].endswith("stems/full.ogg") + assert rest["stems"][1]["default"] is False + + +def test_rest_matches_the_ws_for_a_LEGACY_original_audio_pack(tmp_path): + """The one CodeRabbit caught, and the one that matters most in practice. + + load_song falls back to the DEPRECATED `original_audio:` key when a pack has + no reserved `full` stem — which is every pack written before feedpak 1.15.0, + i.e. most of a real library. My first version of this payload reimplemented + the full-mix rule from extract_meta and silently returned None for them: REST + would say "no full mix" while the WS said there was one. The plugin would then + preload a graph WITHOUT the pristine mix and, because the signature still + matched, never rebuild — unity playback silently downgraded to the lossy + recombination. + + The payload now calls load_song itself, so this cannot drift. Pinned anyway. + """ + pak = _pak(tmp_path, [ + {"id": "guitar", "file": "stems/guitar.ogg"}, + {"id": "bass", "file": "stems/bass.ogg"}, + ], name="Legacy Pack.feedpak", original_audio="original/full.ogg") + + rest = _payload(tmp_path, pak) + assert rest == _ws_payload(tmp_path, pak) + assert rest["full_mix_url"] is not None, ( + "a pre-1.15.0 pack's full mix must survive — dropping it downgrades unity " + "playback to the lossy stem recombination, silently" + ) + assert rest["full_mix_url"].endswith("original/full.ogg") + + +def test_rest_matches_the_ws_for_a_single_full_pack(tmp_path): + # Its ONE stem IS the mixdown: nothing to be pristine against, so `full` stays + # the sole playable stem and no separate mixdown is surfaced. + pak = _pak(tmp_path, [{"id": "full", "file": "stems/full.ogg"}], name="Single.feedpak") + rest = _payload(tmp_path, pak) + assert rest == _ws_payload(tmp_path, pak) + assert [s["id"] for s in rest["stems"]] == ["full"] + assert rest["full_mix_url"] is None + + +def test_a_broken_pack_yields_an_empty_list_not_an_error(tmp_path): + # Preloading is an optimisation: an unreadable pack must fall back to the + # normal WS-driven path, never break the song-info request. + from routers.song import _playable_stems_payload + import appstate + cache = tmp_path / "cache" + cache.mkdir() + appstate.sloppak_cache_dir = cache + (tmp_path / "bad.feedpak").write_bytes(b"not a zip") + assert _playable_stems_payload("bad.feedpak", tmp_path) == {"stems": [], "full_mix_url": None}