blender/blender
170
528
Fork 806
Code Issues 6.6k Pull Requests 956 Projects 21 Wiki Activity

Refactor: combine insert_keyframe() and insert_key_rna() into a single function #122053

Merged
Nathan Vegdahl merged 49 commits from nathanvegdahl/blender:combine_keying_functions into main 2024-06-11 16:43:08 +02:00
Conversation 8 Commits 49 Files Changed 9 +3 -1
1 changed files with 3 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 17aab9da89 - Show all commits

4
source/blender/animrig/ANIM_keyframing.hh
View File

@ -138,7 +138,9 @@ 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 current scene time is used.
* If not given, the evaluation time from `anim_eval_context` is used instead.
nathanvegdahl marked this conversation as resolved Outdated
Sybren A. Stüvel commented 2024-06-11 11:34:49 +02:00
Outdated
Review
Copy Link

"the current scene time" is ill defined here. There is no parameter that gives you this information unambiguously. I think this assumes that anim_eval_context.eval_time is always the scene time (which it may or may not be), but that assumption is not documented here. This means that the reader wouldn't know where this value comes from.

"the current scene time" is ill defined here. There is no parameter that gives you this information unambiguously. I think this assumes that `anim_eval_context.eval_time` is always the scene time (which it may or may not be), but that assumption is not documented here. This means that the reader wouldn't know where this value comes from.
👍 1
Nathan Vegdahl commented 2024-06-11 12:31:37 +02:00
Outdated
Review
Copy Link

Ah! Thanks for catching that. I wrote those docs when I thought AnimationEvalContext represented something more fundamental/core than it actually does.

Ah! Thanks for catching that. I wrote those docs when I thought `AnimationEvalContext` represented something more fundamental/core than it actually does.
* 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.