Page MenuHome

CollectionProperty: Keyframe Insert Using Element Name
Needs RevisionPublic

Authored by Wayde Moss (GuiltyGhost) on Apr 24 2020, 10:25 AM.

Details

Summary

Problem
Currently inserting keyframes on a CollectionProperty uses the rna path similar to "collection[n].value". This leads to fcurves depending on the element's index. If the collection is modified, items added or removed, then all fcurves with a higher index are invalidated, now associated with the wrong element. Animations that relied on the old indices are now corrupted. Fixing them is non trivial due to the unreliable index-based association.

Solution
By changing the rna_path built to use the element's name ("collection["element name"].value"), collection modifications do not corrupt existing animations. This is optional and applies to CollectionProperties constructed using the newly added subtype='DICTIONARY'.

Limitations

  1. In the Graph Editor and DopeSheet editor, the channels will have the name "Value" instead of something meaningful. This is a minor problem since we can manually group those channels to give them more meaningful names. This only applies when the keyed property is referenced from the named element. If the property is part of the same named element, it appears as "Value (Element Name)" which is good.
  2. The CollectionProperty doesn't enforce unique names. This is minor since addon creators can handle this behaviour.

Here's a blend file just to view the effect on keyframed properties. There's a script that will create the new subtype and provide a panel for some debug prints.

Here's the print results from the attached blend file. Being able to access a CollectionProperty element by name is existing behavior.

test_dictionary[0].name = "D_item"
test_dictionary["D_item"].name = "D_item"

test_collection[0].name = "C_item"
test_collection["C_item"].name = "C_item"

Diff Detail

Repository
rB Blender
Branch
arcpatch-D7511_1 (branched from master)
Build Status
Buildable 7848
Build 7848: arc lint + arc unit

Event Timeline

Wayde Moss (GuiltyGhost) requested review of this revision.Apr 24 2020, 10:25 AM
Wayde Moss (GuiltyGhost) created this revision.
Wayde Moss (GuiltyGhost) updated this revision to Diff 24073.EditedApr 26 2020, 3:01 AM

...I didn't mean to update this diff using these commits, apologies. There should only be the first commit considered for this patch.

Although this helps me with my immediate use case, I now realize a proper DictionaryProperty type would be ideal. Since collection properties are just lists, I assume lookups are linear despite supporting calls like collection["element name"] already. So if we have have N items in a collection and all have an associated Fcurve, then the worst case is N^2 for evaluating the animation.

What happens when somebody is actually using a CollectionProperty as a list, and wants to use indices?

  • revert accidental unrelated changes
  • works for when property in same group as name property
Wayde Moss (GuiltyGhost) edited the summary of this revision. (Show Details)May 2 2020, 1:50 AM

Here's the python result from the attached blend file.

test_dictionary[0].name = "D_item"
test_dictionary["D_item"].name = "D_item"

test_collection[0].name = "C_item"
test_collection["C_item"].name = "C_item"

So nothing has changed. The index access and named access existed before the patch already.

_

If you meant: if they want the data path to resolve to using indices, then they would just not use the dictionary subtype.

Brecht Van Lommel (brecht) requested changes to this revision.Mon, May 25, 12:07 PM

We have a different mechanism for this for builtin RNA properties, and I think this should be changed to use the same mechanism.

RNA_def_struct_name_property marks a property of the struct as the name (not necessarily called name but usually it is).

So rather than defining a collection as a dictionary, you'd define property as a name and then any collection of structs with a name property would use it for keyframing.

This revision now requires changes to proceed.Mon, May 25, 12:07 PM

Will do. I didn't have time this week but I'll get on it.