GNOME Bugzilla – Bug 768345
Generator stuck in a loop
Last modified: 2016-08-13 08:08:25 UTC
I’m getting the same puzzles over and over. I start gnome-sudoku and click “Very Hard”. Expected behavior: I get a new puzzle that I have never seen before. Observed behavior: I get the same puzzle each time I start the program. ..8 ..7 1.. ... 8.. .4. 5.6 1.. 3.. ..3 ..4 ..9 7.. ... ..3 9.. 7.. 4.. ..1 ..2 6.5 .6. ..8 ... ..2 3.. 8.. I click “New Puzzle”, “Very Hard”. Observed behavior: I get a different puzzle, again, it’s always the same. ... .9. ... .1. 8.3 .4. 6.. ... ..2 .9. .5. .3. 5.4 ... 7.9 ..2 .4. 1.. ..9 .8. 4.. .4. ... .6. 3.. 5.4 ..7 I have not tried reproducing the problem on a different user profile. What information do I need to provide so that this could be investigated? Alternatively, how can I get the generator unstuck?
Oh what the hell. I looked at the source for qqwing. It uses the C runtime library pseudo-random number generator, rand(). It expects the client code to seed that. The command-line sudoku generator in main.cpp does that. gnome-sudoku does not. Additionally, gnome-sudoku keeps a directory of all puzzles ever solved and has a function to determine if a particular puzzle is contained in that directory (SudokuBoard.is_finished), but this function is never called since the removal of SudokuStore in 0e364d4 (“Remove SudokuStore and the puzzle data files” by Parin Porecha on 2014-08-06). A simple, albeit not quite correct, workaround would be to seed the generator in lib/qqwing-wrapper.cpp, function qqwing_generate_puzzle. As before, I don’t have a GNOME 3.21 development environment handy, but the patch that will follow fixes the problem for me on 3.18.2 as packaged by Ubuntu.
Created attachment 331005 [details] [review] Seed the random number generator before generating each puzzle Rebased to git://git.gnome.org/gnome-sudoku:master as of 2016-06-26 (commit 283f24c).
Hmm, scratch that. Seeding the generator with current time leads to duplicates if several puzzles are generated in quick succession, which is what happens when printing multiple puzzles.
Created attachment 331009 [details] [review] Seed the random number generator once on start This is a bit more invasive. I seed the random generator in the only C++-based module in this program, expose that in the QQwing wrapper class, and that through the SudokuGenerator class all the way up to Sudoku.main. I tried putting the call to QQwing.seed in the SudokuGenerator’s initializer function, class constructor, or static constructor. It doesn’t help, because that class is never constructed but only used for its static functions. I do not understand vala well enough to make the QQwing namespace available to the main class; nor is it necessarily a good idea.
Wow. How did we not notice this.
Review of attachment 331009 [details] [review]: Thanks for this patch. I'm going to go with a simpler approach that doesn't require punching through so many layers and modifying the vala files, but your help with this was much appreciated.
The following fixes have been pushed: 7d404be qqwing-wrapper: seed RNG 8ba9104 Switch to C++ 11 7b237c6 qqwing-wrapper: Drop using statements
Created attachment 331294 [details] [review] qqwing-wrapper: seed RNG So we don't repeat same puzzles each time Sudoku is started. Credit to Yuri Khan for discovering and debugging this issue (and for submitting a different fix).
Created attachment 331295 [details] [review] Switch to C++ 11 We could go all the way to C++ 14, but we don't need it for anything. I want C++ 11 to use std::call_once.
Created attachment 331296 [details] [review] qqwing-wrapper: Drop using statements These make the code harder to read.
The following fix has been pushed: 30065ef qqwing-wrapper: Seed RNG
Created attachment 331297 [details] [review] qqwing-wrapper: Seed RNG This is ugly. On the master branch, we will use std::once_flag, but for gnome-3-20 let's avoid adding a dependency on C++ 11.
The following fix has been pushed: 672deab Fixup for the previous commit
Created attachment 331298 [details] [review] Fixup for the previous commit once_flag needs to be static or this gets called multiple times....
*** Bug 768751 has been marked as a duplicate of this bug. ***
*** Bug 769758 has been marked as a duplicate of this bug. ***