Keyframing Bugfix:

This commit attempts to fix some of the bugs which were causing grief with some Durian animation tests. 

In one of those files, the order in which F-Curves were stored was seriously messed up; causing problems with some F-Curves still existing but unable to be edited (i.e. still showing up in the Object/Action summaries but nowhere else) since the standard assumptions for the way the data was stored had been violated.

I've recoded the code that ensures that when F-Curves get added to Action Groups (and the Action that contains these) it ends up in the right places, since it was very likely that all the F-Curves would only ever get added near the end of the list. 

Hopefully this is enough to prevent these problems reoccurring, though I have a feeling there may still be one or two buggy tools which caused the problems in the first place. 

1 files changed, 42 insertions, 75 deletions

diff --git a/source/blender/blenkernel/intern/action.c b/source/blender/blenkernel/intern/action.c
index ace1292f813..303cd208b7c 100644
--- a/source/blender/blenkernel/intern/action.c
+++ b/source/blender/blenkernel/intern/action.c
@@ -254,101 +254,68 @@ void set_active_action_group (bAction *act, bActionGroup *agrp, short select)
  *	- always adds at the end of the group 
  */
 void action_groups_add_channel (bAction *act, bActionGroup *agrp, FCurve *fcurve)
-{
-	FCurve *fcu;
-	short done=0;
-	
+{	
 	/* sanity checks */
 	if (ELEM3(NULL, act, agrp, fcurve))
 		return;
 	
-	/* if no channels, just add to two lists at the same time */
+	/* if no channels anywhere, just add to two lists at the same time */
 	if (act->curves.first == NULL) {
 		fcurve->next = fcurve->prev = NULL;
 		
 		agrp->channels.first = agrp->channels.last = fcurve;
 		act->curves.first = act->curves.last = fcurve;
-		
-		fcurve->grp= agrp;
-		return;
 	}
 	
-	/* try to find a channel to slot this in before/after */
-	for (fcu= act->curves.first; fcu; fcu= fcu->next) {
-		/* if channel has no group, then we have ungrouped channels, which should always occur after groups */
-		if (fcu->grp == NULL) {
-			BLI_insertlinkbefore(&act->curves, fcu, fcurve);
-			
-			if (agrp->channels.first == NULL)
-				agrp->channels.first= fcurve;
-			agrp->channels.last= fcurve;
+	/* if the group already has channels, the F-Curve can simply be added to the list 
+	 * (i.e. as the last channel in the group)
+	 */
+	else if (agrp->channels.first) {
+		/* if the group's last F-Curve is the action's last F-Curve too, 
+		 * then set the F-Curve as the last for the action first so that
+		 * the lists will be in sync after linking
+		 */
+		if (agrp->channels.last == act->curves.last)
+			act->curves.last= fcurve;
 			
-			done= 1;
-			break;
-		}
+		/* link in the given F-Curve after the last F-Curve in the group,
+		 * which means that it should be able to fit in with the rest of the
+		 * list seamlessly
+		 */
+		BLI_insertlinkafter(&agrp->channels, agrp->channels.last, fcurve);
+	}
+	
+	/* otherwise, need to find the nearest F-Curve in group before/after current to link with */
+	else {
+		bActionGroup *grp;
 		
-		/* if channel has group after current, we can now insert (otherwise we have gone too far) */
-		else if (fcu->grp == agrp->next) {
-			BLI_insertlinkbefore(&act->curves, fcu, fcurve);
-			
-			if (agrp->channels.first == NULL)
-				agrp->channels.first= fcurve;
-			agrp->channels.last= fcurve;
-			
-			done= 1;
-			break;
-		}
+		/* firstly, link this F-Curve to the group */
+		agrp->channels.first = agrp->channels.last = fcurve;
 		
-		/* if channel has group we're targeting, check whether it is the last one of these */
-		else if (fcu->grp == agrp) {
-			if ((fcu->next) && (fcu->next->grp != agrp)) {
-				BLI_insertlinkafter(&act->curves, fcu, fcurve);
-				agrp->channels.last= fcurve;
-				done= 1;
-				break;
-			}
-			else if (fcu->next == NULL) {
-				BLI_addtail(&act->curves, fcurve);
-				agrp->channels.last= fcurve;
-				done= 1;
+		/* step through the groups preceeding this one, finding the F-Curve there to attach this one after */
+		for (grp= agrp->prev; grp; grp= grp->prev) {
+			/* if this group has F-Curves, we want weave the given one in right after the last channel there,
+			 * but via the Action's list not this group's list
+			 *	- this is so that the F-Curve is in the right place in the Action,
+			 *	  but won't be included in the previous group
+			 */
+			if (grp->channels.last) {
+				/* once we've added, break here since we don't need to search any further... */
+				BLI_insertlinkafter(&act->curves, grp->channels.last, fcurve);
 				break;
 			}
 		}
 		
-		/* if channel has group before target, check whether the next one is something after target */
-		else if (fcu->grp == agrp->prev) {
-			if (fcu->next) {
-				if ((fcu->next->grp != fcu->grp) && (fcu->next->grp != agrp)) {
-					BLI_insertlinkafter(&act->curves, fcu, fcurve);
-					
-					agrp->channels.first= fcurve;
-					agrp->channels.last= fcurve;
-					
-					done= 1;
-					break;
-				}
-			}
-			else {
-				BLI_insertlinkafter(&act->curves, fcu, fcurve);
-				
-				agrp->channels.first= fcurve;
-				agrp->channels.last= fcurve;
-				
-				done= 1;
-				break;
-			}
-		}
+		/* if grp is NULL, that means we fell through, and this F-Curve should be added as the new first
+		 * since group is (effectively) the first group. Thus, the existing first F-Curve becomes the 
+		 * second in the chain, etc. etc.
+		 */
+		if (grp == NULL)
+			BLI_insertlinkbefore(&act->curves, act->curves.first, fcurve);
 	}
 	
-	/* only if added, set channel as belonging to this group */
-	if (done) {
-		//printf("FCurve added to group \n");
-		fcurve->grp= agrp;
-	}
-	else {
-		printf("Error: FCurve '%s' couldn't be added to Group '%s' \n", fcurve->rna_path, agrp->name);
-		BLI_addtail(&act->curves, fcurve);
-	}
+	/* set the F-Curve's new group */
+	fcurve->grp= agrp;
 }	
 
 /* Remove the given channel from all groups */