Undo wanted - TransferGoal() and nested instances

mp5gosu

Hi there,

I came across a little problem about cloning nested instances and TransferGoal().
Let my try to describe the problem. I have the following setup:

0_1538768755462_7c4af3fa-7837-4e0b-8efe-242b06707ff3-image.png

"nested instance" is pointing to "direct instance" and this in turn pointing to the reference object.
Now I am trying to make the instances "editable" - quotation marks because it's just relinking & cloning linked objects which has the same effect in the end and saves a bit time and a few lines of code.

My approach is as follows: Get the linked object and replace it with the instance object currently processed. So to speak: Clone the reference, insert it and remove the original instance object. This is because it also works with deeply nested instance hierarchies.

This all works quite well, but there is an annoying problem that I cannot get rid of. When calling undo, the nested instance loses its link. I tried several approaches like using AliasTrans, adding undos in the helper routines, etc. I really have no clue, what is going on. Maybe it is a bug?

Here's my actual code, so I didn't condense it. Hope, it doesn't matter.

From Command_MakeEditable.h (CommandData Plugin)

Bool Execute(BaseDocument* doc) override
	{
		if (!doc)
			return false;

		// Detect Key modifiers
		BaseContainer state;
		GetInputState(BFM_INPUT_MOUSE, BFM_INPUT_MOUSELEFT, state);
		const auto bCtrl = (state.GetInt32(BFM_INPUT_QUALIFIER) & QCTRL) != 0;

		doc->StartUndo();

		// Create Array that holds all objects to operate on
		const AutoAlloc<AtomArray> activeObjects;
		doc->GetActiveObjects(*activeObjects, GETACTIVEOBJECTFLAGS::CHILDREN);

		// empty? quit.
		if (!activeObjects)
			return false;

		for (auto i = 0; i < activeObjects->GetCount(); ++i)
		{
			auto obj = static_cast<BaseObject*>(activeObjects->GetIndex(i));
			if (!obj)
				continue;

			// Make editable magic
			if (obj->IsInstanceOf(Oinstance))
			{
				// Convert a single instance
				auto convertedInstance = g_MakeInstanceEditable(doc, obj, bCtrl);
				if (convertedInstance == nullptr) // Something went wrong, skip
					continue;

				// Insert it into the document
				doc->InsertObject(convertedInstance, obj->GetUp(), obj->GetPred());
				doc->AddUndo(UNDOTYPE::NEWOBJ, convertedInstance);

				// Select the new object
				doc->AddUndo(UNDOTYPE::BITS, convertedInstance);
				convertedInstance->SetBit(BIT_ACTIVE);

				// Update links
				doc->AddUndo(UNDOTYPE::CHANGE, convertedInstance);
				obj->TransferGoal(convertedInstance, false);

				// Remove the original instance object to finally replace it with the converted one
				doc->AddUndo(UNDOTYPE::DELETEOBJ, obj);
				obj->Remove();
				BaseObject::Free(obj);
			}
		}

		doc->EndUndo();
		EventAdd();

		return true;
	}

The helper functions:

inline BaseObject* g_MakeInstanceEditable(BaseDocument* doc, BaseObject* obj, const bool deep = false)
{
	if (obj == nullptr || !doc)
		return nullptr;

	if (obj->IsInstanceOf(Oinstance))
	{
		// Retrieve the linked or root object
		// obj is instance and root is requested, so simply return a copy of the root object
		auto refObj = g_GetInstanceRef(obj, deep);
		if (!refObj)
			return nullptr;

		return static_cast<BaseObject*>(refObj->GetClone(COPYFLAGS::NONE, nullptr));
	}
	return nullptr;
}

// Get shallow or deeply linked reference objects
/**
 * @brief Retrieve the direct or root linked object of an instance. Also works with nested instances
 * @param obj the instance object
 * @param deep if true, the root object of nested instances is returned
 * @return the reference or root object. Can return nullptr.
 */
inline BaseObject* g_GetInstanceRef(BaseObject* obj, const Bool deep = false)
{
	if (!obj->IsInstanceOf(Oinstance))
		return nullptr;

	// Retrieve Link from instance
	GeData data;
	if (!obj->GetParameter(DescID(INSTANCEOBJECT_LINK), data, DESCFLAGS_GET::NONE))
		return nullptr;

	// The document needs to be provided for GetLinkAtom()
	const auto doc = obj->GetDocument();
	if (!doc)
		return nullptr;

	// Get the Atom
	auto linkedEntity = static_cast<BaseObject*>(data.GetLinkAtom(doc, Obase));

	// Get down to the reference object if root object is requested
	while (deep && linkedEntity->IsInstanceOf(Oinstance))
	{
		if (!linkedEntity->GetParameter(DescID(INSTANCEOBJECT_LINK), data, DESCFLAGS_GET::NONE))
			return nullptr;

		linkedEntity = static_cast<BaseObject*>(data.GetLinkAtom(doc, Obase));
	}

	return linkedEntity;
}

Any help here would be highly appreciated!

Thanks in advance,
Robert

m_adam

Hi @mp5gosu, sorry for the delay it took me a while to figure it out this topic.

The first issue is that you are adding an Undo to an object which will be deleted.
So if we get an object called "A", an Instance called "1" pointing to "A" and another instance called "2" pointing to "1", and in the object manager they are ordered as follow A => 1 => 2 and you select all tree and run your script.
In the First iteration, A is passed.
On the second iteration, 1 is converted to a copy of A, "Copy A" is inserted and goal is transferred and then the first issue, you make undo only for the instance "1" while the actual parameter which is changing is in the link in instance "2". Then you delete "1".
So at the end of the first iteration, we get "A", "Copy A" and "2" which is pointing to "Copy A".
On the third iteration, "Copy A "is again copied to "Copy Copy A", you insert this "Copy Copy A", you transfer goal, here, in fact, we don't need any undo since nothing is referring to this object in the scene. And we delete this object.

Now if we press undo. The whole process happens reversely.
So Instance object "2" is recreated, and point to "Copy A" then "Copy Copy A" is deleted. Then Instance Object "1" is recreated and point to "A", but due to improper undo handling of TransferGoal I explained before, TrasnferGoal is not called so "2" still point to "Copy A" which will be then Deleted.

So in order to fix everythings here is a modified version of g_MakeInstanceEditable

inline BaseObject* g_MakeInstanceEditable(BaseDocument* doc, BaseObject* obj, const bool deep = false)
{
	if (obj == nullptr || !doc)
		return nullptr;

	BaseObject* refObj = obj;
	doc->AddUndo(UNDOTYPE::CHANGE, refObj); // We are sure we will change the Goal of this object.
	while (refObj->IsInstanceOf(Oinstance)) // Loop until we get the final obj and not an instance
	{
		// Retrieve the linked or root object
		// obj is instance and root is requested, so simply return a copy of the root object
		refObj = g_GetInstanceRef(refObj, deep);
		if (!refObj)
			return nullptr;
	}

	return static_cast<BaseObject*>(refObj->GetClone(COPYFLAGS::NONE, nullptr));
}

and the main function.

if (!doc)
	return false;

// Detect Key modifiers
BaseContainer state;
GetInputState(BFM_INPUT_MOUSE, BFM_INPUT_MOUSELEFT, state);
const auto bCtrl = (state.GetInt32(BFM_INPUT_QUALIFIER) & QCTRL) != 0;

doc->StartUndo();

// Create Array that holds all objects to operate on
const AutoAlloc<AtomArray> activeObjects;
doc->GetActiveObjects(*activeObjects, GETACTIVEOBJECTFLAGS::CHILDREN);

// empty? quit.
if (!activeObjects)
	return false;

for (auto i = 0; i < activeObjects->GetCount(); ++i)
{
	auto obj = static_cast<BaseObject*>(activeObjects->GetIndex(i));
	if (!obj)
		continue;

	// Make editable magic
	if (obj->IsInstanceOf(Oinstance))
	{
		// Convert a single instance
		auto convertedInstance = g_MakeInstanceEditable(doc, obj, bCtrl);
		if (convertedInstance == nullptr) // Something went wrong, skip
			continue;

		// Insert it into the document
		doc->InsertObject(convertedInstance, obj->GetUp(), obj->GetPred());
		doc->AddUndo(UNDOTYPE::NEWOBJ, convertedInstance);

		// Select the new object
		doc->AddUndo(UNDOTYPE::BITS, convertedInstance);
		convertedInstance->SetBit(BIT_ACTIVE);

		//Undo handle in g_MakeInstanceEditable
		obj->TransferGoal(convertedInstance, false);

		// Remove the original instance object to finally replace it with the converted one
		doc->AddUndo(UNDOTYPE::DELETEOBJ, obj);
		obj->Remove();
		BaseObject::Free(obj);
	}
}

doc->EndUndo();
EventAdd();

return true;

If you have any questions, please let me know.
Cheers,
Maxime!

mp5gosu

Hey Maxime,

thank you very much for the detailed explanation. Makes absolute sense now! It works perfectly now.
I tried a similar approach, but I was adding the Undo in g_MakeInstanceEditable() per object - that was the main culprit.

Cheers,
Robert