Create generic modal functions from GRAPH_OT_decimate
ClosedPublic
Actions

Authored by Christoph Lendenfeld (ChrisLend) on Oct 23 2020, 12:30 PM.

Details

Reviewers

Wayde Moss (GuiltyGhost)
Sybren A. Stüvel (sybren)

Maniphest Tasks

T81785: Implementation: Modal Key Manipulation Operators

Commits

rBd5920744f442: Create generic modal functions from GRAPH_OT_decimate

Summary

This patch extracts the modal functions from GRAPH_OT_decimate and makes them generic so they can be reused by future operators.

Note that to make that work, tGraphSliderOp now contains a function pointer that is used to point to the specific update function of the operator.

Diff Detail

Repository: rB Blender

Event Timeline

Christoph Lendenfeld (ChrisLend) requested review of this revision.Oct 23 2020, 12:30 PM

Christoph Lendenfeld (ChrisLend) created this revision.

Christoph Lendenfeld (ChrisLend) edited the summary of this revision. (Show Details)

Christoph Lendenfeld (ChrisLend) added a task: T81785: Implementation: Modal Key Manipulation Operators.Oct 23 2020, 12:33 PM

Christoph Lendenfeld (ChrisLend) mentioned this in T81785: Implementation: Modal Key Manipulation Operators.

Christoph Lendenfeld (ChrisLend) mentioned this in D9317: Extract functions from GRAPH_OT_decimate.Oct 23 2020, 12:37 PM

Christoph Lendenfeld (ChrisLend) mentioned this in D9314: Generic Slider implementation.

rebase

Split the patch into smaller simpler ones. There are renames (still with a missed dgo->gso), moved code with minor changes, refactored graphkeys_decimate_invoke, and comment cleanups.

source/blender/editors/space_graph/graph_slider_ops.c
395–396	dgo->gso

Gavriel Lavi (Gavriel5578) awarded a token.Nov 5 2021, 3:31 PM

Generalizing some code is good. This patch also changes the API of the operator, though, which isn't described in the patch description. Would it be possible to do one (extracting/generalizing code) without changing the property name from remove_ratio to percentage?

updated patch to latest master
no longer rename anything, will be done in separate patch

LGTM, just one little change. No need to resubmit for review after that.

source/blender/editors/space_graph/graph_slider_ops.c
416	This can just do `return OPERATOR_CANCELLED;`, as it's already known what the value of `invoke_result` is. The advantage is that you can then just look at the `return` statement and instantly know what's returned, without engaging your brain. Not requiring a brain is always good.