feat(keys_highway_3d): add note-colour palettes and selectable camera angles - #794
Conversation
… 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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds selectable note-color palettes and camera presets to the keys_highway_3d plugin. Introduces persisted palette/camera settings, palette-aware rendering, camera rig calculations, live retinting, settings UI controls, expanded tests, README updates, and a version bump. ChangesPalette and Camera Feature
Estimated code review effort: 3 (Moderate) | ~30 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/keys_highway_3d/screen.js (1)
1247-1249: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the stale camera-default comments.
These comments still describe
classicas the default/untouched view, butreadCameraSetting()now defaults tooverheadand the settings UI also selects Overhead by default.Suggested wording
- // Camera-rig presets. 'classic' is the original low, near-telephoto rig - // (numerically identical to the historical constants, so an untouched - // setting keeps the stock framing). y/z/lookY/lookZ are in pre-K world + // Camera-rig presets. 'classic' preserves the original low, + // near-telephoto rig numerically, while new/untouched installs default + // to the overhead reading view. y/z/lookY/lookZ are in pre-K world- // Camera — the default 'classic' preset is a low, near-telephoto rig + // Camera — the 'classic' preset preserves the original low, near-telephoto rigAlso applies to: 1662-1664
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/keys_highway_3d/screen.js` around lines 1247 - 1249, Update the stale camera preset comments in readCameraSetting() and the related Camera-rig preset docs so they no longer say classic is the default or untouched view; the current default is overhead, and the settings UI also selects Overhead by default. Keep the comment wording aligned with the actual behavior in screen.js, especially around the camera preset definitions and default selection logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/keys_highway_3d/screen.js`:
- Around line 2391-2395: In _applyPalette(), the flame rearm logic is forcing
every active slot to _flameTexture(0), which retints visible flames to the wrong
color after a palette switch. Update the flame spawn path to persist each slot’s
MIDI note on the slot object, then use that stored MIDI when reassigning mat.map
during palette rearming instead of hardcoding 0. Make sure the same fix is
applied anywhere else the flame pool is rearmed so active flames keep their
original note color.
---
Nitpick comments:
In `@plugins/keys_highway_3d/screen.js`:
- Around line 1247-1249: Update the stale camera preset comments in
readCameraSetting() and the related Camera-rig preset docs so they no longer say
classic is the default or untouched view; the current default is overhead, and
the settings UI also selects Overhead by default. Keep the comment wording
aligned with the actual behavior in screen.js, especially around the camera
preset definitions and default selection logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f438553-21d9-4c6b-a3d6-00729bef4b5a
📒 Files selected for processing (5)
plugins/keys_highway_3d/README.mdplugins/keys_highway_3d/plugin.jsonplugins/keys_highway_3d/screen.jsplugins/keys_highway_3d/settings.htmlplugins/keys_highway_3d/tests/fx_settings.test.js
| _clearFlameTextures(); | ||
| // Re-arm the pool so no slot keeps rendering a disposed texture | ||
| // (a flame mid-flight briefly re-tints — next spawn sets its | ||
| // true pitch texture). | ||
| for (const s of _flamePool) s.mat.map = _flameTexture(0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Retint active flames with their actual MIDI note.
_applyPalette() assigns every flame slot _flameTexture(0), so a visible hit flame changes to the C/sub-C1 color after a live palette switch instead of the struck note’s color. Store the slot MIDI on spawn and reuse it during palette rearming.
Proposed fix
- for (const s of _flamePool) s.mat.map = _flameTexture(0);
+ for (const s of _flamePool) {
+ s.mat.map = _flameTexture(Number.isFinite(s.midi) ? s.midi : 0);
+ }- _flamePool.push({ sprite, mat, start: -1, baseY: 0 });
+ _flamePool.push({ sprite, mat, start: -1, baseY: 0, midi: 0 }); slot.mat.map = _flameTexture(midi);
+ slot.midi = midi;
slot.start = wallNow;Also applies to: 2547-2550
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/keys_highway_3d/screen.js` around lines 2391 - 2395, In
_applyPalette(), the flame rearm logic is forcing every active slot to
_flameTexture(0), which retints visible flames to the wrong color after a
palette switch. Update the flame spawn path to persist each slot’s MIDI note on
the slot object, then use that stored MIDI when reassigning mat.map during
palette rearming instead of hardcoding 0. Make sure the same fix is applied
anywhere else the flame pool is rearmed so active flames keep their original
note color.
…+ 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>
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 customizable from the settings panel without touching code.
Note colors (settings -> Note colors,
keys3d_bg_palette):Camera (settings -> Camera angle,
keys3d_bg_camera):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 color/camera helpers are covered by unit tests (node --test).
Summary by CodeRabbit