fix: [#1636] Animation flicker on first frame (#1637)

Closes #1636

## Changes:

- Update animations in separate tick from draw()
- Add idempotency token to prevent the same animation from being updated multiple times per frame
- Added new tests

CHANGELOG.md

@@ -9,6 +9,26 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Added
 
+### Changed
+
+### Deprecated
+
+### Removed
+
+### Fixed
+
+- Fixed Animation flicker bug on the first frame when using animations with scale, anchors, or rotation. ([#1636](https://github.com/excaliburjs/Excalibur/issues/1636))
+
+<!--------------------------------- DO NOT EDIT BELOW THIS LINE --------------------------------->
+<!--------------------------------- DO NOT EDIT BELOW THIS LINE --------------------------------->
+<!--------------------------------- DO NOT EDIT BELOW THIS LINE --------------------------------->
+
+## [0.24.4] - 2020-09-02
+
+### Breaking Changes
+
+### Added
+
 - Add new `ex.Screen` abstraction to manage viewport size and resolution independently and all other screen related logic. ([#1617](https://github.com/excaliburjs/Excalibur/issues/1617))
   - New support for the browser fullscreen API
 - Add color blind mode simulation and correction in debug object.
@@ -39,10 +59,6 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed issue where actors that were not in scene still received pointer events ([#1555](https://github.com/excaliburjs/Excalibur/issues/1555))
 - Fixed Scene initialization order when using the lifecycle overrides ([#1553](https://github.com/excaliburjs/Excalibur/issues/1553))
 
-<!--------------------------------- DO NOT EDIT BELOW THIS LINE --------------------------------->
-<!--------------------------------- DO NOT EDIT BELOW THIS LINE --------------------------------->
-<!--------------------------------- DO NOT EDIT BELOW THIS LINE --------------------------------->
-
 ## [0.24.0] - 2020-04-23
 
 ### Breaking Changes

src/engine/Actor.ts

@@ -25,6 +25,7 @@ import { PointerEvent, WheelEvent, PointerDragEvent, PointerEventName } from './
 import { Engine } from './Engine';
 import { Color } from './Drawing/Color';
 import { Sprite } from './Drawing/Sprite';
+import { Animation } from './Drawing/Animation';
 import { Trait } from './Interfaces/Trait';
 import { Drawable } from './Interfaces/Drawable';
 import { CanInitialize, CanUpdate, CanDraw, CanBeKilled } from './Interfaces/LifecycleEvents';
@@ -1129,6 +1130,12 @@ export class ActorImpl extends Class implements Actionable, Eventable, PointerEv
     this._initialize(engine);
     this._preupdate(engine, delta);
 
+    // Tick animations
+    const drawing = this.currentDrawing;
+    if (drawing && drawing instanceof Animation) {
+      drawing.tick(delta, engine.stats.currFrame.id);
+    }
+
     // Update action queue
     this.actionQueue.update(delta);
 

src/engine/Drawing/Animation.ts

@@ -8,10 +8,18 @@ import { Engine } from '../Engine';
 import * as Util from '../Util/Util';
 import { Configurable } from '../Configurable';
 
+export interface HasTick {
+  /**
+   *
+   * @param elapsedMilliseconds The amount of real world time in milliseconds that has elapsed that must be updated in the animation
+   */
+  tick(elapsedMilliseconds: number, idempotencyToken?: number): void;
+}
+
 /**
  * @hidden
  */
-export class AnimationImpl implements Drawable {
+export class AnimationImpl implements Drawable, HasTick {
   /**
    * The sprite frames to play, in order. See [[SpriteSheet.getAnimationForAll]] to quickly
    * generate an [[Animation]].
@@ -28,7 +36,8 @@ export class AnimationImpl implements Drawable {
    */
   public currentFrame: number = 0;
 
-  private _oldTime: number = Date.now();
+  private _timeLeftInFrame: number = 0;
+  private _idempotencyToken: number = -1;
 
   public anchor: Vector = Vector.Zero;
   public rotation: number = 0.0;
@@ -85,6 +94,7 @@ export class AnimationImpl implements Drawable {
     this.sprites = sprites;
     this.speed = speed;
     this._engine = <Engine>engine;
+    this._timeLeftInFrame = this.speed;
 
     if (loop != null) {
       this.loop = loop;
@@ -243,11 +253,24 @@ export class AnimationImpl implements Drawable {
    * calculates whether to change to the frame.
    * @internal
    */
-  public tick() {
-    const time = Date.now();
-    if (time - this._oldTime > this.speed) {
+  public tick(elapsed: number, idempotencyToken?: number) {
+    if (this._idempotencyToken === idempotencyToken) {
+      return;
+    }
+    this._idempotencyToken = idempotencyToken;
+    this._timeLeftInFrame -= elapsed;
+    if (this._timeLeftInFrame <= 0) {
       this.currentFrame = this.loop ? (this.currentFrame + 1) % this.sprites.length : this.currentFrame + 1;
-      this._oldTime = time;
+      this._timeLeftInFrame = this.speed;
+    }
+
+    this._updateValues();
+    const current = this.sprites[this.currentFrame];
+    if (current) {
+      this.width = current.width;
+      this.height = current.height;
+      this.drawWidth = current.drawWidth;
+      this.drawHeight = current.drawHeight;
     }
   }
 
@@ -297,7 +320,6 @@ export class AnimationImpl implements Drawable {
       opacity: options.opacity ?? this._opacity
     };
 
-    this.tick();
     this._updateValues();
     let currSprite: Sprite;
     if (this.currentFrame < this.sprites.length) {

src/engine/Engine.ts

@@ -1175,7 +1175,6 @@ O|===|* >________________>\n\
 
         // reset frame stats (reuse existing instances)
         const frameId = game.stats.prevFrame.id + 1;
-        game.stats.prevFrame.reset(game.stats.currFrame);
         game.stats.currFrame.reset();
         game.stats.currFrame.id = frameId;
         game.stats.currFrame.delta = delta;
@@ -1193,6 +1192,7 @@ O|===|* >________________>\n\
         lastTime = now;
 
         game.emit('postframe', new PostFrameEvent(game, game.stats.currFrame));
+        game.stats.prevFrame.reset(game.stats.currFrame);
       } catch (e) {
         window.cancelAnimationFrame(game._requestId);
         game.stop();

src/spec/AnimationSpec.ts

@@ -53,6 +53,87 @@ describe('An animation', () => {
     expect(animation.sprites.length).toBe(0);
   });
 
+  it('should update the animation width/height and sprite anchor, rotation, scale after tick()', (done) => {
+    const texture = new ex.Texture('base/src/spec/images/SpriteSpec/icon.png', true);
+    texture.load().then(() => {
+      const sprite = new ex.Sprite({
+        image: texture,
+        x: 0,
+        y: 0,
+        width: 62,
+        height: 64,
+        rotation: 0,
+        anchor: new ex.Vector(0.0, 0.0),
+        scale: new ex.Vector(1, 1),
+        flipVertical: false,
+        flipHorizontal: false
+      });
+      const animation = new ex.Animation({
+        engine: engine,
+        sprites: [sprite],
+        speed: 200,
+        loop: false,
+        anchor: new ex.Vector(1, 1),
+        rotation: Math.PI,
+        scale: new ex.Vector(2, 2),
+        flipVertical: true,
+        flipHorizontal: true,
+        width: 100,
+        height: 200
+      });
+
+      animation.tick(10);
+      expect(sprite.scale).toBeVector(animation.scale);
+      expect(sprite.anchor).toBeVector(animation.anchor);
+      expect(sprite.rotation).toBe(animation.rotation);
+
+      expect(animation.width).toBe(sprite.width);
+      expect(animation.height).toBe(sprite.height);
+      expect(animation.drawWidth).toBe(sprite.drawWidth);
+      expect(animation.drawHeight).toBe(sprite.drawHeight);
+      done();
+    });
+  });
+
+  it('should only tick once for an idempotency token', (done) => {
+    const texture = new ex.Texture('base/src/spec/images/SpriteSpec/icon.png', true);
+    texture.load().then(() => {
+      const sprite = new ex.Sprite({
+        image: texture,
+        x: 0,
+        y: 0,
+        width: 62,
+        height: 64,
+        rotation: 0,
+        anchor: new ex.Vector(0.0, 0.0),
+        scale: new ex.Vector(1, 1),
+        flipVertical: false,
+        flipHorizontal: false
+      });
+      const animation = new ex.Animation({
+        engine: engine,
+        sprites: [sprite, sprite],
+        speed: 200,
+        loop: false,
+        anchor: new ex.Vector(1, 1),
+        rotation: Math.PI,
+        scale: new ex.Vector(2, 2),
+        flipVertical: true,
+        flipHorizontal: true,
+        width: 100,
+        height: 200
+      });
+
+      animation.tick(100, 42);
+      animation.tick(100, 42);
+      animation.tick(100, 42);
+      animation.tick(100, 42);
+      animation.tick(100, 42);
+      expect(animation.currentFrame).toBe(0);
+      done();
+    });
+  });
+
   it('should always pass "flipped" state to the current Sprite', () => {
     const mockSprite: any = jasmine.createSpyObj('sprite', ['draw', 'drawWithOptions']);
     mockSprite.anchor = ex.Vector.Half;