Calling Sprite.playAnimation with identical indices breaks the animation (it never finishes)

Repro: (not intended for this primarily, but you can uncomment the playAnimation calls to see the animation work properly)

The reason of the bug can be seen here: Babylon.js/sprite.ts at master · BabylonJS/Babylon.js · GitHub and Babylon.js/sprite.ts at master · BabylonJS/Babylon.js · GitHub

If both indices are identical, this._direction is set to -1 when it should probably be 0. This causes the condition in the second link to always evaluate to false, and so the animation never finishes and any registered callbacks are never notified (which is how I noticed the problem in the first place).

Additionally, the cellIndex is decremented, which can cause graphical glitches if the goal is to display a static sprite for a given period and then use the callback to do something, like switching the cellIndex.

The only reason I did this in the first place was to work around the limitation of requiring indices to be in order, but that’s a separate issue :slight_smile:

Well this is kinda expected :smiley:
For me this is a hack. If you call playAnimation with to and from identical, bad things will happen :smiley:

Let’s maybe focus on what is your original need?

Well, I assumed it would work like you expect from any given interval: The animation takes the toIndex and fromIndex as the interval boundaries, and if they’re equal it’s just a static, one-frame animation :slight_smile:

I used this as a workaround because the API doesn’t support playback of arbitrary frame indices, i.e. cell indices being out-of-order. Due to the nature of the spritesheets and complexity of the animations, I needed to play keyframes in a custom order, which I did by chaining a bunch of single-frame animations.


If I want to animate the sprite using the cell indices 5, 8, 1, 0, 3, 4, 2, 9 (in this order) I can’t define this as a sequence and therefore have to either hack some sort of lookup table into the animate and playAnimation functions, or do what I ended up doing and simply play static animations for every keyframe (5, 5 then 8,8 then 1,1 etc.).

Perhaps there’s a better way, but those were the simplest ways of achieving this that I could find.

And then instead of using animations why not simply relying on setTimeout() and then set the sprite index to whatever you want?

Something like that:

Imagine if you have multiple animations on a spritesheet, and one of them happens to be a static, one frame animation. if you wanted to use a single codepath for playing everything using sprite.playAnimation, you’ll have to duplicate your single frames, which feels…wrong…

1 Like

It does seem that using setTimeout can replicate the desired functionality, though it feels unnecessary to do this when BJS already manages the animation state (and callbacks), as well as the delta time calculation via requestAnimationFrame. I’d also have to clear timeouts manually if the animation is stopped, as opposed to simply using sprite.stopAnimation(). Wouldn’t it be easier not having to deal with the scheduler when we already (implicitly) use the animation loop’s deltaTime anyway?

Considering how simple the “fix” for this “problem” is, I’d much rather rely on the BJS API and remove the unspecified behaviour in this edge case, making sure static animations are handled as well. I can submit a PR if @deltakosh doesn’t object to this as a “hack”, which in my eyes it really isn’t… though of course it’s not my place to decide :slight_smile:

Please do so I can see what you precisely mean :slight_smile: