Refactor: Make asset catalog class members constant

Some of these members are not expected to change throughout the lifetime
of the class instances, so make them constant and only set them via the
constructor. Making them immutable this way helps making clear which
data needs extra attention for thread safety.
This commit is contained in:
Julian Eisel 2024-02-29 17:30:53 +01:00
parent d5bd5415ec
commit 5bc9434893
5 changed files with 19 additions and 22 deletions

View File

@ -40,17 +40,17 @@ using OwningAssetCatalogMap = Map<CatalogID, std::unique_ptr<AssetCatalog>>;
* directory hierarchy). */ * directory hierarchy). */
class AssetCatalogService { class AssetCatalogService {
std::unique_ptr<AssetCatalogCollection> catalog_collection_; std::unique_ptr<AssetCatalogCollection> catalog_collection_;
/** /**
* Cached catalog tree storage. Lazy-created by #AssetCatalogService::catalog_tree(). * Cached catalog tree storage. Lazy-created by #AssetCatalogService::catalog_tree().
*/ */
std::unique_ptr<AssetCatalogTree> catalog_tree_; std::unique_ptr<AssetCatalogTree> catalog_tree_;
std::mutex catalog_tree_mutex_; std::mutex catalog_tree_mutex_;
CatalogFilePath asset_library_root_;
Vector<std::unique_ptr<AssetCatalogCollection>> undo_snapshots_; Vector<std::unique_ptr<AssetCatalogCollection>> undo_snapshots_;
Vector<std::unique_ptr<AssetCatalogCollection>> redo_snapshots_; Vector<std::unique_ptr<AssetCatalogCollection>> redo_snapshots_;
const CatalogFilePath asset_library_root_;
const bool is_read_only_ = false; const bool is_read_only_ = false;
public: public:
@ -59,9 +59,8 @@ class AssetCatalogService {
struct read_only_tag {}; struct read_only_tag {};
public: public:
AssetCatalogService(); explicit AssetCatalogService(const CatalogFilePath &asset_library_root = {});
explicit AssetCatalogService(read_only_tag); explicit AssetCatalogService(read_only_tag);
explicit AssetCatalogService(const CatalogFilePath &asset_library_root);
/** /**
* Set tag indicating that some catalog modifications are unsaved, which could * Set tag indicating that some catalog modifications are unsaved, which could

View File

@ -33,8 +33,9 @@ namespace blender::asset_system {
const CatalogFilePath AssetCatalogService::DEFAULT_CATALOG_FILENAME = "blender_assets.cats.txt"; const CatalogFilePath AssetCatalogService::DEFAULT_CATALOG_FILENAME = "blender_assets.cats.txt";
AssetCatalogService::AssetCatalogService() AssetCatalogService::AssetCatalogService(const CatalogFilePath &asset_library_root)
: catalog_collection_(std::make_unique<AssetCatalogCollection>()) : catalog_collection_(std::make_unique<AssetCatalogCollection>()),
asset_library_root_(asset_library_root)
{ {
} }
@ -43,12 +44,6 @@ AssetCatalogService::AssetCatalogService(read_only_tag) : AssetCatalogService()
const_cast<bool &>(is_read_only_) = true; const_cast<bool &>(is_read_only_) = true;
} }
AssetCatalogService::AssetCatalogService(const CatalogFilePath &asset_library_root)
: AssetCatalogService()
{
asset_library_root_ = asset_library_root;
}
void AssetCatalogService::tag_has_unsaved_changes(AssetCatalog *edited_catalog) void AssetCatalogService::tag_has_unsaved_changes(AssetCatalog *edited_catalog)
{ {
BLI_assert(!is_read_only_); BLI_assert(!is_read_only_);
@ -372,8 +367,7 @@ void AssetCatalogService::load_single_file(const CatalogFilePath &catalog_defini
std::unique_ptr<AssetCatalogDefinitionFile> AssetCatalogService::parse_catalog_file( std::unique_ptr<AssetCatalogDefinitionFile> AssetCatalogService::parse_catalog_file(
const CatalogFilePath &catalog_definition_file_path) const CatalogFilePath &catalog_definition_file_path)
{ {
auto cdf = std::make_unique<AssetCatalogDefinitionFile>(); auto cdf = std::make_unique<AssetCatalogDefinitionFile>(catalog_definition_file_path);
cdf->file_path = catalog_definition_file_path;
/* TODO(Sybren): this might have to move to a higher level when supporting multiple CDFs. */ /* TODO(Sybren): this might have to move to a higher level when supporting multiple CDFs. */
Set<AssetCatalogPath> seen_paths; Set<AssetCatalogPath> seen_paths;
@ -556,8 +550,7 @@ CatalogFilePath AssetCatalogService::find_suitable_cdf_path_for_writing(
std::unique_ptr<AssetCatalogDefinitionFile> AssetCatalogService::construct_cdf_in_memory( std::unique_ptr<AssetCatalogDefinitionFile> AssetCatalogService::construct_cdf_in_memory(
const CatalogFilePath &file_path) const const CatalogFilePath &file_path) const
{ {
auto cdf = std::make_unique<AssetCatalogDefinitionFile>(); auto cdf = std::make_unique<AssetCatalogDefinitionFile>(file_path);
cdf->file_path = file_path;
for (auto &catalog : catalog_collection_->catalogs_.values()) { for (auto &catalog : catalog_collection_->catalogs_.values()) {
cdf->add_new(catalog.get()); cdf->add_new(catalog.get());

View File

@ -151,6 +151,11 @@ std::unique_ptr<AssetCatalog> AssetCatalogDefinitionFile::parse_catalog_line(con
return std::make_unique<AssetCatalog>(catalog_id, catalog_path.cleanup(), simple_name); return std::make_unique<AssetCatalog>(catalog_id, catalog_path.cleanup(), simple_name);
} }
AssetCatalogDefinitionFile::AssetCatalogDefinitionFile(const CatalogFilePath &file_path)
: file_path(file_path)
{
}
bool AssetCatalogDefinitionFile::write_to_disk() const bool AssetCatalogDefinitionFile::write_to_disk() const
{ {
BLI_assert_msg(!this->file_path.empty(), "Writing to CDF requires its file path to be known"); BLI_assert_msg(!this->file_path.empty(), "Writing to CDF requires its file path to be known");

View File

@ -37,10 +37,10 @@ class AssetCatalogDefinitionFile {
const static std::string VERSION_MARKER; const static std::string VERSION_MARKER;
const static std::string HEADER; const static std::string HEADER;
CatalogFilePath file_path; const CatalogFilePath file_path;
public: public:
AssetCatalogDefinitionFile() = default; AssetCatalogDefinitionFile(const CatalogFilePath &file_path);
/** /**
* Write the catalog definitions to the same file they were read from. * Write the catalog definitions to the same file they were read from.

View File

@ -1116,9 +1116,9 @@ class TestableAssetCatalogCollection : public AssetCatalogCollection {
{ {
return catalog_definition_file_.get(); return catalog_definition_file_.get();
} }
AssetCatalogDefinitionFile *allocate_catalog_definition_file() AssetCatalogDefinitionFile *allocate_catalog_definition_file(StringRef file_path)
{ {
catalog_definition_file_ = std::make_unique<AssetCatalogDefinitionFile>(); catalog_definition_file_ = std::make_unique<AssetCatalogDefinitionFile>(file_path);
return get_catalog_definition_file(); return get_catalog_definition_file();
} }
}; };
@ -1185,8 +1185,8 @@ TEST_F(AssetCatalogTest, cat_collection_deep_copy__nonempty_cdf)
catcoll.get_catalogs().add_new(cat2->catalog_id, std::move(cat2)); catcoll.get_catalogs().add_new(cat2->catalog_id, std::move(cat2));
catcoll.get_deleted_catalogs().add_new(cat3->catalog_id, std::move(cat3)); catcoll.get_deleted_catalogs().add_new(cat3->catalog_id, std::move(cat3));
AssetCatalogDefinitionFile *cdf = catcoll.allocate_catalog_definition_file(); AssetCatalogDefinitionFile *cdf = catcoll.allocate_catalog_definition_file(
cdf->file_path = "path/to/somewhere.cats.txt"; "path/to/somewhere.cats.txt");
cdf->add_new(cat1_ptr); cdf->add_new(cat1_ptr);
cdf->add_new(cat2_ptr); cdf->add_new(cat2_ptr);
cdf->add_new(cat3_ptr); cdf->add_new(cat3_ptr);