blender/blender-addons
19
43
Fork 96
Code Issues 634 Pull Requests 27 Projects 1 Releases Wiki Activity

WIP: Animation: operators to update the pose library #104673

Draft
Sybren A. Stüvel wants to merge 10 commits from dr.sybren/blender-addons:pr/poselib-replace-pose into main
pull from: dr.sybren/blender-addons:pr/poselib-replace-pose
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 1 Commits 10 Files Changed 3 +3 -4
1 changed files with 3 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 59ca796663 - Show all commits

7
pose_library/operators.py
View File

@ -329,16 +329,15 @@ class POSELIB_OT_update_pose_asset(Operator):
def _update_existing_fcurve(action: Action, fcu_existing: FCurve, fcu_to_read: FCurve) -> bool: def _update_existing_fcurve(action: Action, fcu_existing: FCurve, fcu_to_read: FCurve) -> bool:
# Update the exsting FCurve. This assumes that it is a valid # Update the exsting FCurve. This assumes that it is a valid
# pose asset, i.e. that the FCurve has a single key. # pose asset, i.e. that the FCurve has a single key.
kp = fcu_existing.keyframe_points[0] keyframe = fcu_existing.keyframe_points[0]
old_value = kp.co.y
dr.sybren marked this conversation as resolved
Nathan Vegdahl commented 2023-06-08 14:40:50 +02:00
Review
Copy Link

Unimportant nit: it doesn't seem like the old_value temporary is necessary.

(fcu_value, by contrast, I think aids readability, since it's short-handing something that's rather long to read.)

Not a big deal either way, though.

Unimportant nit: it doesn't seem like the `old_value` temporary is necessary. (`fcu_value`, by contrast, I think aids readability, since it's short-handing something that's rather long to read.) Not a big deal either way, though.
Sybren A. Stüvel commented 2023-06-08 15:22:50 +02:00
Review
Copy Link

It's indeed not really necessary, but to me it makes the if old_value == fcu_value: read nicer. That can be resolved by renaming kp to keyframe and it overall being less cryptic.

It's indeed not really necessary, but to me it makes the `if old_value == fcu_value:` read nicer. That can be resolved by renaming `kp` to `keyframe` and it overall being less cryptic.
👍 1
# Don't bother updating the FCurve if the value was unchanged. # Don't bother updating the FCurve if the value was unchanged.
fcu_value = fcu_to_read.keyframe_points[0].co.y fcu_value = fcu_to_read.keyframe_points[0].co.y
if abs(old_value - fcu_value) < 0.0001: if keyframe.co.y == fcu_value:
return False return False
dr.sybren marked this conversation as resolved
Nathan Vegdahl commented 2023-06-08 14:35:06 +02:00
Review
Copy Link

Do we expect there to be floating point error involved here? I wouldn't think so, but I could very well be missing something.

But if not, I think it makes more sense to just do a straight == comparison, rather than having an arbitrary epsilon.

Do we expect there to be floating point error involved here? I wouldn't think so, but I could very well be missing something. But if not, I think it makes more sense to just do a straight `==` comparison, rather than having an arbitrary epsilon.
# Use co_ui to move the handles along with the value. # Use co_ui to move the handles along with the value.
kp.co_ui.y = fcu_value keyframe.co_ui.y = fcu_value
fcu_existing.update() fcu_existing.update()
return True return True