GNOME Bugzilla – Bug 735495
[PATCH] Permit to clear board when there's only earmarks.
Last modified: 2014-09-06 15:59:45 UTC
Created attachment 284575 [details] [review] Permit to clear board when there's only earmarks. Sometimes, you find a sudoku grid where you can’t place any number directly. In this case, you use earmarks. In this cases, you should be able to clear the board, and to save the game state… Here is a patch that permits it.
Review of attachment 284575 [details] [review]: Looks good. Since it depends on 'Clear earmarks on reset' patch, I'll test this after it gets fixed.
Review of attachment 284575 [details] [review]: Looks correct. I'm not enthusiastic about the triple for look in is_empty. There's already a counter of how many cells are filled on the board; I think it'd be best to add another counter for earmarks, so that this becomes a simple comparison.
Adding a (public??) counter for a thing used (locally) at only one place looks like a bad idea for my little mind…
I think it can be private.
Well, I don’t know what you have in mind, or it’s so hacky that I cannot sign it. Don’t hesitate.
Created attachment 285525 [details] [review] Allow clearing the board when only earmarks are placed How about this approach?
> How about this approach? Yeah, why not. > @@ -191,7 +191,7 @@ public class SudokuSaver : Object > { > reader.read_element (k); > return_val_if_fail (reader.is_value (), null); > - board.earmarks[row, col, (int) reader.get_int_value ()] = true; > + board.enable_earmark (row, col, (int) reader.get_int_value ()); You’ll need a `reader.get_int_value () + 1` here, or saving bugs.
Ooops, it’s not there. > @@ -96,7 +96,7 @@ private class NumberPicker : Gtk.Grid > public void set_earmarks (int row, int col) > { > for (var i = 0; i < board.max_val; i++) > - set_earmark (row, col, i, board.earmarks[row, col, i]); > + set_earmark (row, col, i, board.is_earmark_enabled(row, col, i)); > } Here I think, a `i + 1` (or a `var i = 1; i <= board.max_val`).
Created attachment 285578 [details] [review] Allow clearing the board when only earmarks are placed Good catch, thanks! I think I've fixed that now.
Created attachment 285579 [details] [review] Allow clearing the board when only earmarks are placed Er, I think I posted the old patch again :)
Attachment 285579 [details] pushed as 560849f - Allow clearing the board when only earmarks are placed