griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #108703: assert when loading "All" asset library

Browse Source 
Error caused by attempting to register multiple catalogs with the same
catalog ID. This would happen when multiple asset libraries would use
the same catalog definition file, and the "All" library would attempt to
merge them all into one library.

Ignore duplicate catalogs, like we already do when reading individual
asset catalog definition files. Log an error instead, or a info log if
the catalog path matches as well (in which case the conflict can be
ignored as it won't be user visible).
This commit is contained in:
Julian Eisel
2024-02-20 15:48:30 +01:00
parent d7c728c2fb
commit debdde985f
3 changed files with 65 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
source/blender/asset_system/AS_asset_catalog.hh
View File

@@ -84,8 +84,13 @@ class AssetCatalogService {
/**
* Duplicate the catalogs from \a other_service into this one. Does not rebuild the tree, this
* needs to be done by the caller (call #rebuild_tree()!).
*
* \note If a catalog from \a other already exists in this collection (identified by catalog ID),
* it will be skipped and \a on_duplicate_items will be called.
*/
void add_from_existing(const AssetCatalogService &other_service);
void add_from_existing(const AssetCatalogService &other_service,
FunctionRef<void(const AssetCatalog &existing,
const AssetCatalog &to_be_ignored)> on_duplicate_items);
/**
* Write the catalog definitions to disk.
@@ -298,11 +303,17 @@ class AssetCatalogCollection {
AssetCatalogCollection(AssetCatalogCollection &&other) noexcept = default;
std::unique_ptr<AssetCatalogCollection> deep_copy() const;
using OnDuplicateCatalogIdFn =
FunctionRef<void(const AssetCatalog &existing, const AssetCatalog &to_be_ignored)>;
/**
* Copy the catalogs from \a other and append them to this collection. Copies no other data
* otherwise.
*
* \note If a catalog from \a other already exists in this collection (identified by catalog ID),
* it will be skipped and \a on_duplicate_items will be called.
*/
void add_catalogs_from_existing(const AssetCatalogCollection &other);
void add_catalogs_from_existing(const AssetCatalogCollection &other,
OnDuplicateCatalogIdFn on_duplicate_items);
protected:
static OwningAssetCatalogMap copy_catalog_map(const OwningAssetCatalogMap &orig);

33
source/blender/asset_system/intern/asset_catalog.cc
View File

@@ -344,9 +344,12 @@ void AssetCatalogService::load_from_disk(const CatalogFilePath &file_or_director
this->rebuild_tree();
}
void AssetCatalogService::add_from_existing(const AssetCatalogService &other_service)
void AssetCatalogService::add_from_existing(
const AssetCatalogService &other_service,
AssetCatalogCollection::OnDuplicateCatalogIdFn on_duplicate_items)
{
catalog_collection_->add_catalogs_from_existing(*other_service.catalog_collection_);
catalog_collection_->add_catalogs_from_existing(*other_service.catalog_collection_,
on_duplicate_items);
}
void AssetCatalogService::load_directory_recursive(const CatalogFilePath &directory_path)
@@ -692,24 +695,40 @@ std::unique_ptr<AssetCatalogCollection> AssetCatalogCollection::deep_copy() cons
return copy;
}
static void copy_catalog_map_into_existing(const OwningAssetCatalogMap &source,
OwningAssetCatalogMap &dest)
static void copy_catalog_map_into_existing(
const OwningAssetCatalogMap &source,
OwningAssetCatalogMap &dest,
AssetCatalogCollection::OnDuplicateCatalogIdFn on_duplicate_items)
{
for (const auto &orig_catalog_uptr : source.values()) {
if (dest.contains(orig_catalog_uptr->catalog_id)) {
if (on_duplicate_items) {
on_duplicate_items(*dest.lookup(orig_catalog_uptr->catalog_id), *orig_catalog_uptr);
}
continue;
}
auto copy_catalog_uptr = std::make_unique<AssetCatalog>(*orig_catalog_uptr);
dest.add_new(copy_catalog_uptr->catalog_id, std::move(copy_catalog_uptr));
}
}
void AssetCatalogCollection::add_catalogs_from_existing(const AssetCatalogCollection &other)
void AssetCatalogCollection::add_catalogs_from_existing(
const AssetCatalogCollection &other,
AssetCatalogCollection::OnDuplicateCatalogIdFn on_duplicate_items)
{
copy_catalog_map_into_existing(other.catalogs_, catalogs_);
copy_catalog_map_into_existing(other.catalogs_, catalogs_, on_duplicate_items);
}
OwningAssetCatalogMap AssetCatalogCollection::copy_catalog_map(const OwningAssetCatalogMap &orig)
{
OwningAssetCatalogMap copy;
copy_catalog_map_into_existing(orig, copy);
copy_catalog_map_into_existing(
orig, copy, /*on_duplicate_items=*/[](const AssetCatalog &, const AssetCatalog &) {
/* `copy` was empty before. If this happens it means there was a duplicate in the `orig`
* catalog map which should've been caught already. */
BLI_assert_unreachable();
});
return copy;
}

27
source/blender/asset_system/intern/asset_library_all.cc
View File

@@ -12,6 +12,10 @@
#include "asset_library_all.hh"
#include "CLG_log.h"
static CLG_LogRef LOG = {"asset_system.all_asset_library"};
namespace blender::asset_system {
AllAssetLibrary::AllAssetLibrary() : AssetLibrary(ASSET_LIBRARY_ALL) {}
@@ -28,9 +32,30 @@ void AllAssetLibrary::rebuild_catalogs_from_nested(const bool reload_nested_cata
if (reload_nested_catalogs) {
nested.catalog_service->reload_catalogs();
}
new_catalog_service->add_from_existing(*nested.catalog_service);
new_catalog_service->add_from_existing(
*nested.catalog_service,
/*on_duplicate_items=*/[](const AssetCatalog &existing,
const AssetCatalog &to_be_ignored) {
if (existing.path == to_be_ignored.path) {
CLOG_INFO(&LOG,
2,
"multiple definitions of catalog %s (path: %s), ignoring duplicate",
existing.catalog_id.str().c_str(),
existing.path.c_str());
}
else {
CLOG_ERROR(&LOG,
"multiple definitions of catalog %s with differing paths (%s vs. %s), "
"ignoring second one",
existing.catalog_id.str().c_str(),
existing.path.c_str(),
to_be_ignored.path.c_str());
}
});
},
false);
new_catalog_service->rebuild_tree();
this->catalog_service = std::move(new_catalog_service);
}