- 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 -
-
@Pitched123 Putting aside whether this should be considered a bug or not, can I ask what your use case is for this?
The reason I ask is because usually when people have setups like this it's…
I'm unable to reproduce with two single-bone armatures, on either Blender 4.1.1 or latest main.
@Billy-Santoso a couple of requests to help track this down:
- Can you double-check that the…
I'm tentatively approving, because this does seem right to me. But, like you, I'm also not sure why that code was there in the first place. According to git, those lines are from @dr.sybren, so let's wait to hear what he says. But maybe he just moved them in a refactor or something, so dunno.
Basically looks good to me. Just a few minor comments.
(Commenting on the line between because Sybren already has unrelated comments on the lines I want.)
This is probably fine as-is given the number of items we're likely dealing with. But just a note that this has quadratic complexity since it's growing the array one item at a time, which involves copying the entire array so far each time.
I'm a little confused why this is necessary. Since we're freshly creating this action, shouldn't it be fine to just unconditionally add a channel bag here, rather than trying to look it up first (which will presumably always fail?).
Generally looking good to me, and seems to work in my testing. Mainly just one comment regarding the semantics of the flag.
I feel like this should be flipped, so that enabling is positive (play) and disabled is negative (don't play). It would also make it match the option presented to the user. That would require some versioning code for older files, and non-zero initialization for new ImageUsers, but I think it might be worth it...? Not totally sure.
Yeah, ultimately it's goofy that fcurve colors are done this way at all. It absolutely should be done at draw time, for a variety of reasons. But the goal right now is just getting the existing…
Maybe something like FCurveEnsureParams could work? It's a bit of a mouthful, but I think it helps communicate better that it's the parameters for either looking up or creating an fcurve. My…