- 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 -
-
Looks good. But I should have been clearer: I also meant for act_keyframes_loop() below to get renamed accordingly. E.g. action_legacy_keyframes_loop().
ID* cache of users to Action Bindings
Had one more comment that I discussed with @dr.sybren in person, regarding making the mutable variant of users() internal-only. But otherwise looks good to me!
Looking good so far. Just some suggestions that I think could help people when reading the code.
I realize this is a preexisting function, but I have no idea what this does from the function name, parameters, and return type. Might be worth adding even just a one-liner doc comment.
Definitely not for this PR, but just noting since I thought of it now: at some point it would be good to put these functions in a namespace like channel_filter so we can drop the cryptic acf_ prefix.
Would now be a good time to rename this to something like act_layered_keyframes_loop(), as well as the corresponding function for legacy actions?
ID* cache of users to Action Bindings
Same as my other comment: I think we should just use a Vector.
ID* cache of users to Action Bindings
I think we should also add a TODO for handling library linked data.
ID* cache of users to Action Bindings
A Set seems overkill to me. The expected number of users is 1, and the presumably overwhelmingly common case will be a small number. So I think it's better to optimize for that. A Vector with a single inline element would be a good fit.
We discussed this at the module meeting, and we agreed that we should be able to address this with versioning code in Blender. Basically, the old code was implicitly clamping values to -180/180…