Added an undo/redo command for deleting SceneObjects and its unit test

Fixed an issue where copying Event handles was causing memory to be freed multiple times
Changed the logic of how GameObjects delayed destruction works to ensure they can be immediately destroyed even if they're queued for normal destruction

BansheeCore/Include/BsComponent.h

@@ -55,6 +55,18 @@ namespace BansheeEngine
 		 * @brief	Called just before the component is destroyed.
 		 */
 		virtual void onDestroyed() {}
+
+		/**
+		 * @brief	Destroys this component.
+		 *
+		 * @param [in]	handle		Game object handle this this object.
+		 * @param [in]	immediate	If true, the object will be deallocated and become unusable
+		 *							right away. Otherwise the deallocation will be delayed to the end of
+		 *							frame (preferred method).
+		 *
+		 * @note	Unlike "destroy", does not remove the component from its parent.
+		 */
+		void destroyInternal(GameObjectHandleBase& handle, bool immediate = false) override;
 	private:
 		Component(const Component& other) { }
 

BansheeCore/Include/BsGameObject.h

@@ -95,6 +95,16 @@ namespace BansheeEngine
 		 */
 		void initialize(const std::shared_ptr<GameObject>& object, UINT64 instanceId);
 
+		/**
+		 * @brief	Destroys this object.
+		 *
+		 * @param [in]	handle		Game object handle to this object.
+		 * @param [in]	immediate	If true, the object will be deallocated and become unusable
+		 *							right away. Otherwise the deallocation will be delayed to the end of
+		 *							frame (preferred method).
+		 */
+		virtual void destroyInternal(GameObjectHandleBase& handle, bool immediate = false) = 0;
+
 	protected:
 		String mName;
 		INT32 mLinkId;

BansheeCore/Include/BsGameObjectHandle.h

@@ -48,7 +48,7 @@ namespace BansheeEngine
 		/**
 		 * @brief	Returns true if the object the handle is pointing to has been destroyed.
 		 *
-		 * @param checkQueued	Game objects can be queued for destruction but no actually destroyed yet, and still accessible. 
+		 * @param checkQueued	Game objects can be queued for destruction but not actually destroyed yet, and still accessible. 
 		 *						If this is false this method will return true only if the object is completely inaccessible (fully destroyed).
 		 *						If this is true this method will return true if object is completely inaccessible or if it is just queued for destruction.
 		 */
@@ -121,6 +121,7 @@ namespace BansheeEngine
 
 	protected:
 		friend class SceneObject;
+		friend class Component;
 		friend class SceneObjectRTTI;
 		friend class GameObjectManager;
 

BansheeCore/Include/BsSceneObject.h

@@ -104,13 +104,14 @@ namespace BansheeEngine
 		/**
 		 * @brief	Destroys this object and any of its held components.
 		 *
+		 * @param [in]	handle		Game object handle to this object.
 		 * @param [in]	immediate	If true, the object will be deallocated and become unusable
 		 *							right away. Otherwise the deallocation will be delayed to the end of
 		 *							frame (preferred method).
 		 *
 		 * @note	Unlike "destroy", does not remove the object from its parent.
 		 */
-		void destroyInternal(bool immediate = false);
+		void destroyInternal(GameObjectHandleBase& handle, bool immediate = false) override;
 
 		/**
 		 * @brief	Recursively enables the provided set of flags on

BansheeCore/Source/BsComponent.cpp

@@ -20,6 +20,19 @@ namespace BansheeEngine
 		SO()->destroyComponent(this, immediate);
 	}
 
+	void Component::destroyInternal(GameObjectHandleBase& handle, bool immediate)
+	{
+		if (immediate)
+		{
+			GameObjectManager::instance().unregisterObject(handle);
+			handle.destroy();
+		}
+		else
+		{
+			GameObjectManager::instance().queueForDestroy(handle);
+		}
+	}
+
 	RTTITypeBase* Component::getRTTIStatic()
 	{
 		return ComponentRTTI::instance();

BansheeCore/Source/BsGameObjectManager.cpp

@@ -63,10 +63,7 @@ namespace BansheeEngine
 	void GameObjectManager::destroyQueuedObjects()
 	{
 		for (auto& objPair : mQueuedForDestroy)
-		{
-			unregisterObject(objPair.second);
-			objPair.second.destroy();
-		}
+			objPair.second->destroyInternal(objPair.second, true);
 
 		mQueuedForDestroy.clear();
 	}

BansheeCore/Source/BsSceneObject.cpp

@@ -27,7 +27,7 @@ namespace BansheeEngine
 		if(!mThisHandle.isDestroyed())
 		{
 			LOGWRN("Object is being deleted without being destroyed first?");
-			destroyInternal(true);
+			destroyInternal(mThisHandle, true);
 		}
 	}
 
@@ -60,43 +60,39 @@ namespace BansheeEngine
 		{
 			if(!mParent.isDestroyed())
 				mParent->removeChild(mThisHandle);
+
+			mParent = nullptr;
 		}
 
-		destroyInternal(immediate);
+		destroyInternal(mThisHandle, immediate);
 	}
 
-	void SceneObject::destroyInternal(bool immediate)
+	void SceneObject::destroyInternal(GameObjectHandleBase& handle, bool immediate)
 	{
-		for(auto iter = mChildren.begin(); iter != mChildren.end(); ++iter)
-			(*iter)->destroyInternal(immediate);
-
-		mChildren.clear();
-
-		for(auto iter = mComponents.begin(); iter != mComponents.end(); ++iter)
+		if (immediate)
 		{
-			(*iter)->_setIsDestroyed();
+			for (auto iter = mChildren.begin(); iter != mChildren.end(); ++iter)
+				(*iter)->destroyInternal(*iter, true);
 
-			if (isInstantiated())
-				(*iter)->onDestroyed();
+			mChildren.clear();
 
-			if (immediate)
+			for (auto iter = mComponents.begin(); iter != mComponents.end(); ++iter)
 			{
-				GameObjectManager::instance().unregisterObject(*iter);
-				(*iter).destroy();
+				(*iter)->_setIsDestroyed();
+
+				if (isInstantiated())
+					(*iter)->onDestroyed();
+
+				(*iter)->destroyInternal(*iter, true);
 			}
-			else
-				GameObjectManager::instance().queueForDestroy(*iter);
-		}
 
-		mComponents.clear();
+			mComponents.clear();
 
-		if (immediate)
-		{
-			GameObjectManager::instance().unregisterObject(mThisHandle);
-			mThisHandle.destroy();
+			GameObjectManager::instance().unregisterObject(handle);
+			handle.destroy();
 		}
 		else
-			GameObjectManager::instance().queueForDestroy(mThisHandle);
+			GameObjectManager::instance().queueForDestroy(handle);
 	}
 
 	void SceneObject::_setInstanceData(GameObjectInstanceDataPtr& other)
@@ -600,17 +596,8 @@ namespace BansheeEngine
 			if (isInstantiated())
 				(*iter)->onDestroyed();
 			
+			(*iter)->destroyInternal(*iter, immediate);
 			mComponents.erase(iter);
-
-			if (immediate)
-			{
-				GameObjectManager::instance().unregisterObject(component);
-				(*iter).destroy();
-			}
-			else
-			{
-				GameObjectManager::instance().queueForDestroy(component);
-			}
 		}
 		else
 			LOGDBG("Trying to remove a component that doesn't exist on this SceneObject.");

BansheeEditor/BansheeEditor.vcxproj

@@ -267,10 +267,12 @@
   <ItemGroup>
     <ClInclude Include="Include\BsBuildDataRTTI.h" />
     <ClInclude Include="Include\BsBuildManager.h" />
+    <ClInclude Include="Include\BsCmdDeleteSO.h" />
     <ClInclude Include="Include\BsCmdEditPlainFieldGO.h" />
     <ClInclude Include="Include\BsCmdInputFieldValueChange.h" />
     <ClInclude Include="Include\BsCmdRecordSO.h" />
     <ClInclude Include="Include\BsCmdReparentSO.h" />
+    <ClInclude Include="Include\BsCmdUtility.h" />
     <ClInclude Include="Include\BsDockManager.h" />
     <ClInclude Include="Include\BsDockManagerLayout.h" />
     <ClInclude Include="Include\BsDockManagerLayoutRTTI.h" />
@@ -347,6 +349,8 @@
     <ClCompile Include="Source\BsBuildManager.cpp" />
     <ClCompile Include="Source\BsCmdRecordSO.cpp" />
     <ClCompile Include="Source\BsCmdReparentSO.cpp" />
+    <ClCompile Include="Source\BsCmdUtility.cpp" />
+    <ClCompile Include="Source\BsCmdDeleteSO.cpp" />
     <ClCompile Include="Source\BsDockManager.cpp" />
     <ClCompile Include="Source\BsDockManagerLayout.cpp" />
     <ClCompile Include="Source\BsDropDownWindow.cpp" />

BansheeEditor/BansheeEditor.vcxproj.filters

@@ -22,12 +22,12 @@
     <Filter Include="Header Files\Win32">
       <UniqueIdentifier>{ee1257c5-4335-401a-938c-adc62b074503}</UniqueIdentifier>
     </Filter>
-    <Filter Include="Source Files\Command">
-      <UniqueIdentifier>{fc5eed3b-3a94-4c0b-b462-636e84615f94}</UniqueIdentifier>
-    </Filter>
     <Filter Include="Source Files\Win32">
       <UniqueIdentifier>{482fa361-f45b-45d4-9d09-05ffb91c39b8}</UniqueIdentifier>
     </Filter>
+    <Filter Include="Source Files\Commands">
+      <UniqueIdentifier>{fc5eed3b-3a94-4c0b-b462-636e84615f94}</UniqueIdentifier>
+    </Filter>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="Include\BsEditorCommand.h">
@@ -261,16 +261,22 @@
     <ClInclude Include="Include\BsGUIStatusBar.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="Include\BsCmdUtility.h">
+      <Filter>Header Files\Commands</Filter>
+    </ClInclude>
+    <ClInclude Include="Include\BsCmdDeleteSO.h">
+      <Filter>Header Files\Commands</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="Source\BsEditorCommand.cpp">
-      <Filter>Source Files\Command</Filter>
+      <Filter>Source Files\Commands</Filter>
     </ClCompile>
     <ClCompile Include="Source\BsCmdReparentSO.cpp">
-      <Filter>Source Files\Command</Filter>
+      <Filter>Source Files\Commands</Filter>
     </ClCompile>
     <ClCompile Include="Source\BsCmdRecordSO.cpp">
-      <Filter>Source Files\Command</Filter>
+      <Filter>Source Files\Commands</Filter>
     </ClCompile>
     <ClCompile Include="Source\Win32\BsVSCodeEditor.cpp">
       <Filter>Source Files\Win32</Filter>
@@ -467,5 +473,11 @@
     <ClCompile Include="Source\BsGUIStatusBar.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="Source\BsCmdUtility.cpp">
+      <Filter>Source Files\Commands</Filter>
+    </ClCompile>
+    <ClCompile Include="Source\BsCmdDeleteSO.cpp">
+      <Filter>Source Files\Commands</Filter>
+    </ClCompile>
   </ItemGroup>
 </Project>

BansheeEditor/Include/BsCmdDeleteSO.h

@@ -0,0 +1,62 @@
+#pragma once
+
+#include "BsEditorPrerequisites.h"
+#include "BsEditorCommand.h"
+#include "BsUndoRedo.h"
+#include "BsCmdUtility.h"
+
+namespace BansheeEngine
+{
+	/**
+	 * @brief	A command used for undo/redo purposes. Deletes a scene object
+	 *			and restores it as an undo operation.
+	 */
+	class CmdDeleteSO : public EditorCommand
+	{
+	public:
+		~CmdDeleteSO();
+
+		/**
+		 * @brief	Creates and executes the command on the provided scene object.
+		 *			Automatically registers the command with undo/redo system.
+		 *
+		 * @param	inputField	Input field to modify the value on.
+		 * @param	value		New value for the field.
+		 */
+		static void execute(const HSceneObject& sceneObject);
+
+		/**
+		 * @copydoc	EditorCommand::commit
+		 */
+		void commit() override;
+
+		/**
+		 * @copydoc	EditorCommand::revert
+		 */
+		void revert() override;
+
+	private:
+		friend class UndoRedo;
+
+		CmdDeleteSO(const HSceneObject& sceneObject);
+
+		/**
+		 * @brief	Saves the state of the specified object, all of its children
+		 *			and components. Make sure to call "clear" when you no longer need
+		 *			the data, or wish to call this method again.
+		 */
+		void recordSO(const HSceneObject& sceneObject);
+
+		/**
+		 * @brief	Clears all the stored data and frees memory.
+		 */
+		void clear();
+
+		HSceneObject mSceneObject;
+		CmdUtility::SceneObjProxy mSceneObjectProxy;
+
+		UINT8* mSerializedObject;
+		UINT32 mSerializedObjectSize;
+		UINT64 mSerializedObjectParentId;
+	};
+}

BansheeEditor/Include/BsCmdRecordSO.h

@@ -3,6 +3,7 @@
 #include "BsEditorPrerequisites.h"
 #include "BsEditorCommand.h"
 #include "BsUndoRedo.h"
+#include "BsCmdUtility.h"
 
 namespace BansheeEngine
 {
@@ -13,22 +14,6 @@ namespace BansheeEngine
 	 */
 	class CmdRecordSO : public EditorCommand
 	{
-		/**
-		 * @brief	Contains stored information about stored scene object instance data,
-		 *			including all of its children and components.
-		 *
-		 * @note	When object is serialized it will receive new instance data (as if it was a new
-		 *			object). But we want to restore the original object completely (including any references
-		 *			other objects might have to it) so we need store the instance data.
-		 */
-		struct SceneObjProxy
-		{
-			GameObjectInstanceDataPtr instanceData;
-
-			Vector<GameObjectInstanceDataPtr> componentInstanceData;
-			Vector<SceneObjProxy> children;
-		};
-
 	public:
 		~CmdRecordSO();
 
@@ -68,23 +53,8 @@ namespace BansheeEngine
 		 */
 		void clear();
 
-		/**
-		 * @brief	Parses the scene object hierarchy and components and generates a
-		 *			hierarchy of instance data required to restore the object identities.
-		 *
-		 * @see		SceneObjProxy
-		 */
-		SceneObjProxy createProxy(const HSceneObject& sceneObject);
-
-		/**
-		 * @brief	Restores original object instance data after deserialization.
-		 *
-		 * @see		SceneObjProxy
-		 */
-		void restoreIds(const HSceneObject& restored);
-
 		HSceneObject mSceneObject;
-		SceneObjProxy mSceneObjectProxy;
+		CmdUtility::SceneObjProxy mSceneObjectProxy;
 
 		UINT8* mSerializedObject;
 		UINT32 mSerializedObjectSize;

BansheeEditor/Include/BsCmdUtility.h

@@ -0,0 +1,45 @@
+#pragma once
+
+#include "BsEditorPrerequisites.h"
+
+namespace BansheeEngine
+{
+	/**
+	 * @brief	Contains various utility methods and structures used by EditorCommand%s.
+	 */
+	class CmdUtility
+	{
+	public:
+		/**
+		 * @brief	Contains stored information about stored scene object instance data,
+		 *			including all of its children and components.
+		 *
+		 * @note	When object is serialized it will receive new instance data (as if it was a new
+		 *			object). But we want to restore the original object completely (including any references
+		 *			other objects might have to it) so we need store the instance data.
+		 */
+		struct SceneObjProxy
+		{
+			GameObjectInstanceDataPtr instanceData;
+
+			Vector<GameObjectInstanceDataPtr> componentInstanceData;
+			Vector<SceneObjProxy> children;
+		};
+
+		/**
+		 * @brief	Parses the scene object hierarchy and components and generates a
+		 *			hierarchy of instance data required to restore the object identities.
+		 */
+		static SceneObjProxy createProxy(const HSceneObject& sceneObject);
+
+		/**
+		 * @brief	Restores original object instance data from the provided scene object proxy
+		 *			that was previously generated using ::createProxy.
+		 *
+		 * @param	restored	New instance of the object.
+		 * @param	proxy		Proxy data containing the original object instance data
+		 *						we want to restore.
+		 */
+		static void restoreIds(const HSceneObject& restored, SceneObjProxy& proxy);
+	};
+}

BansheeEditor/Include/BsEditorTestSuite.h

@@ -74,6 +74,11 @@ namespace BansheeEngine
 		 */
 		void SceneObjectRecord_UndoRedo();
 
+		/**
+		 * @brief	Tests SceneObject delete undo/redo operation.
+		 */
+		void SceneObjectDelete_UndoRedo();
+
 		/**
 		 * @brief	Tests native diff by modifiying an object, generating a diff
 		 *			and re-applying the modifications.

BansheeEditor/Source/BsCmdDeleteSO.cpp

@@ -0,0 +1,85 @@
+#include "BsCmdDeleteSO.h"
+#include "BsSceneObject.h"
+#include "BsComponent.h"
+#include "BsMemorySerializer.h"
+
+namespace BansheeEngine
+{
+	CmdDeleteSO::CmdDeleteSO(const HSceneObject& sceneObject)
+		:mSceneObject(sceneObject), mSerializedObject(nullptr), mSerializedObjectParentId(0), mSerializedObjectSize(0)
+	{
+
+	}
+
+	CmdDeleteSO::~CmdDeleteSO()
+	{
+		mSceneObject = nullptr;
+		clear();
+	}
+
+	void CmdDeleteSO::clear()
+	{
+		mSerializedObjectSize = 0;
+		mSerializedObjectParentId = 0;
+
+		if (mSerializedObject != nullptr)
+		{
+			bs_free(mSerializedObject);
+			mSerializedObject = nullptr;
+		}
+	}
+
+	void CmdDeleteSO::execute(const HSceneObject& sceneObject)
+	{
+		// Register command and commit it
+		CmdDeleteSO* command = new (bs_alloc<CmdDeleteSO>()) CmdDeleteSO(sceneObject);
+		UndoRedo::instance().registerCommand(command);
+		command->commit();
+	}
+
+	void CmdDeleteSO::commit()
+	{
+		clear();
+
+		if (mSceneObject == nullptr || mSceneObject.isDestroyed())
+			return;
+
+		recordSO(mSceneObject);
+		mSceneObject->destroy();
+	}
+
+	void CmdDeleteSO::revert()
+	{
+		if (mSceneObject == nullptr)
+			return;
+
+		HSceneObject parent;
+		if (mSerializedObjectParentId != 0)
+			parent = GameObjectManager::instance().getObject(mSerializedObjectParentId);
+
+		GameObjectManager::instance().setDeserializationMode(GODM_RestoreExternal | GODM_UseNewIds);
+
+		// Object might still only be queued for destruction, but we need to fully destroy it since we're about to replace
+		// the potentially only reference to the old object
+		if (!mSceneObject.isDestroyed())
+			mSceneObject->destroy(true);
+
+		MemorySerializer serializer;
+		std::shared_ptr<SceneObject> restored = std::static_pointer_cast<SceneObject>(serializer.decode(mSerializedObject, mSerializedObjectSize));
+
+		CmdUtility::restoreIds(restored->getHandle(), mSceneObjectProxy);
+		restored->setParent(parent);
+	}
+
+	void CmdDeleteSO::recordSO(const HSceneObject& sceneObject)
+	{
+		MemorySerializer serializer;
+		mSerializedObject = serializer.encode(mSceneObject.get(), mSerializedObjectSize, &bs_alloc);
+
+		HSceneObject parent = mSceneObject->getParent();
+		if (parent != nullptr)
+			mSerializedObjectParentId = parent->getInstanceId();
+
+		mSceneObjectProxy = CmdUtility::createProxy(mSceneObject);
+	}
+}

BansheeEditor/Source/BsCmdRecordSO.cpp

@@ -64,7 +64,7 @@ namespace BansheeEngine
 		MemorySerializer serializer;
 		std::shared_ptr<SceneObject> restored = std::static_pointer_cast<SceneObject>(serializer.decode(mSerializedObject, mSerializedObjectSize));
 
-		restoreIds(restored->getHandle());
+		CmdUtility::restoreIds(restored->getHandle(), mSceneObjectProxy);
 		restored->setParent(parent);
 	}
 
@@ -77,99 +77,6 @@ namespace BansheeEngine
 		if (parent != nullptr)
 			mSerializedObjectParentId = parent->getInstanceId();
 
-		mSceneObjectProxy = createProxy(mSceneObject);
-	}
-
-	CmdRecordSO::SceneObjProxy CmdRecordSO::createProxy(const HSceneObject& sceneObject)
-	{
-		struct TempData
-		{
-			TempData(SceneObjProxy& proxy, const HSceneObject& so)
-				:proxy(proxy), obj(so)
-			{ }
-
-			SceneObjProxy& proxy;
-			HSceneObject obj;
-		};
-
-		SceneObjProxy rootProxy;
-
-		Stack<TempData> todo;
-		todo.push(TempData(rootProxy, sceneObject));
-
-		while (!todo.empty())
-		{
-			TempData data = todo.top();
-			todo.pop();
-
-			data.proxy.instanceData = data.obj->_getInstanceData();
-
-			const Vector<HComponent>& components = data.obj->getComponents();
-			for (auto& component : components)
-				data.proxy.componentInstanceData.push_back(component->_getInstanceData());
-
-			UINT32 numChildren = data.obj->getNumChildren();
-			data.proxy.children.resize(numChildren);
-			for (UINT32 i = 0; i < numChildren; i++)
-			{
-				todo.push(TempData(data.proxy.children[i], data.obj->getChild(i)));
-			}
-		}
-
-		return rootProxy;
-	}
-
-
-	void CmdRecordSO::restoreIds(const HSceneObject& restored)
-	{
-		// Note: This method relies that all restored GameObject handles pointing to the
-		// same object also have the same shared handle data (Since I only update instance
-		// data on a single handle I know exists, and expect all others will be updated
-		// by that as well).
-
-		struct TempData
-		{
-			TempData(SceneObjProxy& proxy, const HSceneObject& restoredObj)
-			:proxy(proxy), restoredObj(restoredObj)
-			{ }
-
-			SceneObjProxy& proxy;
-			HSceneObject restoredObj;
-		};
-
-		Stack<TempData> todo;
-		todo.push(TempData(mSceneObjectProxy, restored));
-
-		while (!todo.empty())
-		{
-			TempData data = todo.top();
-			todo.pop();
-
-			data.restoredObj->_setInstanceData(data.proxy.instanceData);
-
-			// Find components that are still active and swap the old ones with restored ones,
-			// keep any other as is.
-			const Vector<HComponent>& restoredComponents = data.restoredObj->getComponents();
-
-			UINT32 idx = 0;
-			for (auto& restoredComponent : restoredComponents)
-			{
-				restoredComponent->_setInstanceData(data.proxy.componentInstanceData[idx]);
-
-				GameObjectPtr restoredPtr = std::static_pointer_cast<GameObject>(restoredComponent.getInternalPtr());
-				HComponent restoredComponentCopy = restoredComponent; // To remove const
-				restoredComponentCopy._setHandleData(restoredPtr);
-
-				idx++;
-			}
-			
-			// Find children that are still active and swap the old ones with restored ones,
-			// keep any other as is
-			UINT32 restoredNumChildren = data.restoredObj->getNumChildren();
-			for (UINT32 i = 0; i < restoredNumChildren; i++)
-			{
-				todo.push(TempData(data.proxy.children[i], data.restoredObj->getChild(i)));
-			}
-		}
+		mSceneObjectProxy = CmdUtility::createProxy(mSceneObject);
 	}
 }

BansheeEditor/Source/BsCmdUtility.cpp

@@ -0,0 +1,98 @@
+#include "BsCmdUtility.h"
+#include "BsSceneObject.h"
+
+namespace BansheeEngine
+{
+	CmdUtility::SceneObjProxy CmdUtility::createProxy(const HSceneObject& sceneObject)
+	{
+		struct TempData
+		{
+			TempData(SceneObjProxy& proxy, const HSceneObject& so)
+				:proxy(proxy), obj(so)
+			{ }
+
+			SceneObjProxy& proxy;
+			HSceneObject obj;
+		};
+
+		SceneObjProxy rootProxy;
+
+		Stack<TempData> todo;
+		todo.push(TempData(rootProxy, sceneObject));
+
+		while (!todo.empty())
+		{
+			TempData data = todo.top();
+			todo.pop();
+
+			data.proxy.instanceData = data.obj->_getInstanceData();
+
+			const Vector<HComponent>& components = data.obj->getComponents();
+			for (auto& component : components)
+				data.proxy.componentInstanceData.push_back(component->_getInstanceData());
+
+			UINT32 numChildren = data.obj->getNumChildren();
+			data.proxy.children.resize(numChildren);
+			for (UINT32 i = 0; i < numChildren; i++)
+			{
+				todo.push(TempData(data.proxy.children[i], data.obj->getChild(i)));
+			}
+		}
+
+		return rootProxy;
+	}
+
+
+	void CmdUtility::restoreIds(const HSceneObject& restored, SceneObjProxy& proxy)
+	{
+		// Note: This method relies that all restored GameObject handles pointing to the
+		// same object also have the same shared handle data (Since I only update instance
+		// data on a single handle I know exists, and expect all others will be updated
+		// by that as well).
+
+		struct TempData
+		{
+			TempData(SceneObjProxy& proxy, const HSceneObject& restoredObj)
+			:proxy(proxy), restoredObj(restoredObj)
+			{ }
+
+			SceneObjProxy& proxy;
+			HSceneObject restoredObj;
+		};
+
+		Stack<TempData> todo;
+		todo.push(TempData(proxy, restored));
+
+		while (!todo.empty())
+		{
+			TempData data = todo.top();
+			todo.pop();
+
+			data.restoredObj->_setInstanceData(data.proxy.instanceData);
+
+			// Find components that are still active and swap the old ones with restored ones,
+			// keep any other as is.
+			const Vector<HComponent>& restoredComponents = data.restoredObj->getComponents();
+
+			UINT32 idx = 0;
+			for (auto& restoredComponent : restoredComponents)
+			{
+				restoredComponent->_setInstanceData(data.proxy.componentInstanceData[idx]);
+
+				GameObjectPtr restoredPtr = std::static_pointer_cast<GameObject>(restoredComponent.getInternalPtr());
+				HComponent restoredComponentCopy = restoredComponent; // To remove const
+				restoredComponentCopy._setHandleData(restoredPtr);
+
+				idx++;
+			}
+			
+			// Find children that are still active and swap the old ones with restored ones,
+			// keep any other as is
+			UINT32 restoredNumChildren = data.restoredObj->getNumChildren();
+			for (UINT32 i = 0; i < restoredNumChildren; i++)
+			{
+				todo.push(TempData(data.proxy.children[i], data.restoredObj->getChild(i)));
+			}
+		}
+	}
+}

BansheeEditor/Source/BsEditorTestSuite.cpp

@@ -1,6 +1,7 @@
 #include "BsEditorTestSuite.h"
 #include "BsSceneObject.h"
 #include "BsCmdRecordSO.h"
+#include "BsCmdDeleteSO.h"
 #include "BsUndoRedo.h"
 #include "BsRTTIType.h"
 #include "BsGameObjectRTTI.h"
@@ -421,6 +422,7 @@ namespace BansheeEngine
 	EditorTestSuite::EditorTestSuite()
 	{
 		BS_ADD_TEST(EditorTestSuite::SceneObjectRecord_UndoRedo);
+		BS_ADD_TEST(EditorTestSuite::SceneObjectDelete_UndoRedo);
 		BS_ADD_TEST(EditorTestSuite::BinaryDiff);
 		BS_ADD_TEST(EditorTestSuite::TestPrefabDiff);
 		BS_ADD_TEST(EditorTestSuite::TestFrameAlloc)
@@ -470,6 +472,49 @@ namespace BansheeEngine
 		so0_0->destroy();
 	}
 
+	void EditorTestSuite::SceneObjectDelete_UndoRedo()
+	{
+		HSceneObject so0_0 = SceneObject::create("so0_0");
+		HSceneObject so1_0 = SceneObject::create("so1_0");
+		HSceneObject so1_1 = SceneObject::create("so1_1");
+		HSceneObject so2_0 = SceneObject::create("so2_0");
+
+		so1_0->setParent(so0_0);
+		so1_1->setParent(so0_0);
+		so2_0->setParent(so1_0);
+
+		GameObjectHandle<TestComponentA> cmpA1_1 = so1_1->addComponent<TestComponentA>();
+		GameObjectHandle<TestComponentB> cmpB1_1 = so1_1->addComponent<TestComponentB>();
+
+		HSceneObject soExternal = SceneObject::create("soExternal");
+		GameObjectHandle<TestComponentA> cmpExternal = soExternal->addComponent<TestComponentA>();
+
+		cmpA1_1->ref1 = so2_0;
+		cmpA1_1->ref2 = cmpB1_1;
+		cmpB1_1->ref1 = soExternal;
+		cmpB1_1->val1 = "InitialValue";
+		cmpExternal->ref1 = so1_1;
+		cmpExternal->ref2 = cmpA1_1;
+
+		CmdDeleteSO::execute(so0_0);
+		UndoRedo::instance().undo();
+
+		BS_TEST_ASSERT(!so0_0.isDestroyed());
+		BS_TEST_ASSERT(!so1_0.isDestroyed());
+		BS_TEST_ASSERT(!so1_1.isDestroyed());
+		BS_TEST_ASSERT(!so2_0.isDestroyed());
+		BS_TEST_ASSERT(!cmpA1_1.isDestroyed());
+		BS_TEST_ASSERT(!cmpB1_1.isDestroyed());
+		BS_TEST_ASSERT(!cmpA1_1->ref1.isDestroyed());
+		BS_TEST_ASSERT(!cmpA1_1->ref2.isDestroyed());
+		BS_TEST_ASSERT(!cmpB1_1->ref1.isDestroyed());
+		BS_TEST_ASSERT(!cmpExternal->ref1.isDestroyed());
+		BS_TEST_ASSERT(!cmpExternal->ref2.isDestroyed());
+		BS_TEST_ASSERT(cmpB1_1->val1 == "InitialValue");
+
+		so0_0->destroy();
+	}
+
 	void EditorTestSuite::BinaryDiff()
 	{
 		SPtr<TestObjectA> orgObj = bs_shared_ptr<TestObjectA>();

BansheeUtility/Include/BsEvent.h

@@ -12,14 +12,14 @@ namespace BansheeEngine
 	public:
 		BaseConnectionData()
 			:prev(nullptr), next(nullptr), isActive(true),
-			hasHandleLink(true)
+			handleLinks(0)
 		{
 			
 		}
 
 		virtual ~BaseConnectionData()
 		{
-			assert(!hasHandleLink && !isActive);
+			assert(!handleLinks && !isActive);
 		}
 
 		virtual void deactivate()
@@ -30,7 +30,7 @@ namespace BansheeEngine
 		BaseConnectionData* prev;
 		BaseConnectionData* next;
 		bool isActive;
-		bool hasHandleLink;
+		UINT32 handleLinks;
 	};
 
 	/**
@@ -74,9 +74,10 @@ namespace BansheeEngine
 			BS_LOCK_RECURSIVE_MUTEX(mMutex);
 
 			conn->deactivate();
-			conn->hasHandleLink = false;
+			conn->handleLinks--;
 
-			free(conn);
+			if (conn->handleLinks == 0)
+				free(conn);
 		}
 
 		/**
@@ -92,7 +93,7 @@ namespace BansheeEngine
 				BaseConnectionData* next = conn->next;
 				conn->deactivate();
 
-				if (!conn->hasHandleLink)
+				if (conn->handleLinks == 0)
 					free(conn);
 
 				conn = next;
@@ -109,9 +110,9 @@ namespace BansheeEngine
 		{
 			BS_LOCK_RECURSIVE_MUTEX(mMutex);
 
-			conn->hasHandleLink = false;
+			conn->handleLinks--;
 
-			if (!conn->isActive)
+			if (conn->handleLinks == 0 && !conn->isActive)
 				free(conn);
 		}
 
@@ -161,7 +162,9 @@ namespace BansheeEngine
 
 		explicit HEvent(const SPtr<EventInternalData>& eventData, BaseConnectionData* connection)
 			:mConnection(connection), mEventData(eventData)
-		{ }
+		{
+			connection->handleLinks++;
+		}
 
 		~HEvent()
 		{
@@ -197,6 +200,17 @@ namespace BansheeEngine
 			return (mConnection != nullptr ? &Bool_struct::_Member : 0);
 		}
 
+		HEvent& operator=(const HEvent& rhs)
+		{
+			mConnection = rhs.mConnection;
+			mEventData = rhs.mEventData;
+
+			if (mConnection != nullptr)
+				mConnection->handleLinks++;
+
+			return *this;
+		}
+
 	private:
 		BaseConnectionData* mConnection;
 		SPtr<EventInternalData> mEventData;
@@ -254,7 +268,7 @@ namespace BansheeEngine
 					connData->next->prev = nullptr;
 
 				connData->isActive = true;
-				connData->hasHandleLink = true;
+				connData->handleLinks = true;
 			}
 
 			if (connData == nullptr)

TODO.txt

@@ -54,6 +54,13 @@ Code quality improvements:
 ----------------------------------------------------------------------
 Polish
 
+SceneTreeView
+ - Add cut/copy/duplicate/paste functionality (+ appropriate context menu)
+ - Does delete, rename, drag and drop work properly?
+  - Delete + rename should be undoable
+ - ELement cant be deselected by clicking on empty space
+ - Clicking on an already selected element should start rename
+
 Ribek use:
  - Camera, Renderable, Material, Texture inspector
   - Material/Shader has no color type so I cannot know when to display normal vector and when color in inspector
@@ -88,13 +95,8 @@ First screenshot:
 Other polish:
  - C# inspectors for Point/Spot/Directional light
  - C# interface for Font
- - SceneTreeView
-  - Hook up ping effect so it triggers when I select a resource or sceneobject
-   - Add Selection::ping method to both C++ and C# and an Event that triggers when its called
-  - See if it needs other enhancements (rename, delete all work properly? etc.)
-  - Add copy/cut/paste/duplicate (with context menu)
-  - Cannot deselect element by clicking on empty space
-  - Clicking on an already selected element should start rename
+ - Hook up ping effect so that when resource/gameobject/texture field is clicked it triggers a ping in
+   ProjectWindow or HierarchyWindow. (Likely add Selection.Ping method and Selection.OnPing callback)
  - Handle seems to lag behind the selected mesh
  - ProjectLibrary seems to import some files on every start-up
  - Crash on shutdown in mono_gchandle_free
@@ -110,6 +112,8 @@ Other polish:
    - Consider saving this information with the serialized object
  - Make sure to persist EditorSettings
  - Import option inspectors for Texture, Mesh, Font
+ - MenuBar - will likely need a way to mark elements as disabled when not appropriate (e.g. no "frame selected unless scene is focused")
+   - Likely use a user-provided callback to trigger when populating the menus
 
 Stage 2 polish:
  - Inject an icon into an .exe (Win32 specific)