keys_highway_3d: changing the hand filter mid-song resets the run's score #130

Activity

1. cchacs

   added

   bugSomething isn't workingSomething isn't working

   on Oct 2, 2026
2. pullfrog commented on Oct 2, 2026

   

   pullfrogBot

   on Oct 2, 2026 – with Pullfrog

   Last edited by pullfrog

   More actions

   Plan — #130: a hand-filter change keeps the run's score

   Small, well-bounded fix. Every factual claim below was read out of the code at main (d1980ff), not inferred.

   Diagnosis (verified)

   _setHandFilter() — plugins/keys_highway_3d/screen.js:3298-3310:

   function _setHandFilter(next) {
       if (next === _handFilter) return;
       _handFilter = next;
       if (!_notation) return;
       _notation.playable = filterNotationByHand(_notation.masteryPlayable, _handFilter);
       if (!notesGroup) return;
       const curTime = _latestTime;
       buildNoteMeshes();
       _resetScoring();               // ← the bug
       _anchorMissSweep(curTime);
   }

   _resetScoring() (screen.js:3516-3527) clears _hits/_misses/_streak/_bestStreak, both noteKey-keyed judged sets, _sweepCursor.idx = 0, _missFloor = null, _latestTime = 0, and every key flash. The sibling rebuild path _maybeApplyMasteryFilter() (screen.js:3958-3989) is byte-for-byte the same minus that one line, so this is genuinely a one-line divergence.

   Both entry points funnel through _setHandFilter, so one fix covers every reachable surface:

   • the global keys3d:settings event (screen.js:4334-4336), and
   • applySetting('handFilter', …) (screen.js:4510-4514) — Split Screen's per-panel popover, which is the mid-song repro.

   The open question in the issue is answered: no hidden-hand note is ever swept as a miss

   Checked rather than assumed, because it's the one thing that would make dropping the reset unsafe:

   • The sweep is called with _notation.playable, not the base list (screen.js:3844-3847). Hidden-hand notes are filtered out of playable at build time, so they are structurally unreachable by sweepMissed() while hidden — before the fix and after it.
   • The other miss route, judgeHit()'s failure branch, already returns early for a hidden-hand play: matchesHiddenHandNote(_notation.masteryPlayable, …) at screen.js:3486-3489 fires before _misses++.
   • Conversely, notes that a change brings back into view were never judged, so they are absent from _missedNoteKeys; _anchorMissSweep(curTime) puts the cursor past the elapsed tail, so they are not retro-missed, and they score normally once due. That's the intended semantics for switching back to Both.
   • Retained judged keys are the point: a note judged under Both stays judged if you narrow to LH and widen again, so it can't be double-counted — the same argument already written into the mastery path's comment.

   Changes

   1. plugins/keys_highway_3d/screen.js — _setHandFilter()

   Delete the _resetScoring(); call. Keep buildNoteMeshes() and _anchorMissSweep(curTime) exactly as they are, and keep the const curTime = _latestTime; read: the callers are an event handler and applySetting, which have no frame clock, so _latestTime (last drawn song time) is the correct argument. It is no longer read-before-a-zeroing-reset, so the ordering comment goes away with it.

   2. plugins/keys_highway_3d/screen.js — the two stale comments

   • screen.js:3293-3297 (doc comment) justifies the next === _handFilter early return with "can't reset scoring mid-song", which stops being true. The early return still matters and must stay — without it, Split Screen re-applying the same value on every provider refresh would rebuild all note meshes and re-anchor _sweepCursor/_missFloor on every refresh. Re-state the reason as the wasted mesh rebuild and the spurious sweep re-anchor, so nobody "simplifies" the guard away later.
   • screen.js:3307-3308 ("same as the mastery path") becomes accurate only after the fix; replace with what _anchorMissSweep buys — the run totals carry over, playback kept advancing, already-passed notes must not be retroactively missed.
   • Add a short note (mirroring screen.js:3977-3987) that the judged sets survive because they are keyed by noteKey(t, midi), not list index.

   Do not touch _resetScoring() itself — chart load still legitimately calls it at screen.js:3936, and _anchorMissSweep()'s own doc comment (screen.js:3529-3539) describes the reset-then-re-anchor combination that no longer exists on the hand path. Trim only the clause that would now read as stale.

   3. plugins/keys_highway_3d/tests/instance_invariants.test.js — regression test

   Add a source-level invariant beside the existing mastery one (tests/instance_invariants.test.js:67-76), reusing its exact shape — bounded single-wildcard body match terminated by the 8-space closing brace, explicit assert.ok(…, 'not found') guard:

   const fn = src.match(/function _setHandFilter\(next\)\s*\{[\s\S]*?\n {8}\}\n/);
   assert.ok(fn, '_setHandFilter not found');
   assert.doesNotMatch(fn[0], /_resetScoring\(\)/);
   assert.match(fn[0], /_anchorMissSweep\(/);

   Keep the wildcard bounded and scoped to one function body — two chained unbounded [\s\S]* is the ReDoS shape static analysis flags in this repo, and a path.join(__dirname, '..') read is the CWE-22 shape. The suite already reads screen.js via require.resolve('../screen.js').

   4. CHANGELOG.md + version

   • [Unreleased] → ### Fixed, mirroring the sibling keys_highway_3d: every mastery change resets the run's score #94 entry at CHANGELOG.md:20, and citing keys_highway_3d: changing the hand filter mid-song resets the run's score #130.
   • plugins/keys_highway_3d/plugin.json version 0.2.6 → 0.2.7 (same bump PR fix(keys_highway_3d): keep run score across mastery changes #129 made).

   Verification

   node --test plugins/keys_highway_3d/tests/*.test.js     # use the glob; hand-listed subsets omit camera_bridge

   Green today plus the new test. The invariant test fails if the _resetScoring() line is reintroduced, and — by design — also fails loudly if _setHandFilter is re-indented or moved, which is the accepted behaviour of this source-scan shape in this suite.

   Manual repro (needs WebGL, so not automatable here): play a notated keys arrangement to a non-zero hit/streak, flip the hand filter from Split Screen's per-panel popover, confirm the HUD combo/accuracy does not jump and the stats posted at song end cover the whole run.

   Not in scope

   • No change to the neutral hidden-hand handling — it is already correct in both directions.
   • No change to _anchorMissSweep, sweepStartIndex, or the seek path; the hand change inherits their existing semantics.
   • No plugin.json capability/schema change; the fix is inside the renderer.
   • No feedpak surface change, so no FEP is needed.

   Branch fix/130-hand-filter-keeps-run-score off main, targeting main.

   Task list (5/5 completed)

   • Read learnings sections (keys_highway_3d, instance invariants)
   • Inspect _setHandFilter / _maybeApplyMasteryFilter / _resetScoring / _anchorMissSweep in keys_highway_3d/screen.js
   • Inspect tests/instance_invariants.test.js mastery-path invariant test
   • Verify hidden-hand notes cannot be counted as misses while hidden
   • Produce structured plan and post via report_progress

     ^{｜ Implement plan ➔ ｜ View workflow run ｜ via Pullfrog ｜ Using big-pickle (free) ｜ 𝕏}
3. get-flashbacks

   deleted a comment from pullfrog on Oct 9, 2026
4. cchacs

   closed this as completedin #146on Oct 12, 2026