Page MenuHomePhabricator

Undo/Redo is not working
Closed, ResolvedPublic

Description

Currently, clicking on Undo reverses the whole undo stack as all undo stack items have id 0 and the loop iterates until the id is >= 0. A few plugins already use a nasty workaround and manually increase the id, which is supposed to happen automatically. According to some code comments, the EventMapper did this, which is deprecated meanwhile. We need to fix this and remove all the workarounds.

Event Timeline

Okay, it seems like the solution is a compromise. Only the author of some code can actually know, which operation should be undoable in their context. So they should be in charge for increasing the group/object IDs. That's what's happening currently at a few places in our code.

However, the EventMapper also did this for some event types like key presses. As it didn't know nothing about the context, it couldn't know how often the counters should be increased in a meaningful way. I guess that's the reason why there is an extra ExecuteIncrement() method in OperationEvent which actually does an increment, whereas the methods IncCurr[Object/Event]ID() only set a flag that on the next call to ExecuteIncrement() the IDs should be incremented or not. This way it was somehow guaranteed that the mess of the EventMapper was reduced to single increases.

As the EventMapper was removed, nobody can anymore hope that the IDs are somehow magically increased. It seems like the interaction with point sets was/is relying on this.

I try to fix it the following way: Remove OperationEvent::ExecuteIncrement() and the flag stuff as this seems to was only necessary for the EventMapper mess. Remove all calls to ExecuteIncrement(). They were paired with calls to IncCurr[Object/Group]ID() anyway. Instead, increment the IDs already with the IncCurr[Object/Group]ID() methods.

That should fix most of the undo problems. We might have to adapt the point set interaction.

User kislinsk has pushed new remote branch:

bug-19576-FixUndoRedo

[680cda]: Merge branch 'bug-19576-FixUndoRedo'

Merged commits:

2016-03-02 17:43:43 Stefan Kislinskiy [206667]
Fix additions of undo stack items


2016-03-02 16:07:31 Stefan Kislinskiy [b3077c]
Adapt OperationEvent to EventMapper removal.

User kislinsk has pushed new remote branch:

bug-19576-RemovePointSelectionsFromUndoStack

[e8e167]: Merge branch 'bug-19576-RemovePointSelectionsFromUndoStack'

Merged commits:

2016-03-03 11:37:33 Stefan Kislinskiy [542fda]
Remove point selection from undo stack