Page MenuHome

UI: support Copy To Selected and Alt-Click for PropertyGroup members.
ClosedPublic

Authored by Alexander Gavrilov (angavrilov) on Sat, Nov 16, 6:52 PM.

Details

Summary

Rigify uses a property group to contain options of its rigs, so
currently it is impossible to use Alt-Click or Copy To Selected
to change a setting for multiple rigs at the same time.

The main problem here is that there is no efficient way to find
which bone the property group belongs to. To maintain performance,
implement this by checking the active bone if it is known. Copy
Data Path and related features still don't work, as data path
calculation can't use context.

Diff Detail

Repository
rB Blender

Event Timeline

Obviously no real issues from a UI POV. Mainly this is for @Campbell Barton (campbellbarton) to review.

This revision is now accepted and ready to land.Sun, Nov 17, 1:55 AM
Campbell Barton (campbellbarton) requested changes to this revision.Tue, Nov 26, 10:18 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/interface/interface_ops.c
739

It would be good if the code comments here explained whats going on more generally.

That non-ID data-blocks that can have their own ID properties (see use of RNA_def_struct_idprops_func). Edit/Pose/Sequence/Nodes/ViewLayers... etc.

That this would be the location support for other types would be added.

775

Any reason it's necessary to break the else if ... chain?

In the case it doesn't return is there any purpose in running the checks after this block ends?

source/blender/makesrna/intern/rna_access.c
5744–5746

Convention is to use \

This revision now requires changes to proceed.Tue, Nov 26, 10:18 AM
source/blender/editors/interface/interface_ops.c
775

The ptr->owner_id case below is still relevant for properties belonging to ID.

Updated comments, added a couple of simple utilities to hopefully make the code a bit more readable, considering the possibility of adding more similar checks.

Generally seems fine, not sure helper functions are needed though.

source/blender/editors/interface/interface_ops.c
710

having ui_try_ as a prefix is a bit odd, suggest to use CTX_data_pointer_get_type, the caller can check it's not NULL.

720

Not sure this wrapper is needed? callers could use RNA_path_from_struct_to_idproperty directly?

751

It would be good to note here that other types can potentially be added in the future, SequenceStrips for e.g.

Removed ui_try_ helpers.

Generally OK although all things considered I don't think the macros are all that helpful.

Also missing notes on other types where this utility function might be useful.

source/blender/editors/interface/interface_ops.c
710–711

I find these a bit odd, personally I'd just go with the expanded non, macro version.

Infact we could just do:

idpath = RNA_path_from_struct_to_idproperty(&owner_ptr, ptr->data);
if (idpath) {...}

And:

owner_ptr = CTX_data_pointer_get_type(C, "active_pose_bone", &RNA_PoseBone);
if (owner_ptr.data) {...}
This revision is now accepted and ready to land.Mon, Dec 2, 8:04 PM
source/blender/editors/interface/interface_ops.c
710–711

Mainly the issue is that code like that cannot be chained into a non-nesting 'else if' chain, which could be an issue if more cases are expected to be added in the future.