Cleanup: Better follow class layout style guide in asset headers

Move "using" declarations and member variables to the top of the class.
See https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Class_Layout.

Changes access specifiers of some variables from public/protected to
private, there was no point in not having them private.

source/blender/asset_system/AS_asset_catalog.hh

@@ -38,6 +38,13 @@ using OwningAssetCatalogMap = Map<CatalogID, std::unique_ptr<AssetCatalog>>;
 /* Manages the asset catalogs of a single asset library (i.e. of catalogs defined in a single
  * directory hierarchy). */
 class AssetCatalogService {
+  std::unique_ptr<AssetCatalogCollection> catalog_collection_;
+  std::unique_ptr<AssetCatalogTree> catalog_tree_;
+  CatalogFilePath asset_library_root_;
+
+  Vector<std::unique_ptr<AssetCatalogCollection>> undo_snapshots_;
+  Vector<std::unique_ptr<AssetCatalogCollection>> redo_snapshots_;
+
  public:
   static const CatalogFilePath DEFAULT_CATALOG_FILENAME;
 
@@ -164,13 +171,6 @@ class AssetCatalogService {
   bool is_redo_possbile() const;
 
  protected:
-  std::unique_ptr<AssetCatalogCollection> catalog_collection_;
-  std::unique_ptr<AssetCatalogTree> catalog_tree_;
-  CatalogFilePath asset_library_root_;
-
-  Vector<std::unique_ptr<AssetCatalogCollection>> undo_snapshots_;
-  Vector<std::unique_ptr<AssetCatalogCollection>> redo_snapshots_;
-
   void load_directory_recursive(const CatalogFilePath &directory_path);
   void load_single_file(const CatalogFilePath &catalog_definition_file_path);
 
@@ -246,15 +246,6 @@ class AssetCatalogService {
  * struct.
  */
 class AssetCatalogCollection {
-  friend AssetCatalogService;
-
- public:
-  AssetCatalogCollection() = default;
-  AssetCatalogCollection(const AssetCatalogCollection &other) = delete;
-  AssetCatalogCollection(AssetCatalogCollection &&other) noexcept = default;
-
-  std::unique_ptr<AssetCatalogCollection> deep_copy() const;
-
  protected:
   /** All catalogs known, except the known-but-deleted ones. */
   OwningAssetCatalogMap catalogs_;
@@ -271,6 +262,16 @@ class AssetCatalogCollection {
   /** Whether any of the catalogs have unsaved changes. */
   bool has_unsaved_changes_ = false;
 
+  friend AssetCatalogService;
+
+ public:
+  AssetCatalogCollection() = default;
+  AssetCatalogCollection(const AssetCatalogCollection &other) = delete;
+  AssetCatalogCollection(AssetCatalogCollection &&other) noexcept = default;
+
+  std::unique_ptr<AssetCatalogCollection> deep_copy() const;
+
+ protected:
   static OwningAssetCatalogMap copy_catalog_map(const OwningAssetCatalogMap &orig);
 };
 
@@ -278,6 +279,11 @@ class AssetCatalogCollection {
  * Only contains non-owning pointers to the #AssetCatalog instances, so ensure the lifetime of this
  * class is shorter than that of the #`AssetCatalog`s themselves. */
 class AssetCatalogDefinitionFile {
+ protected:
+  /* Catalogs stored in this file. They are mapped by ID to make it possible to query whether a
+   * catalog is already known, without having to find the corresponding `AssetCatalog*`. */
+  Map<CatalogID, AssetCatalog *> catalogs_;
+
  public:
   /* For now this is the only version of the catalog definition files that is supported.
    * Later versioning code may be added to handle older files. */
@@ -290,6 +296,7 @@ class AssetCatalogDefinitionFile {
 
   CatalogFilePath file_path;
 
+ public:
   AssetCatalogDefinitionFile() = default;
 
   /**
@@ -323,10 +330,6 @@ class AssetCatalogDefinitionFile {
       const OwningAssetCatalogMap &catalogs, const OwningAssetCatalogMap &deleted_catalogs) const;
 
  protected:
-  /* Catalogs stored in this file. They are mapped by ID to make it possible to query whether a
-   * catalog is already known, without having to find the corresponding `AssetCatalog*`. */
-  Map<CatalogID, AssetCatalog *> catalogs_;
-
   bool parse_version_line(StringRef line);
   std::unique_ptr<AssetCatalog> parse_catalog_line(StringRef line);
 
@@ -342,9 +345,6 @@ class AssetCatalogDefinitionFile {
  * catalog hierarchy. */
 class AssetCatalog {
  public:
-  AssetCatalog() = default;
-  AssetCatalog(CatalogID catalog_id, const AssetCatalogPath &path, const std::string &simple_name);
-
   CatalogID catalog_id;
   AssetCatalogPath path;
   /**
@@ -376,6 +376,10 @@ class AssetCatalog {
     bool has_unsaved_changes = false;
   } flags;
 
+ public:
+  AssetCatalog() = default;
+  AssetCatalog(CatalogID catalog_id, const AssetCatalogPath &path, const std::string &simple_name);
+
   /**
    * Create a new Catalog with the given path, auto-generating a sensible catalog simple-name.
    *
@@ -420,6 +424,11 @@ using MutableAssetCatalogOrderedSet = std::set<AssetCatalog *, AssetCatalogLessT
  * \see AssetCatalogService::create_catalog_filter()
  */
 class AssetCatalogFilter {
+  const Set<CatalogID> matching_catalog_ids;
+  const Set<CatalogID> known_catalog_ids;
+
+  friend AssetCatalogService;
+
  public:
   bool contains(CatalogID asset_catalog_id) const;
 
@@ -427,10 +436,6 @@ class AssetCatalogFilter {
   bool is_known(CatalogID asset_catalog_id) const;
 
  protected:
-  friend AssetCatalogService;
-  const Set<CatalogID> matching_catalog_ids;
-  const Set<CatalogID> known_catalog_ids;
-
   explicit AssetCatalogFilter(Set<CatalogID> &&matching_catalog_ids,
                               Set<CatalogID> &&known_catalog_ids);
 };

source/blender/asset_system/AS_asset_catalog_path.hh

@@ -34,7 +34,6 @@ namespace blender::asset_system {
 class AssetCatalogPath {
   friend std::ostream &operator<<(std::ostream &stream, const AssetCatalogPath &path_to_append);
 
- private:
   /**
    * The path itself, such as "Agents/Secret/327".
    */

source/blender/asset_system/AS_asset_catalog_tree.hh

@@ -22,14 +22,30 @@ namespace blender::asset_system {
  * Representation of a catalog path in the #AssetCatalogTree.
  */
 class AssetCatalogTreeItem {
-  friend class AssetCatalogTree;
-
  public:
   /** Container for child items. Uses a #std::map to keep items ordered by their name (i.e. their
    * last catalog component). */
   using ChildMap = std::map<std::string, AssetCatalogTreeItem>;
   using ItemIterFn = FunctionRef<void(AssetCatalogTreeItem &)>;
 
+ private:
+  /** Child tree items, ordered by their names. */
+  ChildMap children_;
+  /** The user visible name of this component. */
+  CatalogPathComponent name_;
+  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). */
+  const AssetCatalogTreeItem *parent_ = nullptr;
+
+  friend class AssetCatalogTree;
+
+ public:
   AssetCatalogTreeItem(StringRef name,
                        CatalogID catalog_id,
                        StringRef simple_name,
@@ -49,21 +65,6 @@ class AssetCatalogTreeItem {
    * children. */
   void foreach_child(ItemIterFn callback);
 
- protected:
-  /** Child tree items, ordered by their names. */
-  ChildMap children_;
-  /** The user visible name of this component. */
-  CatalogPathComponent name_;
-  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). */
-  const AssetCatalogTreeItem *parent_ = nullptr;
-
  private:
   static void foreach_item_recursive(ChildMap &children_, ItemIterFn callback);
 };
@@ -72,6 +73,9 @@ class AssetCatalogTree {
   using ChildMap = AssetCatalogTreeItem::ChildMap;
   using ItemIterFn = AssetCatalogTreeItem::ItemIterFn;
 
+  /** Child tree items, ordered by their names. */
+  ChildMap root_items_;
+
  public:
   /** Ensure an item representing \a path is in the tree, adding it if necessary. */
   void insert_item(const AssetCatalog &catalog);
@@ -85,10 +89,6 @@ class AssetCatalogTree {
 
   AssetCatalogTreeItem *find_item(const AssetCatalogPath &path);
   AssetCatalogTreeItem *find_root_item(const AssetCatalogPath &path);
-
- protected:
-  /** Child tree items, ordered by their names. */
-  ChildMap root_items_;
 };
 
 }  // namespace blender::asset_system

source/blender/asset_system/AS_asset_library.hh

@@ -33,13 +33,32 @@ class AssetStorage;
  * The asset library contains catalogs and storage for asset representations. It could be extended
  * to also include asset indexes and more.
  */
-struct AssetLibrary {
+class AssetLibrary {
+  bCallbackFuncStore on_save_callback_store_{};
+
+  /** Storage for assets (better said their representations) that are considered to be part of this
+   * library. Assets are not automatically loaded into this when loading an asset library. Assets
+   * have to be loaded externally and added to this storage via #add_external_asset() or
+   * #add_local_id_asset(). So this really is arbitrary storage as far as #AssetLibrary is
+   * concerned (allowing the API user to manage partial library storage and partial loading, so
+   * only relevant parts of a library are kept in memory).
+   *
+   * For now, multiple parts of Blender just keep adding their own assets to this storage. E.g.
+   * multiple asset browsers might load multiple representations for the same asset into this.
+   * Currently there is just no way to properly identify assets, or keep track of which assets are
+   * already in memory and which not. Neither do we keep track of how many parts of Blender are
+   * using an asset or an asset library, which is needed to know when assets can be freed.
+   */
+  std::unique_ptr<AssetStorage> asset_storage_;
+
+ public:
   /* Controlled by #ED_asset_catalogs_set_save_catalogs_when_file_is_saved,
    * for managing the "Save Catalog Changes" in the quit-confirmation dialog box. */
   static bool save_catalogs_when_file_is_saved;
 
   std::unique_ptr<AssetCatalogService> catalog_service;
 
+ public:
   AssetLibrary();
   ~AssetLibrary();
 
@@ -87,23 +106,6 @@ struct AssetLibrary {
   void on_blend_save_post(Main *bmain, PointerRNA **pointers, int num_pointers);
 
  private:
-  bCallbackFuncStore on_save_callback_store_{};
-
-  /** Storage for assets (better said their representations) that are considered to be part of this
-   * library. Assets are not automatically loaded into this when loading an asset library. Assets
-   * have to be loaded externally and added to this storage via #add_external_asset() or
-   * #add_local_id_asset(). So this really is arbitrary storage as far as #AssetLibrary is
-   * concerned (allowing the API user to manage partial library storage and partial loading, so
-   * only relevant parts of a library are kept in memory).
-   *
-   * For now, multiple parts of Blender just keep adding their own assets to this storage. E.g.
-   * multiple asset browsers might load multiple representations for the same asset into this.
-   * Currently there is just no way to properly identify assets, or keep track of which assets are
-   * already in memory and which not. Neither do we keep track of how many parts of Blender are
-   * using an asset or an asset library, which is needed to know when assets can be freed.
-   */
-  std::unique_ptr<AssetStorage> asset_storage_;
-
   std::optional<int> find_asset_index(const AssetRepresentation &asset);
 };
 

source/blender/asset_system/AS_asset_representation.hh

@@ -22,9 +22,6 @@ struct ID;
 namespace blender::asset_system {
 
 class AssetRepresentation {
-  friend struct AssetLibrary;
-  friend class AssetStorage;
-
   struct ExternalAsset {
     std::string name;
     std::unique_ptr<AssetMetaData> metadata_ = nullptr;
@@ -39,6 +36,9 @@ class AssetRepresentation {
     ID *local_asset_id_ = nullptr; /* Non-owning. */
   };
 
+  friend struct AssetLibrary;
+  friend class AssetStorage;
+
  public:
   /** Constructs an asset representation for an external ID. The asset will not be editable. */
   explicit AssetRepresentation(StringRef name, std::unique_ptr<AssetMetaData> metadata);

source/blender/asset_system/intern/asset_library.cc

@@ -121,8 +121,8 @@ void AS_asset_library_remap_ids(const IDRemapper *mappings)
 namespace blender::asset_system {
 
 AssetLibrary::AssetLibrary()
-    : catalog_service(std::make_unique<AssetCatalogService>()),
-      asset_storage_(std::make_unique<AssetStorage>())
+    : asset_storage_(std::make_unique<AssetStorage>()),
+      catalog_service(std::make_unique<AssetCatalogService>())
 {
 }
 

source/blender/asset_system/intern/asset_library_service.hh

@@ -31,9 +31,22 @@ namespace blender::asset_system {
  *   loaded from a file on disk).
  */
 class AssetLibraryService {
- public:
   using AssetLibraryPtr = std::unique_ptr<AssetLibrary>;
 
+  static std::unique_ptr<AssetLibraryService> instance_;
+
+  /* Mapping absolute path of the library's top-level directory to the AssetLibrary instance. */
+  Map<std::string, AssetLibraryPtr> on_disk_libraries_;
+  /** Library without a known path, i.e. the "Current File" library if the file isn't saved yet. If
+   * the file was saved, a valid path for the library can be determined and #on_disk_libraries_
+   * above should be used. */
+  AssetLibraryPtr current_file_library_;
+
+  /* Handlers for managing the life cycle of the AssetLibraryService instance. */
+  bCallbackFuncStore on_load_callback_store_;
+  static bool atexit_handler_registered_;
+
+ public:
   AssetLibraryService() = default;
   ~AssetLibraryService() = default;
 
@@ -60,19 +73,6 @@ class AssetLibraryService {
   void foreach_loaded_asset_library(FunctionRef<void(AssetLibrary &)> fn) const;
 
  protected:
-  static std::unique_ptr<AssetLibraryService> instance_;
-
-  /* Mapping absolute path of the library's top-level directory to the AssetLibrary instance. */
-  Map<std::string, AssetLibraryPtr> on_disk_libraries_;
-  /** Library without a known path, i.e. the "Current File" library if the file isn't saved yet. If
-   * the file was saved, a valid path for the library can be determined and #on_disk_libraries_
-   * above should be used. */
-  AssetLibraryPtr current_file_library_;
-
-  /* Handlers for managing the life cycle of the AssetLibraryService instance. */
-  bCallbackFuncStore on_load_callback_store_;
-  static bool atexit_handler_registered_;
-
   /** Allocate a new instance of the service and assign it to `instance_`. */
   static void allocate_service_instance();