Răsfoiți Sursa

Bugfix: Multiple fixes regarding project library async import
- Save & Create calls now perform their operations synchronously, as would be expected by the caller
- Updating meta-data will now properly wait for any queued async import operations, otherwise the meta-data changes will be overwritten once the async operations complete

BearishSun 7 ani în urmă
părinte
comite
a552e20afb

+ 183 - 128
Source/EditorCore/Library/BsProjectLibrary.cpp

@@ -317,12 +317,12 @@ namespace bs
 	}
 	}
 
 
 	ProjectLibrary::FileEntry* ProjectLibrary::addResourceInternal(DirectoryEntry* parent, const Path& filePath, 
 	ProjectLibrary::FileEntry* ProjectLibrary::addResourceInternal(DirectoryEntry* parent, const Path& filePath, 
-		const SPtr<ImportOptions>& importOptions, bool forceReimport)
+		const SPtr<ImportOptions>& importOptions, bool forceReimport, bool synchronous)
 	{
 	{
 		FileEntry* newResource = bs_new<FileEntry>(filePath, filePath.getTail(), parent);
 		FileEntry* newResource = bs_new<FileEntry>(filePath, filePath.getTail(), parent);
 		parent->mChildren.push_back(newResource);
 		parent->mChildren.push_back(newResource);
 
 
-		reimportResourceInternal(newResource, importOptions, forceReimport);
+		reimportResourceInternal(newResource, importOptions, forceReimport, false, synchronous);
 		onEntryAdded(newResource->path);
 		onEntryAdded(newResource->path);
 
 
 		return newResource;
 		return newResource;
@@ -410,7 +410,7 @@ namespace bs
 	}
 	}
 
 
 	bool ProjectLibrary::reimportResourceInternal(FileEntry* fileEntry, const SPtr<ImportOptions>& importOptions,
 	bool ProjectLibrary::reimportResourceInternal(FileEntry* fileEntry, const SPtr<ImportOptions>& importOptions,
-		bool forceReimport, bool pruneResourceMetas)
+		bool forceReimport, bool pruneResourceMetas, bool synchronous)
 	{
 	{
 		Path metaPath = fileEntry->path;
 		Path metaPath = fileEntry->path;
 		metaPath.setFilename(metaPath.getFilename() + ".meta");
 		metaPath.setFilename(metaPath.getFilename() + ".meta");
@@ -481,6 +481,14 @@ namespace bs
 				// running it shouldn't be canceled as dependencies still need to wait on it (since cancelling a
 				// running it shouldn't be canceled as dependencies still need to wait on it (since cancelling a
 				// running task doesn't actually stop it). Yet there is currently no good wait to check if task
 				// running task doesn't actually stop it). Yet there is currently no good wait to check if task
 				// is currently running. 
 				// is currently running. 
+
+				// Dependency being imported async but we want the current resource right away. Wait until dependency is
+				// done otherwise when dependency finishes it will overwrite whatever we write now.
+				if(synchronous && dependency)
+				{
+					if (finishQueuedImport(fileEntry, *iterFind->second, true))
+						mQueuedImports.erase(iterFind);
+				}
 			}
 			}
 				
 				
 			// Needs to be pass a weak pointer to worker methods since internally it holds a reference to the task itself, 
 			// Needs to be pass a weak pointer to worker methods since internally it holds a reference to the task itself, 
@@ -554,8 +562,13 @@ namespace bs
 					}
 					}
 				};
 				};
 
 
-				queuedImport->importTask = Task::create("ProjectLibraryImport", importAsync, TaskPriority::Normal,
-					dependency);
+				if(!synchronous)
+				{
+					queuedImport->importTask = Task::create("ProjectLibraryImport", importAsync, TaskPriority::Normal,
+						dependency);
+				}
+				else
+					importAsync();
 			}
 			}
 			else
 			else
 			{
 			{
@@ -597,165 +610,175 @@ namespace bs
 					}
 					}
 				};
 				};
 
 
-				queuedImport->importTask = Task::create("ProjectLibraryImport", importAsync, TaskPriority::Normal,
-					dependency);
+				if(!synchronous)
+				{
+					queuedImport->importTask = Task::create("ProjectLibraryImport", importAsync, TaskPriority::Normal,
+						dependency);
+				}
+				else
+					importAsync();
 			}
 			}
 
 
+			if(!synchronous)
+			{
+				TaskScheduler::instance().addTask(queuedImport->importTask);
+				mQueuedImports[fileEntry] = queuedImport;
+			}
 
 
-			TaskScheduler::instance().addTask(queuedImport->importTask);
-
-			mQueuedImports[fileEntry] = queuedImport;
 			fileEntry->lastUpdateTime = std::time(nullptr);
 			fileEntry->lastUpdateTime = std::time(nullptr);
 
 
+			if(synchronous)
+				finishQueuedImport(fileEntry, *queuedImport, true);
+
 			return true;
 			return true;
 		}
 		}
 
 
 		return false;
 		return false;
 	}
 	}
 
 
-	void ProjectLibrary::_finishQueuedImports(bool wait)
+	bool ProjectLibrary::finishQueuedImport(FileEntry* fileEntry, const QueuedImport& import, bool wait)
 	{
 	{
-		for(auto iter = mQueuedImports.begin(); iter != mQueuedImports.end();)
+		if (import.importTask != nullptr && !import.importTask->isComplete())
 		{
 		{
-			SPtr<QueuedImport> queuedImport = iter->second;
-			if (!queuedImport->importTask->isComplete())
-			{
-				if (wait)
-					queuedImport->importTask->wait();
-				else
-				{
-					++iter;
-					continue;
-				}
-			}
+			if (wait)
+				import.importTask->wait();
+			else
+				return false;
+		}
 
 
-			// The task is done, we can remove it
-			FileEntry* fileEntry = iter->first;
-			iter = mQueuedImports.erase(iter);
+		// We wait on canceled task to finish and then just discard the results because any dependant tasks need to be
+		// aware this tasks exists, so we can't just remove it straight away.
+		if (import.canceled)
+			return true;
 
 
-			// We wait on canceled task to finish and then just discard the results because any dependant tasks need to be
-			// aware this tasks exists, so we can't just remove it straight away.
-			if(queuedImport->canceled)
-			{
-				// Just move on to the next import...
-				continue;
-			}
+		Path metaPath = fileEntry->path;
+		metaPath.setFilename(metaPath.getFilename() + ".meta");
 
 
-			Path metaPath = fileEntry->path;
-			metaPath.setFilename(metaPath.getFilename() + ".meta");
+		Vector<SPtr<ProjectResourceMeta>> existingMetas;
+		if (fileEntry->meta == nullptr) // Build a brand new meta-file
+			fileEntry->meta = ProjectFileMeta::create(import.importOptions);
+		else // Existing meta-file, which needs to be updated
+		{
+			// Remove existing dependencies (they will be re-added later)
+			removeDependencies(fileEntry);
 
 
-			Vector<SPtr<ProjectResourceMeta>> existingMetas;
-			if(fileEntry->meta == nullptr) // Build a brand new meta-file
-				fileEntry->meta = ProjectFileMeta::create(queuedImport->importOptions);
-			else // Existing meta-file, which needs to be updated
-			{
-				// Remove existing dependencies (they will be re-added later)
-				removeDependencies(fileEntry);
+			existingMetas = fileEntry->meta->getAllResourceMetaData();
 
 
-				existingMetas = fileEntry->meta->getAllResourceMetaData();
+			fileEntry->meta->clearResourceMetaData();
+			fileEntry->meta->mImportOptions = import.importOptions;
+		}
 
 
-				fileEntry->meta->clearResourceMetaData();
-				fileEntry->meta->mImportOptions = queuedImport->importOptions;
-			}
+		Path internalResourcesPath = mProjectFolder;
+		internalResourcesPath.append(INTERNAL_RESOURCES_DIR);
 
 
-			Path internalResourcesPath = mProjectFolder;
-			internalResourcesPath.append(INTERNAL_RESOURCES_DIR);
+		// See which sub-resource metas need to be updated, removed or added based on the new resource set
+		bool isFirst = true;
+		for (const auto& entry : import.resources)
+		{
+			// Entries with no resources are sub-resources that used to exist in this file, but haven't been imported
+			// this time
+			if (!entry.resource)
+				continue;
 
 
-			// See which sub-resource metas need to be updated, removed or added based on the new resource set
-			bool isFirst = true;
-			for (auto& entry : queuedImport->resources)
-			{
-				// Entries with no resources are sub-resources that used to exist in this file, but haven't been imported
-				// this time
-				if(!entry.resource)
-					continue;
+			String name = entry.name;
+			Path::stripInvalid(name);
 
 
-				Path::stripInvalid(entry.name);
+			const ProjectResourceIcons icons = generatePreviewIcons(*entry.resource);
 
 
-				const ProjectResourceIcons icons = generatePreviewIcons(*entry.resource);
+			bool foundMeta = false;
+			for (auto iterMeta = existingMetas.begin(); iterMeta != existingMetas.end(); ++iterMeta)
+			{
+				const SPtr<ProjectResourceMeta>& metaEntry = *iterMeta;
 
 
-				bool foundMeta = false;
-				for (auto iterMeta = existingMetas.begin(); iterMeta != existingMetas.end(); ++iterMeta)
+				if (name == metaEntry->getUniqueName())
 				{
 				{
-					const SPtr<ProjectResourceMeta>& metaEntry = *iterMeta;
+					// Make sure the UUID we used for saving the resource matches the current one (should always
+					// be true unless the meta-data somehow changes while the async import is happening)
+					assert(entry.uuid == metaEntry->getUUID());
 
 
-					if (entry.name == metaEntry->getUniqueName())
-					{
-						// Make sure the UUID we used for saving the resource matches the current one (should always
-						// be true unless the meta-data somehow changes while the async import is happening)
-						assert(entry.uuid == metaEntry->getUUID());
-
-						HResource importedResource = gResources()._getResourceHandle(metaEntry->getUUID());
+					HResource importedResource = gResources()._getResourceHandle(metaEntry->getUUID());
 
 
-						gResources().update(importedResource, entry.resource);
+					gResources().update(importedResource, entry.resource);
 
 
-						metaEntry->setPreviewIcons(icons);
-						fileEntry->meta->add(metaEntry);
+					metaEntry->setPreviewIcons(icons);
+					fileEntry->meta->add(metaEntry);
 
 
-						iterMeta = existingMetas.erase(iterMeta);
-						foundMeta = true;
-						break;
-					}
+					iterMeta = existingMetas.erase(iterMeta);
+					foundMeta = true;
+					break;
 				}
 				}
+			}
 
 
-				if (!foundMeta)
+			if (!foundMeta)
+			{
+				HResource importedResource;
+
+				// Native resources are always expected to have a handle since Resources::load was called during
+				// the 'import' step
+				if (import.native)
 				{
 				{
-					HResource importedResource;
+					importedResource = gResources()._getResourceHandle(entry.uuid);
+					gResources().update(importedResource, entry.resource);
+				}
+				else
+					importedResource = gResources()._createResourceHandle(entry.resource, entry.uuid);
 
 
-					// Native resources are always expected to have a handle since Resources::load was called during
-					// the 'import' step
-					if(queuedImport->native)
-					{
-						importedResource = gResources()._getResourceHandle(entry.uuid);
-						gResources().update(importedResource, entry.resource);
-					}
-					else
-						importedResource = gResources()._createResourceHandle(entry.resource, entry.uuid);
+				SPtr<ResourceMetaData> subMeta = entry.resource->getMetaData();
+				const UINT32 typeId = entry.resource->getTypeId();
+				const UUID& UUID = importedResource.getUUID();
 
 
-					SPtr<ResourceMetaData> subMeta = entry.resource->getMetaData();
-					const UINT32 typeId = entry.resource->getTypeId();
-					const UUID& UUID = importedResource.getUUID();
+				SPtr<ProjectResourceMeta> resMeta = ProjectResourceMeta::create(name, UUID, typeId,
+					icons, subMeta);
+				fileEntry->meta->add(resMeta);
+			}
 
 
-					SPtr<ProjectResourceMeta> resMeta = ProjectResourceMeta::create(entry.name, UUID, typeId,
-						icons, subMeta);
-					fileEntry->meta->add(resMeta);
-				}
+			// Keep resource metas that we are not currently using, in case they get restored so their references
+			// don't get broken
+			if (!import.pruneMetas)
+			{
+				for (auto& metaEntry : existingMetas)
+					fileEntry->meta->addInactive(metaEntry);
+			}
 
 
-				// Keep resource metas that we are not currently using, in case they get restored so their references
-				// don't get broken
-				if (!queuedImport->pruneMetas)
-				{
-					for (auto& metaEntry : existingMetas)
-						fileEntry->meta->addInactive(metaEntry);
-				}
+			// Update UUID to path mapping
+			if (isFirst)
+				mUUIDToPath[entry.uuid] = fileEntry->path;
+			else
+				mUUIDToPath[entry.uuid] = fileEntry->path + name;
 
 
-				// Update UUID to path mapping
-				if(isFirst)
-					mUUIDToPath[entry.uuid] = fileEntry->path;
-				else
-					mUUIDToPath[entry.uuid] = fileEntry->path + entry.name;
+			isFirst = false;
+
+			// Register path in manifest
+			const String uuidStr = entry.uuid.toString();
 
 
-				isFirst = false;
+			internalResourcesPath.setFilename(uuidStr + ".asset");
+			mResourceManifest->registerResource(entry.uuid, internalResourcesPath);
+		}
 
 
-				// Register path in manifest
-				const String uuidStr = entry.uuid.toString();
+		// Save the meta file
+		FileEncoder fs(metaPath);
+		fs.encode(fileEntry->meta.get());
 
 
-				internalResourcesPath.setFilename(uuidStr + ".asset");
-				mResourceManifest->registerResource(entry.uuid, internalResourcesPath);
-			}
+		// Register any dependencies this resource depends on
+		addDependencies(fileEntry);
 
 
-			// Save the meta file
-			FileEncoder fs(metaPath);
-			fs.encode(fileEntry->meta.get());
+		// Notify the outside world import is doen
+		onEntryImported(fileEntry->path);
 
 
-			// Register any dependencies this resource depends on
-			addDependencies(fileEntry);
+		// Queue any resources dependant on this one for import
+		reimportDependants(fileEntry->path);
 
 
-			// Notify the outside world import is doen
-			onEntryImported(fileEntry->path);
+		return true;
+	}
 
 
-			// Queue any resources dependant on this one for import
-			reimportDependants(fileEntry->path);
+	void ProjectLibrary::_finishQueuedImports(bool wait)
+	{
+		for(auto iter = mQueuedImports.begin(); iter != mQueuedImports.end();)
+		{
+			if(finishQueuedImport(iter->first, *iter->second, wait))
+				iter = mQueuedImports.erase(iter);
+			else
+				++iter;
 		}
 		}
 	}
 	}
 
 
@@ -1037,7 +1060,18 @@ namespace bs
 
 
 		Path absPath = assetPath.getAbsolute(getResourcesFolder());
 		Path absPath = assetPath.getAbsolute(getResourcesFolder());
 		Resources::instance().save(resource, absPath, false);
 		Resources::instance().save(resource, absPath, false);
-		checkForModifications(absPath);
+
+		Path parentDirPath = absPath.getParent();
+		LibraryEntry* parentEntry = findEntry(parentDirPath);
+
+		// Register parent hierarchy if not found
+		DirectoryEntry* entryParent = nullptr;
+		if (parentEntry == nullptr)
+			createInternalParentHierarchy(absPath, nullptr, &entryParent);
+		else
+			entryParent = static_cast<DirectoryEntry*>(parentEntry);
+
+		addResourceInternal(entryParent, absPath, nullptr, true, true);
 	}
 	}
 
 
 	void ProjectLibrary::saveEntry(const HResource& resource)
 	void ProjectLibrary::saveEntry(const HResource& resource)
@@ -1055,7 +1089,10 @@ namespace bs
 		filePath.makeAbsolute(getResourcesFolder());
 		filePath.makeAbsolute(getResourcesFolder());
 
 
 		Resources::instance().save(resource, filePath, true);
 		Resources::instance().save(resource, filePath, true);
-		checkForModifications(filePath);
+
+		LibraryEntry* fileEntry = findEntry(filePath);
+		if(fileEntry)
+			reimportResourceInternal(static_cast<FileEntry*>(fileEntry), nullptr, true, false, true);
 	}
 	}
 
 
 	void ProjectLibrary::createFolderEntry(const Path& path)
 	void ProjectLibrary::createFolderEntry(const Path& path)
@@ -1344,6 +1381,16 @@ namespace bs
 		}
 		}
 	}
 	}
 
 
+	void ProjectLibrary::waitForQueuedImport(FileEntry* fileEntry)
+	{
+		const auto iterFind = mQueuedImports.find(fileEntry);
+		if (iterFind != mQueuedImports.end())
+		{
+			if (finishQueuedImport(fileEntry, *iterFind->second, true))
+				mQueuedImports.erase(iterFind);
+		}
+	}
+
 	void ProjectLibrary::setIncludeInBuild(const Path& path, bool include)
 	void ProjectLibrary::setIncludeInBuild(const Path& path, bool include)
 	{
 	{
 		LibraryEntry* entry = findEntry(path);
 		LibraryEntry* entry = findEntry(path);
@@ -1351,17 +1398,21 @@ namespace bs
 		if (entry == nullptr || entry->type == LibraryEntryType::Directory)
 		if (entry == nullptr || entry->type == LibraryEntryType::Directory)
 			return;
 			return;
 
 
-		FileEntry* resEntry = static_cast<FileEntry*>(entry);
-		if (resEntry->meta == nullptr)
+		auto fileEntry = static_cast<FileEntry*>(entry);
+
+		// Any queued imports will overwrite the meta file, so make sure they finish first
+		waitForQueuedImport(fileEntry);
+
+		if (fileEntry->meta == nullptr)
 			return;
 			return;
 
 
-		resEntry->meta->setIncludeInBuild(include);
+		fileEntry->meta->setIncludeInBuild(include);
 
 
-		Path metaPath = resEntry->path;
+		Path metaPath = fileEntry->path;
 		metaPath.setFilename(metaPath.getFilename() + ".meta");
 		metaPath.setFilename(metaPath.getFilename() + ".meta");
 
 
 		FileEncoder fs(metaPath);
 		FileEncoder fs(metaPath);
-		fs.encode(resEntry->meta.get());
+		fs.encode(fileEntry->meta.get());
 	}
 	}
 
 
 	void ProjectLibrary::setUserData(const Path& path, const SPtr<IReflectable>& userData)
 	void ProjectLibrary::setUserData(const Path& path, const SPtr<IReflectable>& userData)
@@ -1371,7 +1422,11 @@ namespace bs
 		if (entry == nullptr || entry->type == LibraryEntryType::Directory)
 		if (entry == nullptr || entry->type == LibraryEntryType::Directory)
 			return;
 			return;
 
 
-		FileEntry* fileEntry = static_cast<FileEntry*>(entry);
+		auto fileEntry = static_cast<FileEntry*>(entry);
+
+		// Any queued imports will overwrite the meta file, so make sure they finish first
+		waitForQueuedImport(fileEntry);
+
 		SPtr<ProjectResourceMeta> resMeta = findResourceMeta(path);
 		SPtr<ProjectResourceMeta> resMeta = findResourceMeta(path);
 		
 		
 		if (resMeta == nullptr)
 		if (resMeta == nullptr)

+ 32 - 5
Source/EditorCore/Library/BsProjectLibrary.h

@@ -305,10 +305,14 @@ namespace bs
 		 *								provided default import options are used.
 		 *								provided default import options are used.
 		 * @param[in]	forceReimport	Should the resource be reimported even if we detect no changes. This should be true
 		 * @param[in]	forceReimport	Should the resource be reimported even if we detect no changes. This should be true
 		 *								if import options changed since last import.
 		 *								if import options changed since last import.
+		 * @param[in]	synchronous		If true the import will happen synchronously on the calling thread. If false
+		 *								the import operation will be queued for execution on a worker thread. You
+		 *								then must call _finishQueuedImports() after the worker thread finishes to
+		 *								actually finish the import.
 		 * @return						Newly added resource entry.
 		 * @return						Newly added resource entry.
 		 */
 		 */
 		FileEntry* addResourceInternal(DirectoryEntry* parent, const Path& filePath, 
 		FileEntry* addResourceInternal(DirectoryEntry* parent, const Path& filePath, 
-			const SPtr<ImportOptions>& importOptions = nullptr, bool forceReimport = false);
+			const SPtr<ImportOptions>& importOptions = nullptr, bool forceReimport = false, bool synchronous = false);
 
 
 		/**
 		/**
 		 * Common code for adding a new folder entry to the library.
 		 * Common code for adding a new folder entry to the library.
@@ -338,7 +342,7 @@ namespace bs
 		/**
 		/**
 		 * Triggers a reimport of a resource using the provided import options, if needed. Doesn't import dependencies.
 		 * Triggers a reimport of a resource using the provided import options, if needed. Doesn't import dependencies.
 		 *
 		 *
-		 * @param[in]	file				File entry of the resource to reimport.
+		 * @param[in]	fileEntry			File entry of the resource to reimport.
 		 * @param[in]	importOptions		Optional import options to use when importing the resource. Caller must ensure 
 		 * @param[in]	importOptions		Optional import options to use when importing the resource. Caller must ensure 
 		 *									the import options are of the correct type for the resource in question. If null
 		 *									the import options are of the correct type for the resource in question. If null
 		 *									is provided default import options are used.
 		 *									is provided default import options are used.
@@ -349,10 +353,15 @@ namespace bs
 		 *									resources are eventually restored, references to them will remain valid. If you
 		 *									resources are eventually restored, references to them will remain valid. If you
 		 *									feel that you need to clear this data, set this to true but be aware that you
 		 *									feel that you need to clear this data, set this to true but be aware that you
 		 *									might need to re-apply those references.
 		 *									might need to re-apply those references.
-		 * @return							Returns true if the resource was queued for import, false otherwise.
+		 * @param[in]	synchronous			If true the import will happen synchronously on the calling thread. If false
+		 *									the import operation will be queued for execution on a worker thread. You
+		 *									then must call _finishQueuedImports() after the worker thread finishes to
+		 *									actually finish the import.
+		 * @return							Returns true if the resource was queued for import (or imported, if 
+		 *									synchronous), false otherwise.
 		 */
 		 */
-		bool reimportResourceInternal(FileEntry* file, const SPtr<ImportOptions>& importOptions = nullptr, 
-			bool forceReimport = false, bool pruneResourceMetas = false);
+		bool reimportResourceInternal(FileEntry* fileEntry, const SPtr<ImportOptions>& importOptions = nullptr, 
+			bool forceReimport = false, bool pruneResourceMetas = false, bool synchronous = false);
 
 
 		/**
 		/**
 		 * Creates a full hierarchy of directory entries up to the provided directory, if any are needed.
 		 * Creates a full hierarchy of directory entries up to the provided directory, if any are needed.
@@ -407,6 +416,24 @@ namespace bs
 		/** Deletes all library entries. */
 		/** Deletes all library entries. */
 		void clearEntries();
 		void clearEntries();
 
 
+		/** 
+		 * Finalizes a queued import operation if the import task has finished (or immediately if no task is present). 
+		 *
+		 * @param[in]	fileEntry		File entry for which the import is running.
+		 * @param[in]	import			Structure containing information about the import.
+		 * @param[in]	wait			If true waits until the asynchronous import task finishes before returning. Not
+		 *								relevant if the import task is not present (synchronous import).
+		 * @return						True if the import was finalized. Will be false if the async import task has not
+		 *								yet finished and @p wait is false.
+		 */
+		bool finishQueuedImport(FileEntry* fileEntry, const QueuedImport& import, bool wait);
+
+		/** 
+		 * Checks if there are any queued imports queued for the provided file entry, and if there are waits until they
+		 * finish before returning.s
+		 */
+		void waitForQueuedImport(FileEntry* fileEntry);
+
 		static const char* LIBRARY_ENTRIES_FILENAME;
 		static const char* LIBRARY_ENTRIES_FILENAME;
 		static const char* RESOURCE_MANIFEST_FILENAME;
 		static const char* RESOURCE_MANIFEST_FILENAME;