Fix T53909: Joining armatures did not remap names on merged action data
authorJoshua Leung <aligorith@gmail.com>
Mon, 29 Jan 2018 04:13:23 +0000 (17:13 +1300)
committerJoshua Leung <aligorith@gmail.com>
Mon, 29 Jan 2018 04:22:03 +0000 (17:22 +1300)
Technically this was not a bug, as this functionality was not meant to
work. (Drivers were already handled though, as they are part of the rig)
It was assumed that there was little value in having this functionality
available, as in most pipelines, animation production only begins after
the rig has been locked down (see bug report comments for more details).

On reflection, in most common situations, there's probably no harm in
doing these rna path fixups. This commit takes advantage of some similar
code I recently put in place in the Grease Pencil branch (for joining GP
objects and their layers).

Important Note for Animators/Riggers/TD's:
Please be aware that after joining armatures, some of the animation may
still need to be redone (due to changes in the transform hierarchies/
transform spaces that the animation is applied in). We do not attempt
to correct for these problems, and it is unlikely that we will in future.

source/blender/editors/armature/armature_relations.c

index 14304b158df1f01124ad8aaa10b286af6ccceaa0..dfe7876175fdb43aae1b81a54a60f7fa3cabf81c 100644 (file)
@@ -127,46 +127,36 @@ typedef struct tJoinArmature_AdtFixData {
 /* FIXME: For now, we only care about drivers here. When editing rigs, it's very rare to have animation
  *        on the rigs being edited already, so it should be safe to skip these.
  */
-static void joined_armature_fix_animdata_cb(ID *id, AnimData *adt, void *user_data)
+static void joined_armature_fix_animdata_cb(ID *id, FCurve *fcu, void *user_data)
 {
        tJoinArmature_AdtFixData *afd = (tJoinArmature_AdtFixData *)user_data;
        ID *src_id = &afd->srcArm->id;
        ID *dst_id = &afd->tarArm->id;
        
        GHashIterator gh_iter;
-       FCurve *fcu;
        
        /* Fix paths - If this is the target object, it will have some "dirty" paths */
-       if (id == src_id) {
-               /* Fix drivers */
-               for (fcu = adt->drivers.first; fcu; fcu = fcu->next) {
-                       /* skip driver if it doesn't affect the bones */
-                       if (strstr(fcu->rna_path, "pose.bones[") == NULL) {
-                               continue;
-                       }
+       if ((id == src_id) && strstr(fcu->rna_path, "pose.bones[")) {
+               GHASH_ITER(gh_iter, afd->names_map) {
+                       const char *old_name = BLI_ghashIterator_getKey(&gh_iter);
+                       const char *new_name = BLI_ghashIterator_getValue(&gh_iter);
                        
-                       // FIXME: this is too crude... it just does everything!
-                       GHASH_ITER(gh_iter, afd->names_map) {
-                               const char *old_name = BLI_ghashIterator_getKey(&gh_iter);
-                               const char *new_name = BLI_ghashIterator_getValue(&gh_iter);
+                       /* only remap if changed; this still means there will be some waste if there aren't many drivers/keys */
+                       if (!STREQ(old_name, new_name) && strstr(fcu->rna_path, old_name)) {
+                               fcu->rna_path = BKE_animsys_fix_rna_path_rename(id, fcu->rna_path, "pose.bones",
+                                                                               old_name, new_name, 0, 0, false);
                                
-                               /* only remap if changed; this still means there will be some waste if there aren't many drivers/keys */
-                               if (!STREQ(old_name, new_name) && strstr(fcu->rna_path, old_name)) {
-                                       fcu->rna_path = BKE_animsys_fix_rna_path_rename(id, fcu->rna_path, "pose.bones",
-                                                                                       old_name, new_name, 0, 0, false);
-                                       
-                                       /* we don't want to apply a second remapping on this driver now, 
-                                        * so stop trying names, but keep fixing drivers
-                                        */
-                                       break;
-                               }
+                               /* we don't want to apply a second remapping on this driver now, 
+                                * so stop trying names, but keep fixing drivers
+                                */
+                               break;
                        }
                }
        }
        
        
        /* Driver targets */
-       for (fcu = adt->drivers.first; fcu; fcu = fcu->next) {
+       if (fcu->driver) {
                ChannelDriver *driver = fcu->driver;
                DriverVar *dvar;
                
@@ -370,7 +360,7 @@ int join_armature_exec(bContext *C, wmOperator *op)
                        }
                        
                        /* Fix all the drivers (and animation data) */
-                       BKE_animdata_main_cb(bmain, joined_armature_fix_animdata_cb, &afd);
+                       BKE_fcurves_main_cb(bmain, joined_armature_fix_animdata_cb, &afd);
                        BLI_ghash_free(afd.names_map, MEM_freeN, NULL);
                        
                        /* Only copy over animdata now, after all the remapping has been done,