ValueTree: undoCurrentTransactionOnly() + redo() fails!

General JUCE discussion
1

Hello,

This code fails:

   ValueTree tree ("data");
   UndoManager undo;
   tree.setProperty("T", 4, nullptr);
   
   undo.beginNewTransaction("T change");
   tree.setProperty("T", 5, &undo);
   const int v0 = tree.getProperty("T");
   undo.undoCurrentTransactionOnly();
   const int v1 = tree.getProperty("T");
   undo.redo();
   const int v2 = tree.getProperty("T");
   jassert(v2 == 5); // fails!

I was expecting redo() actually redo the undoCurrentTransactionOnly(), but it doesn’t. Is this a bug?

I’m interested in this because I have problems modifing multiple objects properties by using a mouseDrag:

void mouseDown(const MouseEvent& e) override
{
	undo.beginNewTransaction("anything");	
}
void mouseDrag(const MouseEvent& e) override
{
	undo.undoCurrentTransactionOnly();

	for (auto& current : data)
	{
		current.tree.setProperty("x", e.x, &undo);
	}
}

I know I could create a copy in the mouseDown, and perform the action in the mouseUp. But the undoCurrentTransactionOnly() idea was pretty neat.

I’m using Juce 6.0.5 (I cannot update to 6.0.8 due to the VS2019 bug, I’m waiting for the next release)

2

Looking at the git commit log, I think the current behaviour is intentional. undoCurrentTransactionOnly restores the “previous” redo history when called, which is useful if you need to abort a new change and retain the previous redo stack. For example, the following code will work:

  juce::ValueTree tree("data");
  juce::UndoManager undo;
  tree.setProperty("T", 4, nullptr);

  undo.beginNewTransaction("set T to 1");
  tree.setProperty("T", 1, &undo);

  undo.beginNewTransaction("set T to 2");
  tree.setProperty("T", 2, &undo);

  undo.beginNewTransaction("set T to 3");
  tree.setProperty("T", 3, &undo);

  undo.undo();
  jassert((int)tree.getProperty("T") == 2);

  undo.undo();
  jassert((int)tree.getProperty("T") == 1);

  tree.setProperty("T", 5, &undo);
  undo.undoCurrentTransactionOnly(); // Oops, didn't mean to set T to 5!

  undo.redo();
  jassert((int)tree.getProperty("T") == 2); // We still have the old redo history

  undo.redo();
  jassert((int)tree.getProperty("T") == 3);

It looks like undoCurrentTransactionOnly is supposed to irrevocably cancel any action that is currently in progress. Changing this behaviour may introduce subtle bugs in existing programs, so I don’t think this behaviour should be changed at this point.

3

Thanks for the support.

I will overcome it calling this on my mouseDrag(), preventing it undoes other action that wasn’t the initial one:

   if (um.getNumActionsInCurrentTransaction())
   {
      String name = um.getCurrentTransactionName();
      um.undo();
      um.beginNewTransaction(name);
   }

btw, FR, I wish there was a way to discard lastest actions from a transaction instead. So I could do an initial action “A” in mouseDown(), and do some repetitive actions “B” in mouseDrag() deleting only the last one, then when undo(), “B” and “A” would be in the same transaction.