Fix sycnchronous animation queues

rwaldron#1376 only fixed the problem when animation segments were enqueued in seperate passes through the event loop.  When two animation segments were enqueued on the same pass through the even loop we were starting both segments because the isRunning state was not being set until nextTick.

This should fix all that. I've updated some of the tests which I believe were written in a non-intuitive manner because of this bug. Instead of really understanding the problem I had just written the tests in a way that made them pass. There's probably a name for this sort of behavior. If not, we should invent a name.

Author: dtex

lib/animation.js

@@ -91,7 +91,7 @@ function Animation(target) {
 
   Animation.Segment.call(this);
 
-  this.defaultTarget = target;
+  this.defaultTarget = target || {};
 }
 
 util.inherits(Animation, Emitter);
@@ -152,8 +152,7 @@ Animation.prototype.enqueue = function(opts) {
     opts.target = this.defaultTarget;
   }
 
-  this.segments.push(new Animation.Segment(opts));
-
+  this.segments.push(opts);
 
   /* istanbul ignore if */
   if (!this.paused && !this.isRunning) {
@@ -170,10 +169,16 @@ Animation.prototype.enqueue = function(opts) {
  */
 Animation.prototype.next = function() {
 
-  if (this.segments.length > 0) {
-
-    Object.assign(this, this.segments.shift());
+  if (this.isRunning) {
+    return this;
+  } else {
+    this.isRunning = true;
+  }
 
+  if (this.segments.length > 0) {
+    var segment = new Animation.Segment(this.segments.shift());
+    
+    Object.assign(this, segment);
     this.paused = this.currentSpeed === 0 ? true : false;
 
     if (this.onstart) {
@@ -281,7 +286,6 @@ Animation.prototype.loopFunction = function(loop) {
   // Find the current timeline progress
   var progress = this.calculateProgress(loop.calledAt);
 
-
   // Find the left and right cuePoints/keyFrames;
   var indices = this.findIndices(progress);
 
@@ -320,14 +324,16 @@ Animation.prototype.loopFunction = function(loop) {
       this.endTime = this.startTime + this.scaledDuration;
     } else {
 
-      this.stop();
+      this.isRunning = false;
 
       if (this.oncomplete) {
         process.nextTick(this.oncomplete.bind(this));
       }
 
       if (this.segments.length > 0) {
-        this.next();
+        process.nextTick(() => { this.next(); });
+      } else {
+        this.stop();
       }
     }
   }

test/animation.js

@@ -360,7 +360,32 @@ exports["Animation -- Servo"] = {
     );
 
     test.done();
-  }
+  },
+  
+    enqueueOnSameTick: function(test) {
+      
+      test.expect(7);
+  
+      this.animation = new Animation(this.a);
+  
+      this.normalizeKeyframes = this.sandbox.spy(this.animation, "normalizeKeyframes");
+      
+      test.equal(this.animation.isRunning, false);
+      test.equal(this.animation.segments.length, 0);
+      
+      // This is the first segment so it should be immediately shifted off the queue
+      test.equal(this.animation.enqueue(this.segment.single), this.animation);
+      test.equal(this.animation.segments.length, 0);
+      test.equal(this.animation.isRunning, true);
+      
+      // This is the second segment so it should stay in the queue
+      test.equal(this.animation.enqueue(this.segment.single), this.animation);
+      test.equal(this.animation.segments.length, 1);
+      
+      this.normalizeKeyframes.restore();
+      test.done();
+
+    }
 
 };
 
@@ -436,6 +461,7 @@ exports["Animation"] = {
     test.equal(this.animation.segments.length, 1);
     test.done();
   },
+
   next: function(test) {
     test.expect(12);
 
@@ -687,78 +713,6 @@ exports["Animation"] = {
     test.done();
   },
 
-  loopFunctiononcomplete: function(test) {
-    test.expect(2);
-
-    this.clock = this.sandbox.useFakeTimers();
-    this.tfb = this.sandbox.stub(Animation, "TemporalFallback", function() {});
-
-    var startTime = Date.now();
-    var loop = {
-      calledAt: startTime + 1000
-    };
-
-    this.animation.startTime = startTime;
-    this.animation.fallBackTime = 500;
-    this.animation.speed(1);
-
-
-    this.animation.normalizedKeyFrames = [];
-    this.animation.target = {};
-    this.animation.target[Animation.render] = function() {};
-    this.animation.playLoop = null;
-    this.animation.onloop = this.sandbox.stub();
-    this.animation.next = this.sandbox.stub();
-
-    var animation = this.animation;
-
-    this.sandbox.stub(Animation.prototype, "stop");
-    this.animation.oncomplete = function() {
-      test.equal(Animation.TemporalFallback.callCount, 1);
-      test.equal(this, animation);
-
-      animation.oncomplete = null;
-      test.done();
-    };
-
-    this.animation.loopFunction(loop);
-  },
-
-  loopFunctionNOoncompleteSegmentsRemaining: function(test) {
-    test.expect(1);
-
-    this.clock = this.sandbox.useFakeTimers();
-    this.tfb = this.sandbox.stub(Animation, "TemporalFallback", function() {});
-    this.stop = this.sandbox.stub(Animation.prototype, "stop");
-
-    var startTime = Date.now();
-    var loop = {
-      calledAt: startTime + 1000
-    };
-
-    this.animation.startTime = startTime;
-    this.animation.fallBackTime = 500;
-    this.animation.speed(1);
-
-
-    this.animation.normalizedKeyFrames = [];
-    this.animation.target = {};
-    this.animation.target[Animation.render] = function() {};
-    this.animation.playLoop = null;
-    this.animation.onloop = this.sandbox.stub();
-    this.animation.next = this.sandbox.stub();
-
-    this.nextTick = this.sandbox.stub(process, "nextTick");
-
-    this.animation.next = function() {
-      test.equal(this.stop.callCount, 1);
-      test.done();
-    }.bind(this);
-
-    this.animation.segments = [1, 2, 3, 4];
-    this.animation.loopFunction(loop);
-  },
-
   loopFunctionfallBackTimeWithPlayLoop: function(test) {
     test.expect(1);
 
@@ -794,8 +748,14 @@ exports["Animation"] = {
     test.expect(1);
 
     this.clock = this.sandbox.useFakeTimers();
-    this.normalizeKeyframes = this.sandbox.stub(Animation.prototype, "normalizeKeyframes", function() {
-      this.loopback = 1;
+    this.animation.normalizeKeyframes = this.sandbox.stub(Animation.prototype, "normalizeKeyframes", function() {
+      this.animation.loopback = 1;
+      this.animation.normalizedKeyFrames = [[
+        { value: 90, easing: "linear" },
+        { step: false, easing: "linear", value: 90 },
+        { value: 45, easing: "linear" },
+        { step: 33, easing: "linear", value: 78 }
+      ]];
     }.bind(this));
 
     var startTime = Date.now();
@@ -804,18 +764,20 @@ exports["Animation"] = {
     };
 
     this.animation.startTime = startTime;
-    this.animation.normalizedKeyFrames = [];
     this.animation.target = {};
     this.animation.target[Animation.render] = function() {};
     this.animation.speed(1);
 
     this.animation.metronomic = true;
     this.animation.reverse = false;
     this.animation.loop = true;
+    this.animation.fps = 10;
     this.animation.onloop = this.sandbox.spy();
+    this.animation.normalizeKeyframes();
     this.animation.loopFunction(loop);
 
     test.equal(this.animation.onloop.callCount, 1);
+    this.animation.stop();
     test.done();
   },
 
@@ -1021,8 +983,9 @@ exports["Animation.Segment"] = {
   },
 
   instanceof: function(test) {
-    test.expect(1);
+    test.expect(2);
     test.equal(Animation({}) instanceof Animation, true);
+    test.equal(Animation() instanceof Animation, true);
     test.done();
   },