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;