LibOverride: RNA Apply code: Work around potential duplicates in names of RNA collections of IDs.

While in theory RNA collections of IDs will have unique names in common
use cases, it can happen that there are naming collisions (due to a same
RNA collection having ID pointers to data from different libraries,
having the same name).

This situation is deadly for liboverride applying code, since it rely on
finding which item of the collection to modify by using its name.

To alleviate the problem, this commit changes the way items are searched
for, by adding an extra first check to find an item which matches both
the requested name and index.

While not perfect, this should reduce the breaking cases when production
files get dirty and start having complex mangling of override and linked
data naming.
This commit is contained in:
Bastien Montagne 2023-07-08 16:09:18 +02:00
parent 6dd51353dd
commit a05419f18b
1 changed files with 158 additions and 57 deletions

View File

@ -992,7 +992,54 @@ bool RNA_struct_override_store(Main *bmain,
return changed;
}
static void rna_porperty_override_collection_subitem_lookup(
static void rna_property_override_collection_subitem_name_index_lookup(
PointerRNA *ptr,
PropertyRNA *prop,
const char *item_name,
const int item_index,
PointerRNA *r_ptr_item_name,
PointerRNA *r_ptr_item_index)
{
RNA_POINTER_INVALIDATE(r_ptr_item_name);
RNA_POINTER_INVALIDATE(r_ptr_item_index);
/* First, lookup by index, but only validate if name also matches (or if there is no given name).
*/
if (item_index != -1) {
if (RNA_property_collection_lookup_int(ptr, prop, item_index, r_ptr_item_index)) {
if (item_name != nullptr) {
PropertyRNA *nameprop = r_ptr_item_index->type->nameproperty;
char name[256], *nameptr;
int keylen = static_cast<int>(strlen(item_name));
int namelen;
nameptr = RNA_property_string_get_alloc(
r_ptr_item_index, nameprop, name, sizeof(name), &namelen);
if (keylen == namelen && STREQ(nameptr, item_name)) {
/* Index and name both match. */
*r_ptr_item_name = *r_ptr_item_index;
return;
}
}
}
}
if (item_name == nullptr) {
return;
}
/* Then, lookup by name only. */
if (RNA_property_collection_lookup_string(ptr, prop, item_name, r_ptr_item_name)) {
RNA_POINTER_INVALIDATE(r_ptr_item_index);
return;
}
/* If name lookup failed, `r_ptr_item_name` is invalidated, so if index lookup was sucessful it
* will be the only valid return value. */
}
static void rna_property_override_collection_subitem_lookup(
PointerRNA *ptr_dst,
PointerRNA *ptr_src,
PointerRNA *ptr_storage,
@ -1022,68 +1069,122 @@ static void rna_porperty_override_collection_subitem_lookup(
if (prop_storage != nullptr) {
RNA_POINTER_INVALIDATE(private_ptr_item_storage);
}
if (opop->subitem_local_name != nullptr) {
RNA_property_collection_lookup_string(
ptr_src, prop_src, opop->subitem_local_name, private_ptr_item_src);
if (opop->subitem_reference_name != nullptr &&
RNA_property_collection_lookup_string(
ptr_dst, prop_dst, opop->subitem_reference_name, private_ptr_item_dst))
{
/* This is rather fragile, but the fact that local override IDs may have a different name
* than their linked reference makes it necessary.
* Basically, here we are considering that if we cannot find the original linked ID in
* the local override we are (re-)applying the operations, then it may be because some of
* those operations have already been applied, and we may already have the local ID
* pointer we want to set.
* This happens e.g. during re-sync of an override, since we have already remapped all ID
* pointers to their expected values.
* In that case we simply try to get the property from the local expected name. */
}
else {
RNA_property_collection_lookup_string(
ptr_dst, prop_dst, opop->subitem_local_name, private_ptr_item_dst);
}
PointerRNA ptr_item_dst_name, ptr_item_dst_index;
PointerRNA ptr_item_src_name, ptr_item_src_index;
PointerRNA ptr_item_storage_name, ptr_item_storage_index;
rna_property_override_collection_subitem_name_index_lookup(ptr_src,
prop_src,
opop->subitem_local_name,
opop->subitem_local_index,
&ptr_item_src_name,
&ptr_item_src_index);
rna_property_override_collection_subitem_name_index_lookup(ptr_dst,
prop_dst,
opop->subitem_reference_name,
opop->subitem_reference_index,
&ptr_item_dst_name,
&ptr_item_dst_index);
/* This is rather fragile, but the fact that local override IDs may have a different name
* than their linked reference makes it necessary.
* Basically, here we are considering that if we cannot find the original linked ID in
* the local override we are (re-)applying the operations, then it may be because some of
* those operations have already been applied, and we may already have the local ID
* pointer we want to set.
* This happens e.g. during re-sync of an override, since we have already remapped all ID
* pointers to their expected values.
* In that case we simply try to get the property from the local expected name. */
if (opop->subitem_reference_name != nullptr && opop->subitem_local_name != nullptr &&
ptr_item_dst_name.type == nullptr)
{
rna_property_override_collection_subitem_name_index_lookup(
ptr_dst,
prop_dst,
opop->subitem_local_name,
opop->subitem_reference_index != -1 ? opop->subitem_reference_index :
opop->subitem_local_index,
&ptr_item_dst_name,
&ptr_item_dst_index);
}
else if (opop->subitem_reference_name != nullptr) {
RNA_property_collection_lookup_string(
ptr_src, prop_src, opop->subitem_reference_name, private_ptr_item_src);
RNA_property_collection_lookup_string(
ptr_dst, prop_dst, opop->subitem_reference_name, private_ptr_item_dst);
/* For historical compatibility reasons, we fallback to reference if no local item info is given,
* and vice-versa. */
if (opop->subitem_reference_name == nullptr && opop->subitem_local_name != nullptr) {
rna_property_override_collection_subitem_name_index_lookup(
ptr_dst,
prop_dst,
opop->subitem_local_name,
opop->subitem_reference_index != -1 ? opop->subitem_reference_index :
opop->subitem_local_index,
&ptr_item_dst_name,
&ptr_item_dst_index);
}
else if (opop->subitem_local_index != -1) {
RNA_property_collection_lookup_int(
ptr_src, prop_src, opop->subitem_local_index, private_ptr_item_src);
if (opop->subitem_reference_index != -1) {
RNA_property_collection_lookup_int(
ptr_dst, prop_dst, opop->subitem_reference_index, private_ptr_item_dst);
}
else {
RNA_property_collection_lookup_int(
ptr_dst, prop_dst, opop->subitem_local_index, private_ptr_item_dst);
}
else if (opop->subitem_reference_name != nullptr && opop->subitem_local_name == nullptr) {
rna_property_override_collection_subitem_name_index_lookup(ptr_src,
prop_src,
opop->subitem_reference_name,
opop->subitem_local_index != -1 ?
opop->subitem_local_index :
opop->subitem_reference_index,
&ptr_item_src_name,
&ptr_item_src_index);
}
else if (opop->subitem_reference_index != -1) {
RNA_property_collection_lookup_int(
ptr_src, prop_src, opop->subitem_reference_index, private_ptr_item_src);
RNA_property_collection_lookup_int(
ptr_dst, prop_dst, opop->subitem_reference_index, private_ptr_item_dst);
if (opop->subitem_reference_index == -1 && opop->subitem_local_index != -1) {
rna_property_override_collection_subitem_name_index_lookup(ptr_dst,
prop_dst,
nullptr,
opop->subitem_local_index,
&ptr_item_dst_name,
&ptr_item_dst_index);
}
else if (opop->subitem_reference_index != -1 && opop->subitem_local_index == -1) {
rna_property_override_collection_subitem_name_index_lookup(ptr_src,
prop_src,
nullptr,
opop->subitem_reference_index,
&ptr_item_src_name,
&ptr_item_src_index);
}
/* For storage, simply lookup by name first, and fallback to indices. */
if (prop_storage != nullptr) {
if (opop->subitem_local_name != nullptr) {
RNA_property_collection_lookup_string(
ptr_storage, prop_storage, opop->subitem_local_name, private_ptr_item_storage);
rna_property_override_collection_subitem_name_index_lookup(ptr_storage,
prop_storage,
opop->subitem_local_name,
opop->subitem_local_index,
&ptr_item_storage_name,
&ptr_item_storage_index);
if (ptr_item_storage_name.data == nullptr) {
rna_property_override_collection_subitem_name_index_lookup(ptr_storage,
prop_storage,
opop->subitem_reference_name,
opop->subitem_reference_index,
&ptr_item_storage_name,
&ptr_item_storage_index);
}
else if (opop->subitem_reference_name != nullptr) {
RNA_property_collection_lookup_string(
ptr_storage, prop_storage, opop->subitem_reference_name, private_ptr_item_storage);
if (ptr_item_storage_name.data == nullptr && ptr_item_storage_index.data == nullptr) {
rna_property_override_collection_subitem_name_index_lookup(ptr_storage,
prop_storage,
nullptr,
opop->subitem_local_index,
&ptr_item_storage_name,
&ptr_item_storage_index);
}
else if (opop->subitem_local_index != -1) {
RNA_property_collection_lookup_int(
ptr_storage, prop_storage, opop->subitem_local_index, private_ptr_item_storage);
}
/* Final selection. both matches have to be based on names, or indices, but not a mix of both. */
if (ptr_item_src_name.type != nullptr && ptr_item_dst_name.type != nullptr) {
*private_ptr_item_src = ptr_item_src_name;
*private_ptr_item_dst = ptr_item_dst_name;
if (prop_storage != nullptr) {
*private_ptr_item_storage = ptr_item_storage_name;
}
else if (opop->subitem_reference_index != -1) {
RNA_property_collection_lookup_int(
ptr_storage, prop_storage, opop->subitem_reference_index, private_ptr_item_storage);
}
else if (ptr_item_src_index.type != nullptr && ptr_item_dst_index.type != nullptr) {
*private_ptr_item_src = ptr_item_src_index;
*private_ptr_item_dst = ptr_item_dst_index;
if (prop_storage != nullptr) {
*private_ptr_item_storage = ptr_item_storage_index;
}
}
*r_ptr_item_dst = private_ptr_item_dst;
@ -1204,7 +1305,7 @@ static void rna_property_override_apply_ex(Main *bmain,
* Note that here, src is the local saved ID, and dst is a copy of the linked ID (since we use
* local ID as storage to apply local changes on top of a clean copy of the linked data). */
PointerRNA private_ptr_item_dst, private_ptr_item_src, private_ptr_item_storage;
rna_porperty_override_collection_subitem_lookup(ptr_dst,
rna_property_override_collection_subitem_lookup(ptr_dst,
ptr_src,
ptr_storage,
prop_dst,
@ -1400,7 +1501,7 @@ void RNA_struct_override_apply(Main *bmain,
PointerRNA *ptr_item_dst, *ptr_item_src;
PointerRNA private_ptr_item_dst, private_ptr_item_src;
rna_porperty_override_collection_subitem_lookup(ptr_dst,
rna_property_override_collection_subitem_lookup(ptr_dst,
ptr_src,
nullptr,
prop_dst,