From 5ccec8ec6bed3e0eda1cffaae565fdfaccd2a6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 21 Oct 2021 15:53:12 +0200 Subject: [PATCH] Asset Catalogs: treat first-loaded catalog as main catalog When there are multiple catalogs with the same path (so different UUIDs all mapped to the same catalog path), treat the first-loaded one as the main catalog for that path, and the rest as aliases. This ensures that the UUID of a catalog (as chosen in the tree UI and thus interacted with by users) is stable, regardless of whether by some coincidence later another catalog with the same UUID is created. --- .../blender/blenkernel/BKE_asset_catalog.hh | 15 +++++++-- .../blenkernel/intern/asset_catalog.cc | 25 ++++++++++++--- .../blenkernel/intern/asset_catalog_test.cc | 32 +++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/source/blender/blenkernel/BKE_asset_catalog.hh b/source/blender/blenkernel/BKE_asset_catalog.hh index ca4517707e3..5de74efe2cf 100644 --- a/source/blender/blenkernel/BKE_asset_catalog.hh +++ b/source/blender/blenkernel/BKE_asset_catalog.hh @@ -108,8 +108,12 @@ class AssetCatalogService { /** Return catalog with the given ID. Return nullptr if not found. */ AssetCatalog *find_catalog(CatalogID catalog_id) const; - /** Return first catalog with the given path. Return nullptr if not found. This is not an - * efficient call as it's just a linear search over the catalogs. */ + /** + * Return first catalog with the given path. Return nullptr if not found. This is not an + * efficient call as it's just a linear search over the catalogs. + * + * If there are multiple catalogs with the same path, return the first-loaded one. If there is + * none marked as "first loaded", return the one with the lowest UUID. */ AssetCatalog *find_catalog_by_path(const AssetCatalogPath &path) const; /** @@ -416,7 +420,7 @@ class AssetCatalog { static std::string sensible_simple_name_for_path(const AssetCatalogPath &path); }; -/** Comparator for asset catalogs, ordering by (path, UUID). */ +/** Comparator for asset catalogs, ordering by (path, first_seen, UUID). */ struct AssetCatalogLessThan { bool operator()(const AssetCatalog *lhs, const AssetCatalog *rhs) const { @@ -424,6 +428,10 @@ struct AssetCatalogLessThan { return lhs->path < rhs->path; } + if (lhs->flags.is_first_loaded != rhs->flags.is_first_loaded) { + return lhs->flags.is_first_loaded; + } + return lhs->catalog_id < rhs->catalog_id; } }; @@ -432,6 +440,7 @@ struct AssetCatalogLessThan { * Set that stores catalogs ordered by (path, UUID). * Being a set, duplicates are removed. The catalog's simple name is ignored in this. */ using AssetCatalogOrderedSet = std::set; +using MutableAssetCatalogOrderedSet = std::set; /** * Filter that can determine whether an asset should be visible or not, based on its catalog ID. diff --git a/source/blender/blenkernel/intern/asset_catalog.cc b/source/blender/blenkernel/intern/asset_catalog.cc index aa8f12d0e23..51f8457ebd9 100644 --- a/source/blender/blenkernel/intern/asset_catalog.cc +++ b/source/blender/blenkernel/intern/asset_catalog.cc @@ -24,6 +24,7 @@ #include "BLI_fileops.h" #include "BLI_path_util.h" +#include "BLI_set.hh" #include "BLI_string_ref.hh" #include "DNA_userdef_types.h" @@ -108,13 +109,23 @@ AssetCatalog *AssetCatalogService::find_catalog(CatalogID catalog_id) const AssetCatalog *AssetCatalogService::find_catalog_by_path(const AssetCatalogPath &path) const { + /* Use an AssetCatalogOrderedSet to find the 'best' catalog for this path. This will be the first + * one loaded from disk, or if that does not exist the one with the lowest UUID. This ensures + * stable, predictable results. */ + MutableAssetCatalogOrderedSet ordered_catalogs; + for (const auto &catalog : catalog_collection_->catalogs_.values()) { if (catalog->path == path) { - return catalog.get(); + ordered_catalogs.insert(catalog.get()); } } - return nullptr; + if (ordered_catalogs.empty()) { + return nullptr; + } + + MutableAssetCatalogOrderedSet::iterator best_choice_it = ordered_catalogs.begin(); + return *best_choice_it; } AssetCatalogFilter AssetCatalogService::create_catalog_filter( @@ -309,7 +320,10 @@ std::unique_ptr AssetCatalogService::parse_catalog_f auto cdf = std::make_unique(); cdf->file_path = catalog_definition_file_path; - auto catalog_parsed_callback = [this, catalog_definition_file_path]( + /* TODO(Sybren): this might have to move to a higher level when supporting multiple CDFs. */ + Set seen_paths; + + auto catalog_parsed_callback = [this, catalog_definition_file_path, &seen_paths]( std::unique_ptr catalog) { if (catalog_collection_->catalogs_.contains(catalog->catalog_id)) { /* TODO(@sybren): apparently another CDF was already loaded. This is not supported yet. */ @@ -319,6 +333,8 @@ std::unique_ptr AssetCatalogService::parse_catalog_f return false; } + catalog->flags.is_first_loaded = seen_paths.add(catalog->path); + /* The AssetCatalog pointer is now owned by the AssetCatalogService. */ catalog_collection_->catalogs_.add_new(catalog->catalog_id, std::move(catalog)); return true; @@ -648,7 +664,8 @@ void AssetCatalogTree::insert_item(const AssetCatalog &catalog) /* If full path of this catalog already exists as parent path of a previously read catalog, * we can ensure this tree item's UUID is set here. */ - if (is_last_component && BLI_uuid_is_nil(item.catalog_id_)) { + if (is_last_component && + (BLI_uuid_is_nil(item.catalog_id_) || catalog.flags.is_first_loaded)) { item.catalog_id_ = catalog.catalog_id; } diff --git a/source/blender/blenkernel/intern/asset_catalog_test.cc b/source/blender/blenkernel/intern/asset_catalog_test.cc index 478d2d2b31e..7691658bc57 100644 --- a/source/blender/blenkernel/intern/asset_catalog_test.cc +++ b/source/blender/blenkernel/intern/asset_catalog_test.cc @@ -39,6 +39,7 @@ const bUUID UUID_POSES_RUZENA("79a4f887-ab60-4bd4-94da-d572e27d6aed"); const bUUID UUID_POSES_RUZENA_HAND("81811c31-1a88-4bd7-bb34-c6fc2607a12e"); const bUUID UUID_POSES_RUZENA_FACE("82162c1f-06cc-4d91-a9bf-4f72c104e348"); const bUUID UUID_WITHOUT_SIMPLENAME("d7916a31-6ca9-4909-955f-182ca2b81fa3"); +const bUUID UUID_ANOTHER_RUZENA("00000000-d9fa-4b91-b704-e6af1f1339ef"); /* UUIDs from lib/tests/asset_library/modified_assets.cats.txt */ const bUUID UUID_AGENT_47("c5744ba5-43f5-4f73-8e52-010ad4a61b34"); @@ -290,6 +291,37 @@ TEST_F(AssetCatalogTest, load_single_file) EXPECT_EQ(UUID_POSES_RUZENA, poses_ruzena->catalog_id); EXPECT_EQ("character/Ružena/poselib", poses_ruzena->path.str()); EXPECT_EQ("POSES_RUŽENA", poses_ruzena->simple_name); + + /* Test getting a catalog that aliases an earlier-defined catalog. */ + AssetCatalog *another_ruzena = service.find_catalog(UUID_ANOTHER_RUZENA); + ASSERT_NE(nullptr, another_ruzena); + EXPECT_EQ(UUID_ANOTHER_RUZENA, another_ruzena->catalog_id); + EXPECT_EQ("character/Ružena/poselib", another_ruzena->path.str()); + EXPECT_EQ("Another Ružena", another_ruzena->simple_name); +} + +TEST_F(AssetCatalogTest, is_first_loaded_flag) +{ + AssetCatalogService service(asset_library_root_); + service.load_from_disk(asset_library_root_ + "/" + "blender_assets.cats.txt"); + + AssetCatalog *new_cat = service.create_catalog("never/before/seen/path"); + EXPECT_FALSE(new_cat->flags.is_first_loaded) + << "Adding a catalog at runtime should never mark it as 'first loaded'; " + "only loading from disk is allowed to do that."; + + AssetCatalog *alias_cat = service.create_catalog("character/Ružena/poselib"); + EXPECT_FALSE(alias_cat->flags.is_first_loaded) + << "Adding a new catalog with an already-loaded path should not mark it as 'first loaded'"; + + EXPECT_TRUE(service.find_catalog(UUID_POSES_ELLIE)->flags.is_first_loaded); + EXPECT_TRUE(service.find_catalog(UUID_POSES_ELLIE_WHITESPACE)->flags.is_first_loaded); + EXPECT_TRUE(service.find_catalog(UUID_POSES_RUZENA)->flags.is_first_loaded); + EXPECT_FALSE(service.find_catalog(UUID_ANOTHER_RUZENA)->flags.is_first_loaded); + + AssetCatalog *ruzena = service.find_catalog_by_path("character/Ružena/poselib"); + EXPECT_EQ(UUID_POSES_RUZENA, ruzena->catalog_id) + << "The first-seen definition of a catalog should be returned"; } TEST_F(AssetCatalogTest, insert_item_into_tree)