From abd6881d7bee1db377ef63154d1b9a0082945e48 Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Fri, 13 Oct 2023 15:43:11 +0200 Subject: [PATCH] Fix removing large amount of assets in Editor at once #1484 --- .../Editor/Modules/ContentDatabaseModule.cs | 5 +++-- Source/Editor/Windows/ContentWindow.cs | 3 ++- Source/Engine/Content/Content.cpp | 22 ++++++------------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/Source/Editor/Modules/ContentDatabaseModule.cs b/Source/Editor/Modules/ContentDatabaseModule.cs index eec55c788..8b7371e44 100644 --- a/Source/Editor/Modules/ContentDatabaseModule.cs +++ b/Source/Editor/Modules/ContentDatabaseModule.cs @@ -649,8 +649,6 @@ namespace FlaxEditor.Modules // Special case for folders if (item is ContentFolder folder) { - // TODO: maybe don't remove folders recursive but at once? - // Delete all children if (folder.Children.Count > 0) { @@ -664,6 +662,9 @@ namespace FlaxEditor.Modules // Remove directory if (deletedByUser && Directory.Exists(path)) { + // Flush files removal before removing folder (loaded assets remove file during object destruction in Asset::OnDeleteObject) + FlaxEngine.Scripting.FlushRemovedObjects(); + try { Directory.Delete(path, true); diff --git a/Source/Editor/Windows/ContentWindow.cs b/Source/Editor/Windows/ContentWindow.cs index a06ec839d..c287900e5 100644 --- a/Source/Editor/Windows/ContentWindow.cs +++ b/Source/Editor/Windows/ContentWindow.cs @@ -629,8 +629,9 @@ namespace FlaxEditor.Windows if (items.Count == 0) return; - // TODO: remove items that depend on different items in the list: use wants to remove `folderA` and `folderA/asset.x`, we should just remove `folderA` + // Sort items to remove files first, then folders var toDelete = new List(items); + toDelete.Sort((a, b) => a.IsFolder ? 1 : -1); string msg = toDelete.Count == 1 ? string.Format("Are you sure to delete \'{0}\'?\nThis action cannot be undone. Files will be deleted permanently.", items[0].Path) diff --git a/Source/Engine/Content/Content.cpp b/Source/Engine/Content/Content.cpp index ea627f2c7..c4ce37dcb 100644 --- a/Source/Engine/Content/Content.cpp +++ b/Source/Engine/Content/Content.cpp @@ -521,37 +521,33 @@ Asset* Content::GetAsset(const Guid& id) void Content::DeleteAsset(Asset* asset) { - ScopeLock locker(AssetsLocker); - - // Validate if (asset == nullptr || asset->_deleteFileOnUnload) - { - // Back return; - } LOG(Info, "Deleting asset {0}...", asset->ToString()); + // Ensure that asset is loaded (easier than cancel in-flight loading) + asset->WaitForLoaded(); + // Mark asset for delete queue (delete it after auto unload) asset->_deleteFileOnUnload = true; // Unload - UnloadAsset(asset); + asset->DeleteObject(); } void Content::DeleteAsset(const StringView& path) { - ScopeLock locker(AssetsLocker); - - // Check if is loaded + // Try to delete already loaded asset Asset* asset = GetAsset(path); if (asset != nullptr) { - // Delete asset DeleteAsset(asset); return; } + ScopeLock locker(AssetsLocker); + // Remove from registry AssetInfo info; if (Cache.DeleteAsset(path, &info)) @@ -573,7 +569,6 @@ void Content::deleteFileSafety(const StringView& path, const Guid& id) // Check if given id is invalid if (!id.IsValid()) { - // Cancel operation LOG(Warning, "Cannot remove file \'{0}\'. Given ID is invalid.", path); return; } @@ -585,7 +580,6 @@ void Content::deleteFileSafety(const StringView& path, const Guid& id) storage->CloseFileHandles(); // Close file handle to allow removing it if (!storage->HasAsset(id)) { - // Skip removing LOG(Warning, "Cannot remove file \'{0}\'. It doesn\'t contain asset {1}.", path, id); return; } @@ -790,10 +784,8 @@ bool Content::CloneAssetFile(const StringView& dstPath, const StringView& srcPat void Content::UnloadAsset(Asset* asset) { - // Check input if (asset == nullptr) return; - asset->DeleteObject(); }