Programming help (sort of)

[Covar]

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

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

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.

 

[checkeredhat]

I think this might be in the wrong subforum...

 

[Covar]

I've reported it asking for a move.

 

[blotsfan]

Dude,

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.

 

[Covar]

Yes, but there's more to the game then making 4th quarter free-throws in the paint.

 

[Necronic]

Man I have no idea what's going on here.

And I like it.

 

[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.

 

[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.

 

[GasBandit]

When it comes to programming, I'd defer to stienman.

 

[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.

 

[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.