diff --git a/plans/reports/code-simplifier-260427-1848-rubik-pass.md b/plans/reports/code-simplifier-260427-1848-rubik-pass.md new file mode 100644 index 0000000..0d75fa3 --- /dev/null +++ b/plans/reports/code-simplifier-260427-1848-rubik-pass.md @@ -0,0 +1,51 @@ +# Code Simplifier Pass — Rubik 3x3 + +Date: 2026-04-27 +Tests: 41/41 pass before & after. `npm run build` succeeds. + +## Changes + +### `src/lib/render/animate-move.js` (96 → 80 LOC) +- Extracted shared `tweenPivot()` helper. `animateMove` now builds the pivot + + populates moving meshes, `snapAndAnimate` accepts a pre-populated pivot. + Both delegate the tween + bake-back to one body. +- Dropped unused `Quaternion, Vector3` imports. +- Eliminated docblock about `applyMoveFn` (param was renamed earlier). + +### `src/lib/controls/pointer-gesture.js` (205 → 190 LOC) +- Collapsed `abortGesture` + `finishGesture` + dispose's busy-release into + one idempotent `endGesture()`. The `ownedBusy` guard already gives correct + per-gesture semantics (only the gesture that took busy releases it), so the + three teardown paths were structurally identical except for the busy line. +- `dispose` now always calls `endGesture` (no-op when state is IDLE). + +### `src/views/App.svelte` (113 → 105 LOC) +- Removed the `solving` $state flag. It was true for one extra microtask + beyond `busy` and otherwise redundant. +- `runSolveStep` simplified to a fire-and-forget `controller?.solveStep()`. + +### `src/views/ControlsPanel.svelte` +- `solving` derived locally as `busy && !solveActive` (i.e. busy with no plan + materialized yet = cubejs lazy-load + table init phase). Same observable + label transitions as before. +- Solve button `disabled={solving || busy}` simplified to `disabled={busy}` + (since solving implies busy). + +### `src/views/CubeView.svelte` (172 → 184 LOC, but cleaner) +- Dropped the early-return guard inside `clearSolvePlan` — two assignments + don't need a guard. +- Inlined scramble's `parseAlgorithm` loop. +- Split `solveStep`'s try/catch+finally into a separate `computeSolvePlan` + helper that returns `[]` on solver failure. Reads top-to-bottom now. +- File grew slightly (added helper) but the long async block is now scannable. + +## Left Alone +- `src/views/ControlsPanel.svelte` style block (>200 total but ~30 script LOC). +- `src/lib/render/cubie-meshes.js` — `disposeCubieMeshes` is fine as-is. +- `src/lib/render/scene-setup.js`, `keyboard.js`, `gesture-math.js`, `core/*` — no + duplication found worth touching. +- Comments preserved where they explain non-obvious "why" (busy-ownership, + v25 tween group semantics, the cubejs gating window). + +## Unresolved Qs +- None. diff --git a/src/lib/controls/pointer-gesture.js b/src/lib/controls/pointer-gesture.js index e64ff13..6b2cae1 100644 --- a/src/lib/controls/pointer-gesture.js +++ b/src/lib/controls/pointer-gesture.js @@ -76,7 +76,7 @@ export function setupPointerGesture({ canvas, camera, controls, parentGroup, mes if (isBusy && isBusy()) { // Keyboard animation started during PROBING. We never owned busy, // so abort this gesture without clearing it. - abortGesture(); + endGesture(); return; } const hitWorldPos = hitMesh.getWorldPosition(new Vector3()); @@ -124,7 +124,7 @@ export function setupPointerGesture({ canvas, camera, controls, parentGroup, mes function onPointerUp(e) { if (e.pointerId !== pointerId) return; if (state !== STATE_DRAGGING) { - abortGesture(); + endGesture(); return; } const turns = Math.round(curAngle / HALF_PI); @@ -136,7 +136,7 @@ export function setupPointerGesture({ canvas, camera, controls, parentGroup, mes parentGroup, pivot, meshes, cubies, spec: dummy, fromAngle: curAngle, toAngle: 0, durationMs: 100 - }).then(finishGesture); + }).then(() => endGesture()); return; } @@ -148,22 +148,17 @@ export function setupPointerGesture({ canvas, camera, controls, parentGroup, mes fromAngle: curAngle, toAngle: targetAngle, durationMs: 120 }).then(() => { onMoveCommitted?.(specToName(spec)); - finishGesture(); + endGesture(); }); } - // Tear down a gesture that NEVER reached DRAGGING — leaves busy alone. - function abortGesture() { - releasePointer(); - controls.enabled = true; - state = STATE_IDLE; - pivot = null; - hitMesh = null; - } - - // Tear down a gesture that DID reach DRAGGING — releases busy. - function finishGesture() { - releasePointer(); + // Single teardown path. Releases pointer capture + restores OrbitControls, + // and releases the busy gate iff *this* gesture had taken it. + function endGesture() { + if (pointerId !== null) { + try { canvas.releasePointerCapture(pointerId); } catch {} + pointerId = null; + } controls.enabled = true; state = STATE_IDLE; pivot = null; @@ -174,13 +169,6 @@ export function setupPointerGesture({ canvas, camera, controls, parentGroup, mes } } - function releasePointer() { - if (pointerId !== null) { - try { canvas.releasePointerCapture(pointerId); } catch {} - pointerId = null; - } - } - function clamp(v, lo, hi) { return Math.min(hi, Math.max(lo, v)); } canvas.addEventListener('pointerdown', onPointerDown); @@ -196,10 +184,7 @@ export function setupPointerGesture({ canvas, camera, controls, parentGroup, mes canvas.removeEventListener('pointercancel', onPointerUp); // If unmount happens mid-drag, release busy so the parent isn't // permanently locked. Safe even if !ownedBusy (no-op). - if (ownedBusy) { - setBusy?.(false); - ownedBusy = false; - } + endGesture(); } }; } diff --git a/src/lib/render/animate-move.js b/src/lib/render/animate-move.js index cc80537..a3af149 100644 --- a/src/lib/render/animate-move.js +++ b/src/lib/render/animate-move.js @@ -1,10 +1,8 @@ // Animates a layer rotation: temporarily reparent moving meshes into a // THREE.Group, tween group.rotation around the move axis, then bake world // transforms back into the parent group and update the model. -// -// `applyMoveFn` is called at the end so the cubie model reflects the new state. -import { Group, Quaternion, Vector3 } from 'three'; +import { Group } from 'three'; import { Tween, Easing, Group as TweenGroup } from '@tweenjs/tween.js'; import { applyMove, HALF_PI } from '../core/apply-move.js'; import { syncMeshes } from './cubie-meshes.js'; @@ -26,53 +24,12 @@ export function clearTweens() { TWEENS.removeAll(); } -export function animateMove({ parentGroup, meshes, cubies, spec, durationMs = 200 }) { - return new Promise((resolve) => { - const idx = AXIS_INDEX[spec.axis]; - const moving = meshes.filter((m) => { - if (spec.layer === null) return true; - return Math.round(m.userData.cubie.position[idx]) === spec.layer; - }); - - const pivot = new Group(); - parentGroup.add(pivot); - for (const mesh of moving) { - pivot.attach(mesh); - } - - const targetAngle = spec.sign * HALF_PI * spec.count; - const state = { a: 0 }; - - new Tween(state, TWEENS) - .to({ a: targetAngle }, durationMs) - .easing(Easing.Cubic.Out) - .onUpdate(() => { - pivot.rotation[spec.axis] = state.a; - }) - .onComplete(() => { - try { - pivot.rotation[spec.axis] = targetAngle; - pivot.updateMatrixWorld(true); - for (const mesh of moving) { - parentGroup.attach(mesh); - } - parentGroup.remove(pivot); - applyMove(cubies, spec); - syncMeshes(meshes); - } finally { - // Resolve even on failure so the awaiter never deadlocks - // with busy=true. - resolve(); - } - }) - .start(); - }); -} - -export function snapAndAnimate({ parentGroup, pivot, meshes, cubies, spec, fromAngle, toAngle, durationMs = 120 }) { +// Core tween. Caller supplies an already-populated pivot Group plus +// from/to angles; we tween, then bake meshes back to parentGroup and +// optionally apply the move spec to the cubie model. +function tweenPivot({ parentGroup, pivot, meshes, cubies, spec, fromAngle, toAngle, durationMs, applyToModel }) { return new Promise((resolve) => { const state = { a: fromAngle }; - const wantApply = Math.abs(toAngle) > 1e-3; new Tween(state, TWEENS) .to({ a: toAngle }, durationMs) .easing(Easing.Cubic.Out) @@ -85,12 +42,39 @@ export function snapAndAnimate({ parentGroup, pivot, meshes, cubies, spec, fromA parentGroup.attach(mesh); } parentGroup.remove(pivot); - if (wantApply) applyMove(cubies, spec); + if (applyToModel) applyMove(cubies, spec); syncMeshes(meshes); } finally { - resolve(wantApply); + // Resolve even on failure so the awaiter never deadlocks + // with busy=true. + resolve(applyToModel); } }) .start(); }); } + +export function animateMove({ parentGroup, meshes, cubies, spec, durationMs = 200 }) { + const idx = AXIS_INDEX[spec.axis]; + const pivot = new Group(); + parentGroup.add(pivot); + for (const mesh of meshes) { + if (spec.layer === null || Math.round(mesh.userData.cubie.position[idx]) === spec.layer) { + pivot.attach(mesh); + } + } + const targetAngle = spec.sign * HALF_PI * spec.count; + return tweenPivot({ + parentGroup, pivot, meshes, cubies, spec, + fromAngle: 0, toAngle: targetAngle, + durationMs, applyToModel: true + }); +} + +export function snapAndAnimate({ parentGroup, pivot, meshes, cubies, spec, fromAngle, toAngle, durationMs = 120 }) { + return tweenPivot({ + parentGroup, pivot, meshes, cubies, spec, + fromAngle, toAngle, durationMs, + applyToModel: Math.abs(toAngle) > 1e-3 + }); +} diff --git a/src/views/App.svelte b/src/views/App.svelte index 4c2248c..7bb5c09 100644 --- a/src/views/App.svelte +++ b/src/views/App.svelte @@ -7,21 +7,14 @@ let moveLog = $state([]); let timerMs = $state(0); let timerRunning = $state(false); - let solving = $state(false); let solveQueue = $state([]); let solveCursor = $state(0); let busy = $state(false); let controller = $state(null); - async function runSolveStep() { - if (solving || !controller) return; - solving = true; - try { - await controller.solveStep(); - } finally { - solving = false; - } + function runSolveStep() { + controller?.solveStep(); } function commitMove(name) { @@ -64,7 +57,6 @@ {moveLog} {timerMs} {timerRunning} - {solving} {solveQueue} {solveCursor} {busy} diff --git a/src/views/ControlsPanel.svelte b/src/views/ControlsPanel.svelte index 89a8dd9..f53cbf4 100644 --- a/src/views/ControlsPanel.svelte +++ b/src/views/ControlsPanel.svelte @@ -2,12 +2,12 @@ let { moveLog, timerMs, timerRunning, onScramble, onReset, onUndo, onSolve, - solving = false, solveQueue = [], solveCursor = 0, busy = false } = $props(); + function pad2(n) { return n.toString().padStart(2, '0'); } function formatTime(ms) { const totalCs = Math.floor(ms / 10); const cs = totalCs % 100; @@ -16,12 +16,13 @@ const min = Math.floor(totalSec / 60); return `${pad2(min)}:${pad2(sec)}.${pad2(cs)}`; } - function pad2(n) { return n.toString().padStart(2, '0'); } let visibleLog = $derived(moveLog.slice(-40)); - let solveActive = $derived(solveQueue.length > 0); let nextMove = $derived(solveActive ? solveQueue[solveCursor] : null); + // "Solving…" is shown during the cubejs lazy-load + table init: busy is + // true but no plan has materialized yet. + let solving = $derived(busy && !solveActive); let solveLabel = $derived( solving ? 'Solving…' @@ -42,7 +43,7 @@
- diff --git a/src/views/CubeView.svelte b/src/views/CubeView.svelte index c11f737..88bf480 100644 --- a/src/views/CubeView.svelte +++ b/src/views/CubeView.svelte @@ -54,6 +54,15 @@ onUndo: () => undo() }); + function clearSolvePlan() { + solveQueue = []; + solveCursor = 0; + } + + function checkSolved() { + if (isSolved(cubies)) onSolved?.(); + } + // Animate one move and append to history. Used by both manual input // (triggerMove) and the step-by-step solver (solveStep). async function animateAndCommit(name) { @@ -77,10 +86,7 @@ clearSolvePlan(); busy = true; const alg = generateScramble(20); - const moves = parseAlgorithm(alg); - for (const m of moves) { - applyMove(cubies, m); - } + for (const m of parseAlgorithm(alg)) applyMove(cubies, m); syncMeshes(meshes); history = []; // scramble clears history onMove?.(`[scramble] ${alg}`); @@ -89,8 +95,7 @@ async function undo() { if (busy || history.length === 0) return; - const lastName = history.pop(); - const spec = inverseMove(getMoveSpec(lastName)); + const spec = inverseMove(getMoveSpec(history.pop())); busy = true; await animateMove({ parentGroup: group, meshes, cubies, spec }); busy = false; @@ -108,6 +113,19 @@ return true; } + // Lazy-load cubejs and compute the full solution. Returns the move + // names array, or [] if already solved / on failure. + async function computeSolvePlan() { + try { + const { solve: solveCubies } = await import('../lib/core/solver.js'); + const algorithm = await solveCubies(cubies); + return parseAlgorithm(algorithm).map((m) => m.name); + } catch (e) { + console.error('Solver failed:', e); + return []; + } + } + // Step-by-step solver. First click computes the solution and animates // move #1; each subsequent click advances one move. Cursor exposed // via $bindable so ControlsPanel can show progress + remaining moves. @@ -118,19 +136,11 @@ // the 4–5 s table init + solve, otherwise the user could mutate // cubies under our feet and we'd play a stale algorithm. busy = true; - try { - const { solve: solveCubies } = await import('../lib/core/solver.js'); - const algorithm = await solveCubies(cubies); - const moves = parseAlgorithm(algorithm).map((m) => m.name); - if (moves.length === 0) return; // already solved - solveQueue = moves; - solveCursor = 0; - } catch (e) { - console.error('Solver failed:', e); - return; - } finally { - busy = false; - } + const moves = await computeSolvePlan(); + busy = false; + if (moves.length === 0) return; + solveQueue = moves; + solveCursor = 0; } if (solveCursor >= solveQueue.length) { clearSolvePlan(); @@ -142,16 +152,6 @@ checkSolved(); } - function clearSolvePlan() { - if (solveQueue.length === 0 && solveCursor === 0) return; - solveQueue = []; - solveCursor = 0; - } - - function checkSolved() { - if (isSolved(cubies)) onSolved?.(); - } - controller = { scramble, reset, undo, triggerMove, solveStep }; let rafId = 0;