From 72aaecae51d8e59ac28fa49f81d33a7a480e49ea Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Tue, 27 Aug 2024 17:44:50 +0200 Subject: [PATCH] Cleanup: improve some code documentation in the animation code --- source/blender/animrig/ANIM_action.hh | 18 ++++++++++-------- source/blender/animrig/ANIM_keyframing.hh | 9 +++++++-- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/source/blender/animrig/ANIM_action.hh b/source/blender/animrig/ANIM_action.hh index 30905c7a9a9..cffc7b8baf9 100644 --- a/source/blender/animrig/ANIM_action.hh +++ b/source/blender/animrig/ANIM_action.hh @@ -1018,22 +1018,24 @@ Vector fcurves_all(const Action &action); Vector fcurves_all(Action &action); /** - * Get (or add relevant data to be able to do so) an F-Curve from the given - * Action. This assumes that all the destinations are valid. + * Find or create an F-Curve on the given action that matches the given fcurve + * descriptor. * - * NOTE: this function is primarily intended for use with legacy actions, but - * for reasons of expedience it now also works with layered actions under the + * This function is primarily intended for use with legacy actions, but for + * reasons of expedience it now also works with layered actions under the * following limited circumstances: `ptr` must be non-null and must have an - * `owner_id` that already uses `act`. Otherwise this function will return - * nullptr for layered actions. See the comments in the implementation for more - * details. + * `owner_id` that already uses `act`. See the comments in the implementation + * for more details. * * \note This function also ensures that dependency graph relationships are * rebuilt. This is necessary when adding a new F-Curve, as a * previously-unanimated depsgraph component may become animated now. * * \param ptr: RNA pointer for the struct the fcurve is being looked up/created - * for. For legacy actions this is optional and may be null. + * for. For legacy actions this is optional and may be null, but if provided is + * used to do things like set the fcurve color properly. For layered actions + * this parameter is required, and is used to create and assign an appropriate + * slot if needed when creating the fcurve. * * \param fcurve_descriptor: description of the fcurve to lookup/create. Note * that this is *not* relative to `ptr` (e.g. if `ptr` is not an ID). It should diff --git a/source/blender/animrig/ANIM_keyframing.hh b/source/blender/animrig/ANIM_keyframing.hh index b7d58e38533..9fcd980766d 100644 --- a/source/blender/animrig/ANIM_keyframing.hh +++ b/source/blender/animrig/ANIM_keyframing.hh @@ -126,6 +126,13 @@ void update_autoflags_fcurve_direct(FCurve *fcu, PropertyRNA *prop); * animation data (AnimData, Action, ...) is created if it doesn't already * exist. * + * Note that this function was created as part of an ongoing refactor by merging + * two other functions that were *almost* identical to each other. There are + * still things left over from that which can and should be improved (such as + * the partially redundant `scene_frame` and `anim_eval_context`parameters). + * Additionally, it's a bit of a mega-function now, and can probably be stripped + * down to a clearer core functionality. + * * \param struct_pointer: RNA pointer to the struct to be keyed. This is often * an ID, but not necessarily. For example, pose bones are also common. Note * that if you have an `ID` and want to pass it here for keying, you can create @@ -142,8 +149,6 @@ void update_autoflags_fcurve_direct(FCurve *fcu, PropertyRNA *prop); * \param scene_frame: the frame to insert the keys at. This is in scene time, * not NLA mapped (NLA mapping is already handled internally by this function). * If not given, the evaluation time from `anim_eval_context` is used instead. - * TODO: this redundancy between `scene_frame` and `anim_eval_context` should - * be eliminated. We shouldn't need both! * * \returns A summary of the successful and failed keyframe insertions, with * reasons for the failures.