Ensure 3×3×3 scrambles are fully canonicalized with fulll affixes.

Fixes #95

src/rs/scramble/collapse.rs

@@ -6,6 +6,15 @@ enum CombinedMoves {
     Separate(Move, Move),
 }
 
+fn mod_move(mut r#move: Move, mod_n: i32, mod_offset: i32) -> Option<Move> {
+    let amount = (r#move.amount - mod_offset).rem_euclid(mod_n) + mod_offset;
+    if amount == 0 {
+        return None;
+    }
+    r#move.amount = amount;
+    Some(r#move)
+}
+
 fn combine_adjacent_moves(
     move1: Move,
     mut move2: Move,
@@ -16,12 +25,11 @@ fn combine_adjacent_moves(
         return CombinedMoves::Separate(move1, move2);
     }
 
-    let new_amount = (move2.amount + move1.amount - mod_offset) % mod_n + mod_offset;
-    if new_amount == 0 {
-        return CombinedMoves::Cancelled();
+    move2.amount += move1.amount;
+    match mod_move(move2, mod_n, mod_offset) {
+        Some(move2) => CombinedMoves::Collapsed(move2),
+        None => CombinedMoves::Cancelled(),
     }
-    move2.amount = new_amount;
-    CombinedMoves::Collapsed(move2)
 }
 
 fn pop_final_move(nodes: &mut Vec<AlgNode>) -> Option<Move> {
@@ -40,7 +48,8 @@ fn pop_final_move(nodes: &mut Vec<AlgNode>) -> Option<Move> {
 /// This is a minimal implementation of https://js.cubing.net/cubing/api/classes/alg.Alg.html#experimentalSimplify for collapsing moves between phases.
 /// For face turns and face rotations of a cube, pass:
 /// - `mod_n`: 4
-/// - `mod_offset`: 1
+/// - `mod_offset`: -1
+// TODO: `R4` is not collapsed into an empty alg.
 pub fn collapse_adjacent_moves(alg: Alg, mod_n: i32, mod_offset: i32) -> Alg {
     let mut nodes = Vec::<AlgNode>::new();
 
@@ -52,12 +61,17 @@ pub fn collapse_adjacent_moves(alg: Alg, mod_n: i32, mod_offset: i32) -> Alg {
                     CombinedMoves::Cancelled() => pop_final_move(&mut nodes),
                     CombinedMoves::Collapsed(r#move) => Some(r#move),
                     CombinedMoves::Separate(move1, move2) => {
-                        nodes.push(move1.into());
-                        Some(move2)
+                        match mod_move(move2, mod_n, mod_offset) {
+                            Some(move2) => {
+                                nodes.push(move1.into());
+                                Some(move2)
+                            }
+                            None => Some(move1),
+                        }
                     }
                 }
             } else {
-                Some(new_move)
+                mod_move(new_move, mod_n, mod_offset)
             }
         } else {
             if let Some(pending_move) = maybe_pending_move {
@@ -98,6 +112,11 @@ fn collapse_test() {
         parse_alg!("R2")
     );
 
+    assert_eq!(
+        collapse_adjacent_moves(parse_alg!("U D2 R4 D6' U"), 4, -1),
+        parse_alg!("U2")
+    );
+
     assert_eq!(
         collapse_adjacent_moves(parse_alg!("R F F2 . F R"), 4, -1),
         parse_alg!("R F' . F R")

src/rs/scramble/puzzles/cube3x3x3.rs

@@ -133,12 +133,26 @@ impl Scramble3x3x3TwoPhase {
         pattern: &KPattern,
         constraints: PrefixOrSuffixConstraints,
     ) -> Alg {
-        let (canonical_fsm_pre_moves, canonical_fsm_post_moves) = match constraints {
-            PrefixOrSuffixConstraints::None => (None, None),
+        // TODO: we pass premoves and postmoves to both phases in case the other
+        // turns out to have an empty alg solution. We can handle this better by making
+        // a way to bridge the FSM between phases.
+        let (
+            canonical_fsm_pre_moves_phase1,
+            canonical_fsm_post_moves_phase1,
+            canonical_fsm_pre_moves_phase2,
+            canonical_fsm_post_moves_phase2,
+        ) = match constraints {
+            PrefixOrSuffixConstraints::None => (None, None, None, None),
             PrefixOrSuffixConstraints::ForFMC => {
                 // For the pre-moves, we don't have to specify R' and U' because we know the FSM only depends on the final `F` move.
                 // For similar reasons, we only have to specify R' for the post-moves.
-                (Some(vec![parse_move!("F")]), Some(vec![parse_move!("R'")]))
+                (
+                    Some(vec![parse_move!("F")]),
+                    Some(vec![parse_move!("R'")]),
+                    // TODO: support a way to specify a quantum factor
+                    Some(vec![parse_move!("F2'")]),
+                    Some(vec![parse_move!("R2'")]),
+                )
             }
         };
 
@@ -149,8 +163,8 @@ impl Scramble3x3x3TwoPhase {
                     &phase1_search_pattern,
                     IndividualSearchOptions {
                         min_num_solutions: Some(1),
-                        canonical_fsm_pre_moves,
-                        canonical_fsm_post_moves, // TODO: We currently need to pass this in case phase 2 return the empty alg. Can we handle this in another way?
+                        canonical_fsm_pre_moves: canonical_fsm_pre_moves_phase1,
+                        canonical_fsm_post_moves: canonical_fsm_post_moves_phase1,
                         ..Default::default()
                     },
                 )
@@ -166,6 +180,8 @@ impl Scramble3x3x3TwoPhase {
                     &phase2_search_pattern,
                     IndividualSearchOptions {
                         min_num_solutions: Some(1),
+                        canonical_fsm_pre_moves: canonical_fsm_pre_moves_phase2,
+                        canonical_fsm_post_moves: canonical_fsm_post_moves_phase2,
                         ..Default::default()
                     },
                 )