Resource handle refactor (part 4)

 - Resources properly manages new handle creation by having same resources always share handle data
 - Unloading of used resources is no longer permitted in managed code

BansheeCore/Include/BsResourceHandle.h

@@ -245,6 +245,17 @@ namespace BansheeEngine
 			return std::static_pointer_cast<T>(mData->mPtr); 
 		}
 
+		/**
+		 * @brief	Converts a handle into a weak handle.
+		 */
+		TResourceHandle<T, true> getWeak() const
+		{
+			TResourceHandle<T, true> handle;
+			handle.setHandleData(getHandleData());
+
+			return handle;
+		}
+
 	protected:
 		friend Resources;
 		template<class _T, bool _Weak>
@@ -302,6 +313,17 @@ namespace BansheeEngine
 			addRef();
 		}
 
+		/**
+		 * @brief	Converts a weak handle into a normal handle.
+		 */
+		TResourceHandle<T, false> lock() const
+		{
+			TResourceHandle<Resource, false> handle;
+			handle.setHandleData(getHandleData());
+
+			return handle;
+		}
+
 		using TResourceHandleBase::setHandleData;
 	};
 

BansheeCore/Include/BsResourceHandleRTTI.h

@@ -30,7 +30,7 @@ namespace BansheeEngine
 
 			if(resourceHandle->mData && resourceHandle->mData->mUUID != "")
 			{
-				HResource loadedResource = gResources()._createResourceHandle(resourceHandle->mData->mUUID);
+				HResource loadedResource = gResources()._getResourceHandle(resourceHandle->mData->mUUID);
 
 				resourceHandle->releaseRef();
 				resourceHandle->mData = loadedResource.mData;
@@ -83,7 +83,7 @@ namespace BansheeEngine
 
 			if (resourceHandle->mData && resourceHandle->mData->mUUID != "")
 			{
-				HResource loadedResource = gResources()._createResourceHandle(resourceHandle->mData->mUUID);
+				HResource loadedResource = gResources()._getResourceHandle(resourceHandle->mData->mUUID);
 				resourceHandle->mData = loadedResource.mData;
 			}
 		}

BansheeCore/Include/BsResources.h

@@ -39,7 +39,7 @@ namespace BansheeEngine
 		HResource load(const Path& filePath, bool loadDependencies = true);
 
 		/**
-		 * @copydoc	load
+		 * @copydoc	load(const Path&, bool)
 		 */
 		template <class T>
 		ResourceHandle<T> load(const Path& filePath, bool loadDependencies = true)
@@ -47,6 +47,20 @@ namespace BansheeEngine
 			return static_resource_cast<T>(load(filePath, loadDependencies));
 		}
 
+		/**
+		 * @brief	Loads the resource for the provided weak resource handle, or returns a loaded resource if already loaded.
+		 */
+		HResource load(const WeakResourceHandle<Resource>& handle, bool loadDependencies = true);
+
+		/**
+		 * @copydoc	load(const WeakResourceHandle<T>&, bool)
+		 */
+		template <class T>
+		ResourceHandle<T> load(const WeakResourceHandle<T>& handle, bool loadDependencies = true)
+		{
+			return static_resource_cast<T>(load(handle, loadDependencies));
+		}
+
 		/**
 		 * @brief	Loads the resource asynchronously. Initially returned resource handle will be invalid
 		 *			until resource loading is done.
@@ -77,9 +91,10 @@ namespace BansheeEngine
 		HResource loadFromUUID(const String& uuid, bool async = false, bool loadDependencies = true);
 
 		/**
-		 * @brief	Unloads the resource that is referenced by the handle. Resource is unloaded regardless if it is 
-		 *			referenced or not. Any dependencies held by the resource will also be unloaded, but only if the
-		 *			resource is holding the last reference to them.
+		 * @brief	Unloads the resource that is referenced by the handle. Any dependencies held by the resource will also 
+		 * 			be unloaded, but only if the resource is holding the last reference to them. This method will unload a 
+		 * 			resource even if it is being referenced and the caller must ensure whatever is referencing it can
+		 * 			handle a null resource, or ensure there are no references.
 		 *
 		 * @param	resourceHandle	Handle of the resource to unload.
 		 * 							
@@ -88,6 +103,11 @@ namespace BansheeEngine
 		 */
 		void unload(HResource resource);
 
+		/**
+		 * @copydoc unload(HResource&)
+		 */
+		void unload(WeakResourceHandle<Resource> resource);
+
 		/**
 		 * @brief	Finds all resources that aren't being referenced anywhere and unloads them.
 		 */
@@ -108,7 +128,7 @@ namespace BansheeEngine
 		 *			If saving a core thread resource this is a potentially very slow operation as we must wait on the 
 		 *			core thread and the GPU in order to read the resource.
 		 */
-		void save(HResource resource, const Path& filePath, bool overwrite);
+		void save(const HResource& resource, const Path& filePath, bool overwrite);
 
 		/**
 		 * @brief	Saves an existing resource to its previous location.
@@ -122,7 +142,7 @@ namespace BansheeEngine
 		 *			If saving a core thread resource this is a potentially very slow operation as we must wait on the
 		 *			core thread and the GPU in order to read the resource.
 		 */
-		void save(HResource resource);
+		void save(const HResource& resource);
 
 		/**
 		 * @brief	Updates an existing resource handle with a new resource. Caller must ensure that
@@ -158,10 +178,9 @@ namespace BansheeEngine
 		HResource _createResourceHandle(const ResourcePtr& obj);
 
 		/**
-		 * @brief	Returns an existing handle of a resource that has already been loaded,
-		 *			or is currently being loaded, or creates a new handle for the specified UUID.
+		 * @brief	Returns an existing handle for the specified UUID if one exists, or creates a new one.
 		 */
-		HResource _createResourceHandle(const String& uuid);
+		HResource _getResourceHandle(const String& uuid);
 
 		/**
 		 * @brief	Allows you to set a resource manifest containing UUID <-> file path mapping that is
@@ -212,7 +231,8 @@ namespace BansheeEngine
 		Event<void(const HResource&)> onResourceLoaded;
 
 		/**
-		 * @brief	Called when the resource has been destroyed.
+		 * @brief	Called when the resource has been destroyed. Subscriber should not hold on to the provided resource
+		 * 			reference as it will be destroyed.
 		 *
 		 * @note	It is undefined from which thread this will get called from.
 		 */
@@ -255,6 +275,7 @@ namespace BansheeEngine
 		BS_MUTEX(mInProgressResourcesMutex);
 		BS_MUTEX(mLoadedResourceMutex);
 
+		UnorderedMap<String, WeakResourceHandle<Resource>> mHandles;
 		UnorderedMap<String, HResource> mLoadedResources;
 		UnorderedMap<String, ResourceLoadData*> mInProgressResources; // Resources that are being asynchronously loaded
 		UnorderedMap<String, Vector<ResourceLoadData*>> mDependantLoads;

BansheeCore/Source/BsResources.cpp

@@ -25,12 +25,7 @@ namespace BansheeEngine
 		UnorderedMap<String, HResource> loadedResourcesCopy = mLoadedResources;
 
 		for (auto& loadedResourcePair : loadedResourcesCopy)
-		{
 			unload(loadedResourcePair.second);
-
-			// Invalidate the handle
-			loadedResourcePair.second.setHandleData(nullptr, "");
-		}
 	}
 
 	HResource Resources::load(const Path& filePath, bool loadDependencies)
@@ -44,6 +39,15 @@ namespace BansheeEngine
 		return loadInternal(uuid, filePath, true, loadDependencies);
 	}
 
+	HResource Resources::load(const WeakResourceHandle<Resource>& handle, bool loadDependencies)
+	{
+		if (handle.mData == nullptr)
+			return HResource();
+
+		String uuid = handle.getUUID();
+		return loadFromUUID(uuid, false, loadDependencies);
+	}
+
 	HResource Resources::loadAsync(const Path& filePath, bool loadDependencies)
 	{
 		String uuid;
@@ -110,7 +114,18 @@ namespace BansheeEngine
 		// Not loaded and not in progress, start loading of new resource
 		// (or if already loaded or in progress, load any dependencies)
 		if (!alreadyLoading)
-			outputResource = HResource(UUID);
+		{
+			// Check if the handle already exists
+			BS_LOCK_MUTEX(mLoadedResourceMutex);
+			auto iterFind = mHandles.find(UUID);
+			if (iterFind != mHandles.end())
+				outputResource = iterFind->second.lock();
+			else
+			{
+				outputResource = HResource(UUID);
+				mHandles[UUID] = outputResource.getWeak();
+			}			
+		}
 
 		// We have nowhere to load from, warn and complete load if a file path was provided,
 		// otherwise pass through as we might just want to load from memory. 
@@ -299,11 +314,7 @@ namespace BansheeEngine
 			return;
 
 		if (!resource.isLoaded()) // If it's still loading wait until that finishes
-		{
-			LOGWRN("Performance warning: Unloading a resource that is still in process of loading "
-				   "causes a stall until resource finishes loading.");
 			resource.blockUntilLoaded();
-		}
 
 		Vector<ResourceDependency> dependencies = Utility::findResourceDependencies(*resource.get());
 
@@ -312,12 +323,13 @@ namespace BansheeEngine
 
 		resource->destroy();
 
+		const String& uuid = resource.getUUID();
 		{
 			BS_LOCK_MUTEX(mLoadedResourceMutex);
-			mLoadedResources.erase(resource.getUUID());
+			mLoadedResources.erase(uuid);
 		}
 
-		resource.setHandleData(nullptr, "");
+		resource.setHandleData(nullptr, uuid);
 
 		for (auto& dependency : dependencies)
 		{
@@ -335,6 +347,12 @@ namespace BansheeEngine
 		}
 	}
 
+	void Resources::unload(WeakResourceHandle<Resource> resource)
+	{
+		HResource handle = resource.lock();
+		unload(handle);
+	}
+
 	void Resources::unloadAllUnused()
 	{
 		Vector<HResource> resourcesToUnload;
@@ -357,7 +375,7 @@ namespace BansheeEngine
 		}
 	}
 
-	void Resources::save(HResource resource, const Path& filePath, bool overwrite)
+	void Resources::save(const HResource& resource, const Path& filePath, bool overwrite)
 	{
 		if (resource == nullptr)
 			return;
@@ -389,7 +407,7 @@ namespace BansheeEngine
 		fs.encode(resource.get());
 	}
 
-	void Resources::save(HResource resource)
+	void Resources::save(const HResource& resource)
 	{
 		if (resource == nullptr)
 			return;
@@ -484,12 +502,13 @@ namespace BansheeEngine
 			BS_LOCK_MUTEX(mLoadedResourceMutex);
 
 			mLoadedResources[uuid] = newHandle;
+			mHandles[uuid] = newHandle.getWeak();
 		}
 	
 		return newHandle;
 	}
 
-	HResource Resources::_createResourceHandle(const String& uuid)
+	HResource Resources::_getResourceHandle(const String& uuid)
 	{
 		{
 			BS_LOCK_MUTEX(mInProgressResourcesMutex);
@@ -506,10 +525,20 @@ namespace BansheeEngine
 				{
 					return iterFind->second;
 				}
+
+				auto iterFind3 = mHandles.find(uuid);
+				if (iterFind3 != mHandles.end()) // Not loaded, but handle does exist
+				{
+					return iterFind3->second.lock();
+				}
+
+				// Create new handle
+				HResource handle(uuid);
+				mHandles[uuid] = handle.getWeak();
+
+				return handle;
 			}
 		}
-
-		return HResource(uuid);
 	}
 
 	bool Resources::getFilePathFromUUID(const String& uuid, Path& filePath) const
@@ -574,6 +603,7 @@ namespace BansheeEngine
 					BS_LOCK_MUTEX(mLoadedResourceMutex);
 
 					mLoadedResources[uuid] = resource;
+					mHandles[uuid] = resource.getWeak();
 					resource.setHandleData(myLoadData->loadedData, uuid);
 				}
 

BansheeEditor/Source/BsProjectLibrary.cpp

@@ -437,7 +437,7 @@ namespace BansheeEngine
 
 				if (!isNativeResource)
 				{
-					importedResource = gResources()._createResourceHandle(resource->meta->getUUID());
+					importedResource = gResources()._getResourceHandle(resource->meta->getUUID());
 
 					if (!importedResource.isLoaded(false))
 						unloadWhenDone = true;
@@ -459,10 +459,12 @@ namespace BansheeEngine
 				internalResourcesPath.setFilename(toWString(importedResource.getUUID()) + L".asset");
 				gResources().save(importedResource, internalResourcesPath, true);
 
+				String uuid = importedResource.getUUID();
+
 				if (unloadWhenDone)
 					gResources().unload(importedResource);
 
-				mResourceManifest->registerResource(importedResource.getUUID(), internalResourcesPath);
+				mResourceManifest->registerResource(uuid, internalResourcesPath);
 			}
 
 			resource->lastUpdateTime = std::time(nullptr);

MBansheeEngine/Resources.cs

@@ -22,16 +22,6 @@ namespace BansheeEngine
             return (T)Internal_Load(path);
         }
 
-        /// <summary>
-        /// Unloads a resource, freeing its memory.
-        /// </summary>
-        /// <param name="resource">Resource to unload.</param>
-        public static void Unload(Resource resource)
-        {
-            if (resource != null)
-                Internal_Unload(resource.GetCachedPtr());
-        }
-
         /// <summary>
         /// Unloads all resources that are no longer referenced. Usually the system keeps resources in memory even if
         /// they are no longer referenced to avoid constant loading/unloading if resource is often passed around.
@@ -44,9 +34,6 @@ namespace BansheeEngine
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern Resource Internal_Load(string path);
 
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern void Internal_Unload(IntPtr resourcePtr);
-
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern void Internal_UnloadUnused();
     }

SBansheeEngine/Include/BsManagedResource.h

@@ -77,7 +77,7 @@ namespace BansheeEngine
 
 		MonoObject* mManagedInstance;
 		uint32_t mManagedHandle;
-		HManagedResource mMyHandle;
+		WeakResourceHandle<ManagedResource> mMyHandle;
 
 		/************************************************************************/
 		/* 								RTTI		                     		*/

SBansheeEngine/Include/BsManagedResourceManager.h

@@ -22,14 +22,14 @@ namespace BansheeEngine
 		/**
 		 * @brief	Register a newly created managed resource.
 		 */
-		void registerManagedResource(const HManagedResource& resource);
+		void registerManagedResource(const WeakResourceHandle<ManagedResource>& resource);
 
 		/**
 		 * @brief	Unregister a managed resource that's about to be destroyed.
 		 */
-		void unregisterManagedResource(const HManagedResource& resource);
+		void unregisterManagedResource(const WeakResourceHandle<ManagedResource>& resource);
 
 	private:
-		UnorderedMap<String, HManagedResource> mResources;
+		UnorderedMap<String, WeakResourceHandle<ManagedResource>> mResources;
 	};
 }

SBansheeEngine/Include/BsScriptResources.h

@@ -20,7 +20,6 @@ namespace BansheeEngine
 		/* 								CLR HOOKS						   		*/
 		/************************************************************************/
 		static MonoObject* internal_Load(MonoString* path);
-		static void internal_Unload(ScriptResourceBase* resourcePtr);
 		static void internal_UnloadUnused();
 	};
 }

SBansheeEngine/Source/BsManagedResource.cpp

@@ -118,7 +118,7 @@ namespace BansheeEngine
 	{
 		mManagedInstance = object;
 		mManagedHandle = mono_gchandle_new(mManagedInstance, false);
-		mMyHandle = myHandle;
+		mMyHandle = myHandle.getWeak();
 
 		ScriptManagedResource* scriptInstance;
 		ScriptResourceManager::instance().createScriptResource(object, myHandle, &scriptInstance);

SBansheeEngine/Source/BsManagedResourceManager.cpp

@@ -15,23 +15,22 @@ namespace BansheeEngine
 
 	void ManagedResourceManager::clear()
 	{
-		UnorderedMap<String, HManagedResource> resourceCopy = mResources;
+		UnorderedMap<String, WeakResourceHandle<ManagedResource>> resourceCopy = mResources;
 		for (auto& resourcePair : resourceCopy)
 		{
-			HManagedResource resource = resourcePair.second;
-			if (resource != nullptr && resource.isLoaded())
-				gResources().unload(resource);
+			WeakResourceHandle<ManagedResource> resource = resourcePair.second;
+			gResources().unload((WeakResourceHandle<Resource>&)resource);
 		}
 
 		mResources.clear();
 	}
 
-	void ManagedResourceManager::registerManagedResource(const HManagedResource& resource)
+	void ManagedResourceManager::registerManagedResource(const WeakResourceHandle<ManagedResource>& resource)
 	{
 		mResources.insert(std::make_pair(resource.getUUID(), resource));
 	}
 
-	void ManagedResourceManager::unregisterManagedResource(const HManagedResource& resource)
+	void ManagedResourceManager::unregisterManagedResource(const WeakResourceHandle<ManagedResource>& resource)
 	{
 		mResources.erase(resource.getUUID());
 	}

SBansheeEngine/Source/BsScriptResources.cpp

@@ -17,7 +17,6 @@ namespace BansheeEngine
 	void ScriptResources::initRuntimeData()
 	{
 		metaData.scriptClass->addInternalCall("Internal_Load", &ScriptResources::internal_Load);
-		metaData.scriptClass->addInternalCall("Internal_Unload", &ScriptResources::internal_Unload);
 		metaData.scriptClass->addInternalCall("Internal_UnloadUnused", &ScriptResources::internal_UnloadUnused);
 	}
 
@@ -35,11 +34,6 @@ namespace BansheeEngine
 		return scriptResource->getManagedInstance();
 	}
 
-	void ScriptResources::internal_Unload(ScriptResourceBase* resourcePtr)
-	{
-		gResources().unload(resourcePtr->getGenericHandle());
-	}
-
 	void ScriptResources::internal_UnloadUnused()
 	{
 		gResources().unloadAllUnused();

TODO.txt

@@ -128,6 +128,7 @@ VisualStudio integration
   - VSExpress doesn't support EnvDTE so the only option is to open it using a shell command which doesn't seem to offer precise parameters
   - Community edition should work similarily to Pro, but might have a different executable and/or registry paths
  - Make sure that 3rd party assemblies can be imported in the project, and that they are properly referenced in VS project generation and compilation
+ - Support Visual Studio Code on other platforms
 
 ----------------------------------------------------------------------
 Library window