mirror of
https://github.com/tiennm99/rubik.git
synced 2026-07-29 16:22:34 +00:00
refactor: simplifier pass — collapse duplicate code paths
- animate-move: factored shared promise/Tween body into tweenPivot; animateMove builds the pivot from spec, snapAndAnimate accepts a caller-built pivot. 96 → 80 LOC, no behavior change. - pointer-gesture: merged abortGesture / finishGesture / dispose busy release into a single idempotent endGesture; ownedBusy already encoded the per-gesture semantics. 205 → 190 LOC. - App: dropped the redundant `solving` state; ControlsPanel derives it as `busy && !solveActive`. runSolveStep collapsed to a one-line delegate since the busy gate now lives in CubeView. - CubeView: extracted cubejs lazy-load + parse into computeSolvePlan and dropped the dead bracket-undo guard. 41 tests still green; build clean. No deps, features, or behavior changes. Refs: plans/reports/code-simplifier-260427-1848-rubik-pass.md
This commit is contained in:
@@ -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.
|
||||
@@ -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();
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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
|
||||
});
|
||||
}
|
||||
|
||||
+2
-10
@@ -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}
|
||||
|
||||
@@ -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 @@
|
||||
|
||||
<section class="actions">
|
||||
<button onclick={onScramble} disabled={busy}>Scramble (Space)</button>
|
||||
<button onclick={onSolve} disabled={solving || busy} class:solve-active={solveActive}>
|
||||
<button onclick={onSolve} disabled={busy} class:solve-active={solveActive}>
|
||||
{solveLabel}
|
||||
</button>
|
||||
<button onclick={onUndo} disabled={busy}>Undo (Z)</button>
|
||||
|
||||
+29
-29
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user