C# undo/redo code not doing anything

Question

for my tile editor I have 2 stacks of TileMaps, undo and redo. Every time the user makes a change the state of the map is added to the stack, than the change is made. Here is my undo code:

private void undoToolStripMenuItem_Click(object sender, EventArgs e)
    {
        if (undo.Count != 0)
        {
            redo.Push(tileMap);
            tileMap = undo.Peek();
            undo.Pop();
        }
    }

The map however does not change. Why?

tileMap is simply an object of type TileMap that contains all the layers being drawn. I expect it to undo the last change made. — Raj, Jun 12 '11 at 21:02

score 1 · Answer 1 · answered Jun 12 '11 at 20:36

1

If undo and redo are just Stack objects, then nothing should happen. Instead, you need to add code to actually update the UI when an undo takes place. In this case, I imagine you would redraw the map using the current tileMap as popped off the stack.

answered Jun 12 '11 at 20:36

siride

200,666
4
41
62

I probably should of specified this, but I've set my UI to update whenever it idles. So that's not the problem. – Raj Jun 12 '11 at 21:00
You sure the UI is actually doing its job? Check to make sure it is refreshing from the proper tileMap. Also check to make sure that your undo code is getting called properly. Verify these things using print statements or the debugger, then come back. – siride Jun 12 '11 at 21:13
So if they, then you don't actually have a problem. If the undo code is called and tileMap is indeed updated, and the UI update code eventually gets called and it is using the correct tileMap, then the only thing left is that your UI update code doesn't work. Perhaps there are still references lying around pointing to the tileMap that was replaced when a new one was popped off the undo stack? – siride Jun 12 '11 at 21:22

score 1 · Answer 2 · answered Jun 28 '21 at 12:39

I know this is an old thread, but I have just encountered this very same issue. Turned out the item I was passing to CurrentItem was the same item I was editing in memory, so it was just passing the same object backwards and forwards.

I got round this by creating a Clone function on the object to return a new version of itself, and I did this every time the undo, redo, or additem functions were performed, in both directions. That way, it was always removing the dependency on the object being edited. It's now all working :)

score 0 · Accepted Answer · answered Jun 12 '11 at 21:33

If it's not a UI update issue...

Are your redo and undo references to different stack objects? If they both reference the same stack object, then your code would just push-and-pop the current state.

Or similarly, are you pushing another reference to the same tileMap object, or a copy of it? (i.e. You probably want to Push(tileMap.DeepCopy()))

Also, why do you Peek and then Pop? You can just Pop directly into tileMap.

C# undo/redo code not doing anything

3 Answers3