Cancel Equalize Handles & Snap Keys when no control points are selected
ClosedPublic
Actions

Authored by Colin Basnett (cmbasnett) on Nov 4 2022, 6:52 PM.

Details

Reviewers

Sybren A. Stüvel (sybren)

Commits

rB3836b6ff8cba: Cancel Equalize Handles & Snap Keys when no control points are selected

Summary

The Equalize Handles and Snap Keys operators would allow the user to
invoke them successfully even when they would have no effect due to
there not being any selected control points.

This patch makes it so that an error is displayed when these operators
are invoked with no control points are selected.

The reason this is in the invoke function is because it would be too
expensive to run this check in the poll function since it requires a
linear search through all the keys of all the visible F-Curves.

Diff Detail

Repository

rB Blender

Branch

arcpatch-D16390 (branched from master)

Build Status

Buildable 24555
Build 24555: arc lint + arc unit

Event Timeline

Colin Basnett (cmbasnett) requested review of this revision.Nov 4 2022, 6:52 PM

Colin Basnett (cmbasnett) created this revision.

Harbormaster completed remote builds in B24493: Diff 57257.Nov 4 2022, 6:52 PM

Colin Basnett (cmbasnett) added a reviewer: Sybren A. Stüvel (sybren).Nov 4 2022, 6:53 PM

Colin Basnett (cmbasnett) added a project: Animation & Rigging.

Colin Basnett (cmbasnett) edited the summary of this revision. (Show Details)Nov 4 2022, 6:57 PM

Colin Basnett (cmbasnett) edited the summary of this revision. (Show Details)Nov 4 2022, 6:59 PM

Nice find :)

I agree with putting this check into the invoke() function, because it's quite a heavy operation to go over each and every possible key. You might want to put this into the patch description (and thus the commit message), so that this motivation is explicit.

Just some minor notes, no need to re-review after addressing those.

source/blender/blenkernel/BKE_fcurve.h
436	I know it would be inconsistent with the rest of the code, but I'd still like to have as much `const` as possible. `const struct FCurve *fcu` would be better here IMO.
source/blender/editors/space_graph/graph_edit.c
2356	Since the purpose of this function is to determine this value, and this loop doesn't set it back to `false`, you can add a `break` here.
2365–2367	This seems to be formatted incorrectly. Either configure your IDE to autoformat with clang-format, or run `make format`.