WIP: Animation&Rigging: Extra operators for the sliders #106281
This is work in progress. I wanted to show to see if I'm going in the right direction.
We decided to implement Animaide sliders as added functionalities to Christoph Lendenfeld slider operators. That made things kind of ease for me because all the hard work was already done. I'm basically following the structure he created to add the new functionality.
The main work is in Keyframes_general.c where I added comments, but all the other code on all the other files still have Christoph's old comments from his tools I used as a base.
There are two sliders missing: Wave-Noise and Blend-Frame.
-The first because the way I implemented it on my add-on was by creating a copy of the curve and adding modifiers to it, then using that as a reference for the "Y" values, but maybe it should have noise and wave formulas to accomplish the same. Are there functions in the code I could use for that?
-The second slider missing is Blend-Frame. This one should not be too hard to add but I have a GUI related question. For this tool to work, the user needs to add reference frames. On Animaide I use the "N" panel to have that:
Even if I just use markers I would need operators to add them, but I don't know if commands on a menu is the way to go. I even thought people would like to archive multitud of reference frames for latter use:
This is my first Pull Request ever so bear with me if I'm not doing things right. There is a file, DNA_modifier_types.h, that apparently I added a space by mistake and it is on the latest commit without me realizing it (but don't know how to take it out now).
Disclaimer: I haven't looked at the code yet
For your first question:
Yes for performance reasons I think it would be ideal to calculate a noise function first and then move the keys from that. You might even be able to use the functions from the modifier directly to do that.
It's a bit tricky. When developing an addon you have all the freedoms on how you develop your UX but when in core it should match existing functionality if possible.
One option I see is to use the curve value under the cursor and instead of having a bidirectional slider you just use 0-100%
To remove the space in
DNA_modifier_types.h just delete the space, save and push. If there is no difference in the files, it will not show up in the changed files.
Also a bit of housekeeping:
If you are not ready to have your code reviewed, prefix the title with WIP:
Try to keep your pull requests small, so 1 feature per PR this makes it a lot easier to review, and also means 1 operator that needs more work doesn't stop all the others from getting committed
You can add the Animation&Rigging label to this PR to indicate where it belongs
Having said that it's really awesome to see these tools in core. Nice work!
Thanks for the feedback.
I just changed the title of the PR. I hope I understood correctly. In regard to the PR being too large: Should I submit them again individually? If that is the case, should I just close this one?
In regard to the images: Sorry if me posting them was misleading, I just wanted to show the input I need from the user for the "Reference Frames" (not the slider). I need feedback on how or where the user should input that information. I think the sliders you implemented work nicely with the widget you added, I just need the frame number the user selects as reference. The thing is that because your tools live in the menu, asking for those frames on another part of the interface would feel disjointed and nobody would have a clue what are they for. But the operators that ask for those frames are not sliders, so I don't know if it should put them in the slider menu. The answer to that question is what I was trying to find. Sorry again if putting the add-on images was confussing.
hmmm... Now that I think about it, how could I submit a PR of just one tool if I made all the commits already?
By the way, I forgot to mention, When I was adding the sliders I went one by one adding what I have on the add-on, but then realized you already have some of them so didn't add them to the rest of the files, but they are still on "keyframe_general.c" and "ED_keyframes_edit.h". I should delete them but forgot. I'm referring to ease, blend_neighbor, and tween (I added a "b" to "ease_b" when realized you already had it).
yes splitting it up into 1 operator per pull request is the way to go.
About the reference frames:
I am arguing you don't necessarily need 2 of them. And one reference frame you can get from the current frame. So it would be a "Blend to Current Frame" operator
That is until we get time selection, but that is still on the horizon.
sorry missed the first part of how to split up a PR
it's a bit of work unfortunately, but what I usually do is
- make a new branch for each operator
- copy the relevant code from the browser
Closing This PR since I created separate ones for each slider
Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?