Page MenuHome

Refactor BKE_bpath module.
ClosedPublic

Authored by Bastien Montagne (mont29) on Nov 26 2021, 3:14 PM.

Details

Summary

The main goal of this refactor is to make BPath module use IDTypeInfo,
and move each ID-specific part of the foreach_path looper into their
own IDTypeInfo struct, using a new foreach_path callback.

Additionally, following improvements/cleanups are included:

  • Attempt to get better, more consistent namings.
    • In particular, move from path_visitor to more standard foreach_path.
  • Update and extend documentation.
    • API doc was moved to header, according to recent discussions on this topic.
  • Remove BKE_bpath_relocate_visitor from API, this is specific callback that belongs in lib_id.c user code.
NOTE: This commit is expected to be 100% non-behavioral-change. This implies that several potential further changes were only noted as comments (like using a more generic solution for lib_id_library_local_paths, addressing inconsistencies like path of packed libraries always being skipped, regardless of the BKE_BPATH_FOREACH_PATH_SKIP_PACKED eBPathForeachFlag flag value, etc.).
NOTE: basic unittests were added to master already in rBdcc500e5a265093bc9cc.

NOTE: Expected review is mostly on BKE_bpath.h, i.e. on changes to the API of the module, rest of this patch is essentially monkey work (renaming, moving code around, cleanups, etc.).

Diff Detail

Repository
rB Blender
Branch
bpath_refactor (branched from master)
Build Status
Buildable 18985
Build 18985: arc lint + arc unit

Event Timeline

Bastien Montagne (mont29) requested review of this revision.Nov 26 2021, 3:14 PM
Bastien Montagne (mont29) created this revision.

The API looks fine to me.

source/blender/blenkernel/BKE_bpath.h
41

typo: proerly

106

The existing code doesn't do it, but as a later improvement it would be a nice to verify this size with a macro + static assert.

This revision is now accepted and ready to land.Nov 26 2021, 6:18 PM

I look at the other code but only superficially, seems fine.

Bastien Montagne (mont29) marked 2 inline comments as done.

Minor updates before commit.

source/blender/blenkernel/BKE_bpath.h
106

Good point, will add to comment.

This whole API could be made safer though, the implicit assumptions about string length are not great at all.

This revision was automatically updated to reflect the committed changes.