blender
/
blender
136
413
Fork 596
Code Issues 6.3k Pull Requests 771 Projects 19 Wiki Activity

Animation: Paste Keys in Graph Editor with value offset #104512

Merged
Christoph Lendenfeld merged 12 commits from ChrisLend/blender:copy_paste_frame_range into main 2023-02-23 09:47:00 +01:00
Conversation 2 Commits 12 Files Changed 5 +12 -12
5 changed files with 12 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit ee48094cb8 - Show all commits

2
release/datafiles/locale

@ -1 +1 @@
Subproject commit 4331c8e76c2f42b9fd903716c333d6cdeaa5cebd
Subproject commit 8d04896a130c12eb440b7b9ac33bf9dc152506b5

2
release/scripts/addons

@ -1 +1 @@
Subproject commit b3f0ffc587d197b37eac9a1566d1d24b7bee7d9a
Subproject commit 7eef98cbe5899b5b9011603cb59632cb5105de2e

2
release/scripts/addons_contrib

@ -1 +1 @@
Subproject commit 14ab9273409ea0231d08ba6e86fdc73d4e459e99
Subproject commit 2f5e03e764898c249029bc0ade52f7928555b267

16
source/blender/editors/animation/keyframes_general.c
View File

@ -1223,27 +1223,28 @@ static float paste_get_y_offset(bAnimContext *ac,
bAnimListElem *ale,
const eKeyPasteValueOffset value_offset_mode)
{
float offset;
FCurve *fcu = (FCurve *)ale->data;
const float cfra = BKE_scene_frame_get(ac->scene);
switch (value_offset_mode) {
case KEYFRAME_PASTE_VALUE_OFFSET_CURSOR:
case KEYFRAME_PASTE_VALUE_OFFSET_CURSOR: {
SpaceGraph *sipo = (SpaceGraph *)ac->sl;
offset = sipo->cursorVal - aci->bezt[0].vec[1][1];
const float offset = sipo->cursorVal - aci->bezt[0].vec[1][1];
return offset;
}
dr.sybren marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-14 17:32:04 +01:00
Review
Copy Link

Instead of having a variable offset, and having to track how it's changed in the code below, IMO it's better to just replace this

offset = ...;
break;

with

return ...;
Instead of having a variable `offset`, and having to track how it's changed in the code below, IMO it's better to just replace this ```c offset = ...; break; ``` with ```c return ...; ```
Sybren A. Stüvel commented 2023-02-17 15:12:53 +01:00
Review
Copy Link

With this change, you don't even need the top-level variable float offset any more. If you want to keep a variable between the calculation and the return (can aid in debugging the code), use a const float offset = ... constant scoped to the case statement instead. Or just return sipo->cursorVal - aci->bezt[0].vec[1][1]; if you don't need the extra variable.

With this change, you don't even need the top-level variable `float offset` any more. If you want to keep a variable between the calculation and the `return` (can aid in debugging the code), use a `const float offset = ...` constant scoped to the `case` statement instead. Or just `return sipo->cursorVal - aci->bezt[0].vec[1][1];` if you don't need the extra variable.
case KEYFRAME_PASTE_VALUE_OFFSET_CFRA:
case KEYFRAME_PASTE_VALUE_OFFSET_CFRA: {
const float cfra_y = evaluate_fcurve(fcu, cfra);
offset = cfra_y - aci->bezt[0].vec[1][1];
const float offset = cfra_y - aci->bezt[0].vec[1][1];
return offset;
}
case KEYFRAME_PASTE_VALUE_OFFSET_LEFT_KEY: {
bool replace;
const int fcu_index = BKE_fcurve_bezt_binarysearch_index(
fcu->bezt, cfra, fcu->totvert, &replace);
BezTriple left_key = fcu->bezt[max_ii(fcu_index - 1, 0)];
offset = left_key.vec[1][1] - aci->bezt[0].vec[1][1];
const float offset = left_key.vec[1][1] - aci->bezt[0].vec[1][1];
return offset;
}
@ -1252,12 +1253,11 @@ static float paste_get_y_offset(bAnimContext *ac,
const int fcu_index = BKE_fcurve_bezt_binarysearch_index(
fcu->bezt, cfra, fcu->totvert, &replace);
BezTriple right_key = fcu->bezt[min_ii(fcu_index, fcu->totvert - 1)];
offset = right_key.vec[1][1] - aci->bezt[aci->totvert - 1].vec[1][1];
const float offset = right_key.vec[1][1] - aci->bezt[aci->totvert - 1].vec[1][1];
return offset;
}
case KEYFRAME_PASTE_VALUE_OFFSET_NONE:
ChrisLend marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-17 15:13:35 +01:00
Review
Copy Link

You can remove the default:, that way the compiler can complain when a new value was added to the enum, but not handled here.

You can remove the `default:`, that way the compiler can complain when a new value was added to the enum, but not handled here.
default:
break;
}

2
source/tools

@ -1 +1 @@
Subproject commit e133fc08cd3254bb3d3bd1345028c8486700bca4
Subproject commit 27826b4aed138b7e3aed882d0eab96740c3a55c7