Cleanup: Anim: Replace use of T_AUTOMERGE #111268

Falk David · 2023-08-18T15:39:12+02:00

Falk David commented

2023-08-18 15:39:12 +02:00

In the animation transform code, T_AUTOMERGE was used
to indicate that the duplicated keys need to be merged.

In perparation of #111051, this no longer makes much sense because
in grease pencil we cannot have two keys in the same channel at the same
frame.

This PR replaces the T_AUTOMERGE flag in the animation code to
T_DUPLICATED_KEYS to indicate that the transform is being called
with duplicated keyframes.

In the animation transform code, `T_AUTOMERGE` was used to indicate that the duplicated keys need to be merged. In perparation of #111051, this no longer makes much sense because in grease pencil we cannot have two keys in the same channel at the same frame. This PR replaces the `T_AUTOMERGE` flag in the animation code to `T_DUPLICATED_KEYS` to indicate that the transform is being called with duplicated keyframes.

Falk David added this to the Animation & Rigging project 2023-08-18 15:39:50 +02:00

Falk David requested review from Campbell Barton 2023-08-18 15:39:56 +02:00

Campbell Barton commented

2023-08-23 08:12:59 +02:00

In general this seems OK, however I think it can be made more obvious what the flag does.

The name T_DUPLICATED_KEYS feels a bit vague, suggest either:

This could be named generically: e.g. T_DUPLICATED and document this is currently only used for key-frames (the same setting could be used elsewhere in the future).

Otherwise, if you feel this should only be used for animation-editors, I'd prefer T_DUPLICATED_KEYFRAMES to make it clear it's for key-frames and not shape-keys or something else.

Accepting, no extra review needed.

In general this seems OK, however I think it can be made more obvious what the flag does. The name `T_DUPLICATED_KEYS` feels a bit vague, suggest either: This could be named generically: e.g. `T_DUPLICATED` and document this is currently only used for key-frames (the same setting could be used elsewhere in the future). Otherwise, if you feel this should only be used for animation-editors, I'd prefer `T_DUPLICATED_KEYFRAMES` to make it clear it's for key-frames and not shape-keys or something else. Accepting, no extra review needed.

Campbell Barton approved these changes 2023-08-23 08:13:09 +02:00

Campbell Barton approved these changes 2023-08-23 08:13:20 +02:00

Falk David force-pushed cleanup-anim-use-of-automerge-flag from f7b4bced47 to 07a6a1015a

2023-09-11 14:43:19 +02:00

Compare

Falk David merged commit 5ed35e5665 into main

2023-09-11 14:43:42 +02:00

Falk David referenced this issue from a commit

2023-09-11 14:43:44 +02:00

Cleanup: Anim: Replace use of `T_AUTOMERGE`

Falk David deleted branch cleanup-anim-use-of-automerge-flag

2023-09-11 14:43:44 +02:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Cleanup: Anim: Replace use of T_AUTOMERGE #111268

Cleanup: Anim: Replace use of `T_AUTOMERGE` #111268