Cloning Zelda: Harmful Changes (and a Christmas Miracle)

Yesterday's post, which I called "Harmless Animations", led to me making a stupid – and harmful – change that I should have known better. It's worth talking about what happened, and why.

When I first implemented the loc.Explod class and the four SwordFlash elements, this is what the code that added them to the game screen looked like:

terminate: function swordProj_terminate() {
    game.insertProjectile(new loc.SwordFlashNW({'pos':this.getPos(),'owner':this.owner}));
    game.insertProjectile(new loc.SwordFlashSW({'pos':this.getPos(),'owner':this.owner}));
    game.insertProjectile(new loc.SwordFlashNE({'pos':this.getPos(),'owner':this.owner}));
    game.insertProjectile(new loc.SwordFlashSE({'pos':this.getPos(),'owner':this.owner}));

    this.inherited(arguments);
}

When I was blogging about the new code yesterday, I "streamlined" that function a bit to look like this:

terminate: function swordProj_terminate() {
    var myPos = this.getPos();
    game.insertProjectile(new loc.SwordFlashNW({'pos':myPos,'owner':this.owner}));
    game.insertProjectile(new loc.SwordFlashSW({'pos':myPos,'owner':this.owner}));
    game.insertProjectile(new loc.SwordFlashNE({'pos':myPos,'owner':this.owner}));
    game.insertProjectile(new loc.SwordFlashSE({'pos':myPos,'owner':this.owner}));

    this.inherited(arguments);
}

Do you see what I broke? It's probably not at all obvious... it certainly wasn't to me. I thought that I was optimizing the code by only running getPos() once and passing its value to all four child explods. But last night, I went to change the live code to reflect the blog post, in case anyone wanted to examine the new bits in context. When I did so, my sword flashes stopped behaving right! If the sword hit a monster, all four explods sat still in the exact spot where they were inserted, and if the sword made it to the end of the screen, two of them would come straight back, rather than at their proper angles. What the heck did I do?!?

I really should have known better, since this has tripped me up before. To explain what the difference is in these two code blocks, you need to know what the getPos() method (which is inherited from loc.Sprite) actually does:

getPos: function sprite_pos() {
    return dojo.clone(this.pos);
}

dojo.clone() is a cool little function. In a nutshell, it creates deep copies of JavaScript objects, which is tremendously useful if you want something approximating a struct datatype. This is exactly how I'm doing (x,y) pairs in Canvassa, which I use for both sprite position and velocity:

dojo.declare("loc.Sprite", null, {
    pos: {x:0, y:0},
    vector: {x:0,y:0},
...
})

The problem comes when you try to reuse one of these position values, which is precisely what the second code block tried to do, and is precisely why I created the getPos() method in the first place! Since a sprite's position is an object and not a struct, I need to dojo.clone() it to get a copy of its contained values instead of a copy of its pointer reference (see Jonathan Snook's explanation of why JavaScript works this way). Now, I'm not a n00b programmer. I've been around the block a few times, and have coded in nearly a dozen different languages in my time, and in many of them I was well aware of this Pass-by-Reference/Pass-by-Value distinction. And yet, in an effort to clean up my code, I managed to instantiate four separate SwordFlash explods and give all four of them the same position object! So when the game told them to update their positions, each in turn would modify its position object relative to its personal velocity:

// let's assume the starting position is {x:32, y:64}

// in SwordFlashNW.updatePosition():
this.pos.x += -3;  // {x:29, y:64}
this.pos.y += -3;  // {x:29, y:61}

// SwordFlashSW.updatePosition():
this.pos.x += -3;  // {x:26, y:61}
this.pos.y += 3;  // {x:26, y:64}

// SwordFlashNE.updatePosition():
this.pos.x += 3;  // {x:29, y:64}
this.pos.y += -3;  // {x:29, y:61}

// SwordFlashSE.updatePosition():
this.pos.x += 3;  // {x:32, y:61}
this.pos.y += 3;  // {x:32, y:64}

// end position for all four: {x:32, y:64}

So when all four explods were operating on a single, shared position object, the net result was no movement at all! And when, in the edge case, two of the explods went off the screen and were removed, the other two combined forces to push the position in a single direction. D'OH!

So, hopefully this time I've learned the lesson well enough that it'll sink in: my sprites' pos and vector values are objects, and I need to call getPos() EVERY time I want a non-interfering copy. Sure, it's not as profound a December lesson as "I will honour Christmas in my heart, and try to keep it all the year," but it'll do.

Happy Christmas to all, and to all a good night!