Servo animation enqueue is buggy

[ScreamZ]

Servo animation enqueue is buggy

[dtex]

So you see "2 end" but never "3 start" correct? No errors thrown?

[ScreamZ]

So you see "2 end" but never "3 start" correct? No errors thrown?

That is correct, I can give you more insight if you wish. For some reason, I've added some console logs inside animation.js file and I can see that this.segments.length is going to 1 to 0 then to 2 then directly to 0. I'm trying to investigate.

I suspect some kind of race conditions.

I think this could also be nice to buffer things before everything starts. Adding an autoStart property might be a good choice.

If I can contribute, just let me know. I'm also doing a big work updating typescript definition typings DefinitelyTyped/DefinitelyTyped#67063

[ScreamZ]

@dtex Any idea on that ?

[dtex]

I don't know if it's a race condition, or we're just popping things off the queue too aggressively. I'll set up a test and see if I can find where that's happening.

[ScreamZ]

I don't know if it's a race condition, or we're just popping things off the queue too aggressively. I'll set up a test and see if I can find where that's happening.

Alright thank you very much, Keep me in touch, I would be really pleased because i need this feature, I'm trying debugging on my side.

[ScreamZ]

@dtex Issue happen on

johnny-five/lib/animation.js

Line 94 in 094bf6c

Object.assign(this, new Animation.Segment(this.segments.shift()));

Added this code

Constructor

class Animation extends Emitter {
  constructor(target) {
    super();

    // Necessary to avoid loading temporal unless necessary
    if (!temporal) {
      temporal = require("temporal");
    }

    console.log("Before constructor assign", this.segments?.length)
    Object.assign(this, new Animation.Segment());
    console.log("After constructor assign", this.segments?.length)

    this.defaultTarget = target || {};
  }

Output is

worker:dev: [dev] Before constructor assign undefined
worker:dev: [dev] opts []
worker:dev: [dev] After constructor assign 0

This is initializing the array to 0, which is fine.

Segments shifting in loop

 console.log("Before shift", this.segments.length)
 const firstElement = this.segments.shift()
 console.log("After Shift",this.segments.length)
 Object.assign(this, new Animation.Segment(firstElement));
 console.log("After assign",this.segments.length)

Output is

worker:dev: [dev] Before shift 1
worker:dev: [dev] After Shift 0
worker:dev: [dev] opts [ 'duration', 'keyFrames', 'onstart', 'oncomplete', 'target' ]
worker:dev: [dev] After assign 0
worker:dev: [dev] enqueue 1
worker:dev: [dev] enqueue 2
worker:dev: [dev] enqueue 3
worker:dev: [dev] enqueue 4
worker:dev: [dev] loopFunction 4
worker:dev: [dev] next 4
worker:dev: [dev] Before shift 4
worker:dev: [dev] After Shift 3
worker:dev: [dev] opts [ 'duration', 'keyFrames', 'onstart', 'oncomplete', 'target' ]
worker:dev: [dev] After assign 0
worker:dev: [dev] loopFunction 0
worker:dev: [dev] Stop called

It's 1 because the first instruction is queued, then 1 again because i have a total of 5 enqueued tasks. So when the first trigger 0, the other become 1,2,3,4.

stop is called, so it pops out the this.segments array.

loopFunction console.log is from there and next is from next method.

   if ((this.progress === 1 && !this.reverse) || (progress === this.loopback && this.reverse)) {
      if (this.loop || (this.metronomic && !this.reverse)) {

        if (this.onloop) {
          this.onloop();
        }

        if (this.metronomic) {
          this.reverse = this.reverse ? false : true;
        }

        this.normalizeKeyframes();
        this.progress = this.loopback;
        this.startTime = Date.now() - this.scaledDuration * this.progress;
        this.endTime = this.startTime + this.scaledDuration;
      } else {

        this.isRunning = false;

        if (this.oncomplete) {
          process.nextTick(() => this.oncomplete());
        }

        console.log("loopFunction",this.segments.length)

        if (this.segments.length > 0) {
          process.nextTick(() => this.next());
        } else {
          this.stop();
        }

FIXES

I've been able to fix the code doing this

Object.assign(this, new Animation.Segment(firstElement), {segments: this.segments});

We need to re-apply segments otherwise we crush the segment array each time. Or we need to pass current second to options so options.segments can trigger inside Animation.Segment

I can make a pull request if you think it's okay.