From a777b6639b8f4a204d2a71147682e97f3c1a2c99 Mon Sep 17 00:00:00 2001
From: Joel Therrien <joel@joeltherrien.ca>
Date: Tue, 22 Sep 2020 21:20:32 -0700
Subject: [PATCH] Fix bug where some puzzles did not have unique solutions.

Incidentally, this increased the difficulty.
---
 README.md        |  2 +-
 src/generator.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/README.md b/README.md
index ed8cc53..e93c2ef 100644
--- a/README.md
+++ b/README.md
@@ -9,7 +9,7 @@ Two binaries, `solver` and `generator` will be generated in `target/release/`.
 
 Try running both of them, first with the `-h` flag to see what other arguments they take. 
 * `solver` reads a CSV file for a puzzle, prints it, solves it, and then prints the solved version. Some example CSV files are in the `puzzle` folder.
-* `generator` tries to generate a new puzzle from scratch. You can set a maximum number of hints that it will allow and it will try to generate a puzzle that meets that requirement. You can also optionally write it to a CSV file. Be warned, however, that the puzzles that it generates aren't particularly difficult; most puzzles have at least 25 hints, usually much more. If I work on this in the future I'll want to find ways to improve the difficulty.
+* `generator` tries to generate a new puzzle from scratch. You can set a maximum number of hints that it will allow and it will try to generate a puzzle that meets that requirement. You can also optionally write it to a CSV file. 
 
 Regarding code quality, I could probably have commented more and I certainly should have written more unit tests. 
 I also wish that I didn't rely so heavily on `Rc` & `RefCell`, which provide ways to get around (sometimes necessarily) the compiler's strict rules on references and ownership. 
diff --git a/src/generator.rs b/src/generator.rs
index e35d0d5..3967936 100644
--- a/src/generator.rs
+++ b/src/generator.rs
@@ -7,6 +7,7 @@ use rand_chacha::ChaCha8Rng;
 pub static mut DEBUG : bool = false;
 
 // Extension of SolveStatus
+#[derive(Debug, Eq, PartialEq)]
 pub enum GenerateStatus {
     UniqueSolution,
     Unfinished,
@@ -179,7 +180,7 @@ pub fn generate_grid(rng: &mut ChaCha8Rng) -> (Grid, i32) {
                 return (grid, num_hints);
             }
             GenerateStatus::Unfinished => {panic!("solve_grid should never return UNFINISHED")}
-            GenerateStatus::NoSolution => {continue;}
+            GenerateStatus::NoSolution => {continue;} // unlucky; try again
             GenerateStatus::NotUniqueSolution => {break grid;}
         };
     };
@@ -246,13 +247,13 @@ pub fn generate_grid(rng: &mut ChaCha8Rng) -> (Grid, i32) {
     non_empty_cells.shuffle(rng);
 
     for (_index, cell) in non_empty_cells.iter().enumerate() {
-        let grid_clone = grid.clone();
+        let mut grid_clone = grid.clone();
         let cell_clone = grid_clone.get(cell.x, cell.y).unwrap();
         let cell_clone = &*cell_clone;
 
         cell_clone.delete_value();
 
-        let status = solve_grid(&mut grid);
+        let status = solve_grid(&mut grid_clone);
         match status {
             GenerateStatus::UniqueSolution => { // great; that cell value was not needed
                 num_hints = num_hints - 1;
@@ -261,7 +262,7 @@ pub fn generate_grid(rng: &mut ChaCha8Rng) -> (Grid, i32) {
             }
             GenerateStatus::Unfinished => {panic!("solve_grid should never return UNFINISHED")}
             GenerateStatus::NoSolution => {panic!("Removing constraints should not have set the # of solutions to zero")}
-            GenerateStatus::NotUniqueSolution => {continue;}
+            GenerateStatus::NotUniqueSolution => {continue;} // We can't remove this cell; continue onto the next one (note that grid hasn't been modified)
         };
     }
 
@@ -325,4 +326,55 @@ fn solve_grid_guess(grid: &Grid) -> GenerateStatus{
         GenerateStatus::NoSolution => {panic!("current_status should not be NO_SOLUTION at this point")}
     }
 
+}
+
+
+#[cfg(test)]
+mod tests {
+    use crate::grid::*;
+    use crate::generator::{solve_grid, GenerateStatus};
+
+    #[test]
+    fn test_unique_detection() {
+        // A puzzle was generated that didn't actually have a unique solution; this is to make sure that the
+        // modified solving code can actually detect this case
+        let grid = Grid::new();
+
+        grid.get(0, 0).unwrap().set(9);
+        grid.get(0, 7).unwrap().set(4);
+
+        grid.get(1, 3).unwrap().set(5);
+        grid.get(1, 6).unwrap().set(8);
+
+        grid.get(2, 0).unwrap().set(2);
+        grid.get(2, 1).unwrap().set(4);
+        grid.get(2, 4).unwrap().set(7);
+
+        grid.get(3, 2).unwrap().set(8);
+        grid.get(3, 4).unwrap().set(2);
+        grid.get(3, 6).unwrap().set(9);
+
+        grid.get(4, 3).unwrap().set(6);
+        grid.get(4, 7).unwrap().set(7);
+
+        grid.get(5, 5).unwrap().set(5);
+        grid.get(5, 8).unwrap().set(1);
+
+        grid.get(6, 0).unwrap().set(3);
+        grid.get(6, 4).unwrap().set(8);
+        grid.get(6, 6).unwrap().set(4);
+        grid.get(6, 8).unwrap().set(7);
+
+        grid.get(7, 0).unwrap().set(7);
+        grid.get(7, 4).unwrap().set(1);
+        grid.get(7, 5).unwrap().set(9);
+        grid.get(7, 6).unwrap().set(2);
+
+        grid.get(8, 2).unwrap().set(6);
+
+        let status = solve_grid(&grid);
+
+        assert_eq!(status, GenerateStatus::NotUniqueSolution);
+
+    }
 }
\ No newline at end of file