Fix for invalid ID renaming during prefab diff generation

BansheeCore/Include/BsGameObject.h

@@ -40,7 +40,7 @@ namespace BansheeEngine
 		 *			in the linked prefab. This will be -1 if the object has no prefab link, or if
 		 *			the object is specific to the instance and has no prefab equivalent.
 		 */
-		INT32 getLinkId() const { return mLinkId; }
+		UINT32 getLinkId() const { return mLinkId; }
 
 		/**
 		 * @brief	Gets the name of the object.
@@ -107,7 +107,7 @@ namespace BansheeEngine
 
 	protected:
 		String mName;
-		INT32 mLinkId;
+		UINT32 mLinkId;
 
 	private:
 		friend class Prefab;

BansheeCore/Include/BsGameObjectRTTI.h

@@ -41,8 +41,8 @@ namespace BansheeEngine
 			deserializationData.originalId = instanceId;
 		}
 
-		INT32& getLinkId(GameObject* obj) { return obj->mLinkId; }
-		void setLinkId(GameObject* obj, INT32& linkId) { obj->mLinkId = linkId; }
+		UINT32& getLinkId(GameObject* obj) { return obj->mLinkId; }
+		void setLinkId(GameObject* obj, UINT32& linkId) { obj->mLinkId = linkId; }
 
 	public:
 		/**

BansheeCore/Include/BsPrefabDiff.h

@@ -33,15 +33,15 @@ namespace BansheeEngine
 	 */
 	struct BS_CORE_EXPORT PrefabObjectDiff : public IReflectable
 	{
-		INT32 id;
+		UINT32 id;
 		String name;
 
 		Vector<SPtr<PrefabComponentDiff>> componentDiffs;
-		Vector<INT32> removedComponents;
+		Vector<UINT32> removedComponents;
 		Vector<SPtr<SerializedObject>> addedComponents;
 
 		Vector<SPtr<PrefabObjectDiff>> childDiffs;
-		Vector<INT32> removedChildren;
+		Vector<UINT32> removedChildren;
 		Vector<SPtr<SerializedObject>> addedChildren;
 
 		/************************************************************************/

BansheeCore/Include/BsPrefabUtility.h

@@ -17,7 +17,7 @@ namespace BansheeEngine
 		struct ComponentProxy
 		{
 			GameObjectInstanceDataPtr instanceData;
-			INT32 linkId;
+			UINT32 linkId;
 		};
 
 		/**
@@ -27,7 +27,7 @@ namespace BansheeEngine
 		struct SceneObjectProxy
 		{
 			GameObjectInstanceDataPtr instanceData;
-			INT32 linkId;
+			UINT32 linkId;
 
 			Vector<ComponentProxy> components;
 			Vector<SceneObjectProxy> children;

BansheeCore/Source/BsGameObject.cpp

@@ -5,7 +5,7 @@
 namespace BansheeEngine
 {
 	GameObject::GameObject()
-		:mIsDestroyed(false), mLinkId(-1)
+		:mIsDestroyed(false), mLinkId((UINT32)-1)
 	{ }
 
 	GameObject::~GameObject()

BansheeCore/Source/BsGameObjectManager.cpp

@@ -72,6 +72,9 @@ namespace BansheeEngine
 	{
 		object->initialize(object, mNextAvailableID);
 
+		// If deserialization is active we must ensure all handles pointing to the same object share GameObjectHandleData,
+		// so check if any handles referencing this object have been created. See ::registerUnresolvedHandle for
+		// further explanation.
 		if (mIsDeserializationActive)
 		{
 			assert(originalId != 0 && "You must provide an original ID when registering a deserialized game object.");
@@ -188,6 +191,11 @@ namespace BansheeEngine
 		}
 #endif
 
+		// All handles that are deserialized during a single begin/endDeserialization session pointing to the same object
+		// must share the same GameObjectHandleData as that makes certain operations in other systems much simpler. 
+		// Therefore we store all the unresolved handles, and if a handle pointing to the same object was already
+		// processed, or that object was already created we replace the handle's internal GameObjectHandleData.
+
 		// Update the provided handle to ensure all handles pointing to the same object share the same handle data
 		bool foundHandleData = false;
 

BansheeCore/Source/BsPrefabDiff.cpp

@@ -409,19 +409,19 @@ namespace BansheeEngine
 
 				if (iterFind != linkToInstanceId.end())
 				{
-					if (current.so->getLinkId() != -1)
+					if (child->getLinkId() != -1)
 					{
 						UnorderedMap<UINT32, UINT64>& idMap = iterFind->second;
 
-						auto iterFind2 = idMap.find(current.so->getLinkId());
+						auto iterFind2 = idMap.find(child->getLinkId());
 						if (iterFind2 != idMap.end())
 						{
 							output.push_back(RenamedGameObject());
 							RenamedGameObject& renamedGO = output.back();
-							renamedGO.instanceData = current.so->mInstanceData;
-							renamedGO.originalId = current.so->getInstanceId();
+							renamedGO.instanceData = child->mInstanceData;
+							renamedGO.originalId = child->getInstanceId();
 
-							current.so->mInstanceData->mInstanceId = iterFind2->second;
+							child->mInstanceData->mInstanceId = iterFind2->second;
 						}
 					}
 				}

BansheeCore/Source/BsPrefabUtility.cpp

@@ -49,6 +49,7 @@ namespace BansheeEngine
 		Stack<HSceneObject> todo;
 		todo.push(topLevelObject);
 
+		// Find any prefab instances
 		Vector<HSceneObject> prefabInstanceRoots;
 
 		while (!todo.empty())
@@ -67,6 +68,8 @@ namespace BansheeEngine
 			}
 		}
 
+		// Stores data about the new prefab instance and its original parent and link id
+		// (as those aren't stored in the prefab diff)
 		struct RestoredPrefabInstance
 		{
 			HSceneObject newInstance;
@@ -76,13 +79,20 @@ namespace BansheeEngine
 
 		Vector<RestoredPrefabInstance> newPrefabInstances;
 
+		// For each prefab instance load its reference prefab from the disk and check if it changed. If it has changed
+		// instantiate the prefab and destroy the current instance. Then apply instance specific changes stored in a
+		// prefab diff, if any, as well as restore the original parent and link id (link id of the root prefab instance
+		// belongs to the parent prefab if any). Finally fix any handles pointing to the old objects so that they now point
+		// to the newly instantiated objects. To the outside world it should be transparent that we just destroyed and then
+		// re-created from scratch the entire hierarchy.
+
 		// Need to do this bottom up to ensure I don't destroy the parents before children
 		for (auto iter = prefabInstanceRoots.rbegin(); iter != prefabInstanceRoots.rend(); ++iter)
 		{
 			HSceneObject current = *iter;
 			HPrefab prefabLink = static_resource_cast<Prefab>(gResources().loadFromUUID(current->mPrefabLinkUUID, false, false));
 
-			if (prefabLink != nullptr && prefabLink->getHash() != current->mPrefabHash)
+			if (prefabLink != nullptr /*&& prefabLink->getHash() != current->mPrefabHash*/)
 			{
 				// Save IDs, destroy original, create new, apply diff, restore IDs
 				SceneObjectProxy soProxy;
@@ -98,6 +108,12 @@ namespace BansheeEngine
 				if (prefabDiff != nullptr)
 					prefabDiff->apply(newInstance);
 
+				// When restoring instance IDs it is important to make all the new handles point to the old GameObjectInstanceData.
+				// This is because old handles will have different GameObjectHandleData and we have no easy way of accessing it to
+				// change to which GameObjectInstanceData it points. But the GameObjectManager ensures that all handles deserialized
+				// at once (i.e. during the ::instantiate() call above) will share GameObjectHandleData so we can simply replace
+				// to what they point to, affecting all of the handles to that object. (In another words, we can modify the
+				// new handles at this point, but old ones must keep referencing what they already were.)
 				restoreLinkedInstanceData(newInstance, soProxy, linkedInstanceData);
 				restoreUnlinkedInstanceData(newInstance, soProxy);
 
@@ -127,7 +143,7 @@ namespace BansheeEngine
 
 			for (auto& component : currentSO->mComponents)
 			{
-				if (component->getLinkId() == -1)
+				if (component->getLinkId() == (UINT32)-1)
 					component->mLinkId = startingId++;
 			}
 
@@ -138,7 +154,7 @@ namespace BansheeEngine
 
 				if (!child->hasFlag(SOF_DontSave))
 				{
-					if (child->getLinkId() == -1)
+					if (child->getLinkId() == (UINT32)-1)
 						child->mLinkId = startingId++;
 
 					if(child->mPrefabLinkUUID.empty())
@@ -161,7 +177,7 @@ namespace BansheeEngine
 			todo.pop();
 
 			for (auto& component : currentSO->mComponents)
-				component->mLinkId = -1;
+				component->mLinkId = (UINT32)-1;
 
 			if (recursive)
 			{
@@ -169,7 +185,7 @@ namespace BansheeEngine
 				for (UINT32 i = 0; i < numChildren; i++)
 				{
 					HSceneObject child = currentSO->getChild(i);
-					child->mLinkId = -1;
+					child->mLinkId = (UINT32)-1;
 
 					if (child->mPrefabLinkUUID.empty())
 						todo.push(child);
@@ -194,7 +210,7 @@ namespace BansheeEngine
 		}
 
 		if (topLevelObject == nullptr)
-			return;
+			topLevelObject = sceneObject;
 
 		Stack<HSceneObject> todo;
 		todo.push(topLevelObject);
@@ -237,7 +253,7 @@ namespace BansheeEngine
 		todo.push({so, &output});
 
 		output.instanceData = so->_getInstanceData();
-		output.linkId = -1;
+		output.linkId = (UINT32)-1;
 
 		while (!todo.empty())
 		{
@@ -295,7 +311,7 @@ namespace BansheeEngine
 			Vector<HComponent>& components = current->mComponents;
 			for (auto& component : components)
 			{
-				if (component->getLinkId() != -1)
+				if (component->getLinkId() != (UINT32)-1)
 				{
 					auto iterFind = linkedInstanceData.find(component->getLinkId());
 					if (iterFind != linkedInstanceData.end())
@@ -311,7 +327,7 @@ namespace BansheeEngine
 			{
 				HSceneObject child = current->getChild(i);
 
-				if (child->getLinkId() != -1)
+				if (child->getLinkId() != (UINT32)-1)
 				{
 					auto iterFind = linkedInstanceData.find(child->getLinkId());
 					if (iterFind != linkedInstanceData.end())
@@ -352,7 +368,7 @@ namespace BansheeEngine
 			UINT32 numComponentProxies = (UINT32)current.proxy->components.size();
 			for (auto& component : components)
 			{
-				if (component->getLinkId() == -1)
+				if (component->getLinkId() == (UINT32)-1)
 				{
 					bool foundInstanceData = false;
 					for (; componentProxyIdx < numComponentProxies; componentProxyIdx++)
@@ -378,7 +394,7 @@ namespace BansheeEngine
 			{
 				HSceneObject child = current.so->getChild(i);
 
-				if (child->getLinkId() == -1)
+				if (child->getLinkId() == (UINT32)-1)
 				{
 					bool foundInstanceData = false;
 					for (; childProxyIdx < numChildProxies; childProxyIdx++)