Off-by-one error in UndoManager

#1

I’m pulling my hair out over this issue, can someone point out what I’m doing wrong?

I have a few objects that I’d like to have undo/redo behavior. I’m using a ValueTree instance to keep track of their data with an UndoManager wrapped together in their own class. My issue is that UndoManager::beginNewTransaction doesn’t necessarily start a new transaction, and it will lump together undoActions that I don’t want grouped together. The following code illustrates the issue:

class ProgramState
{
public:
    ProgramState ()
        : valueTree ("ProgramState")
    {

    }
    ~ProgramState () {};
    UndoManager* getUndoManager () { return &undoManager; }
    ValueTree& getValueTree () { return valueTree; }

private:
    UndoManager undoManager;
    ValueTree valueTree;
};

class CustomObject
{
public:
    CustomObject (ProgramState& state, String name)
        : programState (state), id (name), value (0.0)
    {
        ValueTree& objTree = programState.getValueTree ().getOrCreateChildWithName ("id", nullptr);
        objTree.setProperty ("_val", value, nullptr);
    }

    void changeBegin ()
    {
        String nextName (id + " Change");
        programState.getUndoManager ()->beginNewTransaction (nextName);
    }

    void setValue (float val)
    {
        programState.getValueTree ().setProperty ("_val", value, programState.getUndoManager ());
        value = val;
    }

    float getValue () const
    {
        return value;
    }

private:
    ProgramState& programState;
    String id;
    float value;
};

int main ()
{
    ProgramState programState;

    CustomObject object0 (programState, "object_0");
    CustomObject object1 (programState, "object_1");
    CustomObject object2 (programState, "object_2");

    object0.changeBegin ();
    object0.setValue (0.1);

    DBG ("Current Transaction:");
    DBG (programState.getUndoManager ()->getCurrentTransactionName () + "\n");

    object1.changeBegin ();
    object1.setValue (0.2);

    DBG ("Current Transaction:");
    DBG (programState.getUndoManager ()->getCurrentTransactionName () + "\n");

    programState.getUndoManager ()->undo ();

    object2.changeBegin ();
    object2.setValue (0.3);

    DBG ("Current Transaction:");
    DBG (programState.getUndoManager ()->getCurrentTransactionName () + "\n");

    DBG ("Expected: 0.1, 0.0, 0.3");
    DBG ("Actual: " + String (object0.getValue ()) + "," + String (object1.getValue ()) + "," + String (object2.getValue ()));
    return 0;
}

I get the following output:

Current Transaction :
object_0 Change

Current Transaction :
object_0 Change

Current Transaction :
object_2 Change

Expected : 0.1, 0.0, 0.3
Actual : 0.1, 0.2, 0.3

I haven’t dug too deep into how the UndoManager class works, but this isn’t a synchronicity issue. This happens in some GUI components where mouse event callbacks will call beginNewTransaction, and it doesn’t matter how much longer I make those events, the update manager is always off by one.

#2

Well no… it does start a new transaction. The only time it wouldn’t do that is when the current transaction is empty.

#3

Well that’s my problem. I have a non-empty transaction, try and start a new one, but it does not work.

Maybe I’m not using the right terminology here but if you look at the code I’ve posted, what I did was start a new transaction, change something, check what the current transaction is, then repeat the process and call undo(). I should only undo the second action, and not the first, and I should print the name of the second transaction, not the first. But that’s not what happens.

#4

Maybe you really want undoCurrentTransactionOnly() ?

#5

I figured out my problem, had nothing to do with UndoManager, just was returning false from a KeyListener object that I register with all my components which wound up calling undo() more times than was necessary.