feat: add undo-stack on a per map basis

[tjamesmac]

Description

Hoping to resolve #3 Adds the ability to:

• save the undo stack on reload for unsaved maps
• store undo stacks for saved maps

Todo

• [x] undo stack on reload one off (need to push)
• [ ] move effects into undostack

Would appreciate any feedback and whether this has hit the mark! :)

[cpojer]

Here are the replies to your questions from the issue:

The MapObject.id is returning undefined and never returns an actual id. Is this expected? If it is, do you have any suggestions for generating a decent fallback?

If you rebase, now it will be an empty string when you "save" it.

Instead of restoring the previous map with the Restore button, i'm restoring if a previousState is present. When clicking New map I then delete the related localstorage items so a refresh doesn't continually bring back the last map. Does this sound like the right path for the U? Happy to change if I misinterpreted the requirement.

I think that makes the most sense, yes. If you don't hit restore before clicking "new map", we should wipe the undo state.

Also not sure if i'm missing something obvious but the confirmation dialogs for the new/reset map don't seem to work. I've just started calling new map directly

Yes that's fine. The docs page doesn't have those confirmation dialogs but they are in the game.

Lastly, are there tests for the map editor?

There are no tests 🪦 If you write a function (not React) that you want to test, feel free to add a test. Otherwise we are living in a brave world here.

[cpojer]

Hey @tjamesmac, thank you so much for your contributions! I left a few comments, would you mind taking a look?

if the map isn't saved, the undo stack is extended across reloads. Meaning, you can go back to the last refreshed map with ctrl-z.

I think this is ok actually, that's the whole point of replacing the "restore" feature with this one, as long as the id matches.

[tjamesmac]

Hey @cpojer, thanks for the quick review and sorry for the slowish response. Busy weekend!

I've made your suggestions and added a few more fixes for some edge cases. Would appreciate another review!

Also thanks for open sourcing Athena crisis :)

[cpojer]

This is looking really solid! I think there are only two more considerations before it's ready:

• When I edit a map, then reload and hit cmd+z, it restores the map with one state less than expected. Ideally it restores it at the same state as before.
• Now that we are restoring maps with this approach, I think we need to also move effects into the undo stack. Effects are character messages etc. that you can define in the effects panel. Ideally they get versioned with the current state of the map as well. This should allow cleaning up a bit of the code that handles storage as we won't have to make use of a separate storage item for effects.

Just note, we should probably not immediately update undo every time an effect changes since a user can type into the textarea to update the text a character says and it would probably be slow and not fun to add each character to the undo stack.

[tjamesmac]

Hey thanks, got a bit busier than I expected. Will look at shifting the effects into the stack when I get chance

[tjamesmac]

Hey @cpojer , I have a couple of questions!

When I edit a map, then reload and hit cmd+z, it restores the map with one state less than expected. Ideally it restores it at the same state as before.

How do you feel about the previous state (if present) just being restored straight off a reload instead of hitting restore to get back to the previous state? If that's okay I think I can check when getting the initial map state whether I should restore from:

• undostack
• mapobject
• generate a new map

Ideally they get versioned with the current state of the map as well.

Does this mean move the effects into MapData and store that as an entry in the undostack? Instead of adding an Effect as a separate value in a undoEntry?

[cpojer]

How do you feel about the previous state (if present) just being restored straight off a reload instead of hitting restore to get back to the previous state?

I prefer not to do this. In the game, opening the map editor should start you off with a fresh map. I mostly just want undo to work properly in that empty state so that we can make the "Restore Map" button more prominent in the editor in a follow-up. That way we give control to the user.

Does this mean move the effects into MapData and store that as an entry in the undostack? Instead of adding an Effect as a separate value in a undoEntry?

I think we have to change undoEntry to take [MapData, Effects] instead of just MapData. Effects cannot live on MapData directly.