diff options
Diffstat (limited to 'source')
9 files changed, 324 insertions, 71 deletions
diff --git a/source/blender/blenkernel/BKE_asset_catalog.hh b/source/blender/blenkernel/BKE_asset_catalog.hh index cbb15780a68..902c0e414d6 100644 --- a/source/blender/blenkernel/BKE_asset_catalog.hh +++ b/source/blender/blenkernel/BKE_asset_catalog.hh @@ -64,11 +64,13 @@ class AssetCatalogService { explicit AssetCatalogService(const CatalogFilePath &asset_library_root); /** - * Set global tag indicating that some catalog modifications are unsaved that could get lost - * on exit. This tag is not set by internal catalog code, the catalog service user is responsible - * for it. It is cleared by #write_to_disk(). - */ - void tag_has_unsaved_changes(); + * Set tag indicating that some catalog modifications are unsaved, which could + * get lost on exit. This tag is not set by internal catalog code, the catalog + * service user is responsible for it. It is cleared by #write_to_disk(). + * + * This "dirty" state is tracked per catalog, so that it's possible to gracefully load changes + * from disk. Any catalog with unsaved changes will not be overwritten by on-disk changes. */ + void tag_has_unsaved_changes(AssetCatalog *edited_catalog); bool has_unsaved_changes() const; /** Load asset catalog definitions from the files found in the asset library. */ @@ -103,7 +105,7 @@ class AssetCatalogService { * - Already-known on-disk catalogs are ignored (so will be overwritten with our in-memory * data). This includes in-memory marked-as-deleted catalogs. */ - void merge_from_disk_before_writing(); + void reload_catalogs(); /** Return catalog with the given ID. Return nullptr if not found. */ AssetCatalog *find_catalog(CatalogID catalog_id) const; @@ -140,12 +142,6 @@ class AssetCatalogService { void prune_catalogs_by_id(CatalogID catalog_id); /** - * Delete a catalog, without deleting any of its children and without rebuilding the catalog - * tree. This is a lower-level function than #prune_catalogs_by_path. - */ - void delete_catalog_by_id(CatalogID catalog_id); - - /** * Update the catalog path, also updating the catalog path of all sub-catalogs. */ void update_catalog_path(CatalogID catalog_id, const AssetCatalogPath &new_catalog_path); @@ -178,7 +174,6 @@ class AssetCatalogService { Vector<std::unique_ptr<AssetCatalogCollection>> undo_snapshots_; Vector<std::unique_ptr<AssetCatalogCollection>> redo_snapshots_; - bool has_unsaved_changes_ = false; void load_directory_recursive(const CatalogFilePath &directory_path); void load_single_file(const CatalogFilePath &catalog_definition_file_path); @@ -186,6 +181,31 @@ class AssetCatalogService { /** Implementation of #write_to_disk() that doesn't clear the "has unsaved changes" tag. */ bool write_to_disk_ex(const CatalogFilePath &blend_file_path); void untag_has_unsaved_changes(); + bool is_catalog_known_with_unsaved_changes(CatalogID catalog_id) const; + + /** + * Delete catalogs, only keeping them when they are either listed in + * \a catalogs_to_keep or have unsaved changes. + * + * \note Deleted catalogs are hard-deleted, i.e. they just vanish instead of + * remembering them as "deleted". + */ + void purge_catalogs_not_listed(const Set<CatalogID> &catalogs_to_keep); + + /** + * Delete a catalog, without deleting any of its children and without rebuilding the catalog + * tree. The deletion in "Soft", in the sense that the catalog pointer is moved from `catalogs_` + * to `deleted_catalogs_`; the AssetCatalog instance itself is kept in memory. As a result, it + * will be removed from a CDF when saved to disk. + * + * This is a lower-level function than #prune_catalogs_by_path. + */ + void delete_catalog_by_id_soft(CatalogID catalog_id); + + /** + * Hard delete a catalog. This simply removes the catalog from existence. The deletion will not + * be remembered, and reloading the CDF will bring it back. */ + void delete_catalog_by_id_hard(CatalogID catalog_id); std::unique_ptr<AssetCatalogDefinitionFile> parse_catalog_file( const CatalogFilePath &catalog_definition_file_path); @@ -216,6 +236,7 @@ class AssetCatalogService { /* For access by subclasses, as those will not be marked as friend by #AssetCatalogCollection. */ AssetCatalogDefinitionFile *get_catalog_definition_file(); OwningAssetCatalogMap &get_catalogs(); + OwningAssetCatalogMap &get_deleted_catalogs(); }; /** @@ -246,6 +267,9 @@ class AssetCatalogCollection { * The aim is to support an arbitrary number of such files per asset library in the future. */ std::unique_ptr<AssetCatalogDefinitionFile> catalog_definition_file_; + /** Whether any of the catalogs have unsaved changes. */ + bool has_unsaved_changes_ = false; + static OwningAssetCatalogMap copy_catalog_map(const OwningAssetCatalogMap &orig); }; @@ -269,6 +293,7 @@ class AssetCatalogTreeItem { CatalogID get_catalog_id() const; StringRefNull get_simple_name() const; StringRefNull get_name() const; + bool has_unsaved_changes() const; /** Return the full catalog path, defined as the name of this catalog prefixed by the full * catalog path of its parent and a separator. */ AssetCatalogPath catalog_path() const; @@ -287,6 +312,8 @@ class AssetCatalogTreeItem { CatalogID catalog_id_; /** Copy of #AssetCatalog::simple_name. */ std::string simple_name_; + /** Copy of #AssetCatalog::flags.has_unsaved_changes. */ + bool has_unsaved_changes_ = false; /** Pointer back to the parent item. Used to reconstruct the hierarchy from an item (e.g. to * build a path). */ @@ -353,9 +380,14 @@ class AssetCatalogDefinitionFile { bool write_to_disk(const CatalogFilePath &dest_file_path) const; bool contains(CatalogID catalog_id) const; - /* Add a new catalog. Undefined behavior if a catalog with the same ID was already added. */ + /** Add a catalog, overwriting the one with the same catalog ID. */ + void add_overwrite(AssetCatalog *catalog); + /** Add a new catalog. Undefined behavior if a catalog with the same ID was already added. */ void add_new(AssetCatalog *catalog); + /** Remove the catalog from the collection of catalogs stored in this file. */ + void forget(CatalogID catalog_id); + using AssetCatalogParsedFn = FunctionRef<bool(std::unique_ptr<AssetCatalog>)>; void parse_catalog_file(const CatalogFilePath &catalog_definition_file_path, AssetCatalogParsedFn callback); @@ -405,6 +437,14 @@ class AssetCatalog { * load-and-merged with a file that also has these catalogs, the first one in that file is * always sorted first, regardless of the sort order of its UUID. */ bool is_first_loaded = false; + + /* Merging on-disk changes into memory will not overwrite this catalog. + * For example, when a catalog was renamed (i.e. changed path) in this Blender session, + * reloading the catalog definition file should not overwrite that change. + * + * Note that this flag is ignored when is_deleted=true; deleted catalogs that are still in + * memory are considered "unsaved" by definition. */ + bool has_unsaved_changes = false; } flags; /** diff --git a/source/blender/blenkernel/BKE_asset_library.hh b/source/blender/blenkernel/BKE_asset_library.hh index 64c8afecaef..15f7991e75e 100644 --- a/source/blender/blenkernel/BKE_asset_library.hh +++ b/source/blender/blenkernel/BKE_asset_library.hh @@ -49,6 +49,9 @@ struct AssetLibrary { void load(StringRefNull library_root_directory); + /** Load catalogs that have changed on disk. */ + void refresh(); + /** * Update `catalog_simple_name` by looking up the asset's catalog by its ID. * diff --git a/source/blender/blenkernel/intern/asset_catalog.cc b/source/blender/blenkernel/intern/asset_catalog.cc index 9dd5ccdf3cf..a1647426c41 100644 --- a/source/blender/blenkernel/intern/asset_catalog.cc +++ b/source/blender/blenkernel/intern/asset_catalog.cc @@ -67,23 +67,45 @@ AssetCatalogService::AssetCatalogService(const CatalogFilePath &asset_library_ro { } -void AssetCatalogService::tag_has_unsaved_changes() +void AssetCatalogService::tag_has_unsaved_changes(AssetCatalog *edited_catalog) { - has_unsaved_changes_ = true; + if (edited_catalog) { + edited_catalog->flags.has_unsaved_changes = true; + } + BLI_assert(catalog_collection_); + catalog_collection_->has_unsaved_changes_ = true; } void AssetCatalogService::untag_has_unsaved_changes() { - has_unsaved_changes_ = false; + BLI_assert(catalog_collection_); + catalog_collection_->has_unsaved_changes_ = false; + + /* TODO(Sybren): refactor; this is more like "post-write cleanup" than "remove a tag" code. */ + + /* Forget about any deleted catalogs. */ + if (catalog_collection_->catalog_definition_file_) { + for (CatalogID catalog_id : catalog_collection_->deleted_catalogs_.keys()) { + catalog_collection_->catalog_definition_file_->forget(catalog_id); + } + } + catalog_collection_->deleted_catalogs_.clear(); + + /* Mark all remaining catalogs as "without unsaved changes". */ + for (auto &catalog_uptr : catalog_collection_->catalogs_.values()) { + catalog_uptr->flags.has_unsaved_changes = false; + } } bool AssetCatalogService::has_unsaved_changes() const { - return has_unsaved_changes_; + BLI_assert(catalog_collection_); + return catalog_collection_->has_unsaved_changes_; } bool AssetCatalogService::is_empty() const { + BLI_assert(catalog_collection_); return catalog_collection_->catalogs_.is_empty(); } @@ -91,6 +113,10 @@ OwningAssetCatalogMap &AssetCatalogService::get_catalogs() { return catalog_collection_->catalogs_; } +OwningAssetCatalogMap &AssetCatalogService::get_deleted_catalogs() +{ + return catalog_collection_->deleted_catalogs_; +} AssetCatalogDefinitionFile *AssetCatalogService::get_catalog_definition_file() { @@ -155,7 +181,7 @@ AssetCatalogFilter AssetCatalogService::create_catalog_filter( return AssetCatalogFilter(std::move(matching_catalog_ids)); } -void AssetCatalogService::delete_catalog_by_id(const CatalogID catalog_id) +void AssetCatalogService::delete_catalog_by_id_soft(const CatalogID catalog_id) { std::unique_ptr<AssetCatalog> *catalog_uptr_ptr = catalog_collection_->catalogs_.lookup_ptr( catalog_id); @@ -176,6 +202,15 @@ void AssetCatalogService::delete_catalog_by_id(const CatalogID catalog_id) catalog_collection_->catalogs_.remove(catalog_id); } +void AssetCatalogService::delete_catalog_by_id_hard(CatalogID catalog_id) +{ + catalog_collection_->catalogs_.remove(catalog_id); + catalog_collection_->deleted_catalogs_.remove(catalog_id); + + /* TODO(Sybren): adjust this when supporting mulitple CDFs. */ + catalog_collection_->catalog_definition_file_->forget(catalog_id); +} + void AssetCatalogService::prune_catalogs_by_path(const AssetCatalogPath &path) { /* Build a collection of catalog IDs to delete. */ @@ -189,7 +224,7 @@ void AssetCatalogService::prune_catalogs_by_path(const AssetCatalogPath &path) /* Delete the catalogs. */ for (const CatalogID cat_id : catalogs_to_delete) { - this->delete_catalog_by_id(cat_id); + this->delete_catalog_by_id_soft(cat_id); } this->rebuild_tree(); @@ -231,6 +266,7 @@ void AssetCatalogService::update_catalog_path(const CatalogID catalog_id, AssetCatalog *AssetCatalogService::create_catalog(const AssetCatalogPath &catalog_path) { std::unique_ptr<AssetCatalog> catalog = AssetCatalog::from_path(catalog_path); + catalog->flags.has_unsaved_changes = true; /* So we can std::move(catalog) and still use the non-owning pointer: */ AssetCatalog *const catalog_ptr = catalog.get(); @@ -349,7 +385,7 @@ std::unique_ptr<AssetCatalogDefinitionFile> AssetCatalogService::parse_catalog_f return cdf; } -void AssetCatalogService::merge_from_disk_before_writing() +void AssetCatalogService::reload_catalogs() { /* TODO(Sybren): expand to support multiple CDFs. */ AssetCatalogDefinitionFile *const cdf = catalog_collection_->catalog_definition_file_.get(); @@ -357,26 +393,66 @@ void AssetCatalogService::merge_from_disk_before_writing() return; } - auto catalog_parsed_callback = [this](std::unique_ptr<AssetCatalog> catalog) { - const bUUID catalog_id = catalog->catalog_id; + /* Keeps track of the catalog IDs that are seen in the CDF, so that we also know what was deleted + * from the file on disk. */ + Set<CatalogID> cats_in_file; - /* The following two conditions could be or'ed together. Keeping them separated helps when - * adding debug prints, breakpoints, etc. */ - if (catalog_collection_->catalogs_.contains(catalog_id)) { - /* This catalog was already seen, so just ignore it. */ - return false; - } - if (catalog_collection_->deleted_catalogs_.contains(catalog_id)) { - /* This catalog was already seen and subsequently deleted, so just ignore it. */ + auto catalog_parsed_callback = [this, &cats_in_file](std::unique_ptr<AssetCatalog> catalog) { + const CatalogID catalog_id = catalog->catalog_id; + cats_in_file.add(catalog_id); + + const bool should_skip = is_catalog_known_with_unsaved_changes(catalog_id); + if (should_skip) { + /* Do not overwrite unsaved local changes. */ return false; } - /* This is a new catalog, so let's keep it around. */ - catalog_collection_->catalogs_.add_new(catalog_id, std::move(catalog)); + /* This is either a new catalog, or we can just replace the in-memory one with the newly loaded + * one. */ + catalog_collection_->catalogs_.add_overwrite(catalog_id, std::move(catalog)); return true; }; cdf->parse_catalog_file(cdf->file_path, catalog_parsed_callback); + this->purge_catalogs_not_listed(cats_in_file); + this->rebuild_tree(); +} + +void AssetCatalogService::purge_catalogs_not_listed(const Set<CatalogID> &catalogs_to_keep) +{ + Set<CatalogID> cats_to_remove; + for (CatalogID cat_id : this->catalog_collection_->catalogs_.keys()) { + if (catalogs_to_keep.contains(cat_id)) { + continue; + } + if (is_catalog_known_with_unsaved_changes(cat_id)) { + continue; + } + /* This catalog is not on disk, but also not modified, so get rid of it. */ + cats_to_remove.add(cat_id); + } + + for (CatalogID cat_id : cats_to_remove) { + delete_catalog_by_id_hard(cat_id); + } +} + +bool AssetCatalogService::is_catalog_known_with_unsaved_changes(const CatalogID catalog_id) const +{ + if (catalog_collection_->deleted_catalogs_.contains(catalog_id)) { + /* Deleted catalogs are always considered modified, by definition. */ + return true; + } + + const std::unique_ptr<AssetCatalog> *catalog_uptr_ptr = + catalog_collection_->catalogs_.lookup_ptr(catalog_id); + if (!catalog_uptr_ptr) { + /* Catalog is unknown. */ + return false; + } + + const bool has_unsaved_changes = (*catalog_uptr_ptr)->flags.has_unsaved_changes; + return has_unsaved_changes; } bool AssetCatalogService::write_to_disk(const CatalogFilePath &blend_file_path) @@ -386,6 +462,7 @@ bool AssetCatalogService::write_to_disk(const CatalogFilePath &blend_file_path) } untag_has_unsaved_changes(); + rebuild_tree(); return true; } @@ -395,7 +472,7 @@ bool AssetCatalogService::write_to_disk_ex(const CatalogFilePath &blend_file_pat /* - Already loaded a CDF from disk? -> Always write to that file. */ if (catalog_collection_->catalog_definition_file_) { - merge_from_disk_before_writing(); + reload_catalogs(); return catalog_collection_->catalog_definition_file_->write_to_disk(); } @@ -407,7 +484,7 @@ bool AssetCatalogService::write_to_disk_ex(const CatalogFilePath &blend_file_pat const CatalogFilePath cdf_path_to_write = find_suitable_cdf_path_for_writing(blend_file_path); catalog_collection_->catalog_definition_file_ = construct_cdf_in_memory(cdf_path_to_write); - merge_from_disk_before_writing(); + reload_catalogs(); return catalog_collection_->catalog_definition_file_->write_to_disk(); } @@ -507,7 +584,9 @@ void AssetCatalogService::create_missing_catalogs() } /* The parent doesn't exist, so create it and queue it up for checking its parent. */ - create_catalog(parent_path); + AssetCatalog *parent_catalog = create_catalog(parent_path); + parent_catalog->flags.has_unsaved_changes = true; + paths_to_check.insert(parent_path); } @@ -555,6 +634,7 @@ std::unique_ptr<AssetCatalogCollection> AssetCatalogCollection::deep_copy() cons { auto copy = std::make_unique<AssetCatalogCollection>(); + copy->has_unsaved_changes_ = this->has_unsaved_changes_; copy->catalogs_ = copy_catalog_map(this->catalogs_); copy->deleted_catalogs_ = copy_catalog_map(this->deleted_catalogs_); @@ -602,6 +682,10 @@ StringRefNull AssetCatalogTreeItem::get_simple_name() const { return simple_name_; } +bool AssetCatalogTreeItem::has_unsaved_changes() const +{ + return has_unsaved_changes_; +} AssetCatalogPath AssetCatalogTreeItem::catalog_path() const { @@ -668,9 +752,11 @@ 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_) || catalog.flags.is_first_loaded)) { - item.catalog_id_ = catalog.catalog_id; + if (is_last_component) { + if (BLI_uuid_is_nil(item.catalog_id_) || catalog.flags.is_first_loaded) { + item.catalog_id_ = catalog.catalog_id; + } + item.has_unsaved_changes_ = catalog.flags.has_unsaved_changes; } /* Walk further into the path (no matter if a new item was created or not). */ @@ -705,6 +791,16 @@ void AssetCatalogDefinitionFile::add_new(AssetCatalog *catalog) catalogs_.add_new(catalog->catalog_id, catalog); } +void AssetCatalogDefinitionFile::add_overwrite(AssetCatalog *catalog) +{ + catalogs_.add_overwrite(catalog->catalog_id, catalog); +} + +void AssetCatalogDefinitionFile::forget(CatalogID catalog_id) +{ + catalogs_.remove(catalog_id); +} + void AssetCatalogDefinitionFile::parse_catalog_file( const CatalogFilePath &catalog_definition_file_path, AssetCatalogParsedFn catalog_loaded_callback) @@ -742,16 +838,8 @@ void AssetCatalogDefinitionFile::parse_catalog_file( continue; } - if (this->contains(non_owning_ptr->catalog_id)) { - std::cerr << catalog_definition_file_path << ": multiple definitions of catalog " - << non_owning_ptr->catalog_id << " in the same file, using first occurrence." - << std::endl; - /* Don't store 'catalog'; unique_ptr will free its memory. */ - continue; - } - /* The AssetDefinitionFile should include this catalog when writing it back to disk. */ - this->add_new(non_owning_ptr); + this->add_overwrite(non_owning_ptr); } } diff --git a/source/blender/blenkernel/intern/asset_catalog_test.cc b/source/blender/blenkernel/intern/asset_catalog_test.cc index a6e372e4ae6..a2835d7b3b2 100644 --- a/source/blender/blenkernel/intern/asset_catalog_test.cc +++ b/source/blender/blenkernel/intern/asset_catalog_test.cc @@ -35,6 +35,7 @@ const bUUID UUID_ID_WITHOUT_PATH("e34dd2c5-5d2e-4668-9794-1db5de2a4f71"); const bUUID UUID_POSES_ELLIE("df60e1f6-2259-475b-93d9-69a1b4a8db78"); const bUUID UUID_POSES_ELLIE_WHITESPACE("b06132f6-5687-4751-a6dd-392740eb3c46"); const bUUID UUID_POSES_ELLIE_TRAILING_SLASH("3376b94b-a28d-4d05-86c1-bf30b937130d"); +const bUUID UUID_POSES_ELLIE_BACKSLASHES("a51e17ae-34fc-47d5-ba0f-64c2c9b771f7"); 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"); @@ -59,11 +60,21 @@ class TestableAssetCatalogService : public AssetCatalogService { return AssetCatalogService::get_catalog_definition_file(); } + OwningAssetCatalogMap &get_deleted_catalogs() + { + return AssetCatalogService::get_deleted_catalogs(); + } + void create_missing_catalogs() { AssetCatalogService::create_missing_catalogs(); } + void delete_catalog_by_id_soft(CatalogID catalog_id) + { + AssetCatalogService::delete_catalog_by_id_soft(catalog_id); + } + int64_t count_catalogs_with_path(const CatalogFilePath &path) { int64_t count = 0; @@ -305,8 +316,7 @@ TEST_F(AssetCatalogTest, load_catalog_path_backslashes) AssetCatalogService service(asset_library_root_); service.load_from_disk(asset_library_root_ + "/" + "blender_assets.cats.txt"); - const bUUID ELLIE_BACKSLASHES_UUID("a51e17ae-34fc-47d5-ba0f-64c2c9b771f7"); - const AssetCatalog *found_by_id = service.find_catalog(ELLIE_BACKSLASHES_UUID); + const AssetCatalog *found_by_id = service.find_catalog(UUID_POSES_ELLIE_BACKSLASHES); ASSERT_NE(nullptr, found_by_id); EXPECT_EQ(AssetCatalogPath("character/Ellie/backslashes"), found_by_id->path) << "Backslashes should be normalised when loading from disk."; @@ -799,11 +809,11 @@ TEST_F(AssetCatalogTest, delete_catalog_leaf) TEST_F(AssetCatalogTest, delete_catalog_parent_by_id) { - AssetCatalogService service(asset_library_root_); + TestableAssetCatalogService service(asset_library_root_); service.load_from_disk(asset_library_root_ + "/" + "blender_assets.cats.txt"); /* Delete a parent catalog. */ - service.delete_catalog_by_id(UUID_POSES_RUZENA); + service.delete_catalog_by_id_soft(UUID_POSES_RUZENA); /* The catalog should have been deleted, but its children should still be there. */ EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_RUZENA)); @@ -857,7 +867,7 @@ TEST_F(AssetCatalogTest, delete_catalog_write_to_disk) service.load_from_disk(asset_library_root_ + "/" + AssetCatalogService::DEFAULT_CATALOG_FILENAME); - service.delete_catalog_by_id(UUID_POSES_ELLIE); + service.delete_catalog_by_id_soft(UUID_POSES_ELLIE); const CatalogFilePath save_to_path = use_temp_path(); AssetCatalogDefinitionFile *cdf = service.get_catalog_definition_file(); @@ -938,7 +948,9 @@ TEST_F(AssetCatalogTest, merge_catalog_files) * CDF after we loaded it. */ ASSERT_EQ(0, BLI_copy(modified_cdf_file.c_str(), temp_cdf_file.c_str())); - /* Overwrite the modified file. This should merge the on-disk file with our catalogs. */ + /* Overwrite the modified file. This should merge the on-disk file with our catalogs. + * No catalog was marked as "has unsaved changes", so effectively this should not + * save anything, and reload what's on disk. */ service.write_to_disk(cdf_dir + "phony.blend"); AssetCatalogService loaded_service(cdf_dir); @@ -946,16 +958,90 @@ TEST_F(AssetCatalogTest, merge_catalog_files) /* Test that the expected catalogs are there. */ EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE)); - EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE_WHITESPACE)); - EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH)); - EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA)); - EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA_HAND)); EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA_FACE)); EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_AGENT_47)); /* New in the modified file. */ - /* When there are overlaps, the in-memory (i.e. last-saved) paths should win. */ + /* Test that catalogs removed from modified CDF are gone. */ + EXPECT_EQ(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE_WHITESPACE)); + EXPECT_EQ(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH)); + EXPECT_EQ(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA)); + EXPECT_EQ(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA_HAND)); + + /* On-disk changed catalogs should have overridden in-memory not-changed ones. */ const AssetCatalog *ruzena_face = loaded_service.find_catalog(UUID_POSES_RUZENA_FACE); - EXPECT_EQ("character/Ružena/poselib/face", ruzena_face->path.str()); + EXPECT_EQ("character/Ružena/poselib/gezicht", ruzena_face->path.str()); +} + +TEST_F(AssetCatalogTest, refresh_catalogs_with_modification) +{ + const CatalogFilePath cdf_dir = create_temp_path(); + const CatalogFilePath original_cdf_file = asset_library_root_ + "/blender_assets.cats.txt"; + const CatalogFilePath modified_cdf_file = asset_library_root_ + "/catalog_reload_test.cats.txt"; + const CatalogFilePath temp_cdf_file = cdf_dir + "blender_assets.cats.txt"; + ASSERT_EQ(0, BLI_copy(original_cdf_file.c_str(), temp_cdf_file.c_str())); + + /* Load the unmodified, original CDF. */ + TestableAssetCatalogService service(asset_library_root_); + service.load_from_disk(cdf_dir); + + /* === Perfom changes that should be handled gracefully by the reloading code: */ + + /* 1. Delete a subtree of catalogs. */ + service.prune_catalogs_by_id(UUID_POSES_RUZENA); + /* 2. Rename a catalog. */ + service.tag_has_unsaved_changes(service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH)); + service.update_catalog_path(UUID_POSES_ELLIE_TRAILING_SLASH, "character/Ellie/test-value"); + + /* Copy a modified file, to mimic a situation where someone changed the + * CDF after we loaded it. */ + ASSERT_EQ(0, BLI_copy(modified_cdf_file.c_str(), temp_cdf_file.c_str())); + + AssetCatalog *const ellie_whitespace_before_reload = service.find_catalog( + UUID_POSES_ELLIE_WHITESPACE); + + /* This should merge the on-disk file with our catalogs. */ + service.reload_catalogs(); + + /* === Test that the expected catalogs are there. */ + EXPECT_NE(nullptr, service.find_catalog(UUID_POSES_ELLIE)); + EXPECT_NE(nullptr, service.find_catalog(UUID_POSES_ELLIE_WHITESPACE)); + EXPECT_NE(nullptr, service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH)); + + /* === Test changes made to the CDF: */ + + /* Removed from the file. */ + EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_ELLIE_BACKSLASHES)); + /* Added to the file. */ + EXPECT_NE(nullptr, service.find_catalog(UUID_AGENT_47)); + /* Path modified in file. */ + AssetCatalog *ellie_whitespace_after_reload = service.find_catalog(UUID_POSES_ELLIE_WHITESPACE); + EXPECT_EQ(AssetCatalogPath("whitespace from file"), ellie_whitespace_after_reload->path); + EXPECT_NE(ellie_whitespace_after_reload, ellie_whitespace_before_reload); + /* Simple name modified in file. */ + EXPECT_EQ(std::string("Hah simple name after all"), + service.find_catalog(UUID_WITHOUT_SIMPLENAME)->simple_name); + + /* === Test persistence of in-memory changes: */ + + /* This part of the tree we deleted, but still existed in the CDF. They should remain deleted + * after reloading: */ + EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_RUZENA)); + EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_RUZENA_HAND)); + EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_RUZENA_FACE)); + + /* This catalog had its path changed in the test and in the CDF. The change from the test (i.e. + * the in-memory, yet-unsaved change) should persist. */ + EXPECT_EQ(AssetCatalogPath("character/Ellie/test-value"), + service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH)->path); + + /* Overwrite the modified file. This should merge the on-disk file with our catalogs, and clear + * the "has_unsaved_changes" flags. */ + service.write_to_disk(cdf_dir + "phony.blend"); + + EXPECT_FALSE(service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH)->flags.has_unsaved_changes) + << "The catalogs whose path we changed should now be saved"; + EXPECT_TRUE(service.get_deleted_catalogs().is_empty()) + << "Deleted catalogs should not be remembered after saving."; } TEST_F(AssetCatalogTest, backups) @@ -966,9 +1052,9 @@ TEST_F(AssetCatalogTest, backups) ASSERT_EQ(0, BLI_copy(original_cdf_file.c_str(), writable_cdf_file.c_str())); /* Read a CDF, modify, and write it. */ - AssetCatalogService service(cdf_dir); + TestableAssetCatalogService service(cdf_dir); service.load_from_disk(); - service.delete_catalog_by_id(UUID_POSES_ELLIE); + service.delete_catalog_by_id_soft(UUID_POSES_ELLIE); service.write_to_disk(cdf_dir + "phony.blend"); const CatalogFilePath backup_path = writable_cdf_file + "~"; @@ -1100,6 +1186,13 @@ TEST_F(AssetCatalogTest, create_missing_catalogs_after_loading) ASSERT_NE(nullptr, cat_ellie) << "Missing parents should be created immediately after loading."; ASSERT_NE(nullptr, cat_ruzena) << "Missing parents should be created immediately after loading."; + EXPECT_TRUE(cat_char->flags.has_unsaved_changes) + << "Missing parents should be marked as having changes."; + EXPECT_TRUE(cat_ellie->flags.has_unsaved_changes) + << "Missing parents should be marked as having changes."; + EXPECT_TRUE(cat_ruzena->flags.has_unsaved_changes) + << "Missing parents should be marked as having changes."; + AssetCatalogDefinitionFile *cdf = loaded_service.get_catalog_definition_file(); ASSERT_NE(nullptr, cdf); EXPECT_TRUE(cdf->contains(cat_char->catalog_id)) << "Missing parents should be saved to a CDF."; diff --git a/source/blender/blenkernel/intern/asset_library.cc b/source/blender/blenkernel/intern/asset_library.cc index 6c4660ae75d..aae8a289d32 100644 --- a/source/blender/blenkernel/intern/asset_library.cc +++ b/source/blender/blenkernel/intern/asset_library.cc @@ -128,6 +128,11 @@ void AssetLibrary::load(StringRefNull library_root_directory) this->catalog_service = std::move(catalog_service); } +void AssetLibrary::refresh() +{ + this->catalog_service->reload_catalogs(); +} + namespace { void asset_library_on_save_post(struct Main *main, struct PointerRNA **pointers, diff --git a/source/blender/blenkernel/intern/asset_library_service.cc b/source/blender/blenkernel/intern/asset_library_service.cc index aeded0bc128..7cf95ee4cc1 100644 --- a/source/blender/blenkernel/intern/asset_library_service.cc +++ b/source/blender/blenkernel/intern/asset_library_service.cc @@ -77,7 +77,9 @@ AssetLibrary *AssetLibraryService::get_asset_library_on_disk(StringRefNull top_l AssetLibraryPtr *lib_uptr_ptr = on_disk_libraries_.lookup_ptr(top_dir_trailing_slash); if (lib_uptr_ptr != nullptr) { CLOG_INFO(&LOG, 2, "get \"%s\" (cached)", top_dir_trailing_slash.c_str()); - return lib_uptr_ptr->get(); + AssetLibrary *lib = lib_uptr_ptr->get(); + lib->refresh(); + return lib; } AssetLibraryPtr lib_uptr = std::make_unique<AssetLibrary>(); diff --git a/source/blender/blenkernel/intern/asset_library_service_test.cc b/source/blender/blenkernel/intern/asset_library_service_test.cc index ed132d7a8d8..80504bbdc05 100644 --- a/source/blender/blenkernel/intern/asset_library_service_test.cc +++ b/source/blender/blenkernel/intern/asset_library_service_test.cc @@ -30,6 +30,8 @@ namespace blender::bke::tests { +const bUUID UUID_POSES_ELLIE("df60e1f6-2259-475b-93d9-69a1b4a8db78"); + class AssetLibraryServiceTest : public testing::Test { public: CatalogFilePath asset_library_root_; @@ -162,12 +164,18 @@ TEST_F(AssetLibraryServiceTest, has_any_unsaved_catalogs) const bUUID UUID_POSES_ELLIE("df60e1f6-2259-475b-93d9-69a1b4a8db78"); cat_service->prune_catalogs_by_id(UUID_POSES_ELLIE); EXPECT_FALSE(service->has_any_unsaved_catalogs()) - << "Deletion of catalogs via AssetCatalogService should not tag as 'unsaved changes'."; + << "Deletion of catalogs via AssetCatalogService should not automatically tag as 'unsaved " + "changes'."; + + const bUUID UUID_POSES_RUZENA("79a4f887-ab60-4bd4-94da-d572e27d6aed"); + AssetCatalog *cat = cat_service->find_catalog(UUID_POSES_RUZENA); + ASSERT_NE(nullptr, cat) << "Catalog " << UUID_POSES_RUZENA << " should be known"; - cat_service->tag_has_unsaved_changes(); + cat_service->tag_has_unsaved_changes(cat); EXPECT_TRUE(service->has_any_unsaved_catalogs()) << "Tagging as having unsaved changes of a single catalog service should result in unsaved " "changes being reported."; + EXPECT_TRUE(cat->flags.has_unsaved_changes); } TEST_F(AssetLibraryServiceTest, has_any_unsaved_catalogs_after_write) @@ -185,15 +193,19 @@ TEST_F(AssetLibraryServiceTest, has_any_unsaved_catalogs_after_write) << "Unchanged AssetLibrary should have no unsaved catalogs"; AssetCatalogService *const cat_service = lib->catalog_service.get(); - cat_service->tag_has_unsaved_changes(); + AssetCatalog *cat = cat_service->find_catalog(UUID_POSES_ELLIE); + + cat_service->tag_has_unsaved_changes(cat); EXPECT_TRUE(service->has_any_unsaved_catalogs()) << "Tagging as having unsaved changes of a single catalog service should result in unsaved " "changes being reported."; + EXPECT_TRUE(cat->flags.has_unsaved_changes); cat_service->write_to_disk(writable_dir + "dummy_path.blend"); EXPECT_FALSE(service->has_any_unsaved_catalogs()) << "Written AssetCatalogService should have no unsaved catalogs"; + EXPECT_FALSE(cat->flags.has_unsaved_changes); } } // namespace blender::bke::tests diff --git a/source/blender/editors/asset/intern/asset_catalog.cc b/source/blender/editors/asset/intern/asset_catalog.cc index 98f017db20c..af8128ffb8a 100644 --- a/source/blender/editors/asset/intern/asset_catalog.cc +++ b/source/blender/editors/asset/intern/asset_catalog.cc @@ -71,11 +71,11 @@ AssetCatalog *ED_asset_catalog_add(::AssetLibrary *library, AssetCatalogPath fullpath = AssetCatalogPath(parent_path) / unique_name; catalog_service->undo_push(); - catalog_service->tag_has_unsaved_changes(); bke::AssetCatalog *new_catalog = catalog_service->create_catalog(fullpath); if (!new_catalog) { return nullptr; } + catalog_service->tag_has_unsaved_changes(new_catalog); return new_catalog; } @@ -89,7 +89,7 @@ void ED_asset_catalog_remove(::AssetLibrary *library, const CatalogID &catalog_i } catalog_service->undo_push(); - catalog_service->tag_has_unsaved_changes(); + catalog_service->tag_has_unsaved_changes(nullptr); catalog_service->prune_catalogs_by_id(catalog_id); } @@ -103,13 +103,18 @@ void ED_asset_catalog_rename(::AssetLibrary *library, return; } - const AssetCatalog *catalog = catalog_service->find_catalog(catalog_id); + AssetCatalog *catalog = catalog_service->find_catalog(catalog_id); AssetCatalogPath new_path = catalog->path.parent(); new_path = new_path / StringRef(new_name); + if (new_path == catalog->path) { + /* Nothing changed, so don't bother renaming for nothing. */ + return; + } + catalog_service->undo_push(); - catalog_service->tag_has_unsaved_changes(); + catalog_service->tag_has_unsaved_changes(catalog); catalog_service->update_catalog_path(catalog_id, new_path); } diff --git a/source/blender/editors/space_file/asset_catalog_tree_view.cc b/source/blender/editors/space_file/asset_catalog_tree_view.cc index 0eade57934e..7cdf9be689c 100644 --- a/source/blender/editors/space_file/asset_catalog_tree_view.cc +++ b/source/blender/editors/space_file/asset_catalog_tree_view.cc @@ -219,7 +219,12 @@ void AssetCatalogTreeViewItem::on_activate() void AssetCatalogTreeViewItem::build_row(uiLayout &row) { - ui::BasicTreeViewItem::build_row(row); + if (catalog_item_.has_unsaved_changes()) { + uiItemL(&row, (label_ + "*").c_str(), icon); + } + else { + uiItemL(&row, label_.c_str(), icon); + } if (!is_hovered()) { return; |