cubing / cubing.js

🛠 A library for displaying and working with twisty puzzles. Also currently home to the code for Twizzle.
https://js.cubing.net/cubing/
GNU General Public License v3.0
245 stars 44 forks source link

[cubing.js issue] Skewb solver is broken. #249

Closed lgarron closed 1 year ago

lgarron commented 1 year ago

Steps to reproduce the issue

  1. Go to https://alpha.twizzle.net/edit/?alg=R+U+R%27+D2&puzzle=skewb
  2. Alg Tools > Solve

Observed behaviour

app.ts:164 Uncaught (in promise) Error: Tried to apply a transformation for a KPuzzle (PG3D #1) to a different KPuzzle (skewb).
    at KTransformation.applyTransformation (KTransformation.ts:71:13)
    at TrembleSolver.sgsPhaseSolve (tremble.ts:174:39)
    at recur (tremble.ts:101:29)
    at TrembleSolver.solve (tremble.ts:133:7)
    at solveSkewb (skewb.ts:45:35)
    at async Object.solveSkewbToString (api.ts:255:13)

Expected behaviour

The solution is added to the Moves field.

This issue occurs because https://github.com/cubing/cubing.js/blob/99c378f6c03dcca614096f2a70bccc6250eb01d1/src/cubing/search/inside/solve/puzzles/dynamic/sgs-side-events/skewb.sgs.ts#L22 is a different KPuzzle from the regular Skewb def. The easiest fix is probably to also change the KPuzzle instance to the one used by SGS, here:

https://github.com/cubing/cubing.js/blob/99c378f6c03dcca614096f2a70bccc6250eb01d1/src/cubing/search/inside/solve/puzzles/skewb.ts#L32

This probably affects a few other puzzles with custom search defs. (I'm actually a bit surprised it took this long for me to trigger that error! It seems to be rare to accidentally combine transformations from different puzzles.)

Environment

Chrome 107.0.0.0

🖼 Screenshots

No response

Additional info

No response

lgarron commented 1 year ago

Turns out we were already doing this correctly for Megaminx: https://github.com/cubing/cubing.js/blob/e9063918464b0e563e4166bdde8e237b7b9765fa/src/cubing/search/inside/solve/puzzles/megaminx.ts#L41

So Skewb is the only puzzle that needed a fix.