I'm designing an animation system for a game engine and find myself looking at two different methods for updating the frame count. Both are equally valid and provide the same result, advancing to the next frame, and either looping through, or holding at the end as specified by the animation's property.
My first pass
Code:
if (this.currentFrame < assets[this.id][this.animation].sequence.length - 1) {
this.currentFrame++;
} else if (assets[this.id][this.animation.repeat) {
this.currentFrame = 0;
}
My second shot
Code:
this.currentFrame++;
if (this.currentFrame === assets[this.id][this.animation].sequence.length) {
if (assets[this.id][this.animation].repeat) {
this.currentFrame = 0;
} else {
this.currentFrame--;
}
}
I'm curious as to which people find easier to understand, and which I should ultimately implement. and for the record, yes this is JavaScript, assets is a module scoped variable that contains the asset objects and data. `this` is a local reference file to be handed out to the game objects referencing the art asset and tracking state.
#2
checkeredhat
I think this might be in the wrong subforum...
#3
Covar
I've reported it asking for a move.
#4
blotsfan
Dude,
Code:
if (this.currentFrame === assets[this.id][this.animation].sequence.length)
always chokes in the clutch. It just doesn't have the heart to be a champion.
#5
Covar
Yes, but there's more to the game then making 4th quarter free-throws in the paint.
#6
Necronic
Man I have no idea what's going on here.
And I like it.
#7
GasBandit
I have the vaguest understanding (it's been a long time since college after all), but I like the second way better. The variable always gets iterated no matter what when the function is called, and if it doesn't pass the criteria for the first if-then, then it doesn't even have to bother checking the second... just seems more elegant to me. But what do I know, I haven't programmed anything in a great long time, and even then it was never anything more complicated than a halfassed donkey kong clone.
#8
strawman
I don't like the second way because, even if only for a brief moment, you are exceeding your frame boundary.
I don't know how your animation system is implemented, but if there's any concurrency going on, your second pass is bad, bad news and will only lead to heartache and empty nights of debugging the weirdest problems.
Even if there's no concurrency I'd avoid it on the off chance that you reuse the code elsewhere.
#9
GasBandit
When it comes to programming, I'd defer to stienman.
#10
Bowielee
I agree with Steinman. If you're incrementing on the index, you never want to exceed the index cap, otherwise you will end up with overflow errors.
#11
Covar
Thanks everyone, I'll go with the first option it seems.
There should be no chance of going out of bounds, Only the AssetReference advances it's own animation with each entity having it's own reference, but the first method is safer in that regard. Still why take the chance when you don't have to.
Here's the entire engine so far for anyone interested. There's not much yet, but I'll be making a commit with this code soon. It's a pretty big refactor of the entire rendering pipeline, but one that should be much more efficient in terms of resources. It's easy to get away with loading the assets for each entity that uses them when you only have 2 pieces of art. Switching to references will really help once I get beyond the point of test cases and programmer art.