Return to Answer

Minor improvement.

Source Link

edited Jul 8, 2019 at 14:36

16.4k
17
56
61

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);
        }
    }
}

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;
}

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);
        }
    }
}

Source Link

answered Jul 8, 2019 at 14:07

Vaillancourt ♦

16.4k
17
56
61

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);
        }
    }
}