UndoManager undoes the wrong child in a sorted ValueTree

Hi all,

Full code below. Basically I have a ValueTree containing some children with a single property: position. Initial status:

tree
  child0 - position 0
  child1 - position 100

I start a new undo transaction and I add a new child at the bottom of the tree, with position = 50. The new status becomes:

tree
  child0 - position 0
  child1 - position 100
  child2 - position 50 

Then I sort the tree, through a very simple Sorter class (see full code below). New status:

tree
  child0 - position 0
  child2 - position 50 
  child1 - position 100

Finally I decide to undo the child addition (that is child2) with the UndoManager. Expected status:

tree
  child0 - position 0
  child1 - position 100

But I actually get:

tree
  child0 - position 0
  child2 - position 50 

Looks like UndoManager retains the index of the original element (2) without taking the sort operation into account. Is it something done on purpose? Should I avoid sorting and add the child at the right index instead (kinda… inconvenient, though)?

class Sorter
{
public:
    static int compareElements(const ValueTree &a, const ValueTree &b)
    {
    	int64 apos = a.getProperty("position");
    	int64 bpos = b.getProperty("position");
    	return (apos < bpos) ? -1 : ((bpos < apos) ? 1 : 0);   	
    }
};


void print(ValueTree &tree)
{
	std::cout << "-- TREE ----------------------------------------------\n";
	for (ValueTree t : tree)
		std::cout << t.getType().toString() << " - " << t.getProperty("position").toString() << "\n";
	std::cout << "------------------------------------------------------\n";
}


int main (int argc, char** argv)
{
	Sorter sorter;
	UndoManager undoManager;
	ValueTree tree("tree");

	/* Let's add some children. Initial layout:
	- child0 at position 0
	- child1 at position 100 
	*/

	ValueTree child0("child0");
	child0.setProperty("position", 0, nullptr);
	tree.addChild(child0, -1, nullptr);

	ValueTree child1("child1");
	child1.setProperty("position", 100, nullptr);
	tree.addChild(child1, -1, nullptr);

	print(tree);

	undoManager.beginNewTransaction();

	/* Add a new branch at position 50, at the end of the tree. New layout:
	- child0 at position 0
	- child1 at position 100 
	- child2 at position 50	
	*/

	ValueTree child2("child2");
	child2.setProperty("position", 50, nullptr);
	tree.addChild(child2, -1, &undoManager);

	print(tree);

	/* Sort the tree. New layout:
	- child0 at position 0
	- child2 at position 50	
	- child1 at position 100 
	*/

	tree.sort(sorter, nullptr, false);

	print(tree);

	/* Undo the last addition of child2. Expected layout:
	- child0 at position 0
	- child1 at position 100

	Actual layout:

	- child0 at position 0
	- child2 at position 50	
	*/

	undoManager.undo();

	print(tree);
}
tree.sort(sorter, nullptr, false);

If you don’t provide an undomanager to the sort() function, how do you expect it to track the state correctly!?

Oh my, two hours of debugging and the solution was under my nose. Thank you Jules, you and your team are awesome.

1 Like