Asset Catalogs: undo stack for catalog edits
ClosedPublic
Actions

Authored by Sybren A. Stüvel (sybren) on Oct 11 2021, 5:38 PM.

Details

Reviewers

Julian Eisel (Severin)

Maniphest Tasks

T92047: Undo system for catalog edits

Commits

rBa06435e43a65: Asset Catalogs: undo stack for catalog edits

Summary

Add an undo stack for catalog edits. This only implements the backend, no operators or UI yet.

A lot of this->catalogs_ has now been replaced by catalog_collection_->catalogs_. Things are getting a bit long. What would you think @Julian Eisel (Severin) of having a protected function catalogs() that just returns catalog_collection_->catalogs_? And then of course the same for deleted_catalogs_ and catalog_definition_file_.

Also note that T92114: Refactor AssetCatalogservice exists.

Diff Detail

Repository: rB Blender

Event Timeline

Sybren A. Stüvel (sybren) requested review of this revision.Oct 11 2021, 5:38 PM

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

Harbormaster completed remote builds in B17727: Diff 43144.Oct 11 2021, 5:38 PM

Sybren A. Stüvel (sybren) added a project: Asset Browser.Oct 11 2021, 5:51 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)Oct 11 2021, 5:53 PM

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

What would you think @Julian Eisel (Severin) of having a protected function catalogs() that just returns catalog_collection_->catalogs_? And then of course the same for deleted_catalogs_ and catalog_definition_file_.

I think that's perfectly fine. I like such convenience wrappers that strip away implementation details and let you focus on the bits that matter.

source/blender/blenkernel/BKE_asset_catalog.hh
167–168	Could/should this maybe be moved to a different class, say `AssetCatalogUndoHistory`? The catalog service turns more and more into a god object :) I'd rather have it delegate tasks some more.

Julian Eisel (Severin) removed a subscriber: Julian S (julian).Oct 11 2021, 5:59 PM

Sybren A. Stüvel (sybren) marked an inline comment as done.Oct 11 2021, 6:02 PM

Sybren A. Stüvel (sybren) added inline comments.

source/blender/blenkernel/BKE_asset_catalog.hh
167–168	Yup, that's why I created T92114.

Sybren A. Stüvel (sybren) marked an inline comment as done.Oct 11 2021, 6:02 PM

Sybren A. Stüvel (sybren) added a task: T92047: Undo system for catalog edits.Oct 11 2021, 6:45 PM

In D12825#335918, @Julian Eisel (Severin) wrote:

What would you think @Julian Eisel (Severin) of having a protected function catalogs() that just returns catalog_collection_->catalogs_? And then of course the same for deleted_catalogs_ and catalog_definition_file_.

I think that's perfectly fine. I like such convenience wrappers that strip away implementation details and let you focus on the bits that matter.

I'll keep that for T92114: Refactor AssetCatalogservice; there are some more high-prio tasks I want to finish soon, and those are more important right now ;-)

Sybren A. Stüvel (sybren) mentioned this in T92047: Undo system for catalog edits.Oct 12 2021, 10:11 AM

Julian Eisel (Severin) accepted this revision.Oct 12 2021, 12:01 PM

This revision is now accepted and ready to land.Oct 12 2021, 12:01 PM

Closed by commit rBa06435e43a65: Asset Catalogs: undo stack for catalog edits (authored by Sybren A. Stüvel (sybren)). · Explain WhyOct 12 2021, 12:43 PM

This revision was automatically updated to reflect the committed changes.

Sybren A. Stüvel (sybren) added a commit: rBa06435e43a65: Asset Catalogs: undo stack for catalog edits.