Actions repeat forever if not cleared

[herobank110]

Steps to Reproduce

  import { Actor, Color, EasingFunctions, Engine } from 'excalibur';

  const a = new Actor({ x: 10, y: 10, width: 10, height: 10, color: Color.White });
  a.actions
    .easeTo(10, 100, 1000, EasingFunctions.EaseInCubic)
    .delay(250)
    .callMethod(() => {
      // clear to do 2 repeats, but resets position at first delay
      // a.actions.clearActions();
      a.actions
        .easeTo(100, 100, 1000, EasingFunctions.EaseInCubic)
        .delay(250)
        .easeTo(10, 100, 1000, EasingFunctions.EaseInCubic)
        .delay(250)
        .repeat(2);  // this 2nd round of actions loops forever!
    });

  (e => e.add(a) || e.start())(new Engine());

Expected Result

Expected down movement followed by movement right and left twice before stopping.

Actual Result

Down movement, then right and left movement forever.

Environment

• browsers and versions: Safari (14.0.3)
• operating system: MacOS
• Excalibur versions: 0.24.5

Current Workaround

Schedule function calls with setTimeout using pre set delays instead of repeating actions.

[eonarheim]

@herobank110 Thanks for the issue, I have a theory where the bug is coming from in excalibur.

My guess based on your workaround, is that repeat inside the callAction() does indeed do accumulate N+1 actions as you mentioned in the discussion post. The way repeat is implemented is it copies the current actions in the context and re-adds them https://github.com/excaliburjs/Excalibur/blob/main/src/engine/Actions/ActionContext.ts#L257

CallMethod is technically not yet done, so it get's re-added as part of the repeat, which repeats again and again 😢 possibly a fix is to change how these are dequeued in the context? Or change how repeat selects things for repeating?

[eonarheim]

One thought I had was to change up the repeat api to make it clear what is repeating, instead of trying to guess based on the current queue.

What do you think of something like?

 a.actions
    .easeTo(10, 100, 1000, EasingFunctions.EaseInCubic)
    .delay(250)
    .repeat(reContext => {
      // Specific actions to repeat
       reContext
        .easeTo(100, 100, 1000, EasingFunctions.EaseInCubic)
        .delay(250)
        .easeTo(10, 100, 1000, EasingFunctions.EaseInCubic)
        .delay(250)
     }, 2);

[herobank110]

I think it's better like that, it looks more like a loop from most programming languages.

[herobank110]

Also that way you can define all the actions in one go