UndoManager undoes the wrong child in a sorted ValueTree


#1

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);
}

#2
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!?


#3

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