It's possible that the new layer order is exactly the same as the old layer order, in which case we could skip this call to reorder_customdata()
. It's tempting to detect that case and skip this call as an optimization, but I'm not sure there's any practical benefit to doing that? It would arguably arguably just "complexity creep" to optimize things that don't really need to be optimized.
If someone happened to call this method and pass in the root group as parent_group
, then this call to move_node_into()
is just wasted effort. I could add a check to avoid that, but all we'd really be doing is optimizing a case where the calling code was using the wrong method signature (because they should instead use the signature that implicitly adds to the root group).
Indeed! An embarrassing oversight
Good to know, thanks. You might consider adding that to the style guide, especially since the default linter settings in some IDEs (or at least…
Ack, yes, very glad you caught this because I had completely forgotten to deal with it.
Most places that add a layer were adding the layer to the root group anyway, so I added some signatures…
Great, thanks. With this change grow_or_init_customdata()
is now just 2 simple lines, so maybe not worth its weight as a function? Not sure where the Blender team lands on the tradeoffs on that…
Bah, I can't figure out how to do comments on multiple lines of code in Gitea, so I'll just ask my questions here:
- Could the implementation of
grow_customdata()
be simplified even further…
@filedescriptor I ended up jumping right into the coding anyway, because I'm not yet familiar enough with the codebase to have much confidence that an idea will work out. It also wasn't much work…
Thanks for adding yourself as a reviewer, @filedescriptor. But FYI, I plan to file a new PR within the next couple days that would supersede this one, because I thought of a different approach for…