WIP: Get path from nested ID properties on non-ID datastructures (Bones, Sequences, etc.) (T48975)
Needs ReviewPublic
Actions

Authored by Lukas W (geloescht) on Jun 19 2021, 11:08 PM.

Details

Reviewers

Bastien Montagne (mont29)
Sybren A. Stüvel (sybren)

Summary

Proof of concept working towards fixing T48975

Using this patch, a ID property group now stores a reference to the struct that owns it and its type in unused fields of IDProperty::data. This reference is propagated to any property that is added to an IDP_GROUP or IDP_IDPARRAY. rna_path_from_ID_to_idpgroup() uses this ownership reference if it exists to search for the ID property in the right place and to build a path.
Creating and updating the reference after loading a file happens in the idprops() function for each type. It was the easiest place to do add that, otherwise RNA stuff would have to be added in places that doesn't deal with RNA. I can see how something might go wrong here but so far haven't encountered any problems.
The attached patch is a proof of concept and shouldn't be seen as a patch to be committed immediately. My question is if this is acceptable as an approach since it is kind of a hack (but on the other hand the way ID properties are stored already looks like a hack to me).

Diff Detail

Repository: rB Blender

Event Timeline

Lukas W (geloescht) requested review of this revision.Jun 19 2021, 11:08 PM

Lukas W (geloescht) created this revision.

Lukas W (geloescht) mentioned this in T48975: Custom Properties within PropertyGroups cannot be animated when attached to a PoseBone.

Demeter Dzadik (Mets) added a subscriber: Demeter Dzadik (Mets).Oct 7 2021, 3:58 PM

I think that overall the approach can work, and it would be great to have T48975: Custom Properties within PropertyGroups cannot be animated when attached to a PoseBone fixed. Not sure about the exact location where this info is stored, but changing that would only require changes to IDP_GetPropertyOwner(), IDP_GetPropertyOwnerType(), and IDP_SetPropertyOwner().

Code wise, I think this could be simplified:

if (IDP_GetPropertyOwner(group)) {
  IDP_SetPropertyOwner(prop, IDP_GetPropertyOwner(group), IDP_GetPropertyOwnerType(group));
}

to something like this:

IDP_SetPropertyOwner(prop, IDP_GetPropertyOwner(group), IDP_GetPropertyOwnerType(group));

That'll also set the owner to NULL in case there is no owner.

There are more code improvements possible, but before we go into that, I'd much rather first see @Bastien Montagne (mont29)'s view on the overall concept of the patch.