Bug in Undo Manager ( i think )

Hello,
Most probably everyone who has undo manager in their plugins will get my point. I checked the demo app for the valuethreestate and also a few others. I also added the a few lines of code not to create many transactions while a slider is being rotated.

in editor i set my buttons to trigger redo and undo events

undoButton.onClick = [this]
{
    if(processor.mUndoManager.canUndo())
    {
        DBG("can Undo ");
        processor.mUndoManager.undo();
    }
    else
    {
        DBG("cannot Undo ");
    }
};
redoButton.onClick = [this]
{
    if(processor.mUndoManager.canRedo())
    {
        DBG("can redo ");
        processor.mUndoManager.redo();
    }else
    {
        DBG("cannot Redo ");
    }
};

and this is the code that i am using in paint method not to create a lot of transactions while rotating a knob. It works well. it starts a transaction when i start rotating the knob, It ends the transaction at the end of the rotation. So when i click on the undo, it goes back to the first value that it had before i click on the knob.

if(processor.undoRedoButtonFlag) // this flag is being set when a parameter changes.
{
    //new changes are coming
   // if the mouse button is down and if this is the first time we click on the mouse button after the last release
    if(Component::isMouseButtonDownAnywhere() && (!isNewTransactionStarted))
    {
       //it will start a transaction and set the "isNewTransactionStarted" true so that i will be able to wait till the button is released and then i can set it false. By doing this it starts a transaction and wait till the whole rotation ends. 
        isNewTransactionStarted=true;
        processor.mUndoManager.beginNewTransaction();
        DBG("new transaction started!");
    }
    else if(Component::isMouseButtonDownAnywhere() && (isNewTransactionStarted))
    {
        // while the button of the mouse is down. i am doing nothing here.
    }
    else if(!Component::isMouseButtonDownAnywhere())
    {
        // here the rotation ends and we wait till next button down trigger
        isNewTransactionStarted=false;
        processor.undoRedoButtonFlag=0;
        DBG("Movement ended!");
    }
}

The thing is , if i run the plugin and move a knob and then click on the undo once, it works fine. If i click on undo again, the canUndo does not return false and also the nextIndex (private value in the undo manager) does not go to zero. After this second click , if i try to redo, it doesn’t work.

When i try to run the plugin and move a few knobs and try not to hit the undo again till the last undo step, everything works fine. I can undo and redo, I can add more actions to it. It works great but the bug (i assume, maybe i couldn’t implement it correctly) that i mentioned above occurs every time i try to replicate it .

My question is that is undo manager works properly in your plugins or does it have this bug and more?

Is there a proper demo or example of the implementation of the undo manager that i can check?

Thanks in advance :pray:

The ValueTreesDemo has an example of undo/redo with a ValueTree, so that might be worth checking.

If you’re using the AudioProcessorValueTreeState, this already has UndoManager support built-in. If you are passing an UndoManager to the constructor of the APVTS and also doing your own undo/redo management, you might see unexpected behaviour.

I already checked it. It is using a timer to start new transactions. I tried it.I didn’t create a separate undo manager instance or a different one. i just used UndoManager class of the juce to create an instance of the undo manager to pass it to APVTS.

The code below is just for preventing multiple transactions while rotating a knob. If i don’t use it, it will create many undo points(every 500ms) while i rotate a knob. it is not the behavior that can be expected from an undo event.

I also tried to delete this section and use a timer just like in demo but still i have the same issue. I assume that you confirm that it is working correctly, right. No one has the bug that i have?

If you’re using the APVTS Attachment classes, beginNewTransaction will be called whenever a control is initially clicked. As far as I can tell, beginNewTransaction is not called elsewhere in the APVTS or ValueTree, so using the APVTS and its attachment classes should give you the behaviour you’re looking for, without needing to call beginNewTransaction yourself.

I just pass the undomanager object to constructor of APVTS and i have these two lines of code above to trigger undo and redo.
I ran the plugin.
rotated a knob.
Clicked on undo once. It works.
Clicked on redo once . it works.
Clicked on undo once. It works.(there should not be any undo left)
Clicked on undo again. after this i clicked on redo but it does not work.
If i move a knob or click on a button, then it starts working again with new transaction. Looks like after hitting the zero steps for undo, a new request for undo deletes the history.

It looks like the undo history will be cleared if the current action cannot be undone without any errors. Perhaps you could try sticking a breakpoint inside UndoManager::undo and check which undo action is failing. Then, you can either avoid adding this action, or try to change it so that it does not fail.

1 Like