Forbid undo-of-new-game after midend_set_config.

This is another situation in which the midend's state, at the time
midend_new_game tries to save it, is not such as to generate a viable
serialisation. In this case, it's because after the user enters a game
id into the Game > Specific or Game > Random Seed dialog box,
midend_set_config will have _already_ started overwriting important
fields of the midend such as me->desc and me->seed, before
midend_new_game is even entered.

In fact this caused an assertion failure (thanks to Lennard Sprong for
reporting it), because one of those fields was set to NULL, so the
resulting serialisation buffer was incomplete, leading to a
deserialisation error later. But I was lucky: if I hadn't been, the
serialisation might have been complete, and valid according to the
deserialisation code, but would have contained a mixture of the new
game's description and the old one's move chain, which would surely
have had far stranger results.

For the moment, I'm fixing this by simply not storing a serialisation
in that situation. Perhaps a nicer approach might be to store one
before starting to overwrite midend fields, and then have
midend_new_game swap it in if it's already been generated. That way
you _could_ still undo past an action like this. But preventing the
assertion failure is a good start.
This commit is contained in:
Simon Tatham
2017-12-09 21:28:41 +00:00
parent 4f8a4f7d7f
commit 5247edd16d

View File

@ -69,6 +69,7 @@ struct midend {
struct midend_state_entry *states; struct midend_state_entry *states;
struct midend_serialise_buf newgame_undo, newgame_redo; struct midend_serialise_buf newgame_undo, newgame_redo;
int newgame_can_store_undo;
game_params *params, *curparams; game_params *params, *curparams;
game_drawstate *drawstate; game_drawstate *drawstate;
@ -166,6 +167,7 @@ midend *midend_new(frontend *fe, const game *ourgame,
me->newgame_undo.size = me->newgame_undo.len = 0; me->newgame_undo.size = me->newgame_undo.len = 0;
me->newgame_redo.buf = NULL; me->newgame_redo.buf = NULL;
me->newgame_redo.size = me->newgame_redo.len = 0; me->newgame_redo.size = me->newgame_redo.len = 0;
me->newgame_can_store_undo = FALSE;
me->params = ourgame->default_params(); me->params = ourgame->default_params();
me->game_id_change_notify_function = NULL; me->game_id_change_notify_function = NULL;
me->game_id_change_notify_ctx = NULL; me->game_id_change_notify_ctx = NULL;
@ -410,17 +412,20 @@ static void newgame_serialise_write(void *ctx, const void *buf, int len)
void midend_new_game(midend *me) void midend_new_game(midend *me)
{ {
me->newgame_undo.len = 0; me->newgame_undo.len = 0;
if (me->nstates != 0) { if (me->newgame_can_store_undo) {
/* /*
* Serialise the whole of the game that we're about to * Serialise the whole of the game that we're about to
* supersede, so that the 'New Game' action can be undone * supersede, so that the 'New Game' action can be undone
* later. But if nstates == 0, that means there _isn't_ a * later.
* current game (not even a starting position), because this *
* is the initial call to midend_new_game when the midend is * We omit this in various situations, such as if there
* first set up; in that situation, we want to avoid writing * _isn't_ a current game (not even a starting position)
* out any serialisation, because it would be useless anyway * because this is the initial call to midend_new_game when
* and just confuse us into thinking we had something to undo * the midend is first set up, or if the midend state has
* to. * already begun to be overwritten by midend_set_config. In
* those situations, we want to avoid writing out any
* serialisation, because they will be either invalid, or
* worse, valid but wrong.
*/ */
midend_purge_states(me); midend_purge_states(me);
midend_serialise(me, newgame_serialise_write, &me->newgame_undo); midend_serialise(me, newgame_serialise_write, &me->newgame_undo);
@ -536,6 +541,8 @@ void midend_new_game(midend *me)
if (me->game_id_change_notify_function) if (me->game_id_change_notify_function)
me->game_id_change_notify_function(me->game_id_change_notify_ctx); me->game_id_change_notify_function(me->game_id_change_notify_ctx);
me->newgame_can_store_undo = TRUE;
} }
int midend_can_undo(midend *me) int midend_can_undo(midend *me)
@ -1680,6 +1687,8 @@ static const char *midend_game_id_int(midend *me, const char *id, int defmode)
sfree(par); sfree(par);
me->newgame_can_store_undo = FALSE;
return NULL; return NULL;
} }