Code Quality Day
https://wiki.blender.org/wiki/Style_Guide/Code_Quality_Day
Large Refactor Projects
Those project will affect a large part of the code base, and likely require several people to work on it.
API Calling Conventions
- Current mirror API is confusing, using the same function with NULL args and a single char.
eg:
ED_mesh_mirror_spatial_table(ob, NULL, NULL, NULL, 'e'); is used to end mirror.
ED_mesh_mirror_spatial_table(ob, NULL, mesh, vec, 'u'); to lookup an item.
ED_mesh_mirror_spatial_table(ob, em, me_eval, NULL, 's'); is used to start mirroring.
Use multiple functions - _create / _destroy / _lookup.
API Organization
- gpuPushAttr/gpuPopAttr: Make this part of GPU_state since it's pushing attributes associated with state, use snake-case naming convention.
File Naming
- Use "utils" instead of "util" (currently both are used 34 util, 60 utils).
eg;
- ED_util.h
- path_util.c
- MOD_util.c
Function Naming
- Use BKE_material_ naming prefix for BKE_material.h T73589
(init_def_material, test_all_objects_materials, give_current_material_p... etc).
- Improve names of BLI_path_util API.
Without reading docstrings the purposes of the following functions is unclear.
- BLI_make_exist
- BLI_make_existing_file
- 'BLI_make_file_string'
Propose to use BLI_path_ prefix (TODO, propose new names).
- Remove uses of the term 'derived' when it doesn't apply to derived-mesh, eg: BKE_object_free_derived_caches BKE_editmesh_free_derivedmesh.
Terminology
- Remove use of the term 'verify' since it's used ambiguously.
- Return an item if it exists, otherwise create a new one (use ensure instead of 'verify').
eg:
- defvert_verify_index -> defvert_ensure_index
- BKE_image_verify_viewer -> BKE_image_ensure_viewer
Most instances can be changed, although there are some exceptions, eg: RNA_define_verify_sdna could be left as is, or renamed to something more explicit.
- Avoid use of the term 'validate' since it's used ambiguously,
from dictionary.com "to make valid; substantiate; confirm".
We have examples that...
- Return false, on invalid data.
- Raise an error and early-exit on invalid data.
- Fix the invalid data, returning true - when the data was modified.
- Correctness check (immutable), (use is_valid instead of 'validate').
- Make something valid (mutable) (use sanitize instead of 'validate').
For data which stores a flag for it's correctness - a function that updates this could be called 'validate', also cache validation sense, in most cases this is not what it's used for.
Note that this is used in many places over the API, although reading over the current usage confirms that this is used confusingly.
- Use shorter terms for function names where there is no ambiguity (which are already used in the API).
- synchronize -> sync
- initialize -> init
- attrib -> attr
- Use _fn as a suffix for callbacks instead of f, cb func,
eg:
- FileList.checkdirf -> check_dir_fn.
- PointerPropertyRNA.typef type_fn
- PointerPropertyRNA.itemf item_fn
- RenderEngine.update_render_passes_cb -> update_render_passes_fn
- bNodeType.freeexecfunc -> free_exec_fn
- Disambiguate term "Line" in BLI_math.h
Currently it's sometimes used for a line-segment (clamped start/end points), other uses extend infinitely in both directions. eg:
closest_to_line_v2 clamps between the two points, where line_point_factor_v3_ex doesn't.
Propose for all clamping versions to use line_segment, or seg in cases where we need to include multiple in a name, eg: isect_seg_seg_v2_point_ex.
Refactoring Files
- Split BKE_boundbox API out of BKE_object (possibly rename BKE_object_boundbox).
- T73630: Remove ID refcounting handling from readfile.c
- D6764: Readfile: refactor/factorize more handling of common ID data.
Documentation
- Document how to add a new property on a high level (DNA, RNA, default values, versioning, preferences, ...).
- T73275: Wiki: Migrating and Integrating Design Documentation