Assembly refresh working (all obvious issues resolved)

BansheeCore/Include/BsGameObject.h

@@ -45,6 +45,19 @@ namespace BansheeEngine
 		 */
 		void setName(const String& name) { mName = name; }
 
+		/**
+		 * @brief	Marks the object as destroyed. Generally this means the object
+		 *			has been queued for destruction but it hasn't occurred yet.
+		 *
+		 * @note	Internal method.
+		 */
+		void _setIsDestroyed() { mIsDestroyed = true; }
+
+		/**
+		 * @brief	Checks if the object has been destroyed.
+		 */
+		bool _getIsDestroyed() const { return mIsDestroyed; }
+
 		/**
 		 * @brief	Replaces the instance data with another objects instance data.
 		 *			This object will basically become the original owner of the provided
@@ -78,6 +91,7 @@ namespace BansheeEngine
 
 	private:
 		GameObjectInstanceDataPtr mInstanceData;
+		bool mIsDestroyed;
 
 		/************************************************************************/
 		/* 								RTTI		                     		*/

BansheeCore/Include/BsGameObjectHandle.h

@@ -44,8 +44,13 @@ 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. 
+		 *						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.
 		 */
-		bool isDestroyed() const { return mData->mPtr == nullptr || mData->mPtr->object == nullptr; }
+		bool isDestroyed(bool checkQueued = false) const 
+		{ return mData->mPtr == nullptr || mData->mPtr->object == nullptr || (checkQueued && mData->mPtr->object->_getIsDestroyed()); }
 
 		/**
 		 * @brief	Returns the instance ID of the object the handle is referencing.

BansheeCore/Source/BsGameObject.cpp

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

BansheeCore/Source/BsSceneObject.cpp

@@ -69,6 +69,7 @@ namespace BansheeEngine
 
 		for(auto iter = mComponents.begin(); iter != mComponents.end(); ++iter)
 		{
+			(*iter)->_setIsDestroyed();
 			(*iter)->onDestroyed();
 
 			if (immediate)
@@ -467,6 +468,7 @@ namespace BansheeEngine
 
 		if(iter != mComponents.end())
 		{
+			(*iter)->_setIsDestroyed();
 			(*iter)->onDestroyed();
 			
 			mComponents.erase(iter);

MBansheeEditor/Scene/SceneWindow.cs

@@ -145,7 +145,7 @@ namespace BansheeEditor
 		    }
 
 		    // TODO - Consider only doing the resize once user stops resizing the widget in order to reduce constant
-		    // render target destroy/create cycle for every little pixel.
+		    // render target destroy/create cycle for every single pixel.
 
 		    camera.aspectRatio = width / (float)height;
 	    }

SBansheeEngine/Include/BsManagedComponent.h

@@ -38,6 +38,7 @@ namespace BansheeEngine
 		String mNamespace;
 		String mTypeName;
 		String mFullTypeName;
+		bool mRequiresReset;
 
 		// We store data of a missing component type in hope it will be restored later
 		bool mMissingType;

SBansheeEngine/Source/BsManagedComponent.cpp

@@ -15,12 +15,13 @@ namespace BansheeEngine
 {
 	ManagedComponent::ManagedComponent()
 		:mManagedInstance(nullptr), mUpdateThunk(nullptr), mOnDestroyThunk(nullptr), mOnInitializedThunk(nullptr), 
-		mOnResetThunk(nullptr), mMissingType(false)
+		mOnResetThunk(nullptr), mMissingType(false), mRequiresReset(true)
 	{ }
 
 	ManagedComponent::ManagedComponent(const HSceneObject& parent, MonoReflectionType* runtimeType)
 		: Component(parent), mManagedInstance(nullptr), mRuntimeType(runtimeType), mUpdateThunk(nullptr), 
-		mOnDestroyThunk(nullptr), mOnInitializedThunk(nullptr), mOnResetThunk(nullptr), mMissingType(false)
+		mOnDestroyThunk(nullptr), mOnInitializedThunk(nullptr), mOnResetThunk(nullptr), mMissingType(false),
+		mRequiresReset(true)
 	{
 		MonoType* monoType = mono_reflection_type_get_type(mRuntimeType);
 		::MonoClass* monoClass = mono_type_get_class(monoType);
@@ -41,11 +42,7 @@ namespace BansheeEngine
 
 	ManagedComponent::~ManagedComponent()
 	{
-		if (mManagedInstance != nullptr)
-		{
-			mManagedInstance = nullptr;
-			mono_gchandle_free(mManagedHandle);
-		}
+
 	}
 
 	ComponentBackupData ManagedComponent::backup(bool clearExisting)
@@ -152,6 +149,7 @@ namespace BansheeEngine
 		}
 
 		mMissingType = missingType;
+		mRequiresReset = true;
 	}
 
 	void ManagedComponent::initialize(MonoObject* object)
@@ -207,7 +205,7 @@ namespace BansheeEngine
 
 	void ManagedComponent::triggerOnReset()
 	{
-		if (mOnResetThunk != nullptr && mManagedInstance != nullptr)
+		if (mRequiresReset && mOnResetThunk != nullptr && mManagedInstance != nullptr)
 		{
 			MonoException* exception = nullptr;
 
@@ -217,6 +215,8 @@ namespace BansheeEngine
 
 			MonoUtil::throwIfException(exception);
 		}
+
+		mRequiresReset = false;
 	}
 
 	void ManagedComponent::onInitialized()
@@ -269,6 +269,12 @@ namespace BansheeEngine
 
 			MonoUtil::throwIfException(exception);
 		}
+
+		if (mManagedInstance != nullptr)
+		{
+			mManagedInstance = nullptr;
+			mono_gchandle_free(mManagedHandle);
+		}
 	}
 
 	RTTITypeBase* ManagedComponent::getRTTIStatic()

SBansheeEngine/Source/BsScriptComponent.cpp

@@ -180,7 +180,11 @@ namespace BansheeEngine
 		ScriptGameObjectBase::beginRefresh();
 
 		ScriptObjectBackup backupData;
-		backupData.data = mManagedComponent->backup(true);
+
+		// It's possible that managed component is destroyed but a reference to it
+		// is still kept. Don't backup such components.
+		if (!mManagedComponent.isDestroyed(true))
+			backupData.data = mManagedComponent->backup(true);
 
 		return backupData;
 	}
@@ -197,7 +201,10 @@ namespace BansheeEngine
 	{
 		mManagedInstance = nullptr;
 
-		if (!mRefreshInProgress)
+		// It's possible that managed component is destroyed but a reference to it
+		// is still kept during assembly refresh. Such components shouldn't be restored
+		// so we delete them.
+		if (!mRefreshInProgress || mManagedComponent.isDestroyed(true))
 			ScriptGameObjectManager::instance().destroyScriptGameObject(this);
 	}
 

TODO.txt

@@ -5,18 +5,19 @@
 When serializing Camera I cannot save the reference to RenderTexture. Make it a Resource?
 Possibly set up automatic refresh in debug mode after initialization? As an ad-hoc unit test
 
-I don't think GUI elements are properly cleaned up after assembly refresh
- - GUIPanelContainer was holding an invalid reference to mGUIPanel
- - GUI elements aren't destroyed when their managed reference is lost. Instead they remain owned by their GUIWidget.
-   - How to get around this when performing assembly refresh?
-   - Destroying them when reference is lost would be too clumsy as I would need to track when they're referenced by GUILayout or GUIArea and similar.
-   - I probably need to ensure that any GUIArea owned by EditorWindow gets destroyed properly.
-     - By definition this should be any area owned by a GUIPanel
-   - Later when I might have GUI elements in non-editor windows I can handle those on a case by case basis
-
 Scene grid disappears and scene view doesn't work after refresh
  - This might be related to GUI issue above, as the old GUI still remains so it might be rendering above the new stuff
 
+Assembly refresh issues:
+When managed component is destroyed its ScriptComponent will only get destroyed when last reference to the managed component is lost. With assembly refresh this happens during refresh, which avoids destroying the ScriptComponent.
+ - I need a way to mark GameObject as destroyed so I can then avoid backing it up and restoring its instance.
+
+New Camera gets its OnReset called twice
+
+Make sure to remove all debug logs
+
+After refresh BansheeRenderer has two extra cameras (4 new ones total, 2 old ones got deleted). 2 of the new ones have empty targets.
+
 <<<<Multi-resource saving>>>>:
  - Modify Font so it doesn't contain a texture, but instead keeps a handle to it
  - Register it in its meta file