Cleanups after informal review with Sybren

source/blender/asset_system/AS_asset_library.hh

@@ -56,7 +56,7 @@ class AssetLibrary {
    */
   std::unique_ptr<AssetStorage> asset_storage_;
 
-  std::function<void()> on_refresh_;
+  std::function<void(AssetLibrary &self)> on_refresh_;
 
   bCallbackFuncStore on_save_callback_store_{};
 

source/blender/asset_system/intern/asset_library.cc

@@ -160,7 +160,7 @@ void AssetLibrary::load_catalogs()
 void AssetLibrary::refresh()
 {
   if (on_refresh_) {
-    on_refresh_();
+    on_refresh_(*this);
   }
 }
 

source/blender/asset_system/intern/asset_library_service.cc

@@ -79,7 +79,8 @@ AssetLibrary *AssetLibraryService::get_asset_library(
       if (!root_path.empty()) {
         return get_asset_library_on_disk(root_path);
       }
-    } break;
+      break;
+    }
   }
 
   return nullptr;
@@ -107,7 +108,7 @@ AssetLibrary *AssetLibraryService::get_asset_library_on_disk(StringRefNull root_
   lib->on_blend_save_handler_register();
   lib->load_catalogs();
   /* Reload catalogs on refresh. */
-  lib->on_refresh_ = [lib]() { lib->catalog_service->reload_catalogs(); };
+  lib->on_refresh_ = [](AssetLibrary &self) { self.catalog_service->reload_catalogs(); };
 
   on_disk_libraries_.add_new(normalized_root_path, std::move(lib_uptr));
   CLOG_INFO(&LOG, 2, "get \"%s\" (loaded)", normalized_root_path.c_str());
@@ -130,6 +131,23 @@ AssetLibrary *AssetLibraryService::get_asset_library_current_file()
   return lib;
 }
 
+static void rebuild_all_library(AssetLibrary &all_library, const bool reload_catalogs)
+{
+  /* Start with empty catalog storage. */
+  all_library.catalog_service = std::make_unique<AssetCatalogService>(
+      AssetCatalogService::read_only_tag());
+
+  AssetLibrary::foreach_loaded(
+      [&](AssetLibrary &nested) {
+        if (reload_catalogs) {
+          nested.catalog_service->reload_catalogs();
+        }
+        all_library.catalog_service->add_from_existing(*nested.catalog_service);
+      },
+      false);
+  all_library.catalog_service->rebuild_tree();
+}
+
 AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain)
 {
   /* (Re-)load all other asset libraries. */
@@ -152,29 +170,14 @@ AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain)
   CLOG_INFO(&LOG, 2, "get all lib (loaded)");
   all_library_ = std::make_unique<AssetLibrary>();
 
-  AssetLibrary &all_library = *all_library_;
-  auto build_catalogs_fn = [&all_library](const bool is_first_load) {
-    /* Start with empty catalog storage. */
-    all_library.catalog_service = std::make_unique<AssetCatalogService>(
-        AssetCatalogService::read_only_tag());
+  /* Don't reload catalogs on this initial read, they've just been loaded above. */
+  rebuild_all_library(*all_library_, /*reload_catlogs=*/false);
 
-    /* (Re-)load catalogs on refresh, and merge them into the all library. */
-    AssetLibrary::foreach_loaded(
-        [&](AssetLibrary &nested) {
-          /* On first load the catalogs were read just above, no need to reload. */
-          if (!is_first_load) {
-            nested.catalog_service->reload_catalogs();
-          }
-          all_library.catalog_service->add_from_existing(*nested.catalog_service);
-        },
-        false);
-    all_library.catalog_service->rebuild_tree();
+  all_library_->on_refresh_ = [](AssetLibrary &all_library) {
+    rebuild_all_library(all_library, /*reload_catalogs=*/true);
   };
 
-  build_catalogs_fn(true);
-  all_library.on_refresh_ = [build_catalogs_fn]() { build_catalogs_fn(false); };
-
-  return &all_library;
+  return all_library_.get();
 }
 
 std::string AssetLibraryService::root_path_from_library_ref(

source/blender/editors/asset/ED_asset_catalog.hh

@@ -7,7 +7,8 @@
  * catalog API provides (which this uses internally).
  *
  * Functions can be expected to not perform any change when #ED_asset_catalogs_read_only() returns
- * true. The caller should check.
+ * true. Generally UI code should disable such functionality in this case, so these functions are
+ * not called at all.
  *
  * Note that `ED_asset_catalog.h` is part of this API.
  */

source/blender/editors/asset/ED_asset_list.hh

@@ -21,10 +21,13 @@ class AssetLibrary;
 
 /**
  * Get the asset library being read into an asset-list and identified using \a library_reference.
- * \note The asset library may be loaded asynchronously, so this may return null until it becomes
- *       available.
+ *
+ * \note The asset library may be allocated and loaded asynchronously, so it's not available right
+ *       after fetching, and this function will return null. The asset list code sends `NC_ASSET |
+ *       ND_ASSET_LIST_READING` notifiers until loading is done, they can be used to continuously
+ *       call this function to retrieve the asset library once available.
  */
-blender::asset_system::AssetLibrary *ED_assetlist_library_get(
+blender::asset_system::AssetLibrary *ED_assetlist_library_get_once_available(
     const AssetLibraryReference &library_reference);
 
 /* Can return false to stop iterating. */

source/blender/editors/asset/intern/asset_list.cc

@@ -461,7 +461,7 @@ void ED_assetlist_iterate(const AssetLibraryReference &library_reference, AssetL
   }
 }
 
-asset_system::AssetLibrary *ED_assetlist_library_get(
+asset_system::AssetLibrary *ED_assetlist_library_get_once_available(
     const AssetLibraryReference &library_reference)
 {
   const AssetList *list = AssetListStorage::lookup_list(library_reference);

source/blender/editors/space_node/add_menu_assets.cc

@@ -87,7 +87,7 @@ static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node
   ED_assetlist_storage_fetch(&all_library_ref, &C);
   ED_assetlist_ensure_previews_job(&all_library_ref, &C);
 
-  asset_system::AssetLibrary *all_library = ED_assetlist_library_get(all_library_ref);
+  asset_system::AssetLibrary *all_library = ED_assetlist_library_get_once_available(all_library_ref);
   if (!all_library) {
     return {};
   }