Repository navigation
feat(highway_3d): general per-panel zoom that works in every camera mode - #139
Conversation
…mera mode The only zoom control was "Locked zoom", which applies only while "Lock camera at frets 1-12" is on and the visible notes are at or below fret 12. In a small split-screen panel the auto-framed view could not be brought closer. Add a cameraZoom setting (0..1, 0.5 = unchanged) that scales the final camera distance in every mode, expose it first in panelControls so Split Screen's Viz popover shows it, and add window.h3dBgSetCameraZoom. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH
At the closest slider setting (0.55x camera distance) the outer fret numbers and strings were clipped by the panel edge, because the fret-row guard only probes the camera's centre column. Limit the near end to 0.7x, which keeps every fret number through 24 on screen. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 5 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Declare dependsOn: 'cameraLockLow' on the cameraLockZoom panel control so hosts such as Split Screen can grey it out until the lock is enabled. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Incremental review of All previously reported findings are resolved in current code: the neutral default ( Files Reviewed (3 files)
Notes
Previous Review Summaries (3 snapshots, latest commit a1e653b)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a1e653b)Status: No Issues Found | Recommendation: Merge Incremental review of Files Reviewed (1 file)
Previous review (commit 126769a)Status: No Issues Found | Recommendation: Merge Incremental review of Files Reviewed (3 files)
Previous review (commit 7c55a4a)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (5 files)
Reviewed by free · Input: 98.9K · Output: 11.6K · Cached: 553.5K |
…Locked zoom Raising the near limit to 0.7 left a single linear blend whose midpoint was 1.075x, so the default slider position moved the camera. Map the slider piecewise (0 -> 0.7, 0.5 -> exactly 1.0, 1 -> 1.45). While the lock is engaged cameraLockZoom has already scaled the distance and the view zoom multiplied on top, reaching 0.385x and 2.1x. Bound the view zoom by the lock multiplier so the product stays within the limits either slider documents. Document the panelControls dependsOn field. Add tests for the neutral default, the monotonic mapping and the product bound. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH
There was a problem hiding this comment.
Caution
The new default is not neutral. CAM_VIEW_ZOOM_MIN = 0.7 paired with CAM_VIEW_ZOOM_MAX = 1.45 gives 0.7 + 0.75 × 0.5 = 1.075 at cameraZoom = 0.5, so every existing user gets a 7.5 % farther camera on upgrade — with no control anywhere to put it back. The constant pair needs one value changed before this merges; everything else is secondary.
Reviewed changes — all three commits at 7c55a4a (5 files, +36/−9), read end-to-end against plugins/highway_3d/screen.js, its two updated source-scan tests, the live Split Screen host, and the camera-distance math traced through camUpdate().
- New
cameraZoomsetting — added toBG_DEFAULTS(:2423) and_BG_FLOAT_KEYS(:2843, so_bgCoerceclamps it to0..1), declared as instance state (:4869), read per panel in_bgLoadSettings()(:8809), and picked up by the settings-change dispatcher (:8504). - Applied to the camera distance —
camUpdate()multiplies thecurDistlerp target by_viewZoomMulafter_fretRowFitBoost(:15702), so it reaches locked and auto framing alike. Two float ops on the per-frame path; no new allocation or DOM work, so the plugin's 60 fps budget is unaffected. - New per-panel control descriptor —
cameraZoom("Zoom (In ↔ Out)", range 0..1, step 0.05, default fromBG_DEFAULTS) headsfactory.panelControls(:16646), which is what Split Screen's Viz ⚙ popover renders.window.h3dBgSetCameraZoom(:2965) is load-bearing, not decorative: the host's_vizPanelSetderivesh3dBgSet+Cap(key)and returns early when the setter is missing, so without it the per-panellocalStoragewrite would never reach_bgEmitChange→_bgLoadSettings(). cameraLockZoomgainsdependsOn: 'cameraLockLow'(:16679) — see the inline note; the field is currently inert.- Version bump + CHANGELOG — 3.34.2 → 3.35.0 and an
[Unreleased]/Addedentry. The new CHANGELOG line correctly preserves the file's CRLF endings. - Two tests updated — the
panelControlskey list and thecurDistlerp regex.
node --test tests/js/*.test.js tests/plugins/*/js/*.test.js passes locally: 1242 pass, 0 fail, matching the PR's claim.
ℹ️ Nitpicks
- The PR body's "Not changed" section still reads "Locked zoom is still listed in the popover with no hint that it depends on the lock checkbox" — commit
7c55a4aadded that hint, so the body describes the pre-7c55a4astate.
big-pickle (free) | 𝕏
Codacy flagged the 1e-9 literals in the cameraZoom bound test. Use a named TOLERANCE constant with no exponent literal; the products under test differ from their limits by far more than float slack, so behaviour is unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH
There was a problem hiding this comment.
All three findings from my previous review are verified resolved against 126769a:
- Neutral default.
camViewZoomMulis now piecewise with a hard1.0×pivot at0.5, andcamViewZoomMul(0.5) === 1is pinned by a real assertion rather than a source-shape regex. Checked by hand:0 → 0.7,0.5 → 1.0,1 → 1.45, monotonic across the sweep,NaN → 1, out-of-range clamped. - Locked-mode stacking.
camBoundViewZoomclamps the product rather than the view zoom alone. I checkedprevLockActiveagainst all three sites that foldlockZoomMulintotgtDist(_applyNoteCamTargetsreturnslockActiveand every caller assigns it; the lookahead path sets it alongsidelockActive; the bootstrap-hold path correctly retains the prior value), so_lockMulNowis true exactly when the lock multiplier is already insidetgtDist. The product invariant holds at the boundaries — atlockMul ≤ 0.7the clamp pins the total tolockMul, atlockMul ∈ [0.7, 1]toMIN, and symmetrically at the far end. The near-end and far-end cases from my comment are both covered. dependsOn. Confirmed real:get-flashbacks/feedBack-plugin-splitscreen#78is merged, and the host's_wireVizControlDepsimplements exactly what the newCLAUDE.mdline claims —input.disabled+opacity: 0.4until the gate is on, with missing / non-toggle / self-referencing / cyclic targets all ignored. The doc no longer overstates anything.
node --test tests/js/*.test.js tests/plugins/*/js/*.test.js: 1245 pass, 0 fail — matches the updated PR body.
Nothing here blocks the merge. Two small follow-ups, both anchored below: a fourth copy of the lock-zoom expression that this file already has an explicit precedent against desyncing, and a user-visible side effect of the new bound that isn't written down anywhere.
big-pickle (free) | 𝕏
There was a problem hiding this comment.
a1e653b is cosmetic and correct — a named TOLERANCE in place of four repeated 1e-9 literals, with the loop bounds unchanged. node --test tests/js/highway_3d_panel_controls.test.js: 4 pass, 0 fail. Nothing to action.
One note for the record, no response needed: the constant is 1 / 1000000 (1e-6), so the tolerance loosened by 1000× rather than just being renamed. That is immaterial here — the smallest gap the assertions exist to catch is 0.385 vs 0.55, five orders of magnitude larger than either value, and total(1, 0) / total(1, 1) are still pinned by exact assert.equal — but the commit subject ("instead of 1e-9 literals") reads as though the value is unchanged.
big-pickle (free) | 𝕏
…raction Fold the four copies of the cameraLockZoom -> multiplier expression (the three locked tgtDist sites and the cameraZoom bound) into camLockZoomMul(), so retuning it cannot desync them. Identical arithmetic; a test pins the mapping (0 -> 0.55, 0.5 -> 1.0, 1 -> 1.45) and that only the helper computes it. Move the cameraZoom paragraph from the Splitscreen controls list to the Camera section and add that Locked zoom and View zoom interact: the further Locked zoom is from its default, the less of View zoom's travel does anything. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH

What
Adds a general camera zoom to 3D Highway that works in every camera mode, and shows it first in Split Screen's per-panel Viz ⚙ popover.
Why: the only zoom control was "Locked zoom" (
cameraLockZoom). It scales the camera distance only while "Lock camera at frets 1–12" is on and the visible notes are at or below fret 12. In a small split-screen panel the normal auto-framed view sits too far back and nothing in the popover could bring it closer, so the zoom slider looked dead.Changes (
plugins/highway_3d)cameraZoomsetting, 0–1. It multiplies the final camera distance in the camera update, after the fret-row fit guard, so it applies in locked and auto modes alike. The slider maps piecewise: 0 → 0.7×, 0.5 → exactly 1.0×, 1 → 1.45×, so the default leaves the framing unchanged (camViewZoomMul).cameraLockZoomhas already scaled the distance and the view zoom multiplies on top, so the view zoom is bounded (camBoundViewZoom) to keep the product within the limits either slider documents. Without the lock, it keeps its full range.panelControlsdescriptors, which Split Screen reads for its Viz popover. Per-panel values use the existingh3d_bg_panel<N>_cameraZoomkeys.cameraLockZoomdeclaresdependsOn: 'cameraLockLow'. Split Screen greys it out until the lock is on (merged as feedBack-plugin-splitscreen#78). Documented inplugins/highway_3d/CLAUDE.md.window.h3dBgSetCameraZoom(v)sets the global value. No slider was added to the global Settings page.CHANGELOG.mdentry.panelControlskey list and thecurDistlerp expression), and added tests for the neutral default, the monotonic mapping, and the bound against Locked zoom.Default behaviour is unchanged. At 0.5 the multiplier is exactly 1.0×.
How it was checked
node --test tests/js/*.test.js tests/plugins/*/js/*.test.js: 1245 pass, 0 fail.feedpak surface
Checklist
CHANGELOG.md[Unreleased]updated (user-visible changes)git commit -s): not done, the commits carry noSigned-off-byline🤖 Generated with Claude Code
https://claude.ai/code/session_01G99dXhcfEFmVR7R4jLpYRH