mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-21 12:21:49 +00:00
main
236 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Matthew Harris Glover
|
270cb39f41
|
Enable Linux nightly AppImage auto-update in the System settings UI (#999)
Some checks are pending
ship-ci / ci (push) Waiting to run
* fix(settings): enable Linux nightly AppImage auto-update in System settings
Fixes the Settings → System "App updates" panel so it actually works on
Linux, and adds Nightly as a selectable channel — previously missing
entirely, so Linux self-update couldn't be reached from this UI at all.
- The channel dropdown no longer gets permanently disabled the moment
the desktop bridge reports 'unsupported', which is the normal state
whenever the channel isn't Nightly on Linux. It stays enabled so the
user can switch to Nightly, the only way out of that state.
- Shows live download progress ("Downloading update… N%") and an
explicit button state machine (Check → grayed out while busy →
Restart now once staged), instead of a frozen "Checking…" during the
~1.5GB background download.
- Renders every status update from the triggering action's own return
value (checkNow()/setChannel()'s result) rather than a separate
follow-up getStatus() call, which can race against other state
changes and show a stale result even after a real success.
- setupAppUpdates() no longer re-syncs the channel to the backend on
every Settings-panel re-render — only once per page load — so a
redundant sync can no longer stomp an in-flight download's state.
- Routes update-flow events into the existing diagnostics.js
console-capture + contribute() snapshot API, so the user's existing
"Export Diagnostics" button now captures the full update decision
trace end to end — no new UI or log file. This diagnostic tracing is
what actually root-caused the bugs above, from real on-device
captures rather than guesswork.
Companion PR in feedBack-desktop (the underlying update engine).
Verified end-to-end on a Steam Deck: channel switch → check → live
download progress → restart button → relaunch onto the new build,
confirmed via a real Export Diagnostics capture showing a clean,
fully-accounted-for trace.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(settings): extract + unit-test the app-update status view; dedupe diag log
- Extract the status→UI state machine from renderFrom into a pure, exported
_appUpdateStatusView() (DOM-free) and cover it with tests/js — settings.js's
large module graph made importing it for a full harness impractical, so the
pure function is the testable seam. Behavior-preserving; renderFrom applies
the returned shape to the DOM exactly as before.
- Dedupe the [update-diag] renderFrom console line so the ~1.5s download poll
no longer floods the diagnostics ring buffer with byte-identical entries;
every real state/percent change still logs, and the structured contribute()
snapshot stays unconditional.
Left the 'audio_engine' diagnostics key as-is: the server export filters
client contributions to loaded plugin ids (diagnostics_bundle.py path-traversal
guard), so a dedicated key would be silently dropped from the bundle.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
---------
Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
|
||
|
Matthew Harris Glover
|
23c509322b
|
Add gamepad/controller support (#1001)
* feat(input): add gamepad/controller support
Adds full gamepad/controller navigation and playback control, driven
by requests from players who use fee[dB]ack on a TV/console setup and
from wheelchair users for whom a controller is far more convenient
than a keyboard + mouse. Confirmed working end-to-end on a Steam Deck
across several rounds of on-device testing.
- static/v3/gamepad.js: polls navigator.getGamepads() and dispatches
synthetic keydown events (Arrow/Enter/Space/Escape) on the focused
element (falling back to document), reusing the app's existing
keyboard pipeline (static/js/shortcuts.js's scope-aware dispatcher,
player shortcuts, text-field/modal guards) instead of a parallel
action-mapping table. Only acts on gamepads reporting the W3C
"standard" mapping — which is what Steam Input presents for the
Deck's built-in controls, both in Gaming Mode and in Desktop Mode
via a non-Steam shortcut — so button order is guaranteed correct
and a non-standard/raw device safely no-ops instead of misfiring.
Handles Steam Input's virtual-pad duplicates (a real controller
plus 1-2 mirrored XInput slots) without spamming connect toasts or
losing input when the live pad isn't at index 0. Xbox-style face
button mapping: bottom face = Space (play/pause, and activates the
focused control), right face = Escape (back), top face reveals the
player screen's tool rail (focuses it into visibility via the
existing CSS :focus-within rule). D-pad/stick repeat while held,
mirroring OS keyboard auto-repeat.
- static/v3/gamepad-nav.js: fills the one real gap in that reuse
strategy — no screen but the song library grid had any arrow-key
navigation, and Chromium doesn't run native Enter/Space button
activation for untrusted synthetic events even when dispatched at
the focused element. Gated entirely on `!e.isTrusted`, so it only
ever reacts to gamepad-originated events and never touches real
keyboard/mouse users: emulates Tab-order (the sidebar + active
screen's real, already-focusable buttons/links) for Arrow keys,
explicitly .click()s the focused element for Enter/Space, and gives
Escape a consistent "go back" behavior — an existing in-screen back
button if one's visible (reusing each screen's own drill-down logic
for free), else the main menu. Every branch defers via
`e.defaultPrevented` to any screen that already handles the key
itself (the song grid, the player, settings), so nothing here
overrides existing behavior.
- static/v3/songs.js: adds real 2D d-pad/arrow-key navigation to the
song library's virtualized grid (only a slice of the library is
ever in the DOM), including fetching/scrolling off-screen rows into
view and correcting for the sticky filter toolbar's occlusion.
- static/v3/index.html: wires up the two new scripts.
* chore: regenerate stale tailwind.min.css
Rebuilt in a fresh clone (not the local working copy). Several plugin
directories (audio_engine, plugin_manager, community_charts, etc.) are
gitignored locally but present on disk from checking out plugin repos
for local dev/testing — Tailwind's content scan picks them up
regardless, so a rebuild against the contaminated local working copy
bakes in extra utility classes that don't belong in the real,
git-tracked build. A clean checkout reproduces CI's expected output
exactly.
* fix(gamepad): check all matching back buttons, not just the first
document.querySelector on the combined [data-ap-back], [data-albums-back],
#v3-pl-back selector only ever inspects the first match in DOM order —
since screens stay in the DOM (hidden, not removed) when you navigate
away, a hidden back button from an unrelated screen could sort before
the one that's actually visible, incorrectly falling through to
showScreen('v3-home') instead of clicking it. Uses querySelectorAll +
find(visible) instead.
* fix(gamepad): address CodeRabbit findings on connect/disconnect and grid nav
- gamepad.js: anyLiveConnectedPad -> anyLiveStandardPad, filtering by
mapping === 'standard' like firstLiveStandardPad already does, and
applied at the top of the gamepadconnected handler too. A still-
connected non-standard raw mirror could otherwise mask the real
pad's disconnect (toast never fires, polling never stops).
- songs.js _gpMove: an unset cursor now always seeds at index 0
before the first press, instead of applying that press's delta
immediately (ArrowDown/Right previously skipped straight past row
0; Left/Up only looked right by accident of clamping). Matches the
existing convention in shortcuts.js's legacy _handleLibArrowNav.
- songs.js _gpBlockedTarget: form-control/button blocking now
requires the element to be visible (offsetParent !== null), not
just present. Screens stay in the DOM hidden (not removed) when you
navigate away, so a real button focused on some other now-hidden
screen could leave document.activeElement pointing at it and block
all grid navigation indefinitely. (An el.closest('#v3-songs') scope
was tried first and reverted — it fixed that case but broke
blocking for the topbar search input, which lives outside
#v3-songs's DOM subtree even while v3-songs is active; visibility
is the distinction that actually matters, not DOM nesting.)
Skipped two CodeRabbit suggestions, verified against current code:
gating songs.js's grid keydown listener to synthetic-only events
would regress the real keyboard accessibility this PR intentionally
added (v3-songs' grid had none before); renaming the _gp* helpers to
drop their underscore prefix would break from this codebase's own
established module-private naming convention.
Verified in-browser: first arrow press lands on index 0, stale hidden
focus no longer blocks grid nav, the topbar search input still
correctly blocks it, and normal nav resumes after blur.
* test(gamepad): unit-cover the controller + nav state machines
- gamepad.test.js (10): standard-mapping filter, Steam Input duplicate-slot
dedup, disconnect masking, button edge-detection, d-pad/stick repeat timing,
analog deadzone — driven via a fake navigator + manual rAF queue.
- gamepad_nav.test.js (10): !isTrusted/defaultPrevented gating, arrow focus
traversal + clamping, hidden-element skipping, Enter/Space click activation
(not into text fields/body), Escape visible-back-button vs home fallback.
songs.js grid nav is left to on-device coverage (async + windowed-DOM heavy).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
---------
Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Joe Hallahan
|
05be9ebdbe
|
Add new chart-transform plugin capability (#1000)
* Chart-transform plugin capability * PR comments * Cleanup * Fix markdown * CodeRabbit feedback Signed-off-by: Joe <jphinspace@gmail.com> --------- Signed-off-by: Joe <jphinspace@gmail.com> Co-authored-by: Byron Gamatos <xasiklas@gmail.com> |
||
|
K. O. A.
|
39d1a8cb9b
|
feat(v3): one-click "Not split" library filter + piano stem pill (#1010)
Finding un-split songs took five taps (cycle each stem pill to its "lacks" state) — and was quietly wrong even then: the drawer offered five of the canonical six stems, so a piano-only song lacked all five listed and matched a hand-built "not split" filter despite being split. The stems section gains a "Not split" toggle that sets stem_lacks to every instrument stem in one tap (the same lacks-ALL query Stem Splitter's missing-stems view runs, backend semantics unchanged), and piano joins the pill row (already in the backend's allowed set). (Rebuilt on current main after #1003/#810/#92e78be rewrote the drawer region — the original branch conflicted whole-file.) Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
ChrisBeWithYou
|
1745b13ba7
|
feat(playlists): flag songs that are not in your current tuning (#1009)
* feat(playlists): flag songs that are not in your current tuning
Making the library's tuning filter instrument-aware does not repair playlists
already built under the old guitar-first behaviour. Those keep their
wrong-tuning songs, so a player still hits a surprise retune mid-practice and
reasonably concludes nothing was fixed.
Adds a per-playlist check: each row is marked against the player's current
tuning, with a summary ("3 of 24 songs are not in your tuning"), a filter to
show only those, and an explicit removal that lists every affected song by
title and states they stay in the library. Flagging is the feature -- nothing
is ever removed without being asked for, and removal reuses the existing
per-song DELETE rather than adding a bulk destructive endpoint.
Reuses the tuner capability's coverage report and `window.feedBack
.workingTuning`, the same pair the library cards already score against,
rather than introducing another source of truth.
Two deliberate departures:
- A coverage report reads "not covered" both for a real mismatch and for a
bail-out it could not evaluate. Only a report carrying an actual reason
counts as a mismatch; the rest render as unknown. This differs from the
library grid, which paints every not-covered song amber -- acceptable on a
grid, not on a hand-curated playlist where a false warning costs trust.
- With no tuning perspective available it makes no claim at all, rather than
defaulting to guitar and reproducing the original bug in a new place.
Playlist rows carry `tuning_offsets` and `bass_only`; a tuning *name* cannot
be scored, since two "Custom Tuning" rows are different tunings.
Fully correct once the instrument-aware tuning filter lands. That dependency
is confined to `rowTuningForCheck()` in static/v3/playlists.js, marked SEAM.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFDokqh2H6mEjk1Kgbi6JW
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
* build(tailwind): regenerate for the playlist tuning-check classes
CI's tailwind-fresh gate rebuilds static/tailwind.min.css and hard-fails if
the committed file differs. The new chip/summary/filter markup introduces
classes the previous build never saw.
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
* fix(playlists): stay within the shipped Tailwind class set
Reverts the regenerated static/tailwind.min.css and reworks the tuning-check
markup to use only classes already in the committed sheet.
Regenerating that file is not reproducible off CI: nothing pins tailwindcss,
autoprefixer or caniuse-lite, so a local `npx -y tailwindcss@3.4.19` resolves
different browser data and rewrites unrelated bytes -- a clean checkout of
main rebuilds with the -webkit-backdrop-filter prefixes dropped. Committing
that output fails the tailwind-fresh gate no matter how many times it is
regenerated.
Six utilities were new: bg-fb-good/10, border-fb-accent/50,
hover:bg-fb-accent/10, list-disc, list-inside, max-h-48, plus gap-x-3/gap-y-2.
Substituted bg-fb-good/30, the amber border already used by the mismatch
state, hover:bg-fb-card, a literal bullet in a div, max-h-32 and gap-3. Visual
intent is unchanged.
The removal-confirm test pinned the <li> markup; it now accepts either
wrapper, since what it guards is that every song is named and escaped ahead
of any DELETE, not which element wraps it.
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
* Use instrument tuning in playlist checks
---------
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
ChrisBeWithYou
|
cc75cb876a
|
fix(library): tuning filter answers for your instrument, not always guitar (#1003)
* fix(library): tuning filter answers for your instrument, not always guitar The library indexed exactly one tuning per song, chosen guitar-first (lead > rhythm > combo, bass only as a last resort), and nothing consulted the player's instrument. A bassist filtering by tuning was shown the guitar chart's tuning, so playlists built by tuning contained songs needing a retune. Reported by a tester building bass practice sets; Covet "Shibuya" is the clean case, with a custom guitar tuning over a standard bass chart. Indexes each arrangement role's own tuning and makes the facet, filter, sort and labels answer for one perspective. `guitar-lead` reads the original unprefixed columns and adds no payload keys, so the default response is unchanged. The same defect existed inside guitar -- lead and rhythm charts can disagree -- so perspective is three-valued (guitar-lead, guitar-rhythm, bass) driven by one PERSPECTIVES table rather than parallel column families. Songs with no chart for the perspective fall back to the song-level tuning rather than vanishing (18 of 59 packs in the test library have no bass chart), but the fallback is marked inferred in the facet counts and on the row instead of being silently coalesced. "Only real charts" reuses the existing `arrangements_has` filter rather than adding one. Bass-specific handling, from measured content: - Bass tuning arrays are padded to six entries; charts never reference string index 4 or 5. Truncated to four before naming and grouping. - Grouping uses a canonical open-pitch key, so [-2,0,0,0] and [-2,0,0,0,0,0] are one facet row instead of two. - Offsets above +1 semitone are refused a name. Bassists tune down, near never up; one pack ships [5,5,5,5,4,4] (A-D-G-C, unplayable, and its own notes sit in the song's real key under standard tuning). Naming that would send a player to retune to a tuning that does not exist. Rhythm deliberately does not truncate -- padding is a bass finding, and cutting a seven-string array would invent a tuning the chart lacks. Adds an opt-in `tuning_match=playable` mode alongside exact match: a chart is offered when your lowest open pitch is at or below its lowest open pitch, so a five-string bass covers four-string standard and drop-D with no retune. Open strings only -- note range is not indexed and the scan stays manifest-only -- so it fails conservative: unknown low pitch is excluded, and the upper bound is unchecked and documented rather than guessed. Existing installs would otherwise never populate: the tree-signature fast path reports "unchanged" forever on a settled library. Rows with NULL marker columns re-extract, and the fast path is disabled until that backfill converges (writes use '' rather than NULL, so it self-clears). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFDokqh2H6mEjk1Kgbi6JW Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> * test(v3): accept the tuning-perspective indirection in the badge guard The album-art badge now reads shownTuningName(), so the source-pattern guard no longer matched the inline `tuning_name || tuning` form and CI went red. Accept the helper, and pin the helper's own fallback in a companion test so the guard still fails if a guitar player's tuning label is ever dropped. Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> --------- Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
f0d9c3abc0
|
feat(playlists): manual drag order + Sort A-Z for the playlist list (#1004)
Playlists could only ever be listed alphabetically (system playlists first). Users who group playlists by purpose had no way to put the ones they reach for daily at the front. Adds a nullable `position` column and orders by `(system_key IS NULL), (position IS NULL), position, name COLLATE NOCASE`, so manually-ordered playlists lead, unpositioned ones keep sorting alphabetically behind them, and system playlists stay pinned first. Drag-reorder mirrors the existing within-playlist song reorder, adapted for grid tiles (insert side decided on the horizontal midpoint since tiles flow left-to-right then wrap). System playlists are neither drag sources nor drop targets. `POST /api/playlists/reorder` requires an exact permutation of the current non-system ids, so a duplicate, omission, extra, unknown id, or a system id is rejected rather than silently producing duplicate positions; booleans are rejected explicitly because `sorted([True, 2]) == sorted([1, 2])` would otherwise slip through the permutation check. `POST /api/playlists/sort-alpha` clears the manual order again. Claude-Session: https://claude.ai/code/session_01SFDokqh2H6mEjk1Kgbi6JW Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
3717e4338d
|
fix(plugins): restore window.esc for out-of-tree plugins (#986)
Some checks are pending
ship-ci / ci (push) Waiting to run
app.js exported `esc` as an implicit global back when it was a classic
script. a9fce29 made it an ES module and
|
||
|
Byron Gamatos
|
2f2a095e4c
|
fix(venue): fly in once per set, not before every song (#978)
Some checks are pending
ship-ci / ci (push) Waiting to run
Tester, mid-gig: "the second song in the gig started when the first one ended. But it showed the flyover intro again." The flyover is arriving at the venue, and you arrive once. #968 stopped it replaying on an arrangement SWITCH (same filename), but a gig's song 2 is a genuinely different file, so it took the full-teardown path and played the arrival flyover again — the camera flew in from the back of the room before every track of the set. The play queue now answers isContinuation(): false for the first song of a set (or a standalone play — an arrival), true for song 2..N. onSongLoaded carries the room over to the new song's loop on a continuation, and only a real arrival plays the intro. Verified on the built AppImage: isContinuation goes false (song 1) -> true (song 2) across an advance, and song 2 no longer flies in. Also confirmed NOT a bug, same session: "didn't show the author for the second song." The credits card shows on a queue advance whenever the song carries authors — reproduced with a song that has them as the advanced-to track. The tester's song 2 simply had no `authors:` metadata (most auto-converted feedpaks don't). No code change. Tests: isContinuation across start/advance/clear, and that onSongLoaded gates the flyover on the continuation check. Both fail on pre-fix source. JS 1214/1214. |
||
|
Byron Gamatos
|
e14ef64224
|
fix(playback): the song queue must survive a playSong wrapper that drops options (#977)
Some checks are pending
ship-ci / ci (push) Waiting to run
Tester: "Passports does not advance in the song queue." The play queue tells playSong "don't clear the queue I'm driving" by passing options.fromQueue. But window.playSong is wrapped by a CHAIN of plugins — nam_tone, midi_amp, fretboard, invert_highway, tabview — and each wrapper forwards only (filename, arrangement), silently dropping the options object. So fromQueue never reached playSong: it cleared the queue the instant its first song started, and a gig/album/playlist never advanced. Reproduced on the real build via a queue.start + a hooked clear(): the queue went inactive with 0 remaining immediately after start, and the clear stack ran through nam_tone -> midi_amp -> invert_highway -> fretboard -> session.js. Fixing six plugin wrappers is whack-a-mole and the next plugin re-breaks it. Fix it at the source instead: the queue raises an out-of-band flag (_consumeInternalPlay, one-shot) beside the wrapper chain, not through it, and playSong's clear-guard honours it. options.fromQueue stays as the in-band path. The flag is consumed on read so a later MANUAL play still abandons the queue. Verified on the real build: the gig queue stays active after start and advances on song:ended (Iron Maiden -> Blind Guardian), and a manual play still clears. Tests drive the real clear-guard against the queue for: a dropped-options wrapper (the bug), the one-shot manual-play-still-clears invariant, and the in-band fromQueue path on its own. All 3 fail on the pre-fix source. JS 1211/1211. |
||
|
Byron Gamatos
|
917d81c2d2
|
fix(highway): a SUPERSEDED renderer init is not a FAILED one (#970)
Starting a gig dropped the player onto the fallback 2D highway with no venue.
startGig() calls setViz('venue'), which installs the 3D renderer — whose init is
async — and then immediately starts its play queue. playSong() re-initialises
that same renderer a tick later. A renderer mints a fresh readyPromise per
init() and rejects the previous one with "superseded"; 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)
The guard now also checks the PROMISE identity: a rejection from an init cycle
the renderer has already moved on from is ignored. The renderer-identity guard
stays (a rejection for a renderer since REPLACED is also not ours), and a
genuine failure of the CURRENT cycle still reverts — both init() call sites go
through _setRenderer, which re-wires the handler every time, so the new cycle is
always watched.
Reproduced and fixed against the real build:
before: vizSelection=default viz-picker=default venue=inactive viz:reverted
after: vizSelection=venue viz-picker=venue venue=ACTIVE (no revert)
Also widens the paused-frame throttle's opt-out. The throttle fires whenever the
CHART CLOCK is stalled — not only on a pause, but through a count-in and the
credits/author overlay too. Its opt-out only asked "is a crowd video rolling",
but the venue scene animates on a clock of its own with no pack at all (backdrop
breathe, parallax, haze drift, warmth pulse — Math.sin(t) in the draw loop), so
that motion was still being throttled. It now claims frames for both sources; a
plain 3D highway with no venue reads motion mode 'off' and keeps the #654 GPU
saving.
HONEST CAVEAT on that second part: I could not get the throttle to fire in a
reproduction. A control run on the shipped code showed 100 draws/sec while
paused, not the ~10/sec a firing throttle would give — so the change is
defensible on its own terms (a stalled clock is genuinely not a static picture)
but it does NOT have a demonstrated symptom behind it. The viz fix above does.
Tests: the superseded guard, and that the throttle opt-out covers both motion
sources. All fail against the pre-fix source. eslint 0 errors; JS 1207/1207.
|
||
|
Byron Gamatos
|
4e0e3c5417
|
fix(venue/highway): flyover replay on arrangement switch, venue on Virtuoso, and the paused throttle starving the venue (#968)
* fix(venue): don't replay the flyover on an arrangement switch; keep the venue off other screens Two bugs from a live career session. 1. CHANGING ARRANGEMENT REPLAYED THE ARRIVAL FLYOVER. changeArrangement() reloads the song through the normal load path, so highway.js re-emits `song:loaded` — same filename, new arrangement. The venue could not tell that from a fresh arrival, so it reset the machine and flew the camera in from the back of the room again, mid-set, every time the player switched lead -> rhythm. The player is already on stage. onSongLoaded now compares the filename. A repeat of the song already on stage keeps the video pipeline running and only re-syncs the mood: the performance restarts, so the loop follows the reset machine with a quiet crossfade, never the intro. A genuinely different song still gets the full teardown + flyover. 2. THE VENUE SHOWED UP ON THE VIRTUOSO HIGHWAY. The venue was gated purely on `isVenueViz()` — the selected visualization, which is a GLOBAL preference and says nothing about what is on screen. Virtuoso borrows the same highway_3d renderer for its practice charts, so with Venue selected it inherited the backdrop: the crowd and the stage behind a chromatic exercise. Selecting Venue is a preference for the PLAYER; it is not a licence to paint the venue over whatever else happens to be using the renderer. The venue is now gated on viz AND screen (`shouldBeActive`), and follows `screen:changed` — it tears down on leaving the player and rebuilds on return. Nothing else changes: stop() already unbinds the videos from the renderer, so deactivating is enough to clear the backdrop. Tests: both decisions exposed as pure predicates and pinned — arrangement switch vs new song (including the first load, and a malformed payload that must not suppress the flyover forever), and the venue's screen scope. The existing syncViz test encoded the OLD contract (activate regardless of screen), so it now states the new one and additionally asserts the venue does NOT activate on virtuoso. Includes a guard test: with Venue selected AND on the player, the venue IS active — without it, every "not active" assertion could pass vacuously. All 8 new/updated assertions fail against the pre-fix source. eslint clean; JS 1199/1199; pytest 2597 passed. * fix(highway): the paused-frame throttle was throttling the whole venue Pausing the song dropped the venue, the crowd and the stage to ~10 fps — "everything around the highway drops fps by a lot". draw() caps paused frames to one per _PAUSED_FRAME_INTERVAL_MS (100ms), on an assumption stated plainly in highway-constants.js: a heavy WebGL renderer "does a full render every frame even while paused. That is pure waste." That was true when a paused chart was a still picture. The venue broke the assumption. Its video backdrop keeps playing and its crowd reacts on a clock of their own, and BOTH are drawn into the same canvas as the notes — so a throttle aimed at static notes throttled the entire room. The scene only got a texture upload 10 times a second while the transport sat paused. Renderers can now declare that their picture is not static while the chart clock is stopped: an optional needsContinuousFrames(). The throttle is skipped only when it returns exactly true, and the probe fails closed — a renderer that doesn't implement it, or one that throws, keeps the throttle unchanged. So the GPU saving that motivated #654 survives everywhere it was actually valid. highway_3d implements it and claims continuous frames ONLY while a crowd video is genuinely rolling (bound, unpaused, not ended, readyState >= 2). With no venue pack — the common case — the paused scene really is static, so it keeps the throttle and the GPU still idles. Tests extend tests/js/highway_pause_throttle.test.js, which guards this code path source-level (the draw loop owns the rAF + WebGL lifecycle and is deliberately not reproduced in a vm — see the file header). The new guards pin that the capability GATES the early return rather than merely being called near it, that the probe fails closed on absent/non-function/throwing/truthy-but-not- true, and that the 3D renderer keys off the real video elements and can still return false. All 3 fail against the pre-fix source. eslint 0 errors; JS 1202/1202; pytest 2597 passed. |
||
|
Byron Gamatos
|
6272af8d33
|
feat(career): profile passport wall, home career card, shareable PNG card (#955)
* feat(career): profile passport wall, home career card, shareable PNG card Career v3, WS2. The identity artifact leaves the plugin tab: - Profile: #v3-profile-passports-mount (core, one div) filled by career on v3:profile-rendered — per-instrument shelves of earned covers, hours, gig count, open-career link. Absent-not-empty. - Home: the plugin-count stat tile becomes #v3-dash-career-slot with the old stat as fallback content; career replaces it with a trading-card tile (leather + foil shine, badge count, hours, closest-stamp ask) on the existing v3:dashboard-rendered event. - Shareable card: static/js/blob-io.js (downloadBlob lifts the idiom duplicated verbatim in settings-io/diagnostics-export — both refactored; copyImageBlob wraps ClipboardItem, returns false to signal the download fallback). Earned passports get Save/Copy card: a natively-drawn 480×640 canvas (leather, stamp ring, stubs+hours line); copy falls back to download with a notice when the clipboard refuses. - Mount-point convention documented in docs/plugin-v3-ui.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(career): external surfaces stay absent until a passport exists CodeRabbit on #955: a bare commitment produced a zero-passport wall and replaced the dashboard fallback. Docs also now say mounts may hold fallback content and plugins REPLACE, never append. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Byron Gamatos
|
329cc86315
|
fix(sloppak): the full mix is a stem — drop the invented original_audio key (#946)
* fix(sloppak): the full mix is a stem — drop the invented `original_audio` key (#933) Core read, served, and depended on `original_audio:` — a top-level manifest key this repo invented in #583 that the feedpak spec never defined. The format already had a home for the pre-separation mixdown: it is a stem. feedpak 1.15.0 (feedpak-spec#53) RESERVES the id `full` for it, so read it from there. The key existed to work around a bug in our own reader. The packer's comment said so plainly: "we must NOT list the full mix as a playable stem — the player sums every entry in `stems` and does not gate playback on `default`, so a listed full mix plays on top of the stems". Faced with a reader that would double the song, the packer put the mixdown outside `stems` and invented a key to point at it. The fix belongs in the reader, and that is what this is. load_song() now partitions the stem list: `full` comes out as LoadedSloppak.full_mix, the instruments stay in .stems. Nothing that sums stems or draws one fader per stem can see the mixdown, so retaining it is safe — which is what lets the packer put it where the format says it goes. - ws_highway: `song_info` gains full_mix_url / has_full_mix. The old original_audio_url / has_original_audio remain as deprecated aliases for one release so an older stems plugin keeps working (#945). - `stems` on the wire, and stem_ids / stem_count in the library index, are now INSTRUMENT stems only — a separated pack that retains its mixdown no longer advertises a bogus "full" chip or an inflated stem count. - enrichment: fingerprint against the mixdown wherever it lives. This widens coverage — _song_audio_file() previously returned None for any pack without the invented key, so fingerprinting silently did nothing for nearly every pack. - sloppak: `original_audio:` is still READ as a deprecated fallback, because every pack in the wild carries it and would otherwise lose its pristine mix. tools/migrate_full_mix_stem.py rewrites those packs into the spec shape (original/full.ogg -> stems/full.ogg, add the `full` stem at default:off, drop the key); the fallback and the aliases die with #945. The spec gate keeps the debt honest: the grandfather entry now tracks #945, and the gate fails if it goes stale. Verified: spec gate OK (4/4, incl. ingesting the spec's new example pack that retains `full`); 2493 python tests, 995 js tests; migrator round-tripped over real packs from the library and the results pass the spec's reference validator. * fix(migrate): discover directory-form packs instead of silently skipping them iter_packs() searched only files, so a directory-form pack (`song.sloppak/`, the authoring shape) was walked INTO and never yielded — silently missed by a run that's meant to be exhaustive. Discover suffix-named directories too (yielded whole, not descended into), and route packs through migrate_pack/verify_pack. Directory packs are REPORTED as `dir-form-unsupported`, not rewritten in place: a single-file pack is replaced atomically (a fully-built temp archive swapped in with one os.replace), but a populated directory can't be swapped that way, so an interrupted in-place rewrite could leave an authoring pack half-migrated. The status is a problem status, so it counts against the run's exit code and shows in the summary — the operator re-packs or migrates it as a `.feedpak` instead of it vanishing from the report. Addresses a CodeRabbit review finding. Signed-off-by: Kris Anderson <topkoa@gmail.com> * fix(migrate): verify requires an explicit `off` on a retained full mix verify_zip accepted any non-truthy `default` on a multi-stem `full` (missing, empty, boolean, `false`/`no`/`0`, malformed) as "ok". But core defaults an ABSENT `default` to True — ON (lib/sloppak.py: `s.get("default", True)`) — and treats an empty/unrecognized string as ON too, so a migrated-shape pack whose `full` stem has a missing or blank default beside instrument stems would actually play the mixdown on open and double the song. verify was certifying that as safe. Require an explicit normalized `off` beside instrument stems: `on`-ish values are reported `full-stem-default-on` (actively plays), everything that is not a normalized `off` is reported `full-stem-default-not-off`. The migrator already writes the literal `off`, so its own output is unaffected; this also certifies the pack is in the tool's canonical, most-portable shape. The len>1 gate is kept, so a sole `full` stem (which IS the audio) is not policed. Adds parametrized coverage for missing / empty / boolean / off-ish / malformed defaults, and a sole-full-stem case. Addresses a CodeRabbit review finding. Signed-off-by: Kris Anderson <topkoa@gmail.com> --------- Signed-off-by: Kris Anderson <topkoa@gmail.com> Co-authored-by: Kris Anderson <topkoa@gmail.com> |
||
|
Byron Gamatos
|
3e57ba0345
|
fix(career): hours polish — recency stamp + non-2xx POST is a failure (#947)
CodeRabbit follow-up on #942: - add_play_seconds() now stamps last_played_at (like touch_position): an unscored play that ran to the natural end WAS played — recent / Continue ordering must see it. Resume position stays untouched. - stats-recorder post() treats non-2xx as failure: a 4xx/5xx JSON error body parsed as an object read as success, silently dropping the accrued seconds instead of re-queuing them. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Byron Gamatos
|
0fc6a4beed
|
feat(career): hours-per-genre odometer — honest wall-clock play time (#942)
Career v2, WS2. Nothing measured play time before (the achievements plugin's final-position shortcut double-counts loops and mis-reads seeks). Now: - stats-recorder.js accrues WALL-CLOCK seconds across song:play/resume ↔ pause/stop/ended spans (single spans clamp at 2h against suspend inflation) and piggybacks them as `seconds` on the POSTs it already sends; failed POSTs restore the accumulator; a session reset flushes first so time can't re-attribute to the next song/arrangement. - POST /api/stats accepts optional `seconds` (finite, 0 < s ≤ 6h) on the scored and position branches, plus a new seconds-only branch for unscored plays that ran to the natural end — banks time WITHOUT touching the resume position (song:ended must not overwrite Continue) and still counts as playing today for the streak. - song_stats gains additive idempotent `seconds_total`; record_session/ touch_position accrue, new add_play_seconds() for the seconds-only path; the legacy-encoding stats merge sums seconds across duplicates. - Passports surface it: "14.2 h in Blues" under the badge stamp and on the shelf cover sub-line — a true fact that only grows, never a target or a meter (Stage 5 post-cap, per the career design). Tests: seconds accrual/validation/seconds-only branch (stats API), per-instrument-and-genre summing (career), fmtHours formatting (vm). Full suites: pytest 2480, JS 1165. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Byron Gamatos
|
d26347981c
|
feat(career): badge ceremony — the crowd erupts, the stamp drops (#941)
Earning a genre badge now stages the full moment (career v2, WS1):
- venue-crowd.js gains a public celebrate(): machine.force('ecstatic')
commits instantly (bypassing STABLE_MS/DWELL_MS; the stamped
lastSwitchAt makes the dwell window HOLD the forced state before the
real perf machine reasserts) + a cheer stinger via the same
_lastStingerAt=-Infinity bypass the end-of-song reaction uses. If a
stinger/intro owns the idle layer, the ecstatic loop is queued via
_pendingLoop exactly like onPerformanceState. No-op without a
manifest/active venue.
- career detectNewBadges() calls badgeCeremony(): crowd first, then a
body-appended full-screen overlay 300ms later (it cannot live in
#pp-overlay — #plugin-career is display:none during playback): dimmed
backdrop, the bronze stamp slamming in with a shine sweep, a 42-piece
canvas confetti burst, click-or-4s dismiss.
- prefers-reduced-motion: chime + fbNotify only, no overlay.
Tests: machine.force commit/dwell-hold/bogus-state, celebrate export +
no-manifest no-op, celebrate-called-once-per-badge, crowd-absent and
crowd-throwing degradation.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
45caa86ab8
|
Bar pack v4 refresh + crowd sound reactions (#940)
* chore(career): refresh bar pack to v4 + restore crowd-SFX setting Pack v4: per-character desynced animation starts, flyover intro, per-venue reaction sounds (sfx-up/sfx-down in manifest). Settings: re-add the crowd sound reactions toggle that was dropped when settings.html became the passports data panel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(v3): crowd mood-change sound reactions (cheer up / boo down) Port the venue-crowd SFX runtime that the settings toggle and the pack's sfx-up/sfx-down files were built for: on a committed mood transition, play the venue's own cheer (up) or boo (down) one-shot, gated by the feedBack-venue-crowd-sfx setting (default off). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: rebuild tailwind.min.css for settings toggle classes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Byron Gamatos
|
8f1906a0c1
|
Merge pull request #926 from got-feedBack/feat/tuning-midis-followups
tuningMidis follow-ups: NaN/Inf guard in freqs_to_midis + v3 badge adopts exact midis |
||
|
gionnibgud
|
3050c7b1d3 |
feat(v3): reword the fullscreen setting + note the macOS launch caveat
Retitle the toggle "Fullscreen" (from "Start in fullscreen") and reword the description to "Run fee[dB]ack in fullscreen mode. On macOS, changes take effect on the next launch." The macOS note is honest about a native-fullscreen limitation: AppKit drops the first programmatic fullscreen-enter on a window created windowed, so on macOS the desktop side applies the pref at next launch rather than live. Windows/Linux apply it live on the first toggle. The note is self-scoping text (no platform-detection code needed). Signed-off-by: gionnibgud <gionnibgud@gmail.com> |
||
|
gionnibgud
|
f8012a8ce4 |
feat(v3): add desktop-only "Start in fullscreen" system option
Adds a "Start in fullscreen" toggle to the Settings → System panel,
addressing the desktop request in feedBack-desktop#97: users want the
app to launch fullscreen without hitting the OS hotkey every time.
The block ships hidden and is gated exactly like the App-updates block:
setupWindowOptions() only unhides + wires it when the feedBack-desktop
bridge exposes window.feedBackDesktop.window.{getStartFullscreen,
setStartFullscreen}. Web/Docker builds have no such bridge, so the
section never appears there. Persistence lives desktop-side because
only the Electron main process can read the pref at window-creation
time — core just proxies through the bridge.
The desktop bridge + launch behaviour land in a follow-up
feedBack-desktop PR.
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
|
||
|
K. O. A.
|
503716acbf
|
Merge pull request #928 from got-feedBack/feat/panes-core
feat(panes): detachable panes — pop a plugin's real panel out into its own window |
||
|
topkoa
|
41bb4482fe |
docs(panes): say which repo the desktop half lives in
Two comments pointed at `main.ts` and `pane-hosts.ts` as though they were in this repo. They are not — they are in got-feedback/feedBack-desktop, and a contributor reading only this codebase would go looking for files that do not exist. Named the repo and the paths, and said the part that actually matters: nothing here depends on that code. In a plain browser a pane window is simply a pop-up; the desktop side only upgrades it. And the frame-name prefix is a contract across two repos with no build-time link between them, so the comment IS the link — worth saying out loud. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
0955f0b6f2 |
fix(panes): keep a pane window in step with theme + interface scale
The pane window got a ONE-TIME snapshot of the app's theme classes and the interface-scale custom property. The app changes both at runtime — Interface size emits `scale:changed`, the theme emits `theme:changed` / `v3:cosmetics-applied` — so an already-open pane went on rendering at the old scale, in the old palette, the moment the user touched either. "Looks identical" has to keep being true, not merely start out true. A pane window now follows those three events for as long as it is open, and stops on unplace(). The inline style is assigned wholesale rather than merged: unlike the class lists (where pane.html's own `fb-pane-window` must survive), there is nothing in the pane document's inline style to preserve — and concatenating on every change would grow the attribute without bound as the user dragged the scale slider. Also: the dock's focus() always smooth-scrolled, ignoring prefers-reduced-motion — which panes.css already honours for the card's flash animation. A smooth scroll is motion too, and someone who asked for less of it meant this as well. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
3a50e593bf |
fix(panes): fail fast when the pane window is unreachable; validate opts.header
Two from review. 1. _whenReady's own comment said a SecurityError means the pop-out is not reachable from this realm and "no amount of waiting will fix it" — and then it waited the full 10s deadline anyway. Ten seconds of a detached panel and a half-popped-out UI, for a condition we had already diagnosed as fatal. It now gives up after a 1s grace instead. Not instantly, deliberately: a throw *during* the navigation from about:blank to /pane would otherwise take down a pop-out that was about to work perfectly. A second is far more than that transition needs and far less than a user should spend staring at a detached panel. 2. attachChip() took opts.header on trust. It's a public plugin API, and a truthy non-Element header (a selector string, a wrapper object, a ref) is an easy mistake — one that surfaced as a confusing DOM exception from deep inside core instead of a TypeError naming the offending pane. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
5049be0523 |
fix(panes): the dock is born empty, so say so
panes.css hides an empty dock (.fb-pane-dock.is-empty { display: none }), but
the element was created without the class — so between creation and the first
card it was a visible-to-CSS, announced-to-screen-readers role="region"
landmark containing nothing.
Harmless in practice today (the dock is created lazily, on the same tick as the
card that prompted it), but the CSS contract should hold from first paint rather
than from the first _syncEmpty(), and any future caller of dock() gets the right
thing for free.
Found by CodeRabbit on #928.
Signed-off-by: topkoa <topkoa@gmail.com>
|
||
|
topkoa
|
de2a42bd35 |
fix(panes): coerce plugin-supplied pane sizes to numbers
`spec.width` / `spec.height` are plugin-controlled, and the window host builds
window.open()'s feature string by concatenation:
'popup,width=' + spec.width + ',height=' + spec.height
`spec.width || 380` passed anything truthy straight through. So a width of
'300,menubar=1' would not merely be an invalid size — it would inject window
features. Less dramatically, any non-numeric value produced a malformed feature
string and a pane that failed to open for no visible reason.
They now go through _size(): Number, round, reject anything not finite and
positive, clamp to 120..4000. A hostile or careless value falls back to the
default instead of reaching window.open() at all.
Verified against the obvious inputs: '300,menubar=1' -> 380 (default), '300' ->
300, 0/-50/NaN/{}/'abc' -> 380, 1e9 -> 4000, 5 -> 120.
Signed-off-by: topkoa <topkoa@gmail.com>
|
||
|
topkoa
|
671aba950c |
chore(panes): drop the leftover adoption diagnostics
A ~15-line console.info dumping computed styles, sizes, child counts and the element's inline style on every single pop-out. It was instrumentation written to chase the "panel comes home dead" bug, and it should have gone out with the rest of the debugging — it survived the cleanup. Removed rather than downgraded to console.debug: nothing here is worth keeping even behind a flag. The failures it was built to diagnose are all handled and commented now, and the paths that can still go wrong (window never loads, adopt throws) already log a console.error that says what happened. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
95d6d8a46e |
fix(panes): keep panes.css last in the pane window's cascade
_copyStyles appended the app's stylesheets to the pane document — which already links panes.css — so they landed AFTER it. In the app document panes.css loads last, after tailwind/style/v3, and its rules win ties. In the pane window that order was silently inverted, letting core styles override the pane chrome and the .fb-paned placement rules. Cascade order is not a detail here. "Looks identical" has to include the order things are said in, or the same markup with the same sheets can still render differently. The clones now go in BEFORE pane.html's own link, preserving their relative order among themselves and leaving panes.css last, exactly as in the app. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
f43779c99e |
fix(panes): detach the element when the pop-out starts, not when it lands
The window host's place() is asynchronous — it opens the window, waits for /pane to load, and only then adopts the element in. But the manager emits `panes:opened` as soon as place() returns, and the chip reacts by putting its "popped out" stub where the element used to be. So for that gap the user saw BOTH: the real panel still sitting in its original spot, and a stub next to it claiming the panel had left. On a window that never loads, that lasts the full 10s readiness timeout. Detach the element as soon as we commit to moving it. That is not destructive: the node keeps its owner document, its listeners and its closures — it is simply out of the tree, waiting for a document to be adopted into. And if the window never loads, closePane() puts it straight back at its home, which is exactly what the failure path already does. The dock host has no such gap; its place() moves the element synchronously. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
82aa8a757e |
fix(panes): harden the persisted host map against unsafe pane ids
A pane id is plugin-controlled, and it becomes a KEY in the persisted
{ paneId: hostId } map. `__proto__` and friends are not ids, they are booby
traps:
- `map['__proto__'] = 'window'` on a plain object corrupts the map, and
can reach Object.prototype.
- `map[id]` on a polluted (or hand-edited) object can return a value straight
off the prototype chain for a pane that was never remembered at all — so a
pane could be "restored" to a host nobody ever put it in.
Three layers, because each is a one-liner:
- Reject `__proto__` / `constructor` / `prototype` as pane ids at
registration, so they never reach storage.
- Re-key whatever comes out of localStorage onto a null-prototype object, so
a corrupt or hand-edited value cannot smuggle a prototype in.
- Read with an own-property check.
Also from the same review:
- Removed `window.__fbPaneWindows`. It was exposed for pane-desktop.js back
when that file needed to reach the window handles; the rewrite dropped that
need and nothing has referenced it since. Dead global, and its comment
described a collaborator that no longer exists.
- Corrected the /pane cache comment. It claimed a stale page would leave the
window blank, which stopped being true when the readiness check gained a
`doc.body` fallback — it would still work, just without the pane window's
own layout. A comment that describes a failure mode the code no longer has
is worse than no comment.
Signed-off-by: topkoa <topkoa@gmail.com>
|
||
|
topkoa
|
cb425ed48d |
fix(panes): don't hide a docked pane; don't force display; restore visibility
Six more findings from CodeRabbit on #928. Three are real bugs. 1. THE CHIP HID DOCKED PANES. `_onOpened` decided "did the pane take my element?" from `ownerDocument !== document`. That is true for a pane in a pop-out window — and false for a pane moved into the DOCK, which lives in this very document. So docking a pane stamped `.fb-pane-detached` (display:none !important) onto the panel the user was looking at, and put the stub next to it instead of at its home. The element cannot answer this question — `isConnected` is true in a pane window, `ownerDocument` is this one in the dock. Both were live bugs. Ask the manager, which knows exactly what it handed to the host: `panes.elementOf(id)`. That holds for every host, and for reconciling after the fact (detail == null), which is what a plugin rebuilding its panel mid-pop-out triggers. 2. `.fb-paned` FORCED `display: block !important`. A panel that is `display:flex` or `grid` would be silently re-laid-out while detached — the exact opposite of "placement only", and precisely the kind of surprise this feature exists to avoid. Removed. Making a hidden panel visible is a separate job, and it now belongs to the manager, which does it without touching the panel's display MODE: clear `hidden`, and clear an inline `display:none` if that is how the panel hides. 3. VISIBILITY IS NOW RESTORED. The hosts used to set `el.hidden = false` and never put it back, so the docs' "core only changes placement" was a lie and a panel's hidden state was quietly lost. The manager stashes both `hidden` and the inline `display` on open and restores them on dock: a panel that was closed when you opened its pane from the tray goes back to being closed; one that was open stays open. Plus: - The launcher rebuilt its whole list on every panes:opened/closed — including the one fired by clicking a button in that list — destroying the button under the user's finger and dropping focus to <body>. It now restores focus to the toggled pane's button. - `_copyStyles` cloned every stylesheet link, including the panes.css that pane.html already loads. Skip sheets the pane document already has. - Docs: the chip may route to the DOCK, not always a window (it goes through detach() → the host router). `header` precedence was documented backwards — an explicit `header` wins. And the visibility contract above is now written down rather than being a surprise. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
859b0036e5 |
fix(panes): review fixes — stranded elements, duplicate listener, class clobber
Three real findings from CodeRabbit on #928, all in current code. 1. closePane() adopted the element out of the pane window ONLY when its original home was still connected. If the panel never had a parent (a plugin that builds it lazily and hands it straight over) or its container was torn down while the pane was out (a screen change), the whole block was skipped — leaving the element inside a window we then close, which strips every listener in its subtree. That is exactly the "comes home dead" failure this ordering exists to prevent; the guard just moved it from the common path to the rare one, where it is far harder to spot. Adopting and re-homing are two different jobs and only one of them is allowed to fail. Adopt UNCONDITIONALLY — that is what rescues the element — and insert only when there is somewhere to insert it. With no home the element ends up owned by this document but not in it: detached, intact, listeners alive, ready for the plugin to re-insert. 2. The pane window's `beforeunload` handler was registered TWICE, comment block and all — a bad scripted edit on my part. Harmless (the handler is idempotent via panes.isOpen) but dead duplicate code. Also fixed the stale comment further down that still claimed there was no beforeunload listener at all. 3. _copyStyles ASSIGNED className on the pane document's <html> and <body> instead of merging. pane.html sets `class="fb-pane-window"` on <html>, and panes.css hangs the pane window's own chrome off exactly that — so copying the app's classes over it silently took the pane window's own layout with them. Merge both class lists, and append the interface-scale inline style rather than replacing the attribute. Also guarded the docs' integration example behind a feedBack.panes check: the doc says the API is optional, and then showed an example that would throw on a host without it. Not applicable (reviewed against |
||
|
topkoa
|
a7348052ae |
fix(panes): give the pop-out stub a focus ring
.fb-pane-stub is a <button>, and both of its sibling controls (.fb-pane-chip, .fb-pane-card-btn) have an explicit :focus-visible outline. It didn't, so keyboard focus fell back to the UA default and looked inconsistent next to them. Found by CodeRabbit on #928. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
1e5282e27e |
fix(panes): get the element out before the pane window's document dies
Docking a popped-out panel brought it home DEAD. It rendered perfectly —
right markup, right size, right place — and every control in it was inert:
the close button, the sliders, the presets, even the pop-out chip. A
photograph of a panel.
Closing a pane window tears down its document, and the panel was still
inside it. The node itself survives (the manager holds a reference), but
every event listener in its subtree goes with the document that hosted
them. Two paths did this:
1. closePane() called the host's unplace() — which closes the window —
BEFORE adopting the element back. Order is now reversed, and the
comment says why so nobody helpfully "tidies" it back.
2. The user closing the pane window themselves was only noticed by the
`closed` poll, which by definition runs AFTER the document is gone.
The window now gets a `beforeunload` listener that brings the element
home while its document is still alive.
That listener has to be attached AFTER /pane loads: window.open() hands
back a throwaway about:blank document, and anything registered on it is
discarded when the real page replaces it. This is the same trap that made
the pane window blank in the first place — adopt into about:blank and the
panel is destroyed a moment later — and it is now handled in both places.
The `closed` poll stays, but only as a last-resort net for a CRASHED pane
window, where nothing can be saved.
Also fixed while chasing this:
- The chip stamped `.fb-pane-detached` (display:none !important) onto the
element to hide it in the main window — and that element is the one we
move, so the class travelled with it and blanked the pane window. The
chip now only hides an element the pane did NOT take, and marks the hole
with its stub otherwise. "Did not take" is an ownerDocument test, not
isConnected: a panel sitting in a pane window IS connected, just not
here, and a plugin that rebuilds its panel (Camera Director does, on
every mode change) re-runs attachChip while popped out.
- The stub was inserted "before the element", which is nowhere — the
element has left the document. The manager now hands over the element's
recorded home, and the stub goes there.
- GET /pane sent no cache headers. A stale copy is especially nasty here:
the opener waits for an element inside that page before adopting, so an
old cached version means the pane window just sits there blank.
Verified in the desktop app: pop out, use the controls in the pane window,
dock back, use them again. Panel comes home alive.
Signed-off-by: topkoa <topkoa@gmail.com>
|
||
|
OmikronApex
|
81ef11d855 |
fix(v3): floor accuracy percentages so 100% means all notes hit
Math.round let 431/433 (99.54%) display as 100%. Floor at every accuracy display site (HUD, library badges, dashboard, lessons, profile, playlists, calibration overlay); stored fractions and mastery thresholds unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
topkoa
|
9e9f0fdac6 |
fix(panes): adopt into the real pane document, not about:blank
Both pop-outs opened blank. window.open() returns immediately, and the window it hands back already has a document — an `about:blank` one, whose readyState is 'complete'. So the host cheerfully adopted the panel into THAT, it worked for a few milliseconds, and then /pane finished loading, replaced the document, and took the panel with it. Blank window, vanished element. Waiting for 'load' is no better: it may already have fired for about:blank before we could listen. So don't trust readyState and don't trust 'load' — wait for the one thing that exists only in the document we actually want: pane.html's #fb-pane-root. Poll for it (guarding the cross-document window while it is mid-swap), give up after 10s, and on failure bring the element home rather than stranding it in a window that never loaded. Also drop the popup's 'beforeunload' listener: it was registered on the about:blank window and discarded along with it, so it never fired. The `closed` poll is what notices a user shutting a pane window — as it must be anyway, since a crashed renderer never says goodbye either. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
188bdaa837 |
feat(panes)!: move the real element, instead of rebuilding it
The first cut of this got the model wrong. A pane was a SECOND
implementation of the plugin's panel — its own sliders, its own styling,
driven over a cross-realm bridge (ctx, a state store, capability RPC,
mirrorGlobal, a stream sampler). Popping out gave you something that
resembled the panel you popped, and every feature it did not reimplement
(presets, tabs, EQ, language) was simply gone.
What a user wants from "pop this out" is the thing they popped out.
So: MOVE THE REAL ELEMENT. Same-origin windows can adopt each other's
nodes, and an adopted node keeps its event listeners and its closures.
The panel goes on running the plugin's own code, against the plugin's own
state, in the plugin's own realm — it is merely being DISPLAYED in another
window. Copy the app's stylesheets into that window and it looks identical
too, because it is identical.
The plugin's side collapses to two lines:
feedBack.panes.register({ id, title, element: () => panelEl });
feedBack.panes.attachChip(panelEl, id);
and everything comes along: the CSS, the listeners, the presets, the
state. Nothing to keep in step, because there is no second copy.
Deleted, all of it now pointless: pane-bridge (ctx + transports), pane-hub
(the cross-realm server), pane-runtime (the pane realm's boot), pane-streams
(the rAF sampler that existed because an AnalyserNode can't cross a window),
pane-mirror (mirrorGlobal), pane-plugins + the manifest `panes[]` key and its
server-side validation, panes.state(), and both built-in demo panes. ~1200
lines. None of it was wrong — it was all correct machinery for the wrong
problem.
Consequences worth knowing:
- The window MUST be opened by the renderer with window.open(), not by the
desktop's main process: a window we did not open gives this realm no handle
to its document, and without the handle there is nothing to adopt into.
Electron turns the same-origin window.open() into a real BrowserWindow
anyway (setWindowOpenHandler → 'allow'), so we get the OS window AND the
live DOM link. The desktop side finds it by frame name.
- `.fb-paned` neutralises PLACEMENT only (position/inset/width/z-index/shadow).
A plugin panel is nearly always a fixed overlay pinned to a corner of the
app; alone in a 380px window that positioning is nonsense. Colours, borders,
padding, fonts and the panel's own internal layout are untouched — the whole
promise is that what you popped out is what you get.
- The element is returned to its EXACT home on dock: same parent, same position
among its siblings.
- The plugin's code still runs in the main window. So a document.body
.appendChild() inside a panel (a tooltip, a popover) lands in the main
window, not the pane — anchor to the panel instead. And a continuously
animating panel may run slowly while the main window is backgrounded, since
its rAF lives there. Both documented.
Signed-off-by: topkoa <topkoa@gmail.com>
|
||
|
topkoa
|
330995588c |
feat(panes): panes.state(id) — let a plugin apply its own pane's values
mirrorGlobal covers the case where a pane drives a plain global that some renderer reads each frame. It does not cover the far more common one: a plugin whose MAIN-realm code is the authority — it clamps, it persists, it emits events, it owns the audio graph or the camera rig — and which must therefore APPLY the pane's values itself rather than have core splat them somewhere. Camera Director is the case that forced this. Its brain is the sole writer of the camera store, the sole broadcaster on splitscreen's channel, and the only thing that clamps an axis to its legal range. A pane cannot write window.__h3dCamCtl behind its back without desynchronising its presets, its persistence, and the panel's own sliders — and running the brain inside the pane realm would make it a SECOND store writer and a second broadcaster, racing the real one. So: `panes.state(id)` hands the main realm the open pane's store (get/set/all/subscribe). A plugin seeds it on `panes:opened`, subscribes, and applies what comes back through its own API. The pane stays realm-agnostic — it only ever touches ctx.state — and the plugin stays the single source of truth. For that to work, the hub now broadcasts EVERY change to the store, not just the ones a pane asked for: it subscribes to the store on connect rather than echoing pane-originated writes by hand. A value the plugin clamps or corrects therefore reaches the pane window immediately, and there is exactly one path by which state arrives in a pane — so it cannot drift. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
254e26bb3a |
feat(panes): mirrorGlobal, manifest-declared panes, and the plugin docs
Three things a plugin needs before it can actually use panes.
## mirrorGlobal — the camera-director problem
The 3D highways read their free camera from a plain global,
`window.__h3dCamCtl` (highway_3d/FREECAM_BRIDGE.md), once per frame in
_resolveFreeCam(). A camera panel in the main window just writes that
object and the camera moves. A panel in a POP-OUT window cannot:
window.__h3dCamCtl there is a different object in a different realm, and
writing it moves nothing.
So a pane declares one field — `mirrorGlobal: '__h3dCamCtl'` — and
pane-mirror.js (main realm, where the renderers live) copies that pane's
state onto the global whenever it changes. highway_3d, keys_highway_3d
and drum_highway_3d are NOT modified and do not know panes exist.
The rule that makes it work: MUTATE THE OBJECT, NEVER REPLACE IT. A
renderer may be holding the reference, and swapping in a new object would
leave it reading an orphan. Keys the pane doesn't set are left alone
rather than deleted — the global may carry a renderer's own bookkeeping.
Closing the pane deliberately leaves the global as-is: closing the camera
panel should not snap the camera back to a default, which is exactly what
happens today (nobody clears __h3dCamCtl).
## Manifest-declared panes
"panes": [{ "id": "camera_director", "title": "Camera Director",
"script": "panes/camera.js", "mirrorGlobal": "__h3dCamCtl" }]
Declaring a pane beats calling panes.register() from screen.js because it
becomes openable FROM THE RAIL OR THE TRAY WITHOUT THE PLUGIN'S SCREEN
EVER HAVING BEEN VISITED — core registers a stub from the manifest and
fetches the script only when the user opens it. A pane you can only reach
by first navigating to the screen it was meant to replace is not much of a
pane.
The script sets `window.feedBackPane_<id> = { mount, unmount }`, mirroring
the existing window.feedBackViz_<id> convention, and the SAME file is what
a pop-out window loads in its own realm.
`script` is validated as a relpath under the plugin's src/ and served
through the sandboxed /api/plugins/<id>/src/ route — the containment rule
`styles` already has for assets/. Traversal, absolute paths, drive letters,
backslashes and non-.js are rejected; a bad entry is dropped with a warning
rather than failing the whole plugin, because one malformed pane should not
cost the user a working plugin.
Note the projection is written TWICE — _nav_entry() and the /api/plugins
route re-project independently — so panes had to be added to both, plus the
pending branch (a pane can be opened while its plugin is still installing
deps; the script is fetched on open, not at discovery).
## docs/plugin-panes.md
The contract, and the one rule it all hangs on: mount(root, ctx) runs in a
realm that may not have the app in it. Everything comes through ctx, or the
pane works docked and silently dies popped out.
Verified: manifest validation rejects ../.., C:\, non-.js, dupes and
missing fields while passing a good entry; /api/plugins projects panes[] for
all 20 plugins. mirrorGlobal mutates the global IN PLACE — a reference held
the way _resolveFreeCam holds it sees the change, and a renderer's own field
on that object survives — both for a local write and for a write arriving
over the channel from a pop-out realm.
pytest: 2401 passed, 8 failed — all 8 reproduce on a clean main (including
the one in tests/test_plugins.py) and are unrelated.
Signed-off-by: topkoa <topkoa@gmail.com>
|
||
|
topkoa
|
d508380532 |
feat(panes): desktop host — real windows and the system tray
Registers a `desktop` pane host at priority 20, above the browser pop-up host (10) and the dock (0), whenever the Electron bridge exposes feedBackDesktop.panes. A popped-out pane then gets a real BrowserWindow: it remembers where you put it, can float above everything, minimizes to the system tray, and appears in the tray's menu. In a plain browser — or on an older desktop build that predates the bridge — this file registers nothing and the browser pop-up host handles detach exactly as before. Nothing else in the pane system changes. That is what the host registry is for. Two things only this realm can decide, so it owns them: - The user closed a pane window (or it crashed). Close the pane, or the dialog its pop-out chip hid never comes back and the user is left with no way to reach their own UI. - The tray asked to toggle a pane it has no window for. Main cannot know what opening one means — the pane might belong in the dock — so it asks. Unlike a browser pop-up, this host needs no user gesture, so it sets autoRestore: true — a pane you left popped out comes back popped out, where you left it, on the next launch. Pairs with got-feedback/feedBack-desktop#103. Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
fefb9051a4 |
feat(panes): pop-out windows — the pane realm, hub, and remote transport
A pane can now leave the main window entirely. Same `mount(root, ctx)`,
same file, different JS realm — which is what the ctx-only contract in the
previous commit was for.
## A purpose-built document, not the app shell with a flag on it
`GET /pane` serves static/panes/pane.html: the bridge, the runtime, and
the pane's own script. No highway, no library, no v3 shell, no <audio>,
no Tailwind.
The splitscreen follower takes the other road — it reloads the whole app
at `/?ssFollower=1` and hides what it doesn't want — and pays for it with
an anti-flash block that must run before any script parses (index.html),
bail-outs in app.js and shell.js, and ~40 lines of CSS hiding core
elements by id. It loads the entire app to throw it away. A pane window
has nothing to throw away, so it boots in milliseconds and there is
nothing to flash.
The cost is that `window.feedBack` in a pane realm is a deliberate,
documented SUBSET. The runtime installs exactly what a pane is promised —
`panes.register`, and the no-op chip/dock calls a shared script may make
at load — so a pane reaching for something it was never given fails
loudly at authoring time instead of subtly at runtime.
## The channel
BroadcastChannel('feedback-panes'), same origin. This works only because
Electron's setWindowOpenHandler returns `action: 'allow'` for same-origin
URLs: `deny` would push the window to the system browser, a different
Chromium instance, where BroadcastChannel cannot reach it and the pane
would silently never sync. That flag is load-bearing.
hello -> snapshot resync-on-open, always. The snapshot is the only way
the pane realm learns anything.
state main is authoritative. A pane's write is a REQUEST;
main applies it and echoes to every realm, so a
losing write self-corrects instead of splitting brain.
rpc / rpc:reply ctx.call() -> the capability bus, with a 10s deadline.
Without one, a main window that died mid-call leaves
the pane's promise pending forever.
event allowlisted bus events, JSON-safe. A CustomEvent
carrying a DOM node (highway:canvas-replaced does)
would throw on postMessage and take the channel down
for everyone, so detail is round-tripped through JSON.
stream one coalesced message per pane per frame, OVERWRITING
anything not yet flushed. Queueing would build a
backlog: Chromium throttles a backgrounded window, and
the main window is exactly what's backgrounded while
the user looks at the pane.
sub / unsub refcounts the main-realm sampler.
bye both directions.
## The follower clock
The pane extrapolates between broadcasts: anchor + observedRate * elapsed,
capped at 2s. observedRate is learned from the broadcasts themselves
(dt/dwall) so it tracks the speed slider without being told about it, and
seeks/pauses are excluded from the fit — a jump is not a tempo. Capping it
means a dead main window decays into a frozen clock rather than one that
confidently runs away. This is splitscreen's hard-won trick, generalized:
panes just call ctx.playhead().
## Failure modes, all of them
- Main window closes -> `bye {main-closed}` and the pane says so plainly,
rather than showing a frozen playhead that looks live. The host also
closes its windows outright; a pane that cannot be fed should not be on
screen.
- Pane window X'd or crashed -> a `closed` poll reaps it (a crashed
renderer never sends `bye`), the pane closes, and the chip's dialog comes
back. Without this the user's dialog stays hidden with no way back.
- Popup blocked -> a toast, and we bail BEFORE the manager records
anything, so the caller's dialog stays exactly where it was.
- Nobody answers `hello` in 5s -> the window says so instead of spinning.
- A pane with no `script` is a closure in this realm and cannot honestly
cross a window boundary. The window host declines it (canHost) and the
router falls back to the dock.
- A browser blocks window.open() outside a user gesture, so a popped-out
pane cannot be auto-restored on page load — it would only ever produce a
"blocked" toast. Such a pane comes back in the DOCK, and the chip pops it
out again on the next click. (autoRestore: false. The desktop host will
set it true.)
Hosts may now declare `remote: true`, meaning the pane's mount() runs in
another realm: the manager then owns only the authoritative state store and
never calls mount() itself. That is the seam the Electron BrowserWindow +
tray host drops into next, with no change here.
Verified: popped Now Playing and Mixer into real windows. The pane realm has
no window.highway, no capability bus and no <audio>, yet the Mixer renders
its faders via ctx.call('audio-mix','list-faders') across the channel — and
dragging that fader IN THE PANE WINDOW moved the main window's song volume
to 55 and persisted it. Closing the pane window un-hid the mixer dialog,
removed the stub and restored the chip, while the other pane window stayed
open.
Signed-off-by: topkoa <topkoa@gmail.com>
|
||
|
topkoa
|
e5cbea2e9f |
feat(panes): core detachable pane system + pop-out chip
The option-heavy player UIs (mixer, camera director, viz, audio routing)
all live in the rail popovers, which are exclusive: openPopFor() closes
the last one before opening the next. You cannot watch the mixer while
riding the camera, and both vanish the moment you look at the highway.
Add `window.feedBack.panes` — a core registry for live UI that is
authored once as `mount(root, ctx)` and hosted anywhere. Panes are
non-exclusive, and they survive song switches structurally: the dock is a
body child outside every .screen, so the per-song teardown never sees it.
The adoption cost for a plugin is two calls:
feedBack.panes.register({ id, title, icon, mount, unmount });
feedBack.panes.attachChip(myExistingDialogEl, id);
attachChip injects THE standard pop-out chip. Clicking it opens the pane
and hides the plugin's dialog, leaving a stub to bring it back. Core owns
the hide/restore, so every plugin's pop-out looks and behaves the same —
which is the point. It hides via a dedicated .fb-pane-detached class, not
.hidden/[hidden], because the dialogs we attach to already toggle those.
Everything a pane may touch arrives through `ctx` — never a global. That
is what will let the same mount() run inside a pop-out window, a separate
JS realm with no window.feedBack, no window.highway and no audio graph:
ctx.call(domain, cmd, payload) -> the capability bus
ctx.on(event, fn) -> the feedBack bus (allowlisted)
ctx.subscribe(stream, fn) -> playhead / meters
ctx.state.get/set -> persisted, main realm is the only writer
ctx.playhead(), ctx.song(), ctx.toast(), ctx.close()
ctx tracks every subscription it hands out and drops them on unmount, so
a pane cannot leak listeners across a dock/undock cycle.
Streams exist because an AnalyserNode cannot cross a window boundary:
levels are reduced to numbers in the realm that owns the audio graph.
One shared rAF loop, refcounted against live subscriptions, dirty-checked
before fan-out, and stopped dead when the last pane closes.
Hosts register themselves with the manager rather than being imported by
it — the dock lands at priority 0 (the floor, always available), so the
OS pane window can drop in later without this code changing.
Ships two built-in panes: Now Playing (the reference pane — reads the bus,
a stream, and levels, and touches no globals) and Mixer (the same faders
as the rail, via ctx.call('audio-mix', ...), with the chip attached to the
real #mixer-control). Plus a "Panes" rail popover to open panes that have
no dialog of their own; the system tray will mirror that list.
Note the dock sits at z-index 110, not on the docs/plugin-v3-ui.md ladder
(transport 20, rail 30, popovers 40) — those live INSIDE #player's
stacking context, and #player is itself fixed at z-index 100. A dock below
100 is invisible on the one screen panes exist for. Body-level ladder:
#player 100 < dock 110 < toasts 120 < modals 200.
Pop-out windows, the system tray, manifest-declared panes and mirrorGlobal
(the window.__h3dCamCtl proxy the camera director needs) follow.
Signed-off-by: topkoa <topkoa@gmail.com>
|
||
|
Byron Gamatos
|
ea0ca94742
|
feat(career): career plugin — stars, venue tiers, pack downloads (career mode 2/3) (#907)
* feat(career): career plugin — stars from song_stats, venue tiers, pack downloads (career mode PR2) Bundled plugin: per-song stars from best_accuracy (60/75/85% → 1/2/3★), cumulative stars unlock bar → club → arena (data-driven venues.json). Venue packs (UE-rendered crowd loops) download on demand to CONFIG_DIR/plugin_uploads/career/ on a background thread with sha256 + zip-slip validation, served via FileResponse. Career screen (promoted sidebar entry) shows progress and pushes the active venue's manifest into the crowd video layer (v3VenueCrowd, PR1) — degrades cleanly when either side is absent. Pack URLs land in venues.json in PR3. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(career): keep manifest cleanup path alive on delete; badge only for installed venues Codex preflight: nulling _appliedManifestVenue on delete skipped pushCrowdManifest's setManifest(null) cleanup, leaving the crowd layer on a deleted pack; and the 'playing here' badge showed for an override venue whose pack was removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(career): generation-guard in-flight manifest fetches Codex preflight: a manifest fetch resolving after a newer refresh (pack deleted, venue switched) could re-apply a stale pack over the user's newer selection — fetches now carry a generation token and bail when superseded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(career): exclude orphaned song_stats from star totals Codex preflight: scans hide rather than delete stats of removed songs, so stars now apply the same existing-song filter other stats surfaces use (filename IN (SELECT filename FROM songs)). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(career): 50/150 star thresholds + star collection overview Byron's progression tuning: club at 50★, arena at 150★. /state now returns star_detail rows (title/artist joined from the library, stars, best accuracy, next-star threshold) sorted closest-to-next-star first, and the career screen renders a collection panel: tier summary plus a per-song list with a 'N% to next star' practice hint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(career): venue select/unselect UX, intro manifest support, fullmatch guards - 'Play here' now also defaults the visualization to Venue (remembering the prior viz); active venues show 'Leave venue' which restores it and sets the '__none__' override so no installed venue silently reapplies. - Pack manifests may ship an intro block (flyover video + ambience mp3); files validate like loops/stingers, .mp3 added to the serving whitelist. - Codex preflight: whitelist regexes use fullmatch (trailing-newline names could validate but 500 on serving). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(career): let pushCrowdManifest clear the manifest on Leave venue Codex preflight: nulling _appliedManifestVenue before refresh skipped the setManifest(null) cleanup branch, leaving the crowd playing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(career): refresh tailwind output --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Byron Gamatos
|
e779c72396
|
feat(venue): reactive crowd video layer behind the 3D highway (career mode 1/3) (#905)
* feat(venue): reactive crowd video layer behind the 3D highway (career 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>
* fix(venue-crowd): retry renderer binding + preserve mid-stinger transitions
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>
* fix(venue-crowd): per-video load tokens + unbind renderer on stop
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>
* fix(venue-crowd): flush deferred loop on stinger failure, source accuracy 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>
* fix(venue-crowd): requeue mid-crossfade loops preempted by stingers; 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>
* fix(venue-crowd): generation-gate stinger handlers; recrop on video size 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>
* fix(venue-crowd): bail loop-fade completion when a stinger preempted 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>
* fix(venue-crowd): keep rear video layer opaque during crossfades
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>
* fix(venue-crowd): reset active layer with mix on stop
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>
* fix(venue-crowd): reset crowd mood to neutral on song load
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>
* fix(venue-crowd): cancel in-flight fade when a stinger preempts it
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>
* fix(venue-crowd): don't let null accuracy resets wipe the end-of-song 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>
* fix(venue-crowd): abort stale stinger state on song load
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>
* fix(venue-crowd): always detach load listeners, gate only the callback
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>
* feat(venue-crowd): flyover intro with crowd-ambience ducking
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>
* fix(venue-crowd): fall back to the loop when the intro fails to load
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>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
ChrisBeWithYou
|
ffc52f13ce |
Harden freqs_to_midis against NaN/Inf; badges read exact tuningMidis
Follow-ups to #829 (CodeRabbit's review nit + the consumer adoption the PR body promised): - freqs_to_midis: reject non-finite frequencies (NaN/Infinity) — a provider handing one through would otherwise raise inside int(round(...)) and 500 GET /api/tunings. Tests cover nan/inf/-inf alongside the existing garbage cases. - v3 instrument badge: TUNING_NOTE now prefers the exact integer midis the server serves (tuningMidis) over reconstructing the note from the lowest string's frequency via log2 against a hardcoded 440 — which can land a semitone off at non-440 reference pitches. Frequency path kept as the fallback for older cached responses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MS2YFb6UUSwJVV6CmEa25i Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> |
||
|
Byron Gamatos
|
8d3db5f42c
|
fix(nav): nobody may monkey-patch window.showScreen — add screen:changing, make the shell listen (#924) (#925)
Some checks are pending
ship-ci / ci (push) Waiting to run
* fix(nav): the library sometimes showed the legacy screen — map 'home' inside showScreen
Testers: "randomly, when moving to the library from another menu option, the library shows the
old interface — never when a song ends."
━━━ WHAT WAS ACTUALLY HAPPENING ━━━
#home is the PRE-V3 library screen. The v3 shell replaced it with #v3-songs, and the mapping DID
exist — but only inside WRAPPERS on window.showScreen, and only for callers that go through
`window`. THREE independent parties monkey-patch it, each capturing whatever happens to be there
at the time:
app.js publishes the raw function
-> shell.js wraps it, adding the home -> v3-songs mapping
-> the stems plugin wraps it AGAIN (src/main.js:1029), capturing the current value
Plugins load ASYNCHRONOUSLY. The chain links up in whatever order the race settles, and any
capture taken before shell.js installs — or any re-assignment after it — silently drops the
mapping. Hence "randomly".
AND THE INTERNAL CALLERS NEVER TOUCHED window.showScreen AT ALL. closeCurrentSong and the
Esc-from-settings shortcut call the IMPORTED showScreen, which no wrapper ever sees. Reproduced
in a browser: the unwrapped function with 'home' lands on the dead legacy screen EVERY time.
"Never when a song ends" is the tell, and it is what identified the mechanism: closeCurrentSong
resolves its target through _resolvePlayerOrigin(), which ALREADY applies this mapping. That one
path was fine — which is exactly why the bug looked random rather than total.
PRE-EXISTING, not a regression from the module carve: the onclick="showScreen('home')" links and
the wrapper-only mapping both date to 2026-06-22.
━━━ THE FIX ━━━
The guard lives inside showScreen now: ONE place, in the function every caller routes through,
instead of a chain of monkey-patches that must each remember. Wrapper order stops mattering, and
the module-internal callers are covered for the first time.
Verified in a browser: the raw, unwrapped showScreen('home') — which reproduced as #home — now
lands on #v3-songs, and cannot be undone by any wrapper order.
━━━ AND A [P1] I INTRODUCED, WHICH CODEX CAUGHT ━━━
My first cut mapped BOTH 'home' and 'v3-home', copied straight from _resolvePlayerOrigin.
That is correct THERE and wrong HERE. _resolvePlayerOrigin computes where to RETURN TO after a
song, and landing on the Songs list from the dashboard is the right behaviour. But #v3-home is
the v3 DASHBOARD — a real screen that the shell's Home nav, the onboarding tour and the dashboard
re-render listener all target. Redirecting it would have made Home unreachable.
A LEGACY ALIAS IS NOT THE SAME THING AS A RETURN TARGET. Only 'home' is mapped now, and a test
pins that: re-adding 'v3-home' to the guard fails it.
4 tests, bite-tested both ways.
node 1049, pytest 2425, ESLint 0, Codex 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(nav): nobody may monkey-patch window.showScreen — add screen:changing, make the shell listen (#924)
window.showScreen was wrapped by THREE independent parties, each capturing whatever happened to be
there at the time:
app.js publishes the raw function
-> static/v3/shell.js wrapped it (to call syncActive, and to map home -> v3-songs)
-> the stems plugin wrapped it AGAIN (to tear down on leaving the player)
Plugins load ASYNCHRONOUSLY, so the chain linked up in whatever order the race settled. A capture
taken before shell.js installed silently dropped the mapping it carried — and the library opened on
the dead legacy #home screen. Testers saw that as "randomly, the library shows the old interface"
(#923).
#923 fixed the symptom by moving the mapping inside showScreen. This removes the CAUSE: neither
wrapper ever needed to be one.
━━━ TWO EVENTS, AND THE DISTINCTION IS THE WHOLE POINT ━━━
screen:changing emitted BEFORE anything happens. "I am leaving `from`." Teardown/cancel here.
screen:changed emitted after the DOM and data settle. "I am on `id`." Now carries `from`.
screen:changing is new, and it exists because Codex caught me collapsing the two. The stems plugin
tore down its audio graph BEFORE showScreen did anything; screen:changed fires at the very END,
after core awaits library and provider loads — so moving the plugin onto it would have delayed
teardown behind a slow fetch, or skipped it entirely if that fetch threw, and stems would keep
playing on a non-player screen. A test pins the ordering: screen:changing must precede the first
await.
shell.js is a plain screen:changed listener now, like app.js, audio-mixer.js and tour-engine.js
already were. window.showScreen is an unwrapped function again, and tests/js/
no_showscreen_monkeypatch.test.js fails CI if anything in static/ ever assigns to it again — so the
hazard is structurally impossible rather than merely avoided.
━━━ AND A FALLBACK THAT COULD NEVER FIRE ━━━
My retry-if-the-bus-is-late path listened for `slopsmith:capabilities:ready`. Core dispatches
`feedBack:capabilities:ready` (capabilities.js:1536) — the slopsmith: name is the PRE-DMCA event
and nothing has emitted it since the rename. Codex caught it. A guard that cannot fire is worse
than no guard: it reads as protection and is decoration.
(The same dead-event bug turned out to be sitting in THREE of the stems plugin's fallbacks, where
it has silently disabled its lifecycle wiring whenever the bus was late. Fixed in
feedback-plugin-stems#38.)
VERIFIED. A/B against origin/main: the nav highlight and topbar title follow IDENTICALLY with
shell.js as a listener; screen:changing -> screen:changed fire in order with the right {id, from};
window.showScreen is unwrapped; and showScreen('home') still lands on v3-songs.
node 1053, pytest 2425, ESLint 0, Codex 0.
Closes #924
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
f27d4f623c
|
fix(nav): the library sometimes showed the legacy screen — map 'home' inside showScreen (#923)
Testers: "randomly, when moving to the library from another menu option, the library shows the
old interface — never when a song ends."
━━━ WHAT WAS ACTUALLY HAPPENING ━━━
#home is the PRE-V3 library screen. The v3 shell replaced it with #v3-songs, and the mapping DID
exist — but only inside WRAPPERS on window.showScreen, and only for callers that go through
`window`. THREE independent parties monkey-patch it, each capturing whatever happens to be there
at the time:
app.js publishes the raw function
-> shell.js wraps it, adding the home -> v3-songs mapping
-> the stems plugin wraps it AGAIN (src/main.js:1029), capturing the current value
Plugins load ASYNCHRONOUSLY. The chain links up in whatever order the race settles, and any
capture taken before shell.js installs — or any re-assignment after it — silently drops the
mapping. Hence "randomly".
AND THE INTERNAL CALLERS NEVER TOUCHED window.showScreen AT ALL. closeCurrentSong and the
Esc-from-settings shortcut call the IMPORTED showScreen, which no wrapper ever sees. Reproduced
in a browser: the unwrapped function with 'home' lands on the dead legacy screen EVERY time.
"Never when a song ends" is the tell, and it is what identified the mechanism: closeCurrentSong
resolves its target through _resolvePlayerOrigin(), which ALREADY applies this mapping. That one
path was fine — which is exactly why the bug looked random rather than total.
PRE-EXISTING, not a regression from the module carve: the onclick="showScreen('home')" links and
the wrapper-only mapping both date to 2026-06-22.
━━━ THE FIX ━━━
The guard lives inside showScreen now: ONE place, in the function every caller routes through,
instead of a chain of monkey-patches that must each remember. Wrapper order stops mattering, and
the module-internal callers are covered for the first time.
Verified in a browser: the raw, unwrapped showScreen('home') — which reproduced as #home — now
lands on #v3-songs, and cannot be undone by any wrapper order.
━━━ AND A [P1] I INTRODUCED, WHICH CODEX CAUGHT ━━━
My first cut mapped BOTH 'home' and 'v3-home', copied straight from _resolvePlayerOrigin.
That is correct THERE and wrong HERE. _resolvePlayerOrigin computes where to RETURN TO after a
song, and landing on the Songs list from the dashboard is the right behaviour. But #v3-home is
the v3 DASHBOARD — a real screen that the shell's Home nav, the onboarding tour and the dashboard
re-render listener all target. Redirecting it would have made Home unreachable.
A LEGACY ALIAS IS NOT THE SAME THING AS A RETURN TARGET. Only 'home' is mapped now, and a test
pins that: re-adding 'v3-home' to the guard fails it.
4 tests, bite-tested both ways.
node 1049, pytest 2425, ESLint 0, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
57e7db5c2a
|
refactor(app): carve the keyboard-shortcuts subsystem into static/js/shortcuts.js (R3d) (#922)
19 declarations + 23 TOP-LEVEL STATEMENTS. 922 lines. app.js 3,243 -> 2,325 (-28%). The panel registry, both global keydown dispatchers, the library arrow-nav, and the whole plugin-facing shortcut API. ━━━ MOST OF THIS SUBSYSTEM WAS NOT DECLARATIONS ━━━ A declaration-seeded dependency closure reports this cluster as 10 names, 246 lines. It is 42 statements and 922. window.registerShortcut, createShortcutPanel, getAllShortcuts, unregisterShortcut, clearWindowShortcuts, the panel registry, and BOTH global keydown dispatchers are bare TOP-LEVEL STATEMENTS at app.js's top level. A call-graph scan sees NONE of them. That blind spot has now cost three times: * it nearly shipped a dead library A-Z rail (#896) — 43 of library.js's exports were referenced only from app.js's window contract; * it threw "Assignment to constant variable" in the session carve (#921), where the autoplay gate's top-level statements wrote state that had just become a read-only import; * and here it under-reported the slice by 3x. The extractor takes them by construction now — any top-level statement that TOUCHES a moved binding comes along — and the SEED is closed to a FIXED POINT, because those statements have their own dependencies (_modifiersMatch, _isShortcutActive, _handleLibArrowNav, _gridColumns…) that the declaration closure never walked. Seed -> pull the statements -> the statements need more names -> re-seed. Iterate until it stops growing. ━━━ syncLibrarySong GOES ACROSS THE SEAM, NOT THROUGH AN IMPORT ━━━ The library arrow-nav calls it on Enter. It cannot be imported: syncLibrarySong reaches showScreen/playSong, and a module importing app.js closes a cycle. It is the ONE name here that had to stay behind, so it comes across the host seam — which is exactly what the seam is for. host.js throws loudly if the wiring is ever dropped, and tests/js/host_contract.test.js fails in CI if the hook drifts. VERIFIED. A/B against origin/main in two browsers, IDENTICAL, zero page errors — and driven for real, not merely present: the plugin API (register / unregister / getAll / panels), THE GLOBAL KEYDOWN DISPATCHER actually firing a registered shortcut, that same shortcut correctly SUPPRESSED while typing in a text input, and `?` opening the help modal. node 1045, pytest 2425, ESLint 0 (no-cycle clean), host contract 2/2, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
545e569ad6
|
refactor(app): carve the song session out of app.js — playSong, showScreen, closeCurrentSong (R3d) (#921)
36 declarations + the 4 autoplay/auto-exit gate statements. 359 lines. app.js 3,772 -> 3,242. Bodies VERBATIM. ━━━ THIS WAS "THE UNCUTTABLE HEART", AND IT IS 359 LINES ━━━ At the start of this epic, seeding a dependency closure from count-in, from loops, from section-practice, or from the JUCE seek shim all returned the SAME 178-function, 3,360-line set. playSong and showScreen called each other; everything called them; nothing could be cut anywhere. The conclusion — correct at the time — was that NO closure-based carve could touch it at any seed, and the answer was a host seam. That was true THEN. Every slice taken out since (transport, loops, count-in, section-practice, the library, the edit modal, settings) removed edges, and the strongly-connected component DISSOLVED. This closure is 36 declarations with an interface width of FOUR. The lesson is not that the seam was wrong — the seam is what MADE this possible, by letting the carves proceed against a cyclic core instead of stalling on it. The lesson is to RE-MEASURE. An SCC is a fact about a graph at a moment, not a property of the code. ━━━ THE BUG NO SCAN COULD SEE, AND THE A/B DID ━━━ First cut passed every gate — no-undef clean, no-cycle clean, 1045/1045, pytest green — and THREW IN THE BROWSER: "Assignment to constant variable." window.feedBack.holdAutoplay / holdAutoExit and their two event handlers are TOP-LEVEL STATEMENTS, not declarations. They WRITE this cluster's state (_autoplayHeld, _autoExitTimer, …), and an imported binding is READ-ONLY — so left behind in app.js, every one threw the instant the module existed. A dependency scan that walks DECLARATIONS cannot see them. Mine didn't. This is the same blind spot that nearly shipped a dead library A-Z rail (#896): app.js keeps its public API in top-level statements, and a call-graph is blind to every one of them. The extractor now finds them by construction — any top-level statement that WRITES a moved binding comes with the carve — and the gate statements live beside the machinery they drive, which is where they belonged anyway. ━━━ ZERO OUTSIDE WRITES, BY MOVING THE BOUNDARY RATHER THAN BUILDING MACHINERY ━━━ The autoplay scalars and the wake-lock state were written from outside the cluster, which would have forced a setter or a state container. But the writers — _releaseAutoplay, _acquireWakeLock — plainly belong here. Pulling them in left ZERO outside writes, so every export is a plain import. Same move as settings (#920): measure the writers before you reach for a container. VERIFIED. A/B against origin/main in two browsers, IDENTICAL, zero page errors — including the autoplay gate driven end to end: a plugin HOLDS autoplay, the song loads but does not start, the RELEASE fires it, and a stale release is a no-op. That is the exact machinery that was throwing. node 1045, pytest 2425, ESLint 0 (no-cycle clean), host contract 2/2, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |