Fix T94116: Drivers can have multiple variables with same name

The RNA setter now ensures that driver variables are uniquely named
(within the scope of the driver).

Versioning code has been added to ensure this uniqueness. The last
variable with the non-unique name retains the original name; this
ensures that the driver will still evaluate to the same value as before
this fix.

This also introduces a new blenlib function `BLI_listbase_from_link()`,
which can be used to find the entire list from any item within the list.

Manifest Task: T94116

Reviewed By: mont29, JacquesLucke

Maniphest Tasks: T94116

Differential Revision: https://developer.blender.org/D13594

7 files changed, 116 insertions, 0 deletions

diff --git a/source/blender/blenkernel/BKE_fcurve_driver.h b/source/blender/blenkernel/BKE_fcurve_driver.h
index 18676dfcb38..7e4e0ad3c2a 100644
--- a/source/blender/blenkernel/BKE_fcurve_driver.h
+++ b/source/blender/blenkernel/BKE_fcurve_driver.h
@@ -105,6 +105,13 @@ void driver_change_variable_type(struct DriverVar *dvar, int type);
  */
 void driver_variable_name_validate(struct DriverVar *dvar);
 /**
+ * Ensure the driver variable's name is unique.
+ *
+ * Assumes the driver variable has already been assigned to the driver, so that
+ * the prev/next pointers can be used to find the other variables.
+ */
+void driver_variable_unique_name(struct DriverVar *dvar);
+/**
  * Add a new driver variable.
  */
 struct DriverVar *driver_add_new_variable(struct ChannelDriver *driver);
diff --git a/source/blender/blenkernel/intern/fcurve_driver.c b/source/blender/blenkernel/intern/fcurve_driver.c
index 5496519e53b..ce30f80ba65 100644
--- a/source/blender/blenkernel/intern/fcurve_driver.c
+++ b/source/blender/blenkernel/intern/fcurve_driver.c
@@ -29,6 +29,7 @@
 
 #include "BLI_alloca.h"
 #include "BLI_expr_pylike_eval.h"
+#include "BLI_listbase.h"
 #include "BLI_math.h"
 #include "BLI_string_utils.h"
 #include "BLI_threads.h"
@@ -864,6 +865,12 @@ void driver_variable_name_validate(DriverVar *dvar)
   }
 }
 
+void driver_variable_unique_name(DriverVar *dvar)
+{
+  ListBase variables = BLI_listbase_from_link((Link *)dvar);
+  BLI_uniquename(&variables, dvar, dvar->name, '_', offsetof(DriverVar, name), sizeof(dvar->name));
+}
+
 DriverVar *driver_add_new_variable(ChannelDriver *driver)
 {
   DriverVar *dvar;
diff --git a/source/blender/blenlib/BLI_listbase.h b/source/blender/blenlib/BLI_listbase.h
index 7d808d339e9..a2a6e958213 100644
--- a/source/blender/blenlib/BLI_listbase.h
+++ b/source/blender/blenlib/BLI_listbase.h
@@ -46,6 +46,11 @@ int BLI_findstringindex(const struct ListBase *listbase,
                         const char *id,
                         const int offset) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1);
 
+/**
+ * Return a ListBase representing the entire list the given Link is in.
+ */
+ListBase BLI_listbase_from_link(struct Link *some_link);
+
 /* Find forwards. */
 
 /**
@@ -279,6 +284,23 @@ BLI_INLINE void BLI_listbase_clear(struct ListBase *lb)
 }
 
 /**
+ * Equality check for ListBase.
+ *
+ * This only shallowly compares the ListBase itself (so the first/last
+ * pointers), and does not do any equality checks on the list items.
+ */
+BLI_INLINE bool BLI_listbase_equal(const struct ListBase *a, const struct ListBase *b)
+{
+  if (a == NULL) {
+    return b == NULL;
+  }
+  if (b == NULL) {
+    return false;
+  }
+  return a->first == b->first && a->last == b->last;
+}
+
+/**
  * Create a generic list node containing link to provided data.
  */
 struct LinkData *BLI_genericNodeN(void *data);
@@ -353,3 +375,10 @@ struct LinkData *BLI_genericNodeN(void *data);
 #ifdef __cplusplus
 }
 #endif
+
+#ifdef __cplusplus
+BLI_INLINE bool operator==(const ListBase &a, const ListBase &b)
+{
+  return BLI_listbase_equal(&a, &b);
+}
+#endif
diff --git a/source/blender/blenlib/intern/listbase.c b/source/blender/blenlib/intern/listbase.c
index a166c846ea7..513b08a589d 100644
--- a/source/blender/blenlib/intern/listbase.c
+++ b/source/blender/blenlib/intern/listbase.c
@@ -749,6 +749,26 @@ int BLI_findstringindex(const ListBase *listbase, const char *id, const int offs
   return -1;
 }
 
+ListBase BLI_listbase_from_link(Link *some_link)
+{
+  ListBase list = {some_link, some_link};
+  if (some_link == NULL) {
+    return list;
+  }
+
+  /* Find the first element. */
+  while (((Link *)list.first)->prev != NULL) {
+    list.first = ((Link *)list.first)->prev;
+  }
+
+  /* Find the last element. */
+  while (((Link *)list.last)->next != NULL) {
+    list.last = ((Link *)list.last)->next;
+  }
+
+  return list;
+}
+
 void BLI_duplicatelist(ListBase *dst, const ListBase *src)
 {
   struct Link *dst_link, *src_link;
diff --git a/source/blender/blenlib/tests/BLI_listbase_test.cc b/source/blender/blenlib/tests/BLI_listbase_test.cc
index d66eb214902..9e4d7c7dd36 100644
--- a/source/blender/blenlib/tests/BLI_listbase_test.cc
+++ b/source/blender/blenlib/tests/BLI_listbase_test.cc
@@ -154,6 +154,31 @@ TEST(listbase, FindLinkFromStringOrPointer)
   BLI_freelistN(&lb);
 }
 
+TEST(listbase, FromLink)
+{
+  ListBase lb = {nullptr, nullptr};
+  Link *link1 = static_cast<Link *>(MEM_callocN(sizeof(Link), "link1"));
+  Link *link2 = static_cast<Link *>(MEM_callocN(sizeof(Link), "link2"));
+  Link *link3 = static_cast<Link *>(MEM_callocN(sizeof(Link), "link3"));
+
+  /* NULL safety. */
+  EXPECT_EQ(lb, BLI_listbase_from_link(nullptr));
+
+  /* One link. */
+  BLI_addtail(&lb, link1);
+  EXPECT_EQ(lb, BLI_listbase_from_link(link1));
+
+  /* Two links. */
+  BLI_addtail(&lb, link2);
+  EXPECT_EQ(lb, BLI_listbase_from_link(link2));
+
+  /* Three links, search from middle. */
+  BLI_addtail(&lb, link3);
+  EXPECT_EQ(lb, BLI_listbase_from_link(link2));
+
+  BLI_freelistN(&lb);
+}
+
 /* -------------------------------------------------------------------- */
 /* Sort utilities & test */
 
diff --git a/source/blender/blenloader/intern/versioning_300.c b/source/blender/blenloader/intern/versioning_300.c
index 7a7f12a7e58..6b22d2f97e9 100644
--- a/source/blender/blenloader/intern/versioning_300.c
+++ b/source/blender/blenloader/intern/versioning_300.c
@@ -30,6 +30,7 @@
 #include "BLI_math_vector.h"
 #include "BLI_path_util.h"
 #include "BLI_string.h"
+#include "BLI_string_utils.h"
 #include "BLI_utildefines.h"
 
 #include "DNA_anim_types.h"
@@ -790,6 +791,32 @@ void do_versions_after_linking_300(Main *bmain, ReportList *UNUSED(reports))
    */
   {
     /* Keep this block, even when empty. */
+
+    { /* Ensure driver variable names are unique within the driver. */
+      ID *id;
+      FOREACH_MAIN_ID_BEGIN (bmain, id) {
+        AnimData *adt = BKE_animdata_from_id(id);
+        if (adt == NULL) {
+          continue;
+        }
+        LISTBASE_FOREACH (FCurve *, fcu, &adt->drivers) {
+          ChannelDriver *driver = fcu->driver;
+          /* Ensure the uniqueness front to back. Given a list of identically
+           * named variables, the last one gets to keep its original name. This
+           * matches the evaluation order, and thus shouldn't change the evaluated
+           * value of the driver expression. */
+          LISTBASE_FOREACH (DriverVar *, dvar, &driver->variables) {
+            BLI_uniquename(&driver->variables,
+                           dvar,
+                           dvar->name,
+                           '_',
+                           offsetof(DriverVar, name),
+                           sizeof(dvar->name));
+          }
+        }
+      }
+      FOREACH_MAIN_ID_END;
+    }
   }
 }
 
diff --git a/source/blender/makesrna/intern/rna_fcurve.c b/source/blender/makesrna/intern/rna_fcurve.c
index a38bbd3d6d2..d3175c445a9 100644
--- a/source/blender/makesrna/intern/rna_fcurve.c
+++ b/source/blender/makesrna/intern/rna_fcurve.c
@@ -388,6 +388,7 @@ void rna_DriverVariable_name_set(PointerRNA *ptr, const char *value)
 
   BLI_strncpy_utf8(data->name, value, 64);
   driver_variable_name_validate(data);
+  driver_variable_unique_name(data);
 }
 
 /* ----------- */