Actionscript 3.0 Splice/RemoveChild issue

Question

I have rocks and enemy ninjas moving off the screen. The ninjas are supposed to disappear after being hit by a shuriken once, however they are taking two hits to disappear, although the shurikens disappear every time fine. The code is nearly identical for the shurikens and enemy ninjas, however the ninjas don't seem to work properly. I also get the occasional ninja getting stuck somewhere on the screen and shurikens pass through it.

    //If a rock moves off the screen, it is spliced.
    if (rock[j].x <= -301){
        removeChild(rock[j]);
        rock.splice(j, 1);
        rockNum--;
    }
}

for (var q=shurikens.length - 1; q >= 0; q--){
    for (var w=enemy.length - 1; w >= 0; w--){
        //If the shurikens hit the ninjas.
        if ((shurikens[q]).hitTestObject(enemy[w])){
            removeChild(enemy[w]);
            enemy.splice(w, 1);
            ninjaNum--;
            removeChild(shurikens[q]);
            shurikens.splice(q, 1);
            break;
        }
    }
}

}

Answer 1

How to fix your code, and give you some performance tips:

This code is very "bug friendly": you are modifying the "length" property of array within a for/in loop that relied on the same property, this is really not a wise thing to do. The way I would do it:

// Create an array where to store colliding ninjas and shurikens 
    var depleted:Array = [];
    // Create local references to make code more readable and fast (querying arrays all the time is slow).
    var ninja:DisplayObject;
    var shuriken:DisplayObject;
    // Loop in shurikens (no need to check if shurikens have been depleted, since you query only once each of them
    for (var q:int=shurikens.length - 1; q >= 0; q--){
        shuriken = shurikens[q]; // Assign a shuriken to our variable so you avoid to call further shurikens[q];
        for (var w:int=enemy.length - 1; w >= 0; w--){ // Loop in ninjas
            ninja = enemy[w]; // Assign ninja
            //If the shurikens hit the ninjas. (only if ninjas have not yet been removed)
            // This is the core of our improvement, before calling hitTest that is slow, you first check that ninjas have not already been killed
            if (depleted.indexOf(ninja) == -1 &&
 shuriken.hitTestObject(ninja)){
                // It's a hit with a live ninja. I just add both objects to depleted list.
                depleted.push(ninja);
                depleted.push(shuriken);
                break; // Breaking the loop, makes sure shuriken cannot hit 2 ninjas
            }
        }
    }
// Then, you loop in the list of killed ninjas and depleted shurikens, and remove them from arrays and display list
for each (var depletedObj:DisplayObject in depleted) {
   // First remove object from the relevant array
   if (shurikens.indexOf(depletedObj) != -1) shurikens.splice(shurikens.indexOf(depletedObj), 1); // If it was in the shurikens array remove from there
   else if (enemy.indexOf(depletedObj) != -1) enemy.splice(enemy.indexOf(depletedObj), 1); // If it was in the ninjas array remove from there
   // The do all necessary stuff to remove object from DisplayList (end eventually add it to an object pooling list)
   removeChild(depletedObj);
}
ninjaNum = enemy.length; // Update number of ninjas

Another hint, in case you run this for loops on each frame, if you place ninjas and shurikens in 2 different DisplayObjectContainer, you can first hitTest the 2 large containers, and once they collide, you can run the loops to check fine collisions. Also, in numeric for/in loops, declare variables always as :int. Typing variable makes it faster to access than an untyped variable. You can of course improve this code to make it faster, i.e.: adding a "alive = true" property to shurikens and ninjas, so you do not need to query a third array, etc.