If the game is empty it should not be saved; you should go straight to the New Game screen.

Created attachment 285034 [details] [review]
The saved game is deleted once it is loaded in a new game.

Review of attachment 285034 [details] [review]:

I'm afraid this is not the right approach.
Everytime Sudoku starts, it checks for a savefile to load the game from, and everytime Sudoku is closed, it overwrites the savegame with the progress made.

If you delete the savefile after reading it, it causes this problem -
If Sudoku crashes (or is sent a kill signal), we wouldn't have saved the progress. The next time Sudoku opens, we won't have anything to go back to !
The savegame is no longer there, so we'll no way but to start a new game. User will have lost ALL of his progress :-(

You might want to discuss your approach first :-)

Yes i understand that and agree with you. Actually when the game is closed with an empty board, it doesn't save the game.In the next run it tries to load the saved game from the stack.but somewhere between we need to indicate that the last game was closed with an empty board.Therefore it should load a new game instead.Do i sound correct?

(In reply to comment #4)
> Do i sound correct?

Yes, this is the bug exactly.
What would be your approach ?

Actually i am not getting how would i give that indication when the game is closed with empty board.

(In reply to comment #6)
> Actually i am not getting how would i give that indication when the game is
> closed with empty board.

- Sudoku takes the user to the new game screen when a savefile isn't present.

Try to think how you would use this to solve the bug.

See the savefile will have some or the other file (i mean there will be a stack of saved files) , no matter the last game was closed with an empty board or not.So it might not be appropriate to make decision from savefile existence for this bug.

(In reply to comment #8)
> See the savefile will have some or the other file (i mean there will be a stack
> of saved files)

On what basis did you assume this ?
Have a look at how SudokuSaver class works. It'll clear many things up.

Yes i went through that.Now i am trying to use something like if ( game.board.is_empty () ) show_new_game_screen ();
But again if i use it while loading any game, the empty game needs to be saved in that case.
And if i do not want to save empty game, i should use the above in  window.delete_event.connect.

Am i going a bit correct way?

I think I would just delete the save file when the user clicks Clear Board; wouldn't that work?

(In reply to comment #11)
> I think I would just delete the save file when the user clicks Clear Board;
> wouldn't that work?

Yes Michael that would definitely work.But using that we may left out the case when new puzzle button is pressed and that brings us a new puzzle to solve.Again if we left it with empty board then in the next run it loads the earlier state. We should do one more thing that delete the savefile after some difficulty level has been selected from new game screen. Will that be appropriate method?

Good point, you're right.  So: delete the save file after the board is cleared, and delete the save file after a new game is started.

(In reply to comment #13)
> and delete the save file after a new game is started.

We start a new game everytime Sudoku starts. Doing this will cause the problem mentioned in comment 3.

Delete the savefile when a "fresh" game is started. (i.e. - via new game screen or puzzle complete dialog)

Hm, this is a bit tricky.
We don't have any way to know if the board is a fresh one or the one after being cleared.

Amisha, you might want to add a flag which is set when a fresh game is started, and is reset when progress is made. Save an empty board only if the flag is set.

Parin i agree with you for using some flag.But i doubt that the requirement of bug fix is something else.It needs that game should always start with a new puzzle whenever an empty board is closed(whether from clear board or from new game button).
But according to you ,there will appear the same game when empty board is closed after pressing new game button.

Or,you mean something else?

No, that's exactly what I meant.

If we don't save the empty board of a fresh game, everytime Sudoku is closed, we'll be redirected to the New game screen. This will stop only when you add a value.
If Michael thinks this is fine, I'm okay with this.

Okay Parin ,after the acceptance of both of you i will move ahead for the patch work. :)

(In reply to comment #18) 
> If we don't save the empty board of a fresh game, everytime Sudoku is closed,
> we'll be redirected to the New game screen. This will stop only when you add a
> value.
> If Michael thinks this is fine, I'm okay with this.

Yes, this is what I would expect. Sudoku should never open up to an empty board, since that's equivalent to starting a new game without letting the user choose his difficulty level. It should always either load a game that was already in progress and in which the user has set at least one square or earmark, or else it should go straight to the new game screen.

Well okay.Should i do one thing that, in window.delete_event.connect i make some condition like if gameboard is empty and savefile exist then delete the savefile.

Will that be fair enough?

Created attachment 285162 [details] [review]
Deleting the savefile when an empty board game is closed.

Review of attachment 285162 [details] [review]:

This looks like a good approach; just a couple small things to fix:

::: src/gnome-sudoku.vala
@@ +176,3 @@
                 saver.save_game (game);
+            if (game.board.is_empty () && savegame != null)
+                {

This block is indented wrongly; it should be four spaces.

@@ +177,3 @@
+            if (game.board.is_empty () && savegame != null)
+                {
+                    var file = File.new_for_path (saver.savegame_file);

I'm surprised this works, since savegame_file is a static member of SudokuSaver.  It's not associated with any object so you should write e.g. SudokuSaver.savegame_file and notn saver.savegame_file

I only noticed this because it caused a compiler warning. It's good to scroll up and check these before submitting a new patch:

gnome-sudoku.vala:179.51-179.69: warning: Access to static member `SudokuSaver.savegame_file' with an instance reference
                    var file = File.new_for_path (saver.savegame_file);
                                                  ^^^^^^^^^^^^^^^^^^^

@@ +178,3 @@
+                {
+                    var file = File.new_for_path (saver.savegame_file);
+                    file.delete ();

gnome-sudoku.vala:180.21-180.34: warning: unhandled error `GLib.Error'
                    file.delete ();
                    ^^^^^^^^^^^^^^

So this should go in a try block.  In the catch block, just print a warning if the deletion fails.

try
{
    file.delete ();
}
catch (Error e)
{
    warning ("Failed to delete saved game: %s", e.message);
}

Review of attachment 285162 [details] [review]:

::: src/gnome-sudoku.vala
@@ +175,3 @@
             if (game != null && !game.board.is_empty () && !game.board.complete)
                 saver.save_game (game);
+            if (game.board.is_empty () && savegame != null)

Also, this is a crash if game is null (which it will be if the game opens to the new game screen and then you exit without starting a game). Easiest thing to do would be to return early if the game is null, so that you don't have to check that in both conditions:

        window.delete_event.connect ((event) => {
            if (game == null)
                return false;

            if (!game.board.is_empty () && !game.board.complete)
                saver.save_game (game);

            if (game.board.is_empty () && savegame != null)
            {
                    var file = File.new_for_path (saver.savegame_file);
                    file.delete ();
            }

            return false;
        });

Created attachment 285484 [details] [review]
Deleting the savefile when an empty board game is closed.

Created attachment 285499 [details] [review]
Games when closed with empty board redirects to new game screen on the next run.

Rebased since I just pushed a patch that conflicts with this one

Fixed, thanks!

Attachment 285499 [details] pushed as 14f2e6c - Games when closed with empty board redirects to new game screen on the next run.

Welcome Michael :)