Are there any plans to port PuzzleGeometry? #7
Comments
|
Not at the moment, but this may become necessary to implement server-side 3D rendering in the future. |
I see. I want to rewrite https://github.com/cubing/cubing.js/blob/main/src/cubing/puzzle-geometry/PuzzleGeometry.ts in Rust, and only the logic for generating the puzzle's permutation group (the data structure represented by the GAP output). The code isn't the easiest the follow, so do you have any advice on where I should start? |
|
I don't really understand the code myself, so I can't really be helpful on that front. Perhaps @rokicki can give you a breakdown of what you need? |
|
Let me know your points of confusion and I'll clarify in the code and/or in a separate document. I did go through and document it pretty extensively at one point, but the functionality has grown so much since then that all the additional features are probably obscuring the core ideas, which are really pretty simple. A rewrite (and generalization!) in any language would probably be a welcome thing. Right now only puzzles with the symmetry of the platonic solids are supported, and that's limiting. Also, no jumbling or shapeshifting is supported. It would be nice to support arbitrary external shapes, arbitrary cuts, jumbling, and so forth. |
|
For what it's worth, if we do have a good Rust implementation we can easily swap |
My specific use case suffices for the platonic solids. Perhaps there exists an older commit in the commit history https://github.com/cubing/cubing.js/commits/main/src/cubing/puzzle-geometry/PuzzleGeometry.ts that has a minimal implementation of the |
|
Here's a much older version (2018) with many fewer lines. The structure has not changed (although speed and functionality has been greatly expanded). https://github.com/rokicki/work/blob/master/quat/PuzzleGeometry.js Bringing it over to Rust might be challenging as I just willy-nilly share structures and depend on the garbage collector/etc. And frankly we probably want a Rust version as well, so there may be benefits to collaboratively doing it. But let's see if we can agree on the basic concepts first and go from there. At the simplest level, I just model the outer shell and the cuts in 3D, using quaternions for the math, and considering points "equal" if they are within epsilon. I calculate what the cuts do to a single face, and then "multiply" that across all the faces. I then join "stickers" (parts of faces) into cubies (where a cubie may have 1 to 5 "stickers") and derive orientation. From there I "simulate" rotations to calculate the underlying group as a permutation/orientation representation. |
Thanks for the reference and explanation. I will look over your code and try to translate it to Rust one-to-one. You mentioned in your 2018 version that the code isn't efficient, and that the new version is optimized for speed. Do you know which specific commits from the new version could be applied to the old version to improve its performance while maintaining its smaller and easier to work with file size? Additionally are there any major now-fixed bugs with the older version? |
|
I will stick to trying to port the current version for the sake of actually having types to work with. Raw JavaScript to Rust will take too much time to figure out. |
|
The code may look like it has types but it’s mostly using any.
- https://cube20.org/ <http://cube20.org/> - https://golly.sf.net/
<http://golly.sf.net/> - https://alpha.twizzle.net/
/ -
…On Tue, Dec 3, 2024 at 3:12 PM Arhan Chaudhary ***@***.***> wrote:
I will stick to trying to port the current version for the sake of
actually having types to work with. Raw JavaScript to Rust will take too
much time to figure out.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMOLS2VXNAV4FBL7OXA7CT2DY3EBAVCNFSM6AAAAABSZ3ZIHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMJVG43DKNBXGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
My port to Rust begins here, hopefully I don't run into too many issues. |
|
Let me know if you have any questions! I've been meaning to write a more
generic puzzle geometry (that supports different, and no, symmetries) but
there's always something else to do.
Do you plan to limit yourself to platonic geometries (as I did), or do you
plan to be more general?
Only straight cuts, or also curved cuts?
Do you plan to support blocking?
Do you plan to support only puzzles that always maintain the same shape, or
do you plan to
support puzzles (such as the helicopter) where different shapes are
available?
…-tom
On Fri, Jan 24, 2025 at 7:41 AM Arhan Chaudhary ***@***.***> wrote:
My port to Rust begins here
<https://github.com/ArhanChaudhary/qter/tree/main/src/puzzle_geometry>,
hopefully I don't run into too many issues.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMOLSZX4F42RJRB7TPWRB32MJNJRAVCNFSM6AAAAABSZ3ZIHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJSHAZDOMRZGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Will ask away as needed! 😄 My plan for now is just to dumbly translate the existing cubing.js puzzlegeometry module into Rust. My work won't actually do anything innovative or particularly new, and will support just a subset of what the existing puzzlegeometry module supports. I will start off with porting the minimum amount of code needed to replicate the results of |
|
I am also not entirely sure how the licensing works. Does rewriting puzzlegeometry and incorporating it into my codebase mean I would have to change the license from MIT to GPL? If so then I will gladly do so! |
If it's derived from ours, you can license your own code under any license compatible with either GPL or MPL. However, in order to accept contributions from you we'd currently need your agreement to license under both GPL and MPL. (We may drop the GPL in favor of only MPL at some point, but we haven't yet made concrete plans to do so.) That doesn't have to be done directly in your repo, but it would certainly be simpler if it does. |
|
Sweet, I agree to the licensing terms. I've just changed my repo's license to GPL appropriately. |
|
I noticed that diff --git a/src/cubing/puzzle-geometry/PuzzleGeometry.ts b/src/cubing/puzzle-geometry/PuzzleGeometry.ts
index e581de16..04a2db7a 100644
--- a/src/cubing/puzzle-geometry/PuzzleGeometry.ts
+++ b/src/cubing/puzzle-geometry/PuzzleGeometry.ts
@@ -2129,9 +2129,9 @@ export class PuzzleGeometry {
}
r.push(parsedmove[5]);
}
- } else if (this.options.vertexMoves && !this.options.allMoves) {
+ } else {
const msg = this.movesetgeos[k];
- if (msg[1] !== msg[3]) {
+ if (this.options.vertexMoves && !this.options.allMoves && msg[1] !== msg[3]) {
for (let i = 0; i < slices; i++) {
if (msg[1] !== "v") {
if (this.options.outerBlockMoves) {
@@ -2149,22 +2149,22 @@ export class PuzzleGeometry {
r.push(1);
}
}
- }
- } else {
- for (let i = 0; i <= slices; i++) {
- if (!this.options.allMoves && i + i === slices) {
- continue;
- }
- if (this.options.outerBlockMoves) {
- if (i + i > slices) {
- r.push([i, slices]);
+ } else {
+ for (let i = 0; i <= slices; i++) {
+ if (!this.options.allMoves && i + i === slices) {
+ continue;
+ }
+ if (this.options.outerBlockMoves) {
+ if (i + i > slices) {
+ r.push([i, slices]);
+ } else {
+ r.push([0, i]);
+ }
} else {
- r.push([0, i]);
+ r.push([i, i]);
}
- } else {
- r.push([i, i]);
+ r.push(1);
}
- r.push(1);
}
}
if (this.fixedCubie >= 0) { |
|
All this does is say if a puzzle has vertex moves opposite face moves,
prefer the vertex moves over
the face moves. This is only relevant for tetrahedral puzzles because
every other Platonic solid only
has faces opposite faces and corners opposite corners. What behavior do
you want it to have for
other puzzles?
…On Sat, Jan 25, 2025 at 4:56 PM Arhan Chaudhary ***@***.***> wrote:
I noticed that --vertexmoves does not generate output on non-tetrahedron
twisty puzzles. It looks like output is specially ignored in this case,
possibly erroneously. This is an issue because the input is arbitrary so
the flag has a chance to not output anything if the description
tetrahedron. Would this be an appropriate fix? If so then I can try to PR
this into cubing.js! @rokicki <https://github.com/rokicki>
diff --git a/src/cubing/puzzle-geometry/PuzzleGeometry.ts b/src/cubing/puzzle-geometry/PuzzleGeometry.ts
index e581de16..04a2db7a 100644--- a/src/cubing/puzzle-geometry/PuzzleGeometry.ts+++ b/src/cubing/puzzle-geometry/PuzzleGeometry.ts@@ -2129,9 +2129,9 @@ export class PuzzleGeometry {
}
r.push(parsedmove[5]);
}- } else if (this.options.vertexMoves && !this.options.allMoves) {+ } else {
const msg = this.movesetgeos[k];- if (msg[1] !== msg[3]) {+ if (this.options.vertexMoves && !this.options.allMoves && msg[1] !== msg[3]) {
for (let i = 0; i < slices; i++) {
if (msg[1] !== "v") {
if (this.options.outerBlockMoves) {@@ -2149,22 +2149,22 @@ export class PuzzleGeometry {
r.push(1);
}
}- }- } else {- for (let i = 0; i <= slices; i++) {- if (!this.options.allMoves && i + i === slices) {- continue;- }- if (this.options.outerBlockMoves) {- if (i + i > slices) {- r.push([i, slices]);+ } else {+ for (let i = 0; i <= slices; i++) {+ if (!this.options.allMoves && i + i === slices) {+ continue;+ }+ if (this.options.outerBlockMoves) {+ if (i + i > slices) {+ r.push([i, slices]);+ } else {+ r.push([0, i]);+ }
} else {- r.push([0, i]);+ r.push([i, i]);
}- } else {- r.push([i, i]);+ r.push(1);
}- r.push(1);
}
}
if (this.fixedCubie >= 0) {
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMOLS5W2DIHCIXOWJZEG2L2MQXEFAVCNFSM6AAAAABSZ3ZIHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJUGE2TMOBUGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I want a way to enforce vertex moves only if the puzzle is a tetrahedron, and ignore the flag if not. The way the flag is implemented currently does not allow this. Because puzzlegeometry works on an arbitrary description, for this to work as described I cannot rely on the flag and would instead have to manually check if the geometry is a tetrahedron and if so, disable it. This is annoying to do from the user end, so I think it makes sense for this to be done internally within puzzlegeometry. |
|
Another way to think about why this is useful is for programmatically running puzzlegeometry on a fixed set of settings as in twizzle explorer. This would make enforcing vertex moves only if the puzzle is a tetrahedron, and ignoring the flag otherwise very simple. |
|
I'm trying to review the change and having trouble. If all we want to do
is "ignore" the vertex move
option if the puzzle is not a tetrahedron, can't we just write that? As in:
let effectiveVertexMoveOption = this.options.vertexMoves ;
// if the shape is not a tetrahedron, make sure vertexMoves can be
safely ignored
if (effectiveVertexMoveOptions) {
// do whatever we need to do here to get the basic shape, and turn
effectiveVertexMoveOption to false if
// the shape is not tetrahedron
}
I think this would be clearer than further confusing the existing rather
confusing nesting of if's.
What do you think?
…-tom
On Sat, Jan 25, 2025 at 5:30 PM Arhan Chaudhary ***@***.***> wrote:
Another way to think about why this is useful is for programmatically
running puzzlegeometry on a fixed set of settings as in twizzle explorer.
This would make enforcing vertex moves only if the puzzle is a tetrahedron,
and ignoring the flag otherwise very simple.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMOLS2NOIUCSMDKNWQEU5L2MQ3CNAVCNFSM6AAAAABSZ3ZIHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJUGE3DGOBYGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
The indentation made the diff look weird, here is the same thing without the indent. This does exactly what you suggested. diff --git a/src/cubing/puzzle-geometry/PuzzleGeometry.ts b/src/cubing/puzzle-geometry/PuzzleGeometry.ts
index e581de16..26086132 100644
--- a/src/cubing/puzzle-geometry/PuzzleGeometry.ts
+++ b/src/cubing/puzzle-geometry/PuzzleGeometry.ts
@@ -2129,9 +2129,10 @@ export class PuzzleGeometry {
}
r.push(parsedmove[5]);
}
- } else if (this.options.vertexMoves && !this.options.allMoves) {
+ } else {
const msg = this.movesetgeos[k];
- if (msg[1] !== msg[3]) {
+ const isTetrahedron = msg[1] !== msg[3];
+ if (this.options.vertexMoves && isTetrahedron && !this.options.allMoves) {
for (let i = 0; i < slices; i++) {
if (msg[1] !== "v") {
if (this.options.outerBlockMoves) {
@@ -2149,7 +2150,6 @@ export class PuzzleGeometry {
r.push(1);
}
}
- }
} else {
for (let i = 0; i <= slices; i++) {
if (!this.options.allMoves && i + i === slices) {
@@ -2167,6 +2167,7 @@ export class PuzzleGeometry {
r.push(1);
}
}
+ }
if (this.fixedCubie >= 0) {
const dep = this.keyface3(this.faces[this.cubies[this.fixedCubie][0]])[k];
const newr = [];I'm not exactly sure what |
|
Also I believe I just found a bug. The following only fails with |
|
Definitely a bug. I'll fix it up.
…On Sat, Jan 25, 2025 at 8:58 PM Arhan Chaudhary ***@***.***> wrote:
Also I believe I just found a bug. The following only fails with
--vertexmoves and only with the Jing pyraminx:
# /usr/local/Cellar/bun/1.1.36/bin/bun /Users/arhan/Desktop/cubing.js/src/bin/puzzle-geometry-bin.ts --gap --vertexmoves Jing pyraminx
602 | movenameFamily = movenameFamily.toLowerCase();
603 | if (bits[1] > 1) {
604 | movenamePrefix = String(bits[1] + 1);
605 | }
606 | } else {
607 | throw new Error(
^
error: We only support slice and outer block moves right now. 1
at getmovename (/Users/arhan/Desktop/cubing.js/src/cubing/puzzle-geometry/PuzzleGeometry.ts:607:11)
at getOrbitsDef (/Users/arhan/Desktop/cubing.js/src/cubing/puzzle-geometry/PuzzleGeometry.ts:2666:25)
at writegap (/Users/arhan/Desktop/cubing.js/src/cubing/puzzle-geometry/PuzzleGeometry.ts:2243:21)
at /Users/arhan/Desktop/cubing.js/src/bin/puzzle-geometry-bin.ts:275:20
Bun v1.1.36 (macOS arm64)```
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMOLS5S6V2X6ZEWTDA2QRD2MRTRHAVCNFSM6AAAAABSZ3ZIHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJUGIYTENJQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Ooh, that's much cleaner! Let me make that patch.
…On Sat, Jan 25, 2025 at 8:48 PM Arhan Chaudhary ***@***.***> wrote:
The indentation made the diff look weird, here is the same thing without
the indent. This does exactly what you suggested.
diff --git a/src/cubing/puzzle-geometry/PuzzleGeometry.ts b/src/cubing/puzzle-geometry/PuzzleGeometry.ts
index e581de16..26086132 100644--- a/src/cubing/puzzle-geometry/PuzzleGeometry.ts+++ b/src/cubing/puzzle-geometry/PuzzleGeometry.ts@@ -2129,9 +2129,10 @@ export class PuzzleGeometry {
}
r.push(parsedmove[5]);
}- } else if (this.options.vertexMoves && !this.options.allMoves) {+ } else {
const msg = this.movesetgeos[k];- if (msg[1] !== msg[3]) {+ const isTetrahedron = msg[1] !== msg[3];+ if (this.options.vertexMoves && isTetrahedron && !this.options.allMoves) {
for (let i = 0; i < slices; i++) {
if (msg[1] !== "v") {
if (this.options.outerBlockMoves) {@@ -2149,7 +2150,6 @@ export class PuzzleGeometry {
r.push(1);
}
}- }
} else {
for (let i = 0; i <= slices; i++) {
if (!this.options.allMoves && i + i === slices) {@@ -2167,6 +2167,7 @@ export class PuzzleGeometry {
r.push(1);
}
}+ }
if (this.fixedCubie >= 0) {
const dep = this.keyface3(this.faces[this.cubies[this.fixedCubie][0]])[k];
const newr = [];
I'm not *exactly* sure what const msg = this.movesetgeos[k] represents. I
tested every PGPuzzle and msg[1] !== msg[3] was only true for tetrahedron
puzzles, therefore I use that to test if it is a tetrahedron. The code was
originally written this way so I conservatively used the same expression.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMOLS62B6CZ7L7ST45GU7D2MRSIZAVCNFSM6AAAAABSZ3ZIHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJUGIYTANBXGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Another concern. To me, this is a priority to fix because locations like |
|
The Jing pyraminx/--vertexmoves issue is fixed. Thanks for the report.
WRT piece location, you say that "locations like F U R looks bizarre".
That's not a location
(the # 1 F U R); it's a comment indicating that this piece is affected by
the moves F, U, and R.
It's just a helpful comment to help you identify which piece is which,
mostly for puzzles that
are not cubes.
If there is still confusion, please expand on what you expect to see and
why you expect to see
that.
…-tom
On Sun, Jan 26, 2025 at 4:06 PM Arhan Chaudhary ***@***.***> wrote:
Another concern. describeSet from PermOriSet outputs locations in the
KSolve definition in a different order compared to
https://standards.cubing.net/draft/1/fundamental-cube-terms/#133-piece-location.
For example on the 3x3:
Set CORNERS 8 3
# 1 F U R
# 2 B U R
# 3 B U L
# 4 F U L
...
To me, this is a priority to fix because locations like FUR looks bizarre.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMOLSYEKQPZ2UMRUODMSPL2MV2BLAVCNFSM6AAAAABSZ3ZIHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJUGY2DKMRZGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Thanks, it looks to work now. 🎉 To elaborate, I would like for there to be a "move order priority" when outputting which moves affect each piece, like a Reid order but for any PG puzzle. For my specific use case, I intend to use the intersecting planes to uniquely identify each piece location for the solved-goto instruction in qter. For example I would like to see instead on the 3x3x3: and a similar type of ordering on the 4x4x4 or any PG puzzle. |
|
Where would this move ordering come from? The set of moves might be
arbitrary, and
arbitrarily complex, across many types of puzzles. The comments are only
intended to
be human-consumed; if you want to order them, you are welcome to rewrite
them or to
recalculate them based on the permutation data.
I see no practical way to introduce a general move ordering across all
puzzles, and I'd
strongly like to avoid puzzle-specific stuff (like we have for move naming
already).
But I'm happy to listen to any explicit concrete suggestions you have.
…-tom
On Tue, Jan 28, 2025 at 2:44 PM Arhan Chaudhary ***@***.***> wrote:
Thanks, it looks to work now. 🎉
To elaborate, I would like for there to be a "move order priority" when
outputting which moves affect each piece, like a Reid order but for any PG
puzzle. For my specific use case, I intend to use the intersecting planes
to uniquely identify each piece location for the solved-goto instruction in
qter. For example I would like to see instead on the 3x3x3:
Set CORNERS 8 3
# 1 U F R
# 2 U R B
# 3 U B L
# 4 U L F
# 5 D R F
# 6 D L F
# 7 D L B
# 8 D B R
and a similar type of ordering on the 4x4x4 or any PG puzzle.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMOLS4FNSXMFOYWPCM4CG32NAB4RAVCNFSM6AAAAABSZ3ZIHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRQGIYDMNJUGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
That's understandable. My idea is to add another parameter to I haven't yet looked at how slice moves are ordered so it could be more complicated than just that. But I am confident that I will eventually figure something out. I will keep you updated as I work on the port. |
title
The text was updated successfully, but these errors were encountered: