A plugin reload silently did nothing for scriptType:"module" plugins. ES modules are
evaluated ONCE PER URL PER DOCUMENT, so re-inserting a <script type="module"> whose src
the module map has already seen fires `load` without re-running the body — and the loader
then recorded the reload as applied. A no-op that reported success.
THE ISSUE UNDERSTATES IT. #879 says "upgrades are fine — a new version yields a new URL".
That is true of screen.js and FALSE of the plugin. I drove a real browser through
install(1.0.0) -> upgrade(1.1.0) -> rollback(1.0.0), counting evaluations of src/main.js:
ONE.
Not three, not two. The upgrade re-runs the one-line screen.js shim at its new ?v= URL;
the shim does `import './src/main.js'`; a relative specifier resolves against the base URL
WITH THE QUERY DROPPED; that is the same URL as before; the module map hands back the
already-evaluated v1.0.0 module. The plugin's own code never re-ran. Busting the entry
point cannot fix this, whatever token you hang off it.
So the token goes in the PATH: /api/plugins/<id>/g/<n>/screen.js. From there
'./src/main.js' resolves to /api/plugins/<id>/g/<n>/src/main.js — every relative import
inherits it, at every depth, for free. No import-specifier rewriting (which could never
see `import(expr)` anyway). Same browser drive after the fix: THREE evaluations.
Keyed on the plugin ID, not id@version: EVERY re-load of a module plugin needs a fresh
path, not just a rollback. First load keeps the stable ?v= URL, so the ETag/304 live-edit
caching the R0 rails depend on is untouched. Classic-script plugins are not affected and
never take a /g/ path.
━━━ A PATH REWRITE, NOT TWO MIRRORED ROUTES ━━━
Codex caught this, and it was right. The token shifts the BASE URL, so EVERYTHING the
module graph resolves relatively moves with it — not only imports.
`new URL('../assets/worklet.js', import.meta.url)` from /api/plugins/x/g/1/src/main.js
resolves to /api/plugins/x/g/1/assets/worklet.js. Mirroring only screen.js and src/ would
have fixed imports and 404'd every asset, worklet and wasm file the graph reaches — and
would have broken again the next time someone added a plugin route.
So the /g/<token> segment is STRIPPED BEFORE ROUTING. Every plugin route, present and
future, works under the prefix with no extra wiring. The token is opaque and never joined
into a filesystem path, so containment still rests entirely on the same safe_join.
Codex then caught a [P3] in that: eagerly re-encoding raw_path with latin-1 raises
UnicodeEncodeError on a valid plugin file like src/工具.js, 500ing a request the plain
route serves fine. raw_path is informational and Starlette routes on scope["path"], so the
mutation is simply gone — and leaving raw_path as the client sent it is more truthful for
logs anyway.
TESTS. tests/js/plugin_module_rollback.test.js (5) + 8 in test_plugin_src_route.py:
identical bytes under the prefix, the whole graph one and two levels deep, ASSETS (the
Codex [P2]), every plugin route, non-ASCII filenames (the [P3]), an opaque token, and
containment asserted as PARITY with the un-prefixed route rather than a guessed 404 —
`../screen.js` legitimately 200s on both, because the URL normalises before routing.
All bite-tested: reverting the fix fails the rollback tests, disabling the rewrite fails
the asset tests.
Two harnesses re-anchored on `script.src = _pluginScriptUrl(` — the URL literal they keyed
on now lives in the helper, further down the file, so their slice ran off the end.
node 1045, pytest 2404, ESLint 0, Codex 0.
Closes#879
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(highway_3d): size Butterchurn output canvas buffer to fill the highway
The 3D-highway Butterchurn background set only the output canvas CSS size
and called setRendererSize(), but never sized the canvas DRAWING BUFFER
(canvas.width/height). Butterchurn does not size the output canvas itself
(renderToScreen viewports to the reported size into the default
framebuffer), so the buffer stayed at the browser default 300x150 while the
viewport was the full highway. Only the bottom-left ~300x150 of the pattern
was drawn, then CSS-stretched across the whole highway -- zoomed, soft, and
aspect-wrong, worse the larger the panel.
Add _bcApplySize(cssW, cssH): set the drawing buffer to the device-pixel
render size (round(css * min(DPR, 1.5))), confine every layer (canvas,
backdrop, scrim, tint) to the highway rect, and report the same device px
to setRendererSize so buffer == on-screen viewport. Seed the buffer at
create and switch createVisualizer to pixelRatio:1, textureRatio:1 (DPR is
now folded into the reported size, so buffer == viewport == internal
texsize, no double-counting). render() and resize() both route through it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
* chore(highway_3d): bump to 3.31.5 (3.31.4 taken by #823 on main)
Rebased onto main; #823 already shipped 3.31.4 (per-panel camera), so this
Butterchurn buffer-sizing fix advances to 3.31.5.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
Opening the keys highway could silently switch the user's configured MIDI
device. Two coupled defects in the plugin's MIDI selection:
1. _midiAutoConnect only consulted the plugin's own localStorage pick
(keys3d_midi_pick); with none saved it fell straight through to "first
non-loopback device", ignoring the core midi-input domain's global
selection (Settings -> Input Setup, window.slopsmith.midiInput.getSelected()).
2. _midiConnect unconditionally persisted every connect to BOTH the local
pick and the shared domain selection (mi.select). So the first-device
guess got frozen locally and overwrote the global default that other
consumers (drums, Input Setup) rely on.
Make the domain-wide selection the source of truth: _pickMidiTarget now
resolves global -> legacy local pick (fallback + name-recovery for stale
ids) -> first device, and gates the "don't grab a random device" recovery
guard on any configured preference. Gate persistence behind an explicit
`persist` flag so only a deliberate device selection writes the local pick
and the shared global; auto-connect and programmatic (audio-input) opens
open the resolved device for the session without touching either store.
mi.select() is not needed to open (open takes the logicalSourceKey directly),
so dropping it from the auto path costs nothing.
Interim step toward instrument-scoped selection in the midi-input domain
itself (the input_setup wizard is already per-instrument, but the domain
stores a single selection); tracked as a separate core follow-up.
Pure decision logic extracted to _pickMidiTarget and covered by unit tests
in data_layer.test.js.
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
_resolveFreeCam() (keys/drum) and _freeCamFor() (highway_3d) guarded the panel
map lookup with only `i != null`, so a non-integer / negative / string index
from panelIndexFor() could resolve an unintended or inherited property (e.g.
map['toString']) instead of cleanly falling back to the global camera. Gate the
index on `Number.isInteger(i) && i >= 0` before `map[i]`, matching the hardening
already applied in _bgPanelKey(). Extend the resolver tests with float/string
(prototype-key) cases. Behavior change only for malformed indices.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Kris Anderson <topkoa@gmail.com>
Finish the docstring pass for the CamDir bridge functions changed in this PR:
convert the two per-panel _freeCamFor delegating wrappers to JSDoc, matching the
pure _resolveFreeCam / _ssApi helpers. Comment-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Kris Anderson <topkoa@gmail.com>
- _bgPanelKey() treated any non-null panelIndexFor() return as a valid panel id,
so a NaN/non-finite index minted a bogus "panelNaN" localStorage key instead
of falling back to "main". Gate on Number.isInteger(idx) && idx >= 0. (The
camera path is already NaN-safe — panelsMap[NaN] misses and falls through.)
- Add a NaN/negative-index case to the resolver tests (drum 22, keys 57, pass).
- Convert the camera-bridge helpers' comments to JSDoc (_bgPanelKey, _freeCamFor,
_resolveFreeCam, _ssApi across the three plugins) to lift docstring coverage on
the changed surface. Comment/robustness only; no behavior change beyond the
NaN guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Kris Anderson <topkoa@gmail.com>
Follow-up to the _bgPanelKey alias fix: _freeCamFor already treats
panelIndexFor as potentially throwy and catches to keep framing stable, but
_bgPanelKey called it bare. A throwing splitscreen build would take down
background-settings resolution (and the render path) even though the camera
path falls back safely. Wrap the call in try/catch, falling back to 'main'.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Kris Anderson <topkoa@gmail.com>
Three review findings on the per-panel camera work:
- highway_3d: _bgPanelKey() resolved splitscreen via window.feedBackSplitscreen
only, while _freeCamFor() uses the feedBackSplitscreen||slopsmithSplitscreen
alias it claims to "mirror". If the rename lands, per-panel background settings
would silently stop being per-panel while the camera stayed per-panel. Resolve
the alias the same way in _bgPanelKey.
- drum_highway_3d: applyCamera()'s "before first positionCamera()" guard tested
`_camBaseH == null`, but _camBaseH/_camBaseD were initialized to 0, so the guard
never fired (and could apply a base-0 pose for a frame). Initialize to null.
- keys + drum: the PR claimed the Camera Director resolver was unit-checked, but
nothing exercised it. Extract the resolver into pure, exported helpers
(_resolveFreeCam + _ssApi), delegate the per-instance _freeCamFor to them, and
add tests/camera_bridge.test.js covering per-panel select, global fallback,
null-when-absent, throw-safety, and the slopsmith-alias resolution. Drum 15→21,
keys 50→56, all pass; behavior unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Kris Anderson <topkoa@gmail.com>
Address a review note on the free-camera block: the comments described the
bridge as "per-panel-aware" without naming the actual globals. Spell out that
_freeCam comes from _freeCamFor(highwayCanvas) — window.__h3dCamCtlPanels[
panelIndexFor(canvas)] when split, else the global window.__h3dCamCtl, else
null — and update the nearby comment that mentioned only __h3dCamCtl. Comment-
only; no behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Kris Anderson <topkoa@gmail.com>
Make the three 3D highways read the Camera Director bridge per panel so each
splitscreen panel renders its own camera (independent orbit/height/zoom/tilt/pan),
instead of all panels sharing the focused camera.
- Add a shared `_freeCamFor(canvas)` resolver to each highway: prefer this panel's
entry in `window.__h3dCamCtlPanels[panelIndexFor(canvas)]`, fall back to the
global `window.__h3dCamCtl`, else null (100% stock). Defensive on the splitscreen
global name (feedBackSplitscreen || slopsmithSplitscreen), NaN-safe, allocation-free.
- highway_3d (guitar): source `_freeCam` from the resolver (was global-only).
- keys_highway_3d: adopt the bridge for the first time — layer dolly/height/orbit +
pan/pitch offsets onto the pan/zoom follow rig at the camera write.
- drum_highway_3d: adopt the bridge — new per-frame `applyCamera()` folds the static
base pose + kick-pulse dip + free-cam offsets.
- In a follower (popped-out) window there is one panel, so the resolver yields
whatever camera the plugin set in that window; no highway change needed for pop-out.
Camera Director absent → resolver returns null → renderers behave exactly as before.
Bump each plugin patch version. Existing plugin tests pass (drum 15, keys 30); the
keys "default look unchanged" test confirms the stock path is byte-identical.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Kris Anderson <topkoa@gmail.com>
* feat(keys_highway_3d): sharp-layout modes, lane-color opacity, octave lines
Add a Highway layout section to the settings with a rebuilt way to draw
sharps/flats and lanes on the 3D piano highway:
- Sharps & flats layout (keys3d_bg_sharpMode): floating (original raised
plane), flat (one plane, zero-overlap piano-shaped tiled lanes with the
naturals evened out), and realistic (one plane, bars sized like the
physical keys). All geometry lives in pure laneSpanFlat()/laneSpanReal()
helpers. Default: realistic.
- Lane color opacity (keys3d_bg_laneOpacity, 0-1): fades the pitch-class
lane tint; at 0 it is a dark floor with guide lines only at the key-block
boundaries (E-F and each octave), toward 1 full vivid colored lanes. The
strips, per-lane separators and block lines crossfade with the value.
Default: 0.
- Octave separators (keys3d_bg_octaveGaps, default on) and Octave line
contrast (keys3d_bg_octaveContrast, 0-1): the B->C octave line is a dark
layer scaled by lane opacity plus a bright layer scaled by its inverse,
so it auto-shifts dark->bright as the lanes fade.
Settings re-read on init() so they apply on the next chart build. All other
behavior (MIDI scoring, palettes, camera, themes, hit feedback) is unchanged.
Unit tests cover the new defaults, the sharp-mode setting, and the lane
geometry (tiling/evening for flat, uniform/overlap for realistic).
Signed-off-by: LegionaryLeader <legionaryleader@gmail.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Update plugins/keys_highway_3d/settings.html
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Update plugins/keys_highway_3d/screen.js
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* fix(keys_highway_3d): don't trim active-range boundary lane when neighbor sharp is out of range
laneSpanFlat() trimmed a white key's edge for its neighboring black
key's lane even when that neighbor midi fell outside
range.activeLow..range.activeHigh — the neighbor's lane is never
drawn (see the activeLow/activeHigh skip around the lane-strip loop),
so the trim left a dark, unfilled sliver at the active-range boundary
with no sharp lane to fill it. Gate the trim on the neighbor being
in-range; callers that don't pass a range (e.g. the raw-tiling unit
tests) keep the prior unconditional-trim behavior.
Also add the CHANGELOG entry for this PR's feature set, following the
existing keys_highway_3d wording convention (no plugin-local
CHANGELOG exists; plugin.json was already bumped 0.1.2 -> 0.2.0 by
the original commits).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Signed-off-by: LegionaryLeader <legionaryleader@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
`injectPlayerButton()` anchored the injected Tuner button with
`controls.querySelector('button:last-child')`, which can match a NESTED
button that is not a direct child of `#player-controls`. `insertBefore(btn,
nestedButton)` then throws `NotFoundError` (the reference node must be a
direct child); since injection runs from the tuner's `screen:changed`
handler, the throw propagated out of the player-screen transition and
stalled its render. The v3 path was already safe (plugin-control slot);
only the classic anchor was bad.
Use `:scope > button:last-of-type` (direct child only) with a
`parentNode === controls` guard before insertBefore, falling back to
appendChild. Bump plugins/tuner 1.3.3 → 1.3.4.
Test: tests/plugins/tuner/js/inject_player_button.test.js — extracts the
real function and runs it over a faithful DOM model whose insertBefore
enforces the direct-child invariant; the nested-last-button case reproduces
the throw on the old anchor and passes on the new one (5 tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: byrongamatos <xasiklas@gmail.com>
* feat(keys_highway_3d): add note-colour palettes and selectable camera angles
Give the 3D keys highway player-facing view options and a tuned default
look, so the piano highway is readable out of the box and customisable
from the settings panel without touching code.
Note colours (settings -> Note colours, `keys3d_bg_palette`):
- Octaves (new default): each octave its own hue climbing the rainbow,
darker sharps, so pitch height reads at a glance on any note range.
- Rainbow: the original per-pitch table (colours unchanged).
- Vivid / Pastel: per-pitch variants.
- Emerald / Ice: single-hue two-tone (uniform naturals, darker sharps).
The pick drives the notes, key glow, lane guides and hit flames, live.
Camera (settings -> Camera angle, `keys3d_bg_camera`):
- Classic (the original low rig) / Elevated / Overhead (new default).
- Height, distance and tilt fine-tune sliders nudge the base vantage the
auto-pan/zoom follow-motion orbits; presets apply live.
The new defaults are opinionated for plug-and-play (octaves palette,
overhead camera, tilt -0.6); anyone who prefers the original look can
pick Rainbow + Classic. Settings changes are re-read on init() so they
apply on return from the settings screen, not only after a relaunch.
Scoring, hit-timing and MIDI handling are untouched -- these are purely
visual. Numeric FX keys clamp to declared ranges (FX_RANGES); the pure
colour/camera helpers are covered by unit tests (node --test).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: LegionaryLeader <legionaryleader@gmail.com>
* fix(keys_highway_3d): make Classic camera reproduce the original rig + drop per-frame camera alloc
Review follow-ups on the palettes/camera feature:
- "Classic" preset now reproduces the historical rig exactly. The tuned
plug-and-play downward aim (camTilt -0.6 x CAM_TILT_UNITS = -33) is baked
into CAM_PRESETS.overhead.lookY, and camTilt now defaults to 0 (neutral).
The default overhead look is byte-identical (effective lookY still -33),
but "pick Classic for the original look" is now actually true instead of
leaving a -33 down-tilt applied. settings.html tilt slider defaults to 0.
- _rig() writes into a hoisted reusable object instead of allocating a fresh
{y,z,lookY,lookZ} literal every frame, honoring the module's documented
"no per-frame allocations in draw()" discipline. Callers read it
synchronously and never retain it, so one shared instance is safe.
Tests updated for the neutral camTilt default; adds an invariant test that
the default overhead framing is unchanged and Classic + neutral tilt == the
historical LOOK_Y. Full JS suite green (1069 pass).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Signed-off-by: LegionaryLeader <legionaryleader@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
Switching the active window / alt-tabbing away (most often on Windows) can
trigger a GPU context reset. The 3D highway's WebGL renderer had no
webglcontextlost handler, so a lost context was left to escalate into a
render-process crash -- matching the intermittent "randomly crashes when I
change windows" desktop reports.
The renderer now binds webglcontextlost/webglcontextrestored on its own WebGL
canvas (ren.domElement): the loss is preventDefault()'d so the browser keeps the
context restorable, draw() bails while the context is down so no GL work runs on
a dead context, and on restore the viewport is re-applied and rendering resumes
(Three re-uploads scene resources on the next frame). Listeners are removed in
teardown.
Root cause is a strong hypothesis -- the crash is intermittent and
unreproducible -- but the fix is low-risk and additive and closes a real gap:
there was no context-loss handling anywhere in the renderer.
plugins/highway_3d 3.31.2 -> 3.31.3. Tests:
tests/js/highway_3d_context_loss.test.js (source-contract, like the other
highway_* tests). The sibling keys_highway_3d / drum_highway_3d renderers share
the same gap -- follow-up in their repos.
Claude-Session: https://claude.ai/code/session_01UR2Cr7GEu3yMY7SrfxH6c1
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The guitar/bass highway_3d renderer self-detects panel-canvas size changes
in its draw() loop and re-runs applySize() every frame, because the
splitscreen host overrides hw.resize and never calls renderer.resize().
The drum and keys highways lacked that fallback — they only re-framed when
the host explicitly called resize(w, h) — so their panels stayed framed for
the pre-fullscreen size while the guitar/bass panels adapted. Symptom: a
too-small, off-center highway in the drum/keys panels after maximizing a
split-screen session.
Port highway_3d's per-frame drift check into both draw() loops: re-apply on
backing-store change (canvas.width/height) AND on CSS-box drift
(clientWidth/clientHeight vs the last applied logical size, throttled to
every 10th frame). Track _lastHwW/_lastHwH + _appliedW/_appliedH per
instance and reset them in destroy() so a reused instance re-frames on the
next song.
plugins/drum_highway_3d -> 0.3.1, plugins/keys_highway_3d -> 0.1.1.
Tests: tests/js/drum_keys_highway_3d_resize_reframe.test.js.
Signed-off-by: Kris Anderson <topkoa@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Removes the Floating Button and Tuning Visibility settings sections and finishes retiring their still-live config: drops the disabledTunings menu filter and showFloatingButton gate from screen.js/ui.js and their persistence in routes.py (retired keys are stripped on write). Repositions the tuner panel opened from the v3 sidebar Plugins popover to anchor beside it via the host's stable plugin-control slot API (falling back to the popover id), clamped to the viewport so it can't open off-screen, and re-anchored on resize. Updates tuner config tests to the retired-key behavior; plugins/tuner 1.3.2 -> 1.3.3.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Disc radius 3.6->4.3, cymbal 3.0->3.6 (heights proportional) — hands-on
feedback said the gems were hard to read at the default camera. All
variant shapes key off these radii; accents still fit the lane gap.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
- BG_STYLES port (off/particles/lights/geometric; lights use the
pitch-class palette) mounted behind the scene; _bgGetAnalyser/
_bgReadBands (stems-first, one-shot #audio fallback, permanent-failure
latch); Ambience intensity + Audio-reactive settings; remounts on
style/intensity change
- Score-FX overlay canvas (drum_highway_3d pattern): +1 pops at the
scored key, ring pulse every 10-combo, milestone bursts at 25/50/100,
red wash on 3+ streak break (wrong notes AND swept misses); cleared
when idle, removed in teardown
- Tests: style id validation + FX defaults (30 total)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
- BG_STYLES port (off/particles/lights/geometric) into a renderOrder -1
group; _bgGetAnalyser/_bgReadBands (stems-first, one-shot #audio
fallback, permanent-failure latch, 5ms bands cache); Ambience
intensity slider + Audio-reactive toggle; remounts on style/intensity/
palette change and across kit-change scene rebuilds
- Score-FX overlay canvas (guitar drawScoreFx adapted to internal
scoring): +1 pops at the struck lane, ring pulse every 10-combo,
milestone bursts at 25/50/100, red wash on 3+ streak break; pooled,
cleared when idle, removed in teardown
- butterchurn/image/video deliberately out of scope
- Tests: style id validation + FX defaults (15 total)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
- Pooled spark bursts (PORTED highway_3d, pool 160) at the struck lane,
timing-colored via _timingHex/_classifyTiming (±50ms window, inner 40%
= on-time); streak-scaled counts (streakFx)
- Lane flashes resurrected as pooled additive gauss-tex quads at the hit
line (timing-colored; red for wrong hits); kick = full-width quad
- Kick pulse: camera dip + amber floor wash, exp decay, hitFx-scaled
- Approach highlight: lane stripes brighten toward their next note
- Open hi-hat: hh_open renders a warm ring around the gem (closes TODO);
orthogonal to accent/ghost/flam variants
- Settings: Hit sparks / Timing colours / Streak feedback toggles +
Hit feedback intensity slider (drum_h3d_bg_*, live-applying)
- All FX resources pooled/shared, disposed in BOTH teardown paths
- Tests: _classifyTiming boundaries + FX defaults (10 total)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
- setPixelRatio at last: DPR (cap 2; 1.25 when >1 instance) x host
adaptive bundle.renderScale — HiDPI displays were rendering at CSS
resolution and upscaling (soft/aliased)
- Bloom: port _bloomEnsure/_bloomDispose (UnrealBloomPass 0.65/0.5/0.82,
MSAA HalfFloat target, ACES<->None switch); hit-line, flames and
consume-glow benefit immediately; direct render is the degrade path
- First settings panel: settings.html (graphics category) with a live
Glow (bloom) toggle; FX scaffold (FX_DEFAULTS/readFxSettings/
window.keys3dSetFx, keys3d_bg_* keys, keys3d:settings event)
- Combo/accuracy/best-streak DOM HUD (drum_highway_3d pattern), gated on
a live MIDI session
- tests/fx_settings.test.js (3 tests; vm harness)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
- Consume host adaptive-quality bundle.renderScale; fold into DPR like
highway_3d (splitscreen proxy: >1 instance caps baseDPR at 1.25)
- Bloom: port _bloomEnsure/_bloomDispose (UnrealBloomPass 0.65/0.5/0.82,
MSAA HalfFloat target, ACES<->None tone-mapping switch); rebuilt across
the kit-change renderer recreation; direct render is the degrade path
- FX settings scaffold: FX_DEFAULTS + readFxSettings + drumH3dSetFx
(drum_h3d_bg_* keys), Graphics section in settings.html (bloom toggle,
default ON, live-applies)
- _applyLaneFlashes dead-code comment updated (visual consumer lands in
the hit-FX PR)
- __test export + tests/data_layer.test.js (vm harness, 8 tests) —
picked up by the CI glob from the bundling PR; README refreshed
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The retrace after the label-swap fix showed getParameters unchanged
(~2.5s / ~4% throttled main thread) — the real driver is Three r158+'s
transparent-DoubleSide two-pass path: renderBufferDirect renders such
objects back side then front side, setting material.needsUpdate BOTH
times, i.e. a full getParameters/program-cache lookup twice per object
per frame, plus double draw calls. (Found by reading the two-pass
branch in the vendored three.module.min.js right next to the
getParameters call site.)
All 18 transparent DoubleSide materials in this renderer are flat
unlit quads — technique markers, sustain rails, chord frames, lane
planes, halo bars — where the two-pass self-occlusion ordering buys
nothing. Declare forceSinglePass: true on all of them.
Also corrects the _setLabelMap comment's churn attribution (that fix
removes the label-swap contribution; this one removes the dominant
source). Plugin 3.31.1 -> 3.31.2.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A 4x-CPU-throttled retrace (the honest weak-hardware proxy) surfaced
three residual per-frame costs; stack attribution pinned each:
- getParameters/getProgramCacheKey (~4% of main thread): every pooled
label sprite map swap set material.needsUpdate, bumping
material.version and forcing full program re-resolution next render.
Swapping between two non-null cached textures never changes the
compiled program (USE_MAP define unchanged) — new _setLabelMap()
helper only flags needsUpdate on a null<->texture transition, used at
all 7 swap sites.
- getBoundingClientRect (~1.2%): the 3D highway's per-frame canvas-size
self-check forced a layout read every frame. The CSS-box drift read
now runs every 10th frame (or when the wrap isn't pinned); the
backing-store comparison stays per-frame with cheap property reads
and forces an immediate box read + applySize when it fires.
- set textContent: the core 60 Hz HUD clock rewrote hud-time (and
getElementById'd it) every tick for a display that changes 1/s — now
write-on-change with a cached element ref.
(The remaining textContent writer in the trace is notedetect's
badges.js — external repo, to be filed there.)
tests/js: resize-reframe shape test updated for the hoisted _bsChanged
gate, incl. an assertion that the throttle can never delay the
backing-store path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- isVisible() forces a fresh DOM sample (was serving the rAF loop's
throttled cache, contradicting its 'live DOM check' docstring).
- v3 chrome: reconcile the edge-driven overControls hover flag against
matches(':hover') on the throttled ~6 Hz tick — covers a missed
mouseleave (flag stuck true, transport never hides) and a re-created
#player-controls node with lost listeners.
- highway_3d pre-warm now also covers teachFg/teachSd label textures
and the technique sprite factories (mute X, hammer/pull triangles,
bend chevrons, slide arrows) per active-palette string colour, plus
a maintenance note tying new label styles to the warm list.
- Document that the visibility throttle's manual invalidations are
latency-only (periodic resample self-heals within ~10 frames), and
why highway_3d keeps its local lowerBoundT (downlevel hosts).
External-repo audit (finding 1): staffview, tabview, piano, drums,
keys_highway_3d, drum_highway_3d grepped — no cross-frame bundle
retention or bundle-identity checks found.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Initial commit — clean relaunch
* Initial commit — clean relaunch
* Remove external game/format terminology from docs and code
Reword references to the external game and its proprietary file formats
in comments, docstrings, UI text and identifiers; no behaviour change.
* Repoint dead slopsmith URLs -> got-feedback
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(midi): consume core midi-input domain + session robustness fixes (#2)
* feat(midi): consume the core midi-input domain instead of private requestMIDIAccess (#881)
Route Web-MIDI device access through window.slopsmith.midiInput
(discover/list/select/open/close) rather than a private
navigator.requestMIDIAccess(). One shared device-access boundary with
piano/drums/onboarding; retires this plugin's private Web-MIDI. The
note-detection 'midi' exact-verdict provider role and the audio-input
source export are preserved (now backed by midi-input source data). Saved
pick ({id,name}) stays compatible — domain sourceId == the old MIDIInput.id.
Live listener is the domain handle's addListener/removeListener; the async
init/resume/pause gate and device-vanish handling are kept.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): stop exporting MIDI into the audio-input domain
Keys MIDI now lives in the dedicated midi-input domain (#881). Continuing to
register pseudonymized 'midi-input-N' sources into audio-input polluted audio
device pickers — notably the onboarding guitar audio-input dropdown showed MIDI
devices under cryptic names. Unregister any leftovers and register none.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): guard async connect races, denied-discovery latch, key consistency
Codex preflight findings on the midi-input consumption:
- Don't latch `_midiReady` when discover() resolves denied/unavailable — only
on a handled outcome — so reopening retries the permission prompt.
- Add a generation guard to async `_midiConnect`: a slower open() from an
earlier selection can resolve after a device/None switch and install a stale
handle/listener. Discard (and close) superseded opens.
- Carry the domain `logicalSourceKey` on the source/selection descriptor and use
it for select/open/close instead of synthesizing `web-midi::<id>`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): invalidate pending opens on detach; clear phantom selection; manifest
Codex re-review (round 2):
- A detach driven by device removal (sources-changed) didn't advance
_midiConnectSeq, so a pending _midiConnect open could resume and install a
handle/listener for a now-gone source. Bump the generation inside _midiDetach
and capture myGen after it, so any later detach supersedes an in-flight open.
- If mi.open() yields no handle, _midiInput stayed set — a phantom connected
device the render loop's miss-sweeping would penalize. Clear it on the
no-handle and catch paths.
- plugin.json: replace the stale audio-input provider role (the removed
MIDI-into-audio-input export bridge) with the midi-input requester it now uses.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): make _midiResume idempotent under multiple live instances
Codex re-review (round 3): with the domain addListener API, a second live
renderer instance (splitscreen/overlapping lifetimes) calling _midiResume() while
already active could register the same listener again and double-deliver a MIDI
note to the focused instance (hit + duplicate misses). Return early when already
active rather than relying on the provider's Set-backed de-dup.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): release domain session on teardown; counter-free legacy source cleanup
Codex re-review (round 4):
- [P2] Last-instance teardown only detached the listener and never released the
midi-input domain session, leaking the requester ref. Add _midiReleaseSession()
(delegates to _midiDetach: close + null + generation bump) at the destroy site;
re-mount's _midiInit auto-connects from the saved pick.
- [P3] The legacy audio-input MIDI-source cleanup looped over the module-local
_aiRegisteredCount, which resets to 0 on an in-page upgrade so the prior build's
'keys-midi:input-N' entries were never unregistered. Iterate a fixed bound over
the known sourceId pattern instead (unregister of an absent source is a no-op).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): gate miss-sweep on live handle; don't open without a live renderer
Codex re-review (round 5):
- Miss-sweeping was gated on _midiInput, set as soon as a device is picked, but
the async mi.open() may still be pending (slow / permission prompt) — notes
passing during that window banked false misses. Gate on _midiHandle, truthy
only after a handle is opened and wired.
- A settings-only ensure-init (or a discover resolving after the last instance
was torn down) could open a midi-input session with no renderer to release it.
Gate mi.open() on _instances.size > 0; the pick is saved and a later mount
re-runs auto-connect.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): guard open-failure clobber + don't clobber saved device on unplug
Bringing keys-highway-3d in line with the piano/drums hardening:
- The _midiConnect catch cleared _midiInput unconditionally; a stale older open's
rejection could wipe a newer connect's _midiInput/_midiHandle (and leak the
handle). Only clear when myGen === _midiConnectSeq.
- The sources-changed recovery called _midiAutoConnect(), whose fallback persists
a substitute device — overwriting the user's saved pick on a transient
multi-device unplug. Parameterize _midiAutoConnect(allowFallback); recovery
passes false (reconnect the saved device only, never a persisted fallback).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): keep first-hotplug working; don't tear down a live session on re-init
Codex round-11:
- The unplug-recovery _midiAutoConnect(false) skipped ALL fallbacks, which broke
first-hotplug (open visualizer with no keyboard, plug one in → never picked →
no connect). Skip the substitute only when a saved pick exists but is absent
(preserve it); allow the fallback when nothing was ever picked.
- _midiInit re-ran _midiAutoConnect on every ready re-init (settings/splitscreen),
tearing down the live handle + releasing held keys. Only re-connect when there's
no live handle.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(midi): consume core midi-input domain + session robustness fixes (#2)
* feat(midi): consume the core midi-input domain instead of private requestMIDIAccess (#881)
Route Web-MIDI device access through window.slopsmith.midiInput
(discover/list/select/open/close) rather than a private
navigator.requestMIDIAccess(). One shared device-access boundary with
piano/drums/keys/onboarding. Saved pick ({id,name}, with legacy id-only
fallback) stays compatible — domain sourceId == the old MIDIInput.id. The
async in-flight init guard, _midiActive teardown gate, device-vanish handling,
and settings device APIs are preserved; the live listener is the domain
handle's addListener/removeListener. Degrades to no-MIDI when the domain
is absent.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): guard async connect races, denied-discovery latch, key consistency
Codex preflight findings on the midi-input consumption:
- Don't latch `_midiReady` when discover() resolves denied/unavailable — only
on a handled outcome — so reopening retries the permission prompt.
- Add a generation guard to async `_midiConnect`: a slower open() from an
earlier selection can resolve after a device/None switch and install a stale
handle/listener. Discard (and close) superseded opens.
- Carry the domain `logicalSourceKey` on the source/selection descriptor and use
it for select/open/close instead of synthesizing `web-midi::<id>`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): invalidate pending opens on detach; clear phantom selection; manifest
Codex re-review (round 2):
- A detach driven by device removal (sources-changed) didn't advance
_midiConnectSeq, so a pending _midiConnect open could resume and install a
handle/listener for a now-gone source. Bump the generation inside _midiDetach
and capture myGen after it, so any later detach supersedes an in-flight open.
- If mi.open() yields no handle, _midiInput stayed set — a phantom connected
device the render loop's miss-sweeping would penalize. Clear it on the
no-handle and catch paths.
- Declare the midi-input requester capability (degrade-noop) in plugin.json.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): make _midiResume idempotent under multiple live instances
Proactively mirror the keys-highway-3d round-3 fix (same multi-instance lifecycle
and identical code): a second live renderer instance calling _midiResume() while
already active could double-register the listener and double-deliver a MIDI hit.
Return early when already active.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): release the domain session on final teardown
Mirror the keys-highway-3d round-4 fix: last-instance teardown only detached the
listener and never released the midi-input domain session, leaking the requester
ref. Add _midiReleaseSession() (delegates to _midiDetach: close + null +
generation bump) at the destroy site; re-mount's _midiInit auto-connects.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): gate miss-sweep on live handle; don't open without a live renderer
Codex re-review (round 5):
- [P2] drumH3dEnsureMidiInit (settings-only, no renderer) auto-connected and
opened a midi-input session that destroy()/_midiReleaseSession would never run
to release — held until reload. Gate mi.open() on _instances.size > 0; the pick
is saved and a later renderer mount re-runs auto-connect.
- Mirror the keys-highway-3d miss-sweep fix: gate accumulation on _midiHandle
(live wired session), not _midiInput (set before the async open resolves), so
notes during a pending open don't bank false misses.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): guard open-failure clobber + reconnect saved device on replug
Bringing drum-highway-3d in line with the piano/drums hardening:
- The _midiConnect catch cleared _midiInput unconditionally; a stale older open's
rejection could wipe a newer connect's _midiInput/_midiHandle. Only clear when
myGen === _midiConnectSeq.
- The sources-changed handler only refreshed the list (never reconnected), so a
replug of the saved device didn't reattach. Reconnect on sources-changed via
_midiAutoConnect(false) — saved device only, no fallback, so a transient unplug
can't switch to / persist another input.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): keep first-hotplug working; don't tear down a live session on re-init
Same two round-11 fixes as keys-highway-3d (shared structure): the recovery
_midiAutoConnect(false) now allows a fallback when nothing was ever picked
(first-hotplug) while still preserving a saved-but-absent pick; and _midiInit
only re-connects when there's no live handle.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): select by logicalSourceKey, not provider-local sourceId (#4)
Codex review of the midi-input migration: resolving a selection with
find(s => s.id === id) treats the provider-local sourceId as globally unique,
so when the core midi-input domain aggregates multiple providers two devices
sharing a sourceId can't be distinguished — the wrong device opens and unplug
detection can miss that the selected key vanished.
Thread the globally-unique logicalSourceKey through save/read, _midiConnect,
auto-connect reconnect, the public keysH3dSetMidiInput entry, and the
capability _aiOpen path, preferring the key and keeping the bare sourceId /
name only as a legacy fallback. Mirrors the already-merged drums/piano
migration (_midiResolveSaved). Self-reviewed against that reference.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(midi): select by logicalSourceKey, not provider-local sourceId (#3)
Codex review of the midi-input migration (same issue as keys-highway-3d):
resolving a selection with find(s => s.id === id) treats the provider-local
sourceId as globally unique, so when the core midi-input domain aggregates
multiple providers two devices sharing a sourceId can't be distinguished —
the wrong device opens and unplug detection can miss the vanished key.
Thread the globally-unique logicalSourceKey through save/read, _midiConnect,
the auto-connect reconnect, and the public drumH3dSetMidiInput entry,
preferring the key with the bare sourceId / name only as a legacy fallback.
Mirrors the already-merged drums/piano migration. Self-reviewed.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: add plugin metadata (category, description, thumbnail) (#5)
* chore: add plugin metadata (category/description/thumbnail)
Refs got-feedback/feedBack#571
* chore: add placeholder thumbnail
Refs got-feedback/feedBack#571
* chore: add plugin metadata (category, description, thumbnail) (#4)
* chore: add plugin metadata (category/description/thumbnail)
Refs got-feedback/feedBack#571
* chore: add placeholder thumbnail
Refs got-feedback/feedBack#571
* fix(viz): register feedBackViz_<id> so the player viz picker lists this plugin (#5)
After the slopsmith->feedBack rename the factory was still registered as
window.slopsmithViz_<id>; the host viz picker only looks up
window.feedBackViz_<id> (no slopsmithViz alias exists) and silently skipped
this plugin. Add the feedBackViz_ global (aliased to the existing
slopsmithViz_ one for back-compat).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(viz): register feedBackViz_<id> so the player viz picker lists this plugin (#6)
After the slopsmith->feedBack rename the factory was still registered as
window.slopsmithViz_<id>; the host viz picker only looks up
window.feedBackViz_<id> (no slopsmithViz alias exists) and silently skipped
this plugin. Add the feedBackViz_ global (aliased to the existing
slopsmithViz_ one for back-compat).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(plugins): bundle drum_highway_3d + keys_highway_3d as in-tree core plugins
Import both 3D highway plugins from their standalone repos via git subtree
(history preserved), ahead of the visual-parity epic that ports the guitar
highway's polish to them.
- .gitignore: !plugins/drum_highway_3d/ + !plugins/keys_highway_3d/ exceptions
- keys plugin.json: "bundled": true (drum already had it)
- CI: JS test step gains 'plugins/*/tests/*.test.js' (+20 keys tests)
- static/tailwind.min.css regenerated (core build scans plugins/**)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(drum_highway_3d): narrow Auto-mode predicate so bundling can't steal guitar arrangements
has_drum_tab is pack-level; first-match-wins Auto order sorts this plugin
before highway_3d. Claim only drum arrangements, or packs nothing more
specific can render (Codex preflight P2).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: CHANGELOG — note the deliberate Auto-predicate narrowing
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(drum_highway_3d): MIDI lifecycle — no false misses on mid-song connect, promote survivor on destroy
- _missSweepFloor exempts notes that passed before the device wired up
from the miss sweep (lowered on seek-back; cleared with scoring resets)
- destroy() promotes a surviving instance to _activeInstance so
splitscreen panel teardown doesn't drop all MIDI routing
(Codex preflight round 2, both pre-existing in the imported code)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Sin <deathlysin@outlook.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Static-analysis follow-ups to the trace-backed fixes; each is cheap
insurance on machines where the profiled headroom doesn't exist.
- highway.js: _makeBundle now mutates one persistent per-instance
object instead of allocating a fresh ~35-field bundle every rAF
frame (xN under splitscreen). Object identity is stable and
meaningless; array fields still swap reference on chart changes,
which field-identity caches rely on. Contract documented in both
CLAUDE.mds.
- highway.js: new bsearchTime (lower-bound on .time) windows the
default 2D renderer's beat-line scan (was O(all beats) per frame);
bundle.lowerBoundT / bundle.lowerBoundTime expose the searches to
custom viz so they stop reimplementing visible-window culling.
- highway_3d: localStorage 'h3d_full_sus' polled at ~1 Hz instead of
every frame (synchronous storage read on the hot path).
- highway_3d: drawLyrics caches the measureText row layout keyed on
(lyrics ref, line index, shown count, font size, width) — per-frame
work is now just drawing over cached widths.
- tests/js: bundle source-shape assertions widened to accept the
assignment form ([:=]) alongside the old object-literal form.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Trace showed frame spikes from Three.js first-use costs mid-song:
shader program compilation (getParameters/getProgramCacheKey) and lazy
texture uploads (texSubImage2D) whenever a chord name, section banner,
or fret label first appeared.
- ren.compile(scene, cam) after initScene (pools already warmed by
feedBack#226, board built, background mounted) so programs compile
during the load spinner.
- Pre-rasterise + GPU-upload (ren.initTexture) the deterministic txtMat
entries: fret numbers 0-24 in the noteFret/fretRow/ghostFret combos
the per-frame paths request.
- Chart-dependent labels (chord template names, section names) prewarm
once on the first draw() after each init, when bundle arrays are
guaranteed populated.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Working-tuning follow-ups:
- Mic-verify used _state.currentSongOffsets for the 'verified' stamp, but for a
MANUALLY-selected tuning (tuner opened off a song) that's a stale/different
song's tuning — so verify could mark the WRONG tuning verified. It now derives
the verified offsets from the tuning actually being checked (its target freqs;
the player's reference pitch cancels in the ratio), so 'verified' always
attaches to the tuning the player confirmed. Explicit offsets still win.
- Adds docs/working-tuning-on-device-tests.md: the checklist for the parts that
can't be covered headlessly — the auto-open/gate flow, both-directions prompts,
mic-verify detection, and the tuner-mic-vs-note_detect ASIO/exclusive-mode
contention flagged in the design charrette.
Test: mic-verify with no explicit offsets / no song context derives the correct
offsets (Drop-D). 55 tuner tests green.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(tuner): mic-verify — promote the working tuning assumed→verified via a per-string check (working-tuning PR 9b)
Adds the choreographed per-string mic verification the design reserved for
'verified' provenance (audio-engine's honesty rule — nothing else may claim it):
the player plays each string, and once every one reads in tune (±6 cents) and
holds stable for 8 frames, the tuner stamps the working tuning
provenance:'verified' + verifiedStrings via workingTuning.set.
- screen.js: a pure verify state machine (verifyStart/verifyFeed/verifyCancel/
verifyState, exposed on the tuner API) + the set-verified writer; cancels on close.
- ui.js: updateUI feeds each processed frame (matched string + cents) into the
session; a "Verify tuning" button + per-string progress + status, shown for a
selected (non-free) tuning.
Pairs with the 9a lifecycle: a 'verified' decays back to 'assumed' on the next
song load, so mic-verify is a per-session confidence boost, never a sticky claim.
Tests: tests/js/tuner_auto_open.test.js +4 (all-strings->verified, out-of-tune
never completes, streak resets on drift, API exposed / only it claims verified) —
33/33. The state machine is headless-verified with synthetic frames; the real
per-string mic detection + the button flow need an on-device pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
* fix(tuner): mic-verify writes the confirmed tuning + no clobber + stricter streak (PR #670 review)
Review fixes for mic-verify (working-tuning PR 9b):
- 'verified' could attach to STALE offsets: _publishVerified stamped provenance
without writing offsets, so the slot's pre-tuning offsets got marked verified.
verifyStart(targets, offsets) now captures the confirmed tuning's offsets, and
_publishVerified writes offsets + stringCount + instrument + referencePitch +
verifiedStrings ATOMICALLY with provenance:'verified' into the selected slot
(and refuses to stamp verified with no concrete offsets).
- The assumed publish-on-clear immediately clobbered a just-earned 'verified':
disable() now skips it when a mic-verify wrote verified this session
(_verifiedPublished).
- The per-string streak could accumulate across silence / wrong-string frames.
verifyFeed now requires CONSECUTIVE in-tune frames: the one confirmed string
advances, every other unfinished string resets each frame.
- A mid-verify tuning change (song switch) left stale captured offsets; verify is
now cancelled in _syncCurrentTuning when the song tuning changes.
Tests: verify writes the confirmed offsets (not stale); source-guard for the
no-clobber path. 47 tuner + 77 tuner/capability tests green. Codex-reviewed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
* feat(tuner): gate playback until you've tuned — hold autoplay + no-trap Skip/Back/Esc (working-tuning PR 4)
When the opt-in auto-open fires because a song needs a different tuning, playback
now WAITS behind the tuner instead of starting underneath it — the "tune before
you play" model. Built on a new generic core hook window.feedBack.holdAutoplay()
(mirrors holdAutoExit): the tuner claims the hold synchronously on song:loading
(beating the song:ready autostart) and releases it — or a 12s fail-open backstop
does — so a wedged plugin can never strand a song. Generation-guarded; manual
Play always wins.
No one-way trap:
- Skip = "I've tuned" -> plays and records the song's tuning as the instrument's
current working tuning (the explicit write-point PR 3 left as 'assumed').
- Back to library / Esc -> leave the song, record nothing (reuses requestExitSong;
Esc is the existing player shortcut).
- The in-panel x is dropped for an auto-open — Skip/Back/Esc are the dismiss
surface. This also keeps the write honest: Skip is the only on-player dismiss
that records, so leaving never falsely records a tuning.
Stacked on #660 (working-tuning PR 3). Core app.js gains only the generic hook
(a test asserts it never references the tuner's internals); shell-agnostic.
Needs a desktop smoke-test that the tuner mic doesn't contend with note_detect's
scoring input under ASIO/exclusive mode (per the design charrette).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
* fix(tuner): backstop can't cut off tuning + gate race/token hardening (PR #666 review)
Review fixes for the autoplay gate:
- The 12s fail-open backstop could start playback UNDER a legitimately-open tuner
(a slow / mic-verify retune > 12s). holdAutoplay()'s release now carries a
.settle() that cancels the backstop; the tuner calls it once the tuner is
confirmed open (_gateClaimed), so the hold becomes deliberate and only a
dismiss / song switch releases it. (Fail-open still covers "claimed but wedged
before deciding".)
- The async song:ready handler could release a NEWER song's gate after its await
(global _gateClaimed, no guard). It now snapshots _autoOpenGeneration and bails
if a newer song took over.
- holdAutoplay guarded by song generation, not per-hold — a stale release from an
earlier hold could clear a later one. Each hold now mints a unique token that
release()/settle() must match.
Tests: source-level assertions for the token, settle(), the settle-on-open call,
and the song:ready gen-guard. 45 tuner+speed tests green. Codex-reviewed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
* feat(v3): flag library songs by working-tuning match (working-tuning PR 6)
Each song's tuning chip in the v3 library grid is now coloured by whether your
CURRENT working tuning covers it: green = play it now, amber = needs a retune
(with a matching tooltip). Uses the tuner plugin's coverageReport (async), so it
runs as a post-paint decoration pass — chips render instantly, then colour a tick
later; a token cancels a superseded pass so scrolling stays snappy. Re-flags on
working-tuning-changed (retune / instrument swap / reset), no re-fetch.
Fully feature-detected: without the tuner coverage API + the host workingTuning
state, the chips render exactly as before. v3-only, single file (static/v3/songs.js).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
* fix(tuner/library): correct bass matching + memoize player tuning (PR #668 review)
Review fixes for working-tuning PR 6 (library tuning-match chips):
- Bass songs were scored against the guitar tuning. The chip passed no arrangement
to coverageReport, so isBassArrangement fell back to guitar — a 4-string bass
drop-D read as guitar could FALSE-MATCH a drop-D guitar player (green). songCard
now flags a bass-only song (every arrangement name matches /\bbass\b/) with
data-tuning-bass, and decorateTuningChips passes arrangement 'Bass'/'Lead' so
coverage uses the right base pitches. Mixed guitar+bass songs → guitar (the
song-level tuning is the guitar one); least-wrong given one tuning per song.
- Per-chip /api/settings fetch storm. coverageReport()→_playerTuning() fetched
/api/settings once per visible chip per grid paint (~60). _playerTuning is now
memoized (the player's tuning is song-independent) so all callers share one read;
invalidated on instrument:changed / working-tuning-changed, with a 3s TTL so a
settings write that doesn't emit an event still heals. A transient fetch failure
is NOT cached (next read retries) — else one hiccup would freeze coverage.
Tests: player tuning shared across songs (one fetch); transient-failure retry
(fails without the fix). The prior #680 dedup test updated for the memoized behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
* Tuner: passive "different tuning" badge cue that names the retune
Building on the coverage check: when you enter a song your current
instrument doesn't cover, the topbar tuner badge gets an amber ring + a
tooltip naming the change (e.g. "retune B->A", or "the reference pitch"
for an A440 vs A432 mismatch). Advisory only -- it never auto-opens the
panel; recomputed on song:ready, cleared on song-load / leaving the
player.
Refactors the coverage check into a structured report
(window._tunerAutoOpen.coverageReport -> { covered, retune:[{from,to}],
reference, cantCover }); the boolean gate now wraps it. The cue is
CSS-free (inline ring + native tooltip, no Tailwind rebuild) and no-ops
when the tuner plugin is absent.
Touches static/v3/badges.js (cue) + plugins/tuner/screen.js (report).
v3-only. Stacked on #656 (issue E stage 2.5/3). The splitscreen-suppress
and no-usable-input guards move to E2 (the playback gate).
Tests: tests/js/tuner_auto_open.test.js (report names the strings,
reference mismatch, badge wiring).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
* feat(tuner): read/write the live per-instrument working tuning — both-directions retune prompt (working-tuning PR 3) (#660)
The §4 coverage check compared each song against the player's fixed
instrument-profile tuning, so the tuner only ever prompted *away* from a "home"
tuning (E -> Drop C#) and stayed silent coming back (Drop C# -> E), even though
the player had physically retuned.
_playerTuning() now reads the host's live per-instrument working tuning
(window.feedBack.workingTuning, keyed by the selected instrument from
/api/settings) instead of re-deriving from the static settings tuning, so
coverage is measured against what the instrument is ACTUALLY in and prompts both
directions. On clearing an auto-opened tuner, _publishWorkingTuning() writes that
song's tuning as the instrument's live working tuning ('assumed' — PR 4's
explicit "I tuned / Skip" refines the write-point), so the next song is judged
against where the player now is.
Per-instrument (guitar vs bass tracked separately). Feature-detected: falls back
to the static /api/settings tuning when the working-tuning capability is absent,
so the 27 existing coverage tests are unchanged. Builds on PR 1 (host
workingTuning) + PR 2 (instrument->chart routing).
Tests: tests/js/tuner_auto_open.test.js — +2 (both-directions coverage via a live
Drop-D working tuning; publish-on-clear targets the right instrument slot); 29
pass total.
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tuner): transactional open + fail-closed auto-open config (tuner-E #655 review) (#681)
Two review fixes for the auto-open opt-in+persist stage:
- enable() wasn't transactional. The panel (with the ×/Skip buttons) is shown
before `await _tunerAudio.start()`, and `_state.enabled` was only set after it.
A ×/Skip dismiss during that await hit disable() with wasEnabled=false, then
enable() completed and flipped enabled on — an enabled-but-hidden zombie. Guard
the open with an `_openGen` token bumped on every enable()/disable(); after the
audio-start await, bail if superseded instead of enabling. Closes#675.
- Config wasn't fail-closed. routes.py normalized the opt-in with
bool(data.get("autoOpenOnTuningChange", False)), so "false"/"0"/junk coerced to
True. Accept only a real JSON boolean. Closes#676.
Tests: tuner_auto_open.test.js (dismiss-mid-open stays disabled — fails without
the token guard), test_config.py (auto-open default-false + fail-closed on
non-bool). 34 JS + 24 config tests green.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(tuner): coverage stays conservative when the instrument is unknown (tuner-E #656 review) (#682)
_playerTuning() is documented as conservative ("missing data → not covered → still
prompt"), but when /api/settings carried no instrument identity (a fresh profile:
_default_settings() omits instrument/string_count/tuning) it invented guitar/6/440/
standard, so an unconfigured player was treated as 6-string E-standard and coverage
suppressed the auto-open (and badge cue) for matching songs. The post-#660 rewrite
only returned null when the whole fetch failed (!s), not when settings existed but
lacked an instrument.
Now return null unless there's a confident identity — any of instrument/string_count/
tuning in settings, or live working-tuning offsets. A configured standard guitar still
covers a standard song (no regression). Closes#677.
Tests: tuner_auto_open.test.js — empty-settings → not covered (fails without the fix);
configured standard guitar → still covered.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(tuner): badge coverage cue staleness + unknown-as-warning + dedupe (tuner-E #657 review) (#683)
Three review fixes for the passive "different tuning" badge cue stage:
- Stale async cue (#678): _refreshCoverageCue awaited coverageReport then wrote the
DOM unconditionally, so a slow /api/settings fetch could restore the previous
song's amber ring after song:loading / leaving the player. Add a monotonic token
bumped on every refresh and both clear paths; apply the awaited report only if the
token still matches.
- "Unknown" rendered as "needs retune" (#679): the plugin returns a conservative
all-false report on a fetch hiccup; the cue painted that as an amber "retune the
reference pitch" ring. Collapse a no-signal report (not covered, no reference /
retune / cantCover) to null (no cue) via _meaningfulReport(). A genuine not-covered
report always carries reference / retune / cantCover, so real cues are preserved.
- Duplicate /api/settings fetch (#680): the auto-open gate and the badge cue both
call coverageReport() per song:ready. Cache the coverage promise per song (keyed by
session + tuning + centOffset) so they share one fetch; invalidate on song:loading,
instrument:changed, and working-tuning-changed so it can't go stale within a song.
Tests: tuner_auto_open.test.js — concurrent reports share one fetch, a new song
refetches (fails without the cache). 34 JS tests green. Codex-reviewed.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
With the opt-in auto-open on, only prompt when the player's current
physical tuning doesn't already cover the song. FeedBack is tune-to-song
(the highway draws tab in the song's tuning), so the check aligns the
song's open-string tuning string-for-string against the player's
instrument:
- An 8-string F# player gets no prompt for a 6-/7-string standard song
(its top strings already match those tunings).
- A song needing an open string the player lacks (e.g. a Drop-A
7-string's low A on an F# 8-string) still prompts.
- A whole-instrument reference difference (A440 vs A432, or an
octave-down centOffset, previously ignored) also prompts.
Reads the player's instrument from core /api/settings (the v3 instrument
selector, a stable physical reference); conservative fallback (prompt)
when undeclared or unavailable, so a real retune is never silently
skipped. v3-only. All in plugins/tuner/screen.js; no core changes.
Stacked on #655. Follow-up E1.6: a passive badge cue that names the
strings to retune, plus splitscreen / no-usable-input guards.
Tests: tests/js/tuner_auto_open.test.js (covered/uncovered, the Drop-A
case, reference mismatch, direct contiguous alignment).
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The tuner self-closes on song:play; autoplay fires it right after a song
switch, so an auto-opened tuner flashed shut ~1s later. An arrangement
switch (which never arms autoplay) instead persisted — the opposite
tester reports, and not the mic.
- New opt-in setting autoOpenOnTuningChange (tuner Settings, default OFF)
- An auto-opened tuner persists: it ignores the autoplay song:play, stray
outside-clicks, and same-screen re-emits, closing only via the new
in-panel x / Skip buttons or leaving the song. A manual open keeps the
classic click-away / play-to-close behaviour.
- Adds the panel's first in-box close (x + contextual Skip).
- All in the tuner plugin; no core app.js changes.
Default (opt-in vs opt-out) is teed up for Byron to flip one boolean.
Staged follow-ups: E1.5 = instrument-coverage smart prompting + badge
cue; E2 = holdAutoplay gate.
Tests: tests/js/tuner_auto_open.test.js (opt-in gate, persist mode,
play/click-proofing).
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The Reset handler forced base.enabled = true after copying _ASPECT_DEFAULTS
(where enabled is false) — a leftover from when enabled controlled panel
visibility. Visibility is now independent (Shift+A / ×), so drop the override
and let Reset restore the defaults exactly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
camUpdate registered every pane each frame regardless of whether the tuner
had ever been opened, so window.__h3dAspectPanes could grow unbounded (prune
runs only while the panel is open) and it ran even for users who never opt
in. Gate _aspectRegisterPane behind __h3dAspectPanelOpen (same gate as the
readout). The pane key is still resolved every frame so saved overrides keep
applying; only the picker bookkeeping is deferred until the panel is open.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
- Memoize _resolveTuneFor per pane, invalidated by a revision bumped on every
tune mutation (all writes funnel through _aspectPersist). Panes with an
override no longer rebuild the merged object every frame; panes without one
still return the base directly.
- _aspectNowMs falls back to Date.now() when the Performance API is absent, so
pane/readout pruning still works in older/borrowed contexts.
- _setAspectPanelVisible prunes stale panes before the first dropdown build, so
panes from a prior song/split don't flash until the first RAF tick.
- Rename _abShortcutRegistered/_registerAspectAbShortcut to
_tunerShortcutRegistered/_registerTunerShortcut — the shortcut opens/closes
the tuner now, it isn't an A/B toggle.
- Fix a stale 'pane1' example in a comment (keys are 'arr:<name>'/'pane:<uid>').
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
When only one pane is live the Target row is hidden, but _aspectEditTarget
could remain a specific pane key — silently routing edits into a hidden
(and persistent arr:*) override in single-player. Reset the edit target to
"" in _aspectBuildTargets whenever the row is hidden (or the selected pane
is gone), so single-pane edits always go to the shared base.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
- Set type="button" on the × close control and the Reset/Copy buttons so
they can never act as submit if the panel is ever nested in a <form>.
- Add aria-label="Target pane" to the Target <select> so screen readers can
identify the control.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
Three fixes from PR review of the per-pane tuner:
- Sync no longer writes back. _syncAspectPanel dispatches synthetic input
events to refresh slider labels; guard those with _aspectSyncing so the
slider handler skips the write. Previously opening/switching a target
populated a full override for every field (defeating sparse inherit) and
spammed localStorage.
- Unchecking "Override held hFOV" on a pane target now clears the override
key (via _aspectClearVal) so the pane re-inherits the base value, instead
of pinning hfovDeg:null in the override. On the base target it still sets
the explicit auto (null).
- _aspectPrunePanes now prunes the matching __h3dAspectReadout slot and drops
a dangling __last, so the readout cache can't grow unbounded as songs and
arrangements churn.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
The Target picker disappeared in split because it keyed panes off the
external splitscreen panel index (panelIndexFor), which isn't always
available — both panes then collapsed to a single 'main' key and the
one-pane row-hide kicked in.
Key panes by arrangement name instead ('arr:Bass'): distinct between split
panes AND stable across songs, with no dependency on the split plugin. A
per-instance id ('pane:N') is the fallback when a pane has no arrangement.
Only arr:* overrides persist to localStorage (instance-id fallback keys are
session-only, so they can't leak a new key each reload). This also gives
nicer semantics — a pane's framing follows its arrangement into the next
song.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
Per-pane overrides were keyed by an ephemeral per-instance id, so leaving a
song and opening another rebuilt the renderer with a new id and the pane's
framing was lost.
Key overrides by the durable split slot again ('main' | 'panel<idx>', via
_bgPanelKey) so the same slot means the same pane across songs, and persist
__panels to localStorage. Keep the anti-flicker fixes that were the actual
cause of the earlier dropdown churn (prune stale panes, rebuild only on a
pane-set change, never rebuild while the select is focused). The slot key is
latched to the last real slot so a transient null from panelIndexFor during
a song/layout transition can't flip it to 'main' and drop the override for a
frame; it resets in destroy() for instance reuse in another slot.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
The Target dropdown keyed panes off feedBackSplitscreen.panelIndexFor,
which can return the focused index for any canvas — so both split panes'
keys ping-ponged, rebuilding the <select> every frame (flicker) and
listing wrong/duplicate entries. The registry also never dropped panes
from a prior song or a closed split.
- Key each pane by a stable per-renderer-instance id (_paneUid, assigned
once in init) instead of the split panel index.
- Prune panes not reported within ~1.5s (song change / split teardown).
- Mark the dropdown dirty only when the pane SET changes, not on every
per-frame re-report, and skip rebuilding while the <select> is focused.
- Hide the Target row entirely when there's a single pane.
- Label panes by arrangement name, falling back to "Pane N".
Per-pane overrides are now session-only (keyed by ephemeral instance ids),
so they're no longer persisted to localStorage.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
Two usability gaps in the wide-pane framing tuner:
- No way to dismiss the panel. Add a × close button to the header and make
the Shift+A shortcut open/close the panel (reveal/dismiss). The A/B
enabled toggle now lives as a checkbox in the panel, so closing the panel
no longer changes the framing state.
- Edits hit every split pane at once. Add a Target selector (All panes, or a
specific pane labelled by its arrangement, e.g. "Panel 1 — Rhythm"). Per-
pane edits write a sparse override map (__panels[key]); each renderer
resolves the shared base with its own pane's overrides laid on top via
_resolveTuneFor(paneKey), so one pane can be framed independently. Reset on
a pane clears its override (re-inherits the base); Copy exports the resolved
values for the selected target. The live readout is keyed per pane.
Panes are discovered from the existing per-panel key (_bgPanelKey /
feedBackSplitscreen.panelIndexFor) and self-register each frame for the
picker. Overrides persist to localStorage alongside the base.
Tests extended in tests/js/highway_3d_wide_fov.test.js.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
