AudioProcessorValueTreeState & UndoManager usage

Audio Plugins
21

I’m a bit confused, in the description it says that AudioProcessorUndoAttachment is not intended for AudioProcessorValueTreeState based projects, but you’re saying that it should work fine?

22

You’re right. @yairadix should maybe edit this comment. it was very early before we had a product with AudioProcessorValueTreeState released.

TL;DR:
The implantation on the gist is agnostic to your parameter system. we use it on older products and newer with great success.

Few caveats to remember:

  • it based on AudioProcessorListener. I’m not sure about future changes as seen here . but it simply keeps track on any parameter changes within the plug-in (not from the host, which is a good thing).

  • you should use begin/end gestures properly for sliders to get a proper behavior. (same goes with AudioProcessorValueTree from my experience).

  • A/B switch or single transaction for entire states aren’t being registered that way.

23

I wrote that description naively assuming that AudioProcessorValueTreeState's own undo attachment should work fine. Nowadays, knowing that it doesn’t work well I recommend AudioProcessorUndoAttachment everywhere.

3 Likes
24

It’s working great so far with my AudioProcessorValueTreeState setup!! Bunches and bunches of thanks Yair! :slight_smile:

25

https://github.com/WeAreROLI/JUCE/commit/2e51654958df26d9c374e961758180cc842e0dfa

This commit should address the issues using an UndoManager with an AudioProcessorValueTreeState.

  • An undoable action is no longer pushed onto the stack when the plug-in is loaded
  • Redoing actions works correctly
  • New transactions are started when you interact with an attachment

Since this alters the behaviour of the UndoManager this is technically a breaking change, but I doubt anyone was relying upon the old way of doing things.

6 Likes
26

thanks tom! everything works just fine!

27

HI.
Working with undomanager and valueTreeState (latest version), i have a randomiser function that changes the values of multiple parameters (notifying host). Undo manager undos all the changes made with just one undo() which is convenient. I thought it would undo them one by one, now I am curious how does it work since im not doing any perform() or beginNewTransaction()?

28

I dont expect it to work correctly if you don’t call beginNewTransaction() when the randomizer button is clicked.

it is probably undoing all the changes you made since the last beginNewTransaction() (so since you previously interact with a slider or something?). also, if you don’t call beginNewTransaction, then successive clicks on your randomiser button will get grouped together no?

29

Yes you’re right. All of them gets into one group and i can call beginNewTransaction() after each randomize action to separate them. So only changing sliders that are attached will create a single transaction, and if we want to call setValueNotifyingHost() ourselves we should call beginNewTransaction() after that. Got it.

30

no, you have to call beginNewTransaction() before the action, not after.

void buttonClicked (Button*)
{
    beginNewTransaction();
    setValueNotifyingHost()
}
1 Like
31

Thanks for this! I noticed this introduced a regression where initializing an AudioProcessorValueTreeState with a null UndoManager will throw assertion failures due to the lack of a null check around the undoManager here https://github.com/WeAreROLI/JUCE/commit/2e51654958df26d9c374e961758180cc842e0dfa#diff-1ec75d23db30b69cffab9854fa6697f4R127

EDIT: Hrm, I’m not sure what the right behavior is or if I’m missing something obvious, but I keep hitting this assertion error whenever I change any param with param.setValue(newValue). The AudioProcessorValueTreeState is being initialized with a a non-null AudioProcessor and UndoManager. Having trouble getting to the bottom of this one.

32

It turns out I was missing this step of the AudioProcessorValueTreeState setup:

state.state = ValueTree(Identifier("My-Identifier"));

@t0m before your commit, I would get no assertion errors at all in my project because the state.isValid() check in copyValueToValueTree() was always returning false, and was thus never descending into the state.setPropertyExcludingListener call. With your changes, it does make the setProperty... call even if the state is invalid (initialized with only the default constructor and given no explicit identifier).

Basically, with the new changes, if someone (a newb like me) is just starting a project and throws a default AudioProcessorValueTreeState constructor on their AudioProcessor, it will throw errors, whereas before the behavior was, “ok, you’ve done that. I won’t complain but I also won’t be doing anything useful.” Now, in that scenario we get obscure-looking errors. I’ll leave it to the team to decide if this is desired, but I would suggest that if this is an undesirable state for an AudioProcessorValueTreeState to be in, that you make it harder, or even impossible, to get into this state by, say, requiring an identifier and initializing the internal ValueTree state in the constructor or something like that. Barring this, if you’d rather allow the bad state but “fail-fast” with an assert error, I would suggest a helpful warning message that mentions explicitly initializing the internal ValueTree state.

Again, thanks for addressing these issues!