diff options
author | Julian Eisel <julian@blender.org> | 2021-02-08 03:30:49 +0300 |
---|---|---|
committer | Julian Eisel <julian@blender.org> | 2021-02-08 12:32:18 +0300 |
commit | 04be1e9980a1f413446353393e713b4660176ca7 (patch) | |
tree | be49ef79022b5f3a074c25ecac3f2ee635823b84 | |
parent | eb7d9e2a1bba53617b4660cde8409496ba35ef74 (diff) |
Code quality: Refactor asset operators using C++
* Attempt to improve readability by using focused, coherent helper classes.
* Replace ListBase with blender::Vector, which is more efficient and has a
better API.
* Split user reporting from error checking.
* Use namespace (as we usually do for C++ code).
* Remove unused headers
-rw-r--r-- | source/blender/editors/asset/asset_ops.cc | 196 |
1 files changed, 104 insertions, 92 deletions
diff --git a/source/blender/editors/asset/asset_ops.cc b/source/blender/editors/asset/asset_ops.cc index 6c4ef8923a2..8ca1b488a1d 100644 --- a/source/blender/editors/asset/asset_ops.cc +++ b/source/blender/editors/asset/asset_ops.cc @@ -18,124 +18,128 @@ * \ingroup edasset */ -#include <string.h> - -#include "BKE_asset.h" #include "BKE_context.h" #include "BKE_report.h" -#include "BLI_listbase.h" -#include "BLI_string_utils.h" -#include "BLI_utildefines.h" - -#include "DNA_asset_types.h" -#include "DNA_userdef_types.h" +#include "BLI_vector.hh" #include "ED_asset.h" -#include "MEM_guardedalloc.h" - #include "RNA_access.h" -#include "RNA_define.h" #include "WM_api.h" #include "WM_types.h" /* -------------------------------------------------------------------- */ -struct AssetMarkResultStats { - int tot_created; - int tot_already_asset; - ID *last_id; -}; +using PointerRNAVec = blender::Vector<PointerRNA>; -static bool asset_ops_poll(bContext *UNUSED(C)) +static bool asset_operation_poll(bContext * /*C*/) { return U.experimental.use_asset_browser; } /** - * Return the IDs to operate on as list of #CollectionPointerLink links. Needs freeing. + * Return the IDs to operate on as PointerRNA vector. Either a single one ("id" context member) or + * multiple ones ("selected_ids" context member). */ -static ListBase /* CollectionPointerLink */ asset_operation_get_ids_from_context(const bContext *C) +static PointerRNAVec asset_operation_get_ids_from_context(const bContext *C) { - ListBase list = {0}; + PointerRNAVec ids; PointerRNA idptr = CTX_data_pointer_get_type(C, "id", &RNA_ID); - if (idptr.data) { - CollectionPointerLink *ctx_link = (CollectionPointerLink *)MEM_callocN(sizeof(*ctx_link), - __func__); - ctx_link->ptr = idptr; - BLI_addtail(&list, ctx_link); + /* Single ID. */ + ids.append(idptr); } else { + ListBase list; CTX_data_selected_ids(C, &list); + LISTBASE_FOREACH (CollectionPointerLink *, link, &list) { + ids.append(link->ptr); + } + BLI_freelistN(&list); } - return list; + return ids; } -static void asset_mark_for_idptr_list(const bContext *C, - const ListBase /* CollectionPointerLink */ *ids, - struct AssetMarkResultStats *r_stats) -{ - memset(r_stats, 0, sizeof(*r_stats)); +/* -------------------------------------------------------------------- */ - LISTBASE_FOREACH (CollectionPointerLink *, ctx_id, ids) { - BLI_assert(RNA_struct_is_ID(ctx_id->ptr.type)); +class AssetMarkHelper { + public: + void operator()(const bContext &C, PointerRNAVec &ids); - ID *id = (ID *)ctx_id->ptr.data; + void reportResults(ReportList &reports) const; + bool wasSuccessful() const; + + private: + struct Stats { + int tot_created = 0; + int tot_already_asset = 0; + ID *last_id = nullptr; + }; + + Stats stats; +}; + +void AssetMarkHelper::operator()(const bContext &C, PointerRNAVec &ids) +{ + for (PointerRNA &ptr : ids) { + BLI_assert(RNA_struct_is_ID(ptr.type)); + + ID *id = static_cast<ID *>(ptr.data); if (id->asset_data) { - r_stats->tot_already_asset++; + stats.tot_already_asset++; continue; } - if (ED_asset_mark_id(C, id)) { - r_stats->last_id = id; - r_stats->tot_created++; + if (ED_asset_mark_id(&C, id)) { + stats.last_id = id; + stats.tot_created++; } } } -static bool asset_mark_results_report(const struct AssetMarkResultStats *stats, - ReportList *reports) +bool AssetMarkHelper::wasSuccessful() const +{ + return stats.tot_created > 0; +} + +void AssetMarkHelper::reportResults(ReportList &reports) const { /* User feedback on failure. */ - if ((stats->tot_created < 1) && (stats->tot_already_asset > 0)) { - BKE_report(reports, - RPT_ERROR, - "Selected data-blocks are already assets (or do not support use as assets)"); - return false; - } - if (stats->tot_created < 1) { - BKE_report(reports, - RPT_ERROR, - "No data-blocks to create assets for found (or do not support use as assets)"); - return false; + if (!wasSuccessful()) { + if ((stats.tot_already_asset > 0)) { + BKE_report(&reports, + RPT_ERROR, + "Selected data-blocks are already assets (or do not support use as assets)"); + } + else { + BKE_report(&reports, + RPT_ERROR, + "No data-blocks to create assets for found (or do not support use as assets)"); + } } - /* User feedback on success. */ - if (stats->tot_created == 1) { + else if (stats.tot_created == 1) { /* If only one data-block: Give more useful message by printing asset name. */ - BKE_reportf(reports, RPT_INFO, "Data-block '%s' is now an asset", stats->last_id->name + 2); + BKE_reportf(&reports, RPT_INFO, "Data-block '%s' is now an asset", stats.last_id->name + 2); } else { - BKE_reportf(reports, RPT_INFO, "%i data-blocks are now assets", stats->tot_created); + BKE_reportf(&reports, RPT_INFO, "%i data-blocks are now assets", stats.tot_created); } - - return true; } static int asset_mark_exec(bContext *C, wmOperator *op) { - ListBase ids = asset_operation_get_ids_from_context(C); + PointerRNAVec ids = asset_operation_get_ids_from_context(C); - struct AssetMarkResultStats stats; - asset_mark_for_idptr_list(C, &ids, &stats); - BLI_freelistN(&ids); + AssetMarkHelper mark_helper; + mark_helper(*C, ids); + mark_helper.reportResults(*op->reports); - if (!asset_mark_results_report(&stats, op->reports)) { + if (!mark_helper.wasSuccessful()) { return OPERATOR_CANCELLED; } @@ -154,67 +158,75 @@ static void ASSET_OT_mark(wmOperatorType *ot) ot->idname = "ASSET_OT_mark"; ot->exec = asset_mark_exec; - ot->poll = asset_ops_poll; + ot->poll = asset_operation_poll; ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO; } /* -------------------------------------------------------------------- */ -struct AssetClearResultStats { - int tot_removed; - ID *last_id; +class AssetClearHelper { + public: + void operator()(PointerRNAVec &ids); + + void reportResults(ReportList &reports) const; + bool wasSuccessful() const; + + private: + struct Stats { + int tot_cleared = 0; + ID *last_id = nullptr; + }; + + Stats stats; }; -static void asset_clear_from_idptr_list(const ListBase /* CollectionPointerLink */ *ids, - AssetClearResultStats *r_stats) +void AssetClearHelper::operator()(PointerRNAVec &ids) { - memset(r_stats, 0, sizeof(*r_stats)); + for (PointerRNA &ptr : ids) { + BLI_assert(RNA_struct_is_ID(ptr.type)); - LISTBASE_FOREACH (CollectionPointerLink *, ctx_id, ids) { - BLI_assert(RNA_struct_is_ID(ctx_id->ptr.type)); - - ID *id = (ID *)ctx_id->ptr.data; + ID *id = static_cast<ID *>(ptr.data); if (!id->asset_data) { continue; } if (ED_asset_clear_id(id)) { - r_stats->tot_removed++; - r_stats->last_id = id; + stats.tot_cleared++; + stats.last_id = id; } } } -static bool asset_clear_result_report(const AssetClearResultStats *stats, ReportList *reports) - +void AssetClearHelper::reportResults(ReportList &reports) const { - if (stats->tot_removed < 1) { - BKE_report(reports, RPT_ERROR, "No asset data-blocks selected/focused"); - return false; + if (!wasSuccessful()) { + BKE_report(&reports, RPT_ERROR, "No asset data-blocks selected/focused"); } - - if (stats->tot_removed == 1) { + else if (stats.tot_cleared == 1) { /* If only one data-block: Give more useful message by printing asset name. */ BKE_reportf( - reports, RPT_INFO, "Data-block '%s' is no asset anymore", stats->last_id->name + 2); + &reports, RPT_INFO, "Data-block '%s' is no asset anymore", stats.last_id->name + 2); } else { - BKE_reportf(reports, RPT_INFO, "%i data-blocks are no assets anymore", stats->tot_removed); + BKE_reportf(&reports, RPT_INFO, "%i data-blocks are no assets anymore", stats.tot_cleared); } +} - return true; +bool AssetClearHelper::wasSuccessful() const +{ + return stats.tot_cleared > 0; } static int asset_clear_exec(bContext *C, wmOperator *op) { - ListBase ids = asset_operation_get_ids_from_context(C); + PointerRNAVec ids = asset_operation_get_ids_from_context(C); - AssetClearResultStats stats; - asset_clear_from_idptr_list(&ids, &stats); - BLI_freelistN(&ids); + AssetClearHelper clear_helper; + clear_helper(ids); + clear_helper.reportResults(*op->reports); - if (!asset_clear_result_report(&stats, op->reports)) { + if (!clear_helper.wasSuccessful()) { return OPERATOR_CANCELLED; } @@ -233,7 +245,7 @@ static void ASSET_OT_clear(wmOperatorType *ot) ot->idname = "ASSET_OT_clear"; ot->exec = asset_clear_exec; - ot->poll = asset_ops_poll; + ot->poll = asset_operation_poll; ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO; } |