Should be able to interrupt and "splice" actions into action queue

[kamranayub]

Think about the AI pathing we did for Kraken. What if in the update loop I wanted to have the ship randomly "look back" and then return back to what it was going to do (moveTo).

I should be able to do something like this using action API:

lookbackTimer: number = 500;
update(engine, delta): void {
  if (this.lookbackTimer <= 0) {
    var actions = [
      new RotateBy(...), // look back
      new RotateBy(...)  // return back to original
    };
    this.actionQueue.queueAfterCurrentAction(actions, interrupt: true/false);
  }
  this.lookbackTimer -= delta;
}

Essentially allow me to insert actions into the queue as it's being processed but also allow me to choose whether I want to wait until the current action is done or interrupt immediately (cancel) and then resume again later. In the case I'm talking about, I'd want to wait until the current rotation/moveTo is done, then lookback, then reverse lookback, then resume action queue. However I could also see interrupting a moveTo (for example in the middle), doing the lookback actions, then resuming the moveTo from where it left off (pause/resume).

[pikooh]

Almost opened this exact issue now. I have the same problem - sending the player to different interaction points and upon reaching them any number of actions should be added to the ActionQueue right then and there. Moving between the interaction points is of course chainable so a "splicing" or queueAfterCurrentAction would be perfect. Is this something you're working on or could/should I try to take a stab at it?

[pikooh]

I'm playing around with this in the ActionQueue:

    public addImmediately(action: IAction, cancelCurrentAction: boolean = false) {
        if (!this.hasNext()) {
            this.add(action);
        } else {
            this._actions.splice(0, 0, action);
            if (cancelCurrentAction && this._currentAction) {
                this._currentAction.stop();
            }
        }
    }

Am I completely on the wrong track or might this be something? Working on some UnitTests right now.

[eonarheim]

@LePhil looks like you're on the right track, keep up the good work

[pikooh]

Alright, I'm very confused right now and hopefully either writing it down will help or you might have an idea on what's wrong.

moveBy is dealing in absolute positions, right? So if I have this:

    expect(actor.pos.x).toBe(0);
    actor.actions.moveBy(100, 0, 100);
    actor.actions.moveBy(200, 0, 100);
    actor.update(engine, 200);
    expect(actor.pos.x).toBe(200);

then that means that after 200ms the actor will be on position 200|0 (x = 200, y = 0).

This seems to work and consists of 2 actions of MoveBy, once from 0|0 to 100|0 and then from 100|0 to 200|0. Behind the scenes there will be those 2 actions added to the actionQueue of the actor and sequentially completed.

Now, if I add an action while one is running, it should be added at the end.

         expect(actor.pos.x).toBe(0); // T = 0
         
         actor.actions.moveBy(100, 0, 100);
         actor.actions.moveBy(200, 0, 100);

         actor.update(engine, 50);
         expect(actor.pos.x).toBe(50); // T = 50

         actor.actions.moveBy(300, 0, 100);

         actor.update(engine, 50);
         expect(actor.pos.x).toBe(100); // T = 100

         actor.update(engine, 100);
         expect(actor.pos.x).toBe(200); // T = 200

         actor.update(engine, 100);
         expect(actor.pos.x).toBe(300); // T = 300

What I expect:

• actor starts at 0|0
• 2 actions are added
• 50ms later, the actor has moved to 50|0 because it has to move 100px in 100ms
• another action is added and queued at the end of the actionQueue
• 50ms later, the actor completed the first action (2 remaining) and is at 100|0
• 100ms later the second action is completed and the actor is at 200|0 because that's the goal of the second action
• another 100ms later the third action is completed and the actor is at 300|0 because that's the goal of the third and last action

What I get when I add and run this test:

• at T = 200 the actor is at 100|0 instead of 200|0
• at T = 300 the actor is at 200|0 instead of 300|0

Now this is happening with the existing code. I'm not sure what is happening, do you have any explanation for this?

[pikooh]

Bump

[eonarheim]

Hi @LePhil I'll take a look tonight, distracted with all the holidays in the US 🎉 🦃

[pikooh]

No worries and happy thanksgiving! 🦃 Speaking about holidays, my little project is supposed to be a christmas present so I'm a bit more anxious about it as I'd be otherwise 🙈 Thanks for taking the time :)

[eonarheim]

@LePhil Definitely a bug. I think I know what the problem here is, and I definitely would expect your test case to be true.

Basically actions don't handle large update values or update boundaries well :( before the next action can activate, a frame must elapse. Basically it is only possible for 1 action to complete per frame (update) given the current implementation.

Normally updates fire at 60FPS (every ~16ms) so at run time things "mostly" work although the accumulated error piles up quickly, see codepen https://codepen.io/eonarheim/pen/LOJeQe?editors=0010

Here is a snippet of the ActionQueue update, and it only concerns itself with the current action and if it completes.

With a slightly modified version of your test and excalibur that logs delta and position it's very clear:

         actor = new ex.Actor(0, 0);
         expect(actor.pos.x).toBe(0); // T = 0
         
         actor.actions.moveBy(100, 0, 100);
         actor.actions.moveBy(200, 0, 100);

         actor.update(engine, 1);
         actor.update(engine, 50);
         actor.update(engine, 1);
         expect(actor.pos.x).toBe(50); // T = 50

         actor.actions.moveBy(300, 0, 100);

         actor.update(engine, 1);
         actor.update(engine, 50);
         actor.update(engine, 1);
         expect(actor.pos.x).toBe(100); // T = 100

         actor.update(engine, 1);
         actor.update(engine, 100);
         actor.update(engine, 1);
         expect(actor.pos.x).toBe(200); // T = 200

         actor.update(engine, 1);
         actor.update(engine, 100);
         actor.update(engine, 1);
         expect(actor.pos.x).toBe(300); // T = 300

Console out:

27 11 2017 18:14:15.810:INFO [HeadlessChrome 0.0.0 (Windows 10 0.0.0)]: Connected on socket _i2DF0JD6Ln_U8vcAAAA with id 24412154
LOG: 'Update (1) ms - Actor pos: (0, 0)'
LOG: 'Update (50) ms - Actor pos: (1, 0)'
LOG: 'Update (1) ms - Actor pos: (51, 0)'
LOG: 'Update (1) ms - Actor pos: (52, 0)'
LOG: 'Update (50) ms - Actor pos: (53, 0)'
LOG: 'Update (1) ms - Actor pos: (103, 0)'
LOG: 'New action q'd'
LOG: 'Update (1) ms - Actor pos: (100, 0)'
LOG: 'Update (100) ms - Actor pos: (101, 0)'
LOG: 'Update (1) ms - Actor pos: (201, 0)'
LOG: 'New action q'd'
LOG: 'Update (1) ms - Actor pos: (200, 0)'
LOG: 'Update (100) ms - Actor pos: (201, 0)'
LOG: 'Update (1) ms - Actor pos: (301, 0)'
LOG: 'New action q'd'
HeadlessChrome 0.0.0 (Windows 10 0.0.0) Action moveBy can move expected over time FAILED
        Expected 52 to be 50.

I think the correct way to fix this is to retool the existing actions api to accurately position actors after updates and calculate the time each action should take.

I have a hack of the moveBy action that seems to work by moving the target actor immediately to the position rather than setting velocity. And it passes the test you provided https://github.com/excaliburjs/Excalibur/tree/bugfix-actionqueue.

[pikooh]

Ah, that would explain it. I've also found another bug, I think - it's not possible to add multiple callMethod actions

         let x = 0;

         actor.actions.callMethod(() => {
            console.log('first');
            x++;
         });
         actor.actions.callMethod(() => {
            console.log('second', x);            
            x++;
         });
         actor.update(engine, 10);
         expect(x).toBe(2);

"Second" never gets called and x is still 1 at the end. I'll figure it out tonight. :+1:

[pikooh]

Ah I see. No matter how many steps are passed to update, it only executes one action. If I call actor.update(engine, 0); twice, it works.

I assume "counting down" the steps and calling _currentAction.update(1) a lot of times and if it's done, go on to the next one until there are no more steps or no more actions would be quite inefficient...

Edit: would actually kind of work but the repeat action does not like this approach very much. Feels way too hacky anyway.

[github-actions[bot]]

This issue hasn't had any recent activity lately and is being marked as stale automatically.

[eonarheim]

Closing for now, recommend using new coroutines for complex behaviors like this

https://excaliburjs.com/docs/coroutines/