shithub: puzzles

Download patch

ref: c0b2f0fc98e87392dcb4dd8faf3076786fc49367
parent: ec84b45ba4a48bd4b50c98fdeffa13f836842be7
author: Ben Harris <bjh21@bjh21.me.uk>
date: Sat Feb 11 16:22:49 EST 2023

Check state is valid at the end of a move in Pearl

A Pearl move string contains a sequence of sub-moves, each of which
can affect the state of the connection between the centre of a square
and one of its edges.  interpret_move() generates these in pairs so
that the two halves of a connection between the centres of adjacent
squares stay in the same state.

If, however, a save file contains mismatched half-moves,
execute_move() should ideally return NULL rather than causing an
assertion failure.  This has to be checked at the end of the whole
move string, so I've arranged for check_completion() to return a
boolean indicating whether the current state (and hence the move
preceding it) is valid.  It now returns 'false' when a connection
stops at a square boundary or when it goes off the board.  These
conditions used to be assertion failures, and now they just cause the
move to be rejected.

This supersedes the check for off-board connections added in 15f4fa8,
since now check_completion() can check for off-board links for the
whole board at once.

This save file trivially demonstrates the problem, causing
"dsf_update_completion: Assertion `state->lines[bc] & F(dir)' failed"
without this fix:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME    :5:Pearl
PARAMS  :5:6x6t0
CPARAMS :5:6x6t0
DESC    :17:BbBfWceBbWaBWWgWB
NSTATES :1:2
STATEPOS:1:2
MOVE    :6:R1,0,0

--- a/pearl.c
+++ b/pearl.c
@@ -1525,25 +1525,30 @@
 
 #define ERROR_CLUE 16
 
-static void dsf_update_completion(game_state *state, int ax, int ay, char dir,
+/* Returns false if the state is invalid. */
+static bool dsf_update_completion(game_state *state, int ax, int ay, char dir,
                                  int *dsf)
 {
     int w = state->shared->w /*, h = state->shared->h */;
     int ac = ay*w+ax, bx, by, bc;
 
-    if (!(state->lines[ac] & dir)) return; /* no link */
+    if (!(state->lines[ac] & dir)) return true; /* no link */
     bx = ax + DX(dir); by = ay + DY(dir);
 
-    assert(INGRID(state, bx, by)); /* should not have a link off grid */
+    if (!INGRID(state, bx, by))
+        return false; /* should not have a link off grid */
 
     bc = by*w+bx;
-    assert(state->lines[bc] & F(dir)); /* should have reciprocal link */
-    if (!(state->lines[bc] & F(dir))) return;
+    if (!(state->lines[bc] & F(dir)))
+        return false; /* should have reciprocal link */
+    if (!(state->lines[bc] & F(dir))) return true;
 
     dsf_merge(dsf, ac, bc);
+    return true;
 }
 
-static void check_completion(game_state *state, bool mark)
+/* Returns false if the state is invalid. */
+static bool check_completion(game_state *state, bool mark)
 {
     int w = state->shared->w, h = state->shared->h, x, y, i, d;
     bool had_error = false;
@@ -1571,8 +1576,11 @@
     /* Build the dsf. */
     for (x = 0; x < w; x++) {
         for (y = 0; y < h; y++) {
-            dsf_update_completion(state, x, y, R, dsf);
-            dsf_update_completion(state, x, y, D, dsf);
+            if (!dsf_update_completion(state, x, y, R, dsf) ||
+                !dsf_update_completion(state, x, y, D, dsf)) {
+                sfree(dsf);
+                return false;
+            }
         }
     }
 
@@ -1727,6 +1735,7 @@
         if (!had_error)
             state->completed = true;
     }
+    return true;
 }
 
 /* completion check:
@@ -2307,16 +2316,6 @@
                 (ret->marks[y*w + x] & (char)l))
                 goto badmove;
 
-            /*
-             * Similarly, if we've ended up with a line or mark going
-             * off the board, that's not acceptable.
-             */
-            for (l = 1; l <= 8; l <<= 1)
-                if (((ret->lines[y*w + x] & (char)l) ||
-                     (ret->marks[y*w + x] & (char)l)) &&
-                    !INGRID(state, x+DX(l), y+DY(l)))
-                    goto badmove;
-
             move += n;
         } else if (strcmp(move, "H") == 0) {
             pearl_solve(ret->shared->w, ret->shared->h,
@@ -2333,7 +2332,7 @@
             goto badmove;
     }
 
-    check_completion(ret, true);
+    if (!check_completion(ret, true)) goto badmove;
 
     return ret;