- Amsterdam, Netherlands
- https://cessen.com
-
Animator, rigger, and software developer. Currently working at the Blender Institute as a developer on Blender's animation system.
Been using Blender since 1998, and worked on Big Buck Bunny and Sintel (two of Blender's open movie projects).
- Joined on
2003-03-21 -
-
This is affecting also the Animation module, would not mind a green light from them first on the general idea
The general idea looks good to me. It makes a good chunk of the code more…
Mostly just me being annoying about comments and documentation. But also a question about the ifdef guards.
As mentioned in my last review, it's not clear to me why anything in this PR should be guarded behind the WITH_ANIM_BAKLAVA flag, since at least conceptually these changes are independent of the changes we're making in that project: these changes are just as relevant/useful to legacy actions as they are to layered actions.
Some documentation here would be good, outlining why you would/should use this over e.g. uiTemplateID() for actions. Right now that's only documented in the PR/commit message, as far as I can tell.
This comment is confusing to me, referring to action, which doesn't exist locally and isn't passed to this function call.
In addition to what I wrote in my comments, I'm also wondering why we need all the ifdefs on WITH_ANIM_BAKLAVA at all. This code doesn't seem to me like it is (or should be) specific to layered actions.