Adding new observer inside the same observable callback causes crash

Bugs
1

Hello,

I’m not sure if this is a bug, but I’ve been doing some tests today, and if I do the next:

const playAnimation = () =>  {
    // Clear observable and stop previous animation
    this._animation.animationGroup.onAnimationGroupEndObservable.clear()
    this._animation.animationGroup.stop()

    // Start mesh animation
    this._animation.animationGroup.start()

    // Trigger observable on animation end
    this._animation.animationGroup.onAnimationGroupEndObservable.add(() => playAnimation())
}

The third iteration triggers the observable lot of times, and the fourth iteration triggers it non stop.
I’ve tried different things and I can’t avoid this situation. notifyIfTriggered is false.

Instead, using a setTimeout, the code above works fine, iteration by iteration.

const playAnimation = () =>  {
    this._animation.animationGroup.stop()

    this._animation.animationGroup.start()

    setTimeout(() => {
        playAnimation()
    }, 5000)  // Some arbitrary time similar to animation end
}

Some idea?
If you think it worth to be reviewed I’ll create a PG.

PS: Waiting a frame before calling the new observer prevents the situation:

const playAnimation = () =>  {
    // Clear observable and stop previous animation
    this._animation.animationGroup.onAnimationGroupEndObservable.clear()
    this._animation.animationGroup.stop()

    // Start mesh animation
    this._animation.animationGroup.start()

    // Trigger observable on animation end, calls callback 1 frame later
    this._animation.animationGroup.onAnimationGroupEndObservable.add(() => {
        setTimeout(playAnimation, 1)
    })
}
2

I could use a PG to see how to explain it :smiley:

1 Like
4

Here you go: https://playground.babylonjs.com/#Z6SWJU#1143

Please uncomment playAnimation. Warning: might crash the scene.

Screenshot 2025-03-22 121029
Screenshot 2025-03-22 121029796×807 193 KB

1 Like
5

Thanks, I didn’t have time to build the PG :sweat_smile:

1 Like
6

let me have a look!

7

ok so not a bug per se!

Here is the explanation:

  • When the animation ends, it calls notifyObservers on the onAnimationGroupEndObservable
  • The observable is doing a loop on its observers
  • In this case one observer is clearing the list of observers WITHIN the loop and then add a new observer while still being in the obsersable loop
  • The observable loop then check the next observer (which is the new playanimation that we added)
  • Infinite loop

This is a classic case of updating a list while running a loop on that list

So the fix to skip to the next event is the good one
We do not overly protect the observable loops because they are widely used within the framework and impact on perf would be terrible

3 Likes
8

Got it, so I keep the setTimeout(() => next, 1), so users wont have to deal with this scenario :slight_smile:

This is done when the animation ends, so there’s not performance impact.

9

Exactly! this is the right approach!