1
\$\begingroup\$

I'm creating a game where waves of enemies are coming to me and I have to kill them.

So far, I'm doing great: it took me hours to come up with this nested loop to check for collision detection and when I ran it the game was running great... but for a few kills only. Sometimes I manage to play it even for a few minutes until it crashes, sometimes I shoot twice and it crashes, and I always get this error:

"Uncaught TypeError: Cannot read property 'x' of undefined
    at collision (collision.js:6)"

I just cannot figure out whats going on. If someone can help me I will be so grateful, I saw similar topic but I couldn't find the answer there.

function collision(){
    for(var l = bullets.length - 1; l >= 0; l--){
        for( var k = enemies.length - 1; k >= 0; k--){
            var e = enemies[k];
            var b = bullets[l];
            if(b.x > e.x && b.x < e.x + e.imgWidth/4 && b.y > e.y && b.y < e.y + 
            e.imgHeight/4 ){
                e.enemiesHealth -= 50;
                b.deletion(); // basically assigns true to b.toDelete
                e.deletion(); // basically assigns true to e.toDelete
            }
            else if (b.toDelete){
                bullets.splice(l, 1);
            }
            else if(e.toDelete && e.enemiesHealth == 0){
                enemies.splice(k, 1);
                kills += 1;
            }
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Could you explain a bit what is the purpose of the deletion() function, and its relation to toDelete? If it is what I think, it looks like there could be potential unexpected behaviour with the code. \$\endgroup\$ – Vaillancourt Jul 8 '19 at 14:09
  • \$\begingroup\$ the deletion function basicly assings toDelete to true. \$\endgroup\$ – Chrissisbeast Jul 8 '19 at 14:11
1
\$\begingroup\$

Looks like you're cutting off the branch over which you're standing.

In order words, it looks like you change the collection over which you're iterating, invalidating the index.

Specifically, this looks fishy:

bullets.splice(l, 1);

If the enemy you check is not the last one you'll check, and the bullet is the last from the list, you'll get an out of rage error (or something) when you'll fetch the bullet again with var b = bullets[l];.

The way you iterate over your array to delete items from it as you go is an appropriate one, but only when you have one loop, not when you have nested loops.

You could modify your code like this to avoid this behaviour:

function collision(){
    for(var l = bullets.length - 1; l >= 0; l--){
        var b = bullets[l];
        var shouldDeleteBullet = false;

        for( var k = enemies.length - 1; k >= 0; k--){
            var e = enemies[k];
            if(b.x > e.x && b.x < e.x + e.imgWidth/4 && b.y > e.y && b.y < e.y + e.imgHeight/4 ){
                e.enemiesHealth -= 50;
                b.deletion();
                e.deletion();
                shouldDeleteBullet = true; // Flag for deletion
                break; // The bullet has been consumed, we can no longer use it.
            }
            else if(e.toDelete && e.enemiesHealth == 0){
                enemies.splice(k, 1);
                kills += 1;
            }
        }

        if (shouldDeleteBullet){
            bullets.splice(l, 1);
        }
    }
}

This should fix the crash that you get.


I'll go a bit further and suggest another thing that might not be working as you expect:

b.deletion();
e.deletion();

... 

if(e.toDelete && e.enemiesHealth == 0){
    enemies.splice(k, 1);
    kills += 1;
}

With this code, if the bullet is the last on the list, or if the enemy is the last on the list being checked, it will not be deleted until the next frame (the next time you iterate over those arrays).

It's not clear why you defer the deletion to later, so I'll suggest you delete the item right away:

function collision(){
    for(var l = bullets.length - 1; l >= 0; l--){
        var b = bullets[l];
        var shouldDeleteBullet = false;

        for( var k = enemies.length - 1; k >= 0; k--){
            var e = enemies[k];
            if(b.x > e.x && b.x < e.x + e.imgWidth/4 && b.y > e.y && b.y < e.y + e.imgHeight/4 ){
                e.enemiesHealth -= 50;

                if (e.enemiesHealth <= 0){ // This will allow a greater variety of damage eventually
                  enemies.splice(k, 1);
                  kills += 1;
                }

                shouldDeleteBullet = true; // Flag for deletion
                break; // The bullet has been consumed, we can no longer use it.
            }
        }

        if (shouldDeleteBullet){
            bullets.splice(l, 1);
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.