Assets: add global `bke::AssetLibraryService` class
ClosedPublic
Actions

Authored by Sybren A. Stüvel (sybren) on Oct 15 2021, 7:08 PM.

Details

Reviewers

Julian Eisel (Severin)

Maniphest Tasks

T92151: Avoid loosing catalog edits on reload or library change

Commits

rBf9113c4be836: Assets: add global `bke::AssetLibraryService` class

Summary

Add blender::bke::AssetLibraryService class that acts like a blendfile-scoped singleton. It's allocated upon the first call to BKE_asset_library_load and destroyed in the LOAD-PRE handler.

The AssetLibraryService ensures that edits to asset catalogs are not lost when the asset browser editor closes (or even reloads). Instead, the AssetLibrary pointers it owns are kept around as long as the blend file is open.

Outside of the scope of this patch: handling the case when the blend file is saved (or saved to a different directory).

Diff Detail

Repository

rB Blender

Branch

temp-asset-service (branched from master)

Build Status

Buildable 17937
Build 17937: arc lint + arc unit

Event Timeline

Sybren A. Stüvel (sybren) requested review of this revision.Oct 15 2021, 7:08 PM

Sybren A. Stüvel (sybren) created this revision.

Harbormaster completed remote builds in B17886: Diff 43394.Oct 15 2021, 7:08 PM

Sybren A. Stüvel (sybren) added a project: Asset Browser.Oct 15 2021, 7:09 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)

Didn't dig too deep, things look as I'd expect them.

One thing that I'd argue - and we could (re)evaluate and change this later on too - is that this shouldn't be named AssetService. I'd prefer AssetSystem, or similar. "Service" for me is something that does one well defined job for me. It doesn't hold own data storage except some smaller stuff it needs for internal work (or if the service is about actually managing some storage). What we design here is the entry point to something bigger than that, something that can contain many services and storage facilities. For me "System" is just much more appropriate.

source/blender/blenkernel/intern/asset_service.hh
42 ↗	(On Diff #43394)	Instead of "NB", I'd just use the Doxygen `\note`.

This revision is now accepted and ready to land.Oct 18 2021, 11:37 AM

Sybren A. Stüvel (sybren) added a task: T92151: Avoid loosing catalog edits on reload or library change.Oct 18 2021, 11:59 AM

In D12885#338755, @Julian Eisel (Severin) wrote:

One thing that I'd argue - and we could (re)evaluate and change this later on too - is that this shouldn't be named AssetService. I'd prefer AssetSystem, or similar. "Service" for me is something that does one well defined job for me. It doesn't hold own data storage except some smaller stuff it needs for internal work (or if the service is about actually managing some storage). What we design here is the entry point to something bigger than that, something that can contain many services and storage facilities. For me "System" is just much more appropriate.

I agree AssetService is not that good a name. I renamed it to AssetLibraryService, as its purpose is to serve AssetLibrary pointers to the rest of the world. For me the "System" is the bigger whole, comprising of all the asset-related code.

Rename AssetService to AssetLibraryService

Harbormaster completed remote builds in B17935: Diff 43477.Oct 18 2021, 12:30 PM

Update documentation for BKE_asset_library_load

Harbormaster completed remote builds in B17937: Diff 43479.Oct 18 2021, 12:33 PM

Sybren A. Stüvel (sybren) retitled this revision from Assets: add global `bke::AssetService` class to Assets: add global `bke::AssetLibraryService` class.Oct 18 2021, 12:34 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)

Closed by commit rBf9113c4be836: Assets: add global `bke::AssetLibraryService` class (authored by Sybren A. Stüvel (sybren)). · Explain WhyOct 18 2021, 2:23 PM

This revision was automatically updated to reflect the committed changes.

Sybren A. Stüvel (sybren) added a commit: rBf9113c4be836: Assets: add global `bke::AssetLibraryService` class.