Fix: correct RNA property access in Rigify

Ensure that Rigify correctly handles RNA properties as RNA properties.
Before, it mixed dictionary-style and attribute-style access, but
since 7276b2009a that is no longer
possible.

This means that the (un)registration of the RNA properties needs to
happen in two places: the add-on itself, and the code it generates in
the rig. To be able to inject the code into a blend file, Rigify needs
to access it as string. This commit introduces a new way of dealing
with this: it just has normal code, and uses Python's
[`inspect.getsource()`][getsource] to get that code as string for
injection into the rig file.

[getsource]: https://docs.python.org/3/library/inspect.html#inspect.getsource

The biggest chunk of code in this commit deals with the 'Copy
Parameters', 'Copy Type & Parameters', and 'Mirror Type & Parameters'
operators. Since it's no longer possible to access RNA properties as
dict-like custom properties, new code was written to recursively copy
`bpy.types.PropertyGroup` instances. Eventually this may end up in
Blender itself, but that would mean writing more thorough checks and
making sure it works in many cases. And it might be easier to
implement this on the C++ side, where it should just be a matter of
exposing one `IDP_CopyPropertyContent()` call to the Python API.
That's all for another Blender release, though -- for now this'll have
to do to fix up Rigify for 5.0.

Fixes: #146675
Pull Request: https://projects.blender.org/blender/blender/pulls/147199

scripts/addons_core/rigify/__init__.py

@@ -473,6 +473,9 @@ class RigifySelectionColors(bpy.types.PropertyGroup):
 class RigifyParameters(bpy.types.PropertyGroup):
     name: StringProperty()
 
+    # NOTE: parameters are dynamically added to this PropertyGroup.
+    # Check `ControlLayersOption` in `layers.py`.
+
 
 class RigifyBoneCollectionReference(bpy.types.PropertyGroup):
     """Reference from a RigifyParameters field to a bone collection."""
@@ -652,7 +655,53 @@ def register():
     for cls in classes:
         register_class(cls)
 
-    # Properties.
+    register_rna_properties()
+
+    prefs = RigifyPreferences.get_instance()
+    prefs.register_feature_sets(True)
+    prefs.update_external_rigs()
+
+    # Add rig parameters
+    register_rig_parameters()
+
+
+def register_rig_parameters():
+    for rig in rig_lists.rigs:
+        rig_module = rig_lists.rigs[rig]['module']
+        rig_class = rig_module.Rig
+        rig_def = rig_class if hasattr(rig_class, 'add_parameters') else rig_module
+        # noinspection PyBroadException
+        try:
+            if hasattr(rig_def, 'add_parameters'):
+                validator = RigifyParameterValidator(RigifyParameters, rig, RIGIFY_PARAMETER_TABLE)
+                rig_def.add_parameters(validator)
+        except Exception:
+            import traceback
+            traceback.print_exc()
+
+
+def unregister():
+    from bpy.utils import unregister_class
+
+    prefs = RigifyPreferences.get_instance()
+    prefs.register_feature_sets(False)
+
+    unregister_rna_properties()
+
+    # Classes.
+    for cls in classes:
+        unregister_class(cls)
+
+    clear_rigify_parameters()
+
+    # Sub-modules.
+    operators.unregister()
+    metarig_menu.unregister()
+    ui.unregister()
+    feature_set_list.unregister()
+
+
+def register_rna_properties() -> None:
     bpy.types.Armature.active_feature_set = EnumProperty(
         items=feature_set_list.feature_set_items,
         name="Feature Set",
@@ -810,35 +859,8 @@ def register():
         name="Rigify Owner Rig",
         description="Rig that owns this object and may delete or overwrite it upon re-generation")
 
-    prefs = RigifyPreferences.get_instance()
-    prefs.register_feature_sets(True)
-    prefs.update_external_rigs()
-
-    # Add rig parameters
-    register_rig_parameters()
-
-
-def register_rig_parameters():
-    for rig in rig_lists.rigs:
-        rig_module = rig_lists.rigs[rig]['module']
-        rig_class = rig_module.Rig
-        rig_def = rig_class if hasattr(rig_class, 'add_parameters') else rig_module
-        # noinspection PyBroadException
-        try:
-            if hasattr(rig_def, 'add_parameters'):
-                validator = RigifyParameterValidator(RigifyParameters, rig, RIGIFY_PARAMETER_TABLE)
-                rig_def.add_parameters(validator)
-        except Exception:
-            import traceback
-            traceback.print_exc()
-
-
-def unregister():
-    from bpy.utils import unregister_class
-
-    prefs = RigifyPreferences.get_instance()
-    prefs.register_feature_sets(False)
 
+def unregister_rna_properties() -> None:
     # Properties on PoseBones and Armature. (Annotated to suppress unknown attribute warnings.)
     pose_bone: typing.Any = bpy.types.PoseBone
 
@@ -877,15 +899,3 @@ def unregister():
     obj_store: typing.Any = bpy.types.Object
 
     del obj_store.rigify_owner_rig
-
-    # Classes.
-    for cls in classes:
-        unregister_class(cls)
-
-    clear_rigify_parameters()
-
-    # Sub-modules.
-    operators.unregister()
-    metarig_menu.unregister()
-    ui.unregister()
-    feature_set_list.unregister()

scripts/addons_core/rigify/operators/copy_mirror_parameters.py

@@ -8,7 +8,7 @@ import importlib
 
 from ..utils.layers import REFS_TOGGLE_SUFFIX, REFS_LIST_SUFFIX, is_collection_ref_list_prop, copy_ref_list
 from ..utils.naming import Side, get_name_base_and_sides, mirror_name
-from ..utils.misc import property_to_python
+from ..utils.misc import propgroup_to_dict, assign_rna_properties
 
 from ..utils.rig import get_rigify_type, get_rigify_params
 from ..rig_lists import get_rig_class
@@ -140,33 +140,36 @@ def copy_rigify_params(from_bone: bpy.types.PoseBone, to_bone: bpy.types.PoseBon
 
     if match_type and rig_type != from_type:
         return False
-    else:
-        rig_type = to_bone.rigify_type = from_type
 
-    # Delete any previously-existing parameters, before copying in the new ones.
-    # Direct assignment to this property, as happens in the code below, is only
-    # possible when there is no pre-existing value. See #135233 for more info.
-    if 'rigify_parameters' in to_bone:
-        del to_bone['rigify_parameters']
+    rig_type = to_bone.rigify_type = from_type
+    if not rig_type:
+        return False
 
-    from_params = from_bone.get('rigify_parameters')
-    if from_params and rig_type:
-        param_dict = property_to_python(from_params)
+    from_params: bpy.types.RigifyParameters = from_bone.rigify_parameters
+    if not from_params:
+        # TODO: check whether this can even happen, given that every bone has this RNA property.
+        return False
 
-        if x_mirror:
-            to_bone['rigify_parameters'] = recursive_mirror(param_dict)
+    # Simple case first: without mirroring, just copy the parameters.
+    if not x_mirror:
+        assign_rna_properties(to_bone.rigify_parameters, from_bone.rigify_parameters)
+        return True
 
-            # Bone collection references must be mirrored specially
-            from_params_typed = get_rigify_params(from_bone)
-            to_params_typed = get_rigify_params(to_bone)
+    # For compatibility with the already-existing recursive_mirror(dict)
+    # function, round-trip the parameters through a dictionary.
+    param_dict: dict[str, object] = propgroup_to_dict(from_params)
+    mirrored_dict: dict[str, object] = recursive_mirror(param_dict)  # type: ignore
+    assign_rna_properties(to_bone.rigify_parameters, mirrored_dict)
 
-            for prop_name in param_dict.keys():
-                if prop_name.endswith(REFS_LIST_SUFFIX):
-                    ref_list = getattr(from_params_typed, prop_name)
-                    if is_collection_ref_list_prop(ref_list):
-                        copy_ref_list(getattr(to_params_typed, prop_name), ref_list, mirror=True)
-        else:
-            to_bone['rigify_parameters'] = param_dict
+    # Bone collection references must be mirrored specially
+    from_params_typed = get_rigify_params(from_bone)
+    to_params_typed = get_rigify_params(to_bone)
+
+    for prop_name in param_dict.keys():
+        if prop_name.endswith(REFS_LIST_SUFFIX):
+            ref_list = getattr(from_params_typed, prop_name)
+            if is_collection_ref_list_prop(ref_list):
+                copy_ref_list(getattr(to_params_typed, prop_name), ref_list, mirror=True)
 
     return True
 

scripts/addons_core/rigify/rig_ui_template.py

@@ -910,7 +910,7 @@ class RigLayers(bpy.types.Panel):
         layout = self.layout
         row_table = collections.defaultdict(list)
         for coll in flatten_children(context.active_object.data.collections):
-            row_id = coll.get('rigify_ui_row', 0)
+            row_id = coll.rigify_ui_row
             if row_id > 0:
                 row_table[row_id].append(coll)
         col = layout.column()
@@ -919,7 +919,7 @@ class RigLayers(bpy.types.Panel):
             row_buttons = row_table[row_id]
             if row_buttons:
                 for coll in row_buttons:
-                    title = coll.get('rigify_ui_title') or coll.name
+                    title = coll.rigify_ui_title or coll.name
                     row2 = row.row()
                     row2.active = coll.is_visible_ancestors
                     row2.prop(coll, 'is_visible', toggle=True, text=title, translate=False)
@@ -1393,6 +1393,9 @@ class ScriptGenerator(base_generate.GeneratorPlugin):
 
         script.write(UI_LAYERS_PANEL)
 
+        # Inject the RNA property (un)register functions.
+        self._write_rna_prop_register_funcs(script)
+
         script.write("\ndef register():\n")
 
         ui_register = OrderedDict.fromkeys(self.ui_register)
@@ -1426,3 +1429,17 @@ class ScriptGenerator(base_generate.GeneratorPlugin):
 
         # Attach the script to the rig
         self.obj['rig_ui'] = script
+
+    def _write_rna_prop_register_funcs(self, script: bpy.types.Text) -> None:
+        """Inject the (un)register_rna_properties functions into the script."""
+        import inspect
+        from . import register_rna_properties, unregister_rna_properties
+
+        register_func_src = inspect.getsource(register_rna_properties)
+        unregister_func_src = inspect.getsource(unregister_rna_properties)
+
+        script.write("\n\n")
+        script.write(register_func_src)
+        script.write("\n\n")
+        script.write(unregister_func_src)
+        script.write("\n")

scripts/addons_core/rigify/utils/misc.py

@@ -272,6 +272,59 @@ def clone_parameters(target):
     return property_to_python(dict(target))
 
 
+def propgroup_to_dict(source: bpy.types.PropertyGroup) -> dict[str, typing.Any]:
+    """Convert a bpy.types.PropertyGroup to a dictionary.
+
+    Note that this follows much of the same logic as `assign_rna_properties()` below.
+    """
+
+    # Precondition check.
+    assert isinstance(source, bpy.types.PropertyGroup), "Source must be PropertyGroup, but is {!r}".format(type(source))
+
+    # Copy the property values one by one.
+    skip_properties = {'rna_type', 'bl_rna'}
+    dictionary = {}
+    for prop in source.bl_rna.properties:
+        attr = prop.identifier
+
+        if attr in skip_properties:
+            continue
+
+        # Un-set properties if necessary:
+        try:
+            is_set = source.is_property_set(attr)
+        except TypeError as ex:
+            raise TypeError("{!s} on {!s}".format('; '.join(ex.args), source)) from None
+        if not is_set:
+            continue
+
+        # Set properties, depending on their type:
+        value = getattr(source, attr)
+        match prop.type:
+            # Directly assignable types:
+            case 'BOOLEAN' | 'INT' | 'FLOAT' | 'ENUM' | 'STRING':
+                dictionary[attr] = value
+
+            # Treat as list-like:
+            case 'COLLECTION':
+                target_coll = [propgroup_to_dict(source_item) for source_item in value]
+                dictionary[attr] = target_coll
+
+            # Pointer properties are treated depending on the type they point
+            # to. PropertyGroups have to be dealt with by recursion, while other
+            # types can be assigned directly.
+            case 'POINTER':
+                if isinstance(value, bpy.types.PropertyGroup):
+                    dictionary[attr] = propgroup_to_dict(value)
+                    continue
+                dictionary[attr] = value
+
+            case _:
+                raise TypeError("no implementation for RNA property {!r} type {!r}".format(prop.identifier, prop.type))
+
+    return dictionary
+
+
 def assign_parameters(target, val_dict=None, **params):
     if val_dict is not None:
         for key in list(target.keys()):
@@ -288,6 +341,92 @@ def assign_parameters(target, val_dict=None, **params):
             raise Exception(f"Couldn't set {key} to {value}: {e}")
 
 
+def assign_rna_properties(target: bpy.types.PropertyGroup,
+                          source: bpy.types.PropertyGroup | dict[str, typing.Any]) -> None:
+    """Basically calling `setattr(target, attribute, value_from_source)` for each property of `target`.
+
+    Note that this follows much of the same logic as `propgroup_to_dict()` above.
+    """
+
+    # Precondition checks.
+    assert isinstance(target, bpy.types.PropertyGroup), "Target must be PropertyGroup, but is {!r}".format(type(target))
+    assert isinstance(source, (bpy.types.PropertyGroup, dict)
+                      ), "Source must be PropertyGroup or dict, but is {!r}".format(type(source))
+    if isinstance(source, bpy.types.PropertyGroup):
+        assert (target.__class__ == source.__class__), "Source and target must be PropertyGroups of the same type."
+
+    def _setattr(prop_identifier, value):
+        """Wrapper around setattr() that has more concrete info in its exception when it fails."""
+        try:
+            setattr(target, prop_identifier, value)
+        except AttributeError as ex:
+            raise AttributeError(
+                "Could not set {!r}.{!s} = {!r} (type={!s}): {!s}".format(
+                    target, prop_identifier, value, type(value), ex)) from None
+
+    # Dynamically construct functions to create an abstraction around dict vs. PropertyGroup.
+    if isinstance(source, dict):
+        def _is_property_set(prop_identifier: str) -> bool:
+            return prop_identifier in source
+
+        def _get_value(prop_identifier: str) -> typing.Any:
+            return source[prop_identifier]
+    else:
+        def _is_property_set(prop_identifier: str) -> bool:
+            return source.is_property_set(prop_identifier)
+
+        def _get_value(prop_identifier: str) -> typing.Any:
+            return getattr(source, prop_identifier)
+
+    # Copy the property values one by one.
+    skip_properties = {'rna_type', 'bl_rna'}
+    for prop in target.bl_rna.properties:
+        attr = prop.identifier
+
+        if attr in skip_properties:
+            continue
+
+        # Un-set properties if necessary:
+        try:
+            is_set = _is_property_set(attr)
+        except TypeError as ex:
+            raise TypeError("{!s} on {!s}".format('; '.join(ex.args), source)) from None
+        if not is_set:
+            target.property_unset(attr)
+            continue
+
+        # Set properties, depending on their type:
+        value = _get_value(attr)
+        match prop.type:
+            # Directly assignable types:
+            case 'BOOLEAN' | 'INT' | 'FLOAT' | 'ENUM' | 'STRING':
+                if target.is_property_readonly(attr):
+                    continue
+                _setattr(attr, value)
+
+            # Treat as list-like:
+            case 'COLLECTION':
+                target_coll = getattr(target, attr)
+                target_coll.clear()
+                for source_item in value:
+                    target_item = target_coll.add()
+                    assign_rna_properties(target_item, source_item)
+
+            # Pointer properties are treated depending on the type they point
+            # to. PropertyGroups have to be dealt with by recursion, while other
+            # types can be assigned directly.
+            case 'POINTER':
+                if isinstance(value, bpy.types.PropertyGroup):
+                    assign_rna_properties(getattr(target, attr), value)
+                    continue
+                if target.is_property_readonly(attr):
+                    continue
+                _setattr(attr, value)
+
+            case _:
+                raise TypeError("no implementation for RNA property {!r} type {!r}".format(prop.identifier, prop.type))
+
+
 def select_object(context: bpy.types.Context, obj: bpy.types.Object, deselect_all=False):
     view_layer = context.view_layer