Fix T67776: Animation/value keyframe slider doesn't appear in dope sheet
ClosedPublic
Actions

Authored by Christoph Lendenfeld (ChrisLend) on Oct 6 2020, 1:03 PM.

Details

Reviewers

Sybren A. Stüvel (sybren)
Luciano Muñoz Sessarego (looch)

Group Reviewers

Animation & Rigging

Maniphest Tasks

T67776: Animation/value keyframe slider doesn't appear in dope sheet

Commits

rB4a633fae8a04: Fix T67776: Animation/value keyframe slider doesn't appear in dope sheet

Summary

Fix for T67776 Animation/value keyframe slider doesn't appear in dope sheet

The bug was that the SACTION_SLIDERS flag would be set when switching into the Shape Key editor in the Dope Sheet

In the task, @Jacques Lucke (JacquesLucke) proposed to split this flag in two, one for sliders with min-max and one for other sliders.

Solution
I thought about this but opted against this solution in the end because, as also mentioned in the task, this has the potential to be quite confusing to the user.

Instead I chose to always display sliders in the Shape Key editor regardless of the set flag and remove the line of code that sets it automatically.

I think this is the best solution because the Shape Key editor doesn't make sense without sliders (hence why the flag was set automatically in the first place)

To indicate to the user that this flag doesn't do anything in this mode anymore, it has the active state set to false when in the Shape Key editor

Limitations
The obvious limitation is that you can't hide the sliders in the Shape Key editor anymore

Diff Detail

Repository: rB Blender

Event Timeline

Christoph Lendenfeld (ChrisLend) requested review of this revision.Oct 6 2020, 1:03 PM

Christoph Lendenfeld (ChrisLend) created this revision.

Christoph Lendenfeld (ChrisLend) added a task: T67776: Animation/value keyframe slider doesn't appear in dope sheet.Oct 6 2020, 1:06 PM

fixed the comment in the python file

Christoph Lendenfeld (ChrisLend) added a project: Animation & Rigging.Oct 6 2020, 1:20 PM

I agree with your solution. Both your code and the original one adhere to the "Sliders should always be shown in the ShapeKey Editor", but the original code overwrote the user's preference, and yours doesn't.

release/scripts/startup/bl_ui/space_dopesheet.py
345	There is no need to mention the task number here. The commit should mention the number, and the Git history can be used to find the commit in which this functionality changed.
345	This comment mentions what happens, which is already clear from the code. The why should be documented, though. Something like "Sliders are always shown in the shapekey editor, regardless of this setting" would be clearer IMO.
source/blender/editors/animation/anim_channels_defines.c
5336–5338	Although I agree with the removal of the superfluous parentheses, it's better to not do that in this patch. Cleanups, refactors, and other non-functional changes have to be committed separately from functional changes. Generally such cleanups are much easier to approve & land in master, and when that's done, the patch with functional changes will be smaller and thus easier to review as well.
5337–5338	Combining `&&` and `\|\|` is ambiguous, and requires parentheses.