Feature Request: Feature toggle for Undo/Redo

[shashikanthgadgay]

Hi,

Currently, the game state stores all the past actions as undo, which bloats up the size of the file incase of large game state.

We would like to have a feature toggle to enable/disable the undo/redo feature, which would help in optimising the game store read and write.

[shashikanthgadgay]

We would like to give it a try by implementing this feature. Our thoughts are

1) By default, Undo/Redo feature would be enabled. 2) Disabling this feature can be in the corresponding game config while initialising server

What do you think about it? Do you have any other suggestions

[delucis]

This is certainly an optimisation we can consider adding. It’s worth noting that undo state is wiped every turn, so it shouldn’t accumulate too much except for long-running turns.

We do already support disabling undo for a move via the undoable field in the long-form move syntax (see Game definition docs), but I don’t think that prevents undo state being saved. Perhaps a good solution here would be not to push G and ctx to the undo stack if the move’s undoable says it can’t be undone (we’d need to add a flag to the undo state like undoable: false in this case).

[nicolodavis]

Another possibility is to implement undo / redo as a plugin.

Something similar to: https://github.com/nicolodavis/boardgame.io/blob/master/src/plugins/plugin-events.ts Plugin docs: https://boardgame.io/documentation/#/plugins

That way we can have undo / redo as opt-in features that you don't pay for in bundle size either if you don't use them.

[shashikanthgadgay]

@delucis

We tried implementing your suggestion. By not pushing the G & ctx of {undoable: false} move into undo array, we are losing a restorable state for the next undoable move.

This approach won't work for us as it is forcing to keep the older states in the undo array.

Is it ok we implement the way we suggested above ?

[delucis]

@shashikanthgadgay Oh, right — of course. So actually, a move that cannot be undone should push its own state, but it should discard any previous state from the stack.

So you might have a state like:

{
  _undo: [state1, state2, state3],
}

Then, after a move that can’t be undone is made, _undo would be overwritten:

{
  _undo: [state4],
}

If it is possible to do this way, it would be great because it’s an optimisation to anyone using moves that can’t be undone instead of relying on the global option. What do you think?

Ultimately @nicolodavis’s suggestion of moving undo/redo entirely to an optional plugin would be the ideal way to handle a global on/off switch. However, there’s some pretty tight coupling currently between the undo state and the flow reducer, so it may not be possible yet to move all that logic to a plugin. If you want to add a global toggle to disable undo/redo entirely, maybe we can add a disableUndo flag to the game config for now?

[shashikanthgadgay]

@delucis We also like the idea of moving undo/redo to an optional plugin, but after going through the code, even we feel there is tight coupling of undo/redo in the reducer.

Meanwhile, we have started working on disableUndo flag in the game config. We will raise a PR soon.

We also like the suggestion you made regarding discarding previous state from stack. We will raise another PR for that.

[shashikanthgadgay]

@delucis Thanks for merging the PR.

@nicolodavis Could you please do a minor release with the changes from the PR. Unfortunately, npm install via Github isn't working.

[delucis]

@shashikanthgadgay #745 will fix installs from GitHub. Until that’s merged you can do:

npm i nicolodavis/boardgame.io#delucis/fix/git-installs