feat(studio): glowing instanced RotarySlider dial (C2) - #20
Conversation
…d ring edge-case tests
- remove speculative InstanceTransform.scale (YAGNI; dummy default scale=1 is identical)
- name HUB_SEGMENTS so its value isn't read as coupled to TICK_COUNT
- document the deliberate toneMapped={false} brightness choice
- test final-tick wrap-around (i/count off-by-one guard) and count=1 boundary
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces a new ChangesRotary Dial Feature
Possibly related PRs
Poem
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
C2: Glowing instanced "Rotary Slider" dial
Who is submitting this PR? (required)
claude-opus-4-8, 1M context). Implementation/review subagents: dispatched at theopustier; the harness does not surface the exact minor version and cannot pin 4.7, so workers most likely ran on Opus 4.8. Disclosed honestly.What problem are you trying to solve?
The studio's central 3D object was a placeholder
torusKnotGeometrywireframe — not the "Rotary Slider" dial the project is named for, and the R3F plan's Phase 2·Step 3 (the namesake dial via instanced meshes) was unchecked. The scene's conceptual centerpiece was missing.What does this PR change?
Replaces the torus-knot core with a glowing instanced rotary dial: 48 emissive box "ticks" evenly spaced on a ring rendered by a single
InstancedMesh(O(1) per-frame — matrices computed once inuseLayoutEffect, only the group rotates), plus a glowing hub. The tick geometry is a pure, unit-tested function (computeDialInstances). Integrated into the post-C1MatrixSceneso C1's intent-pulse "pop" is preserved (the wrapping group still scales on a new intent); the old per-axis tumble is removed to avoid double-spin.Is this change appropriate for the core library?
No. Fork-specific UI for the RotarySlider Feature Studio. Internal fork PR only.
What alternatives did you consider?
useFrame— rejected: defeats the O(1) goal; matrices are static, so they're set once inuseLayoutEffectand only the group transform animates.framer-motion-3dfor the spin — rejected (not installed; unmaintained); a one-lineuseFramegroup rotation suffices.Does this PR contain multiple unrelated changes?
No. One feature: the instanced rotary dial and its integration.
Existing PRs
Environment tested
npm test→ 10 passed (3 dial-geometry + 5 C1 store + 2 dial edge-case tests added in review).npm run build→ Compiled successfully;npx tsc --noEmit→ exit 0.npm run dev→GET / 200, clean compile.Failed to load url ../dialInstances) before implementation.New harness support
N/A.
Evaluation
N/A for skill evals — UI feature. Functional: red→green TDD on the geometry; two independent reviews — spec compliance (re-ran tests, verified the O(1) claim and that C1's pulse source files are byte-unchanged) and code quality (verified the InstancedMesh idiom +
useLayoutEffectflash-avoidance; flagged a speculativescalefield and missing edge-case tests, both since fixed — last-tick wrap-around +count=1now tested).Rigor
Human review
Summary by CodeRabbit
New Features
Refactor