Immutable XwordState

Status: implemented

XwordState is mutable, and we carefully maintain it for the lifetime of a puzzle. This has a few shortcomings:

  • Implementing Undo/Redo is hard. We’d like to keep a history of states.

  • The tests have to implement the “compare old and new states” logic themselves, instead of just using a hypothetical play_state_equal(a, b).

Turning XwordStateFocus into XwordStateAction

Issue #11 is about renaming the XwordStateFocus enum to XwordStateAction, since now it contains PLAY_STATE_DELETE. I propose that we bite the bullet and make XwordStateAction a complete description of the actions that can be taken on a XwordState.

  typedef enum {
    PLAY_STATE_ACTION_NONE,
    PLAY_STATE_ACTION_UP,
    PLAY_STATE_ACTION_DOWN,
    PLAY_STATE_ACTION_LEFT,
    PLAY_STATE_ACTION_RIGHT,
    PLAY_STATE_ACTION_FORWARD,
    PLAY_STATE_ACTION_BACK,
    PLAY_STATE_ACTION_FORWARD_CLUE,
    PLAY_STATE_ACTION_BACK_CLUE,
    PLAY_STATE_ACTION_BACKSPACE,
    PLAY_STATE_ACTION_DELETE,
    PLAY_STATE_ACTION_SWITCH,
    PLAY_STATE_ACTION_GUESS,       /* has data, see below */
    PLAY_STATE_ACTION_SELECT_CLUE, /* has data, see below */
  } XwordStateActionKind;

  typedef struct {
    XwordStateActionKind kind;

    union {
      char *guess;
      ClueId select_clue;
      /* etc. for other actions that take arguments */
    } u;
  } XwordStateAction;

Then we can have a single

XwordState play_state_dispatch(const XwordState *state, XwordStateAction action);

which returns a new state, not the old one modified. It takes the whole action description and acts upon it; we can then remove play_state_clue_selected() et al piecemeal.

Along with “immutable puzzles” below, this would make testing easier, and would open the door for implementing Undo/Redo.

Immutable puzzles

Right now IPuzCrossword owns its IPuzGuesses, but other than setters and getters, the only place where it is used is in the internal ipuz_crossword_game_won_solution. I think we should remove the guesses from the crossword, and instead have

gboolean ipuz_crossword_game_won (IPuzCrossword *xword, IPuzGuesses *guesses);

(Or call it is_solution or whatever.)

Undo/Redo

NOTE: This section is obsolete. We ended up not undoing the actions, but using a PuzzleStack instead.

With the above, we can make XwordState own the IPuzGuesses. Then we can have this undo stack easily:

state 0 -> action -> state 1 -> action -> state 2 -> action -> state 3
 with                 with                 with                 with
guesses              guesses              guesses              guesses

Whether we keep all the states, or just the ones that change the guesses, is up to you. Emacs does the same “collapse undo steps while typing” thing.

Editability and libipuz paraphernalia

I don’t know if you’ll later want to fuse guesses into a crossword to turn them into the crossword’s built-in solution.

Maybe we need an EditState different from XwordState? It seems that the behavior should be different, since the editor will have actions like EDIT_ACTION_TURN_CELL_INTO_BLOCK or EDIT_ACTION_TURN_CELL_INTO_NULL, or whatever actions we have later to add fancy markers to individual grid lines.

(I know, I know, I should get going on the new layout code…)

I think it’s fine to have IPuzCrossword have a mutability API, but it would be easier if the crossword editor app kept it immutable by copying the old xword to a new one and then modifying cells in the new one. This way we can also have an undo stack for the editor.