The python api in rna_mesh.c has a function to rename generic customdata layers. However for customdata layers
which are attributes (i.e. not specialized types) the attribute renaming function needs to be used, as that
can ensure unique names across domains.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- proplayerrenamesviaattrapi (branched from master)
- Build Status
Buildable 22732 Build 22732: arc lint + arc unit
Event Timeline
Comment Actions
I think this is a good change. One might argue that the old API should be deprecated and unchanged, but there is probably a lot of code that uses it, and when it's this simple to make it do the right thing, I'd say this is the right approach.
I'm adding Brecht as a reviewer, since he worked on the initial attribute RNA integration.
| source/blender/makesrna/intern/rna_mesh.c | ||
|---|---|---|
| 170 | The parentheses around ptr->data are unnecessary. | |
Comment Actions
Can we fold this logic into BKE_id_attribute_rename, remove rna_cd_layer_name_set and make CustomData_set_layer_unique_name a static function inside customdata.c?
I think in general we should move towards using BKE_attribute.h instead of BKE_customdata.h where possible, eventually unifying the two into just attributes.
Comment Actions
Hmm, I think we should be careful to avoid using the attribute API for layers that aren't generic and don't have names. That allows us to assume we're working on named attributes with generic data types in the attribute API, which is a helpful simplifying assumption.
So unless these are all really only generic layers, IMO it makes sense to have the split here.
Comment Actions
Ok.
I would like to see things go into the direction where we rename everything to "attribute" and drop the custom data term, not have attribute as a name reserved for generic data type attributes. CustomData then becomes something like "attribute domain data".
But I don't want to hold up this patch over such future design decisions.
Comment Actions
I mostly agree with that long term design, I guess it's just the order of things that I assumed would be different, I figured it was the switch to generic data types that made something an attribute internally.
Then the process would be switching everything to generic data types or moving it out of CustomData/attribute storage. That's one of the ideas with T95965 anyway.
Long term, IMO using CustomData doesn't make much sense at all-- the attributes for all domains could be stored together in some DNA struct like "AttributeStorage" which would interface directly with an attribute API in D15280.
But yeah, that's getting way out of scope for this patch.