Search code examples
arraysp5.js2d-games

The reset of one array affecting a different array


I'm not entirely sure whats going on here. When you shoot all three bullets and the last one leaves the screen the "asteroids" array also resets.

EDIT: Is it because the bullets array ends up being spliced to an undefined when they all leave the screen? Or will it return the base empty array? Even then, it doesn't explain why the asteroids array is voided as well. I also found that the asteroids don't even start falling unless I've shot at least once.

Code on p5web editor

What is it causing this to happen? This is the first time I've coded something this large, code amount wise, but I made sure to use obvious variable names for the most part.


Solution

  • The problem is in your asteroids class:

    check(bullets) {
        for (let i = 0; i < bullets.length; i++) {
            if (dist(bullets[i].x, bullets[i].y, this.pos.x, this.pos.y) < this.r) {
                return false;
            } else {
                return true;
            }
        }
    }
    

    If there are no bullets to check, this function returns undefined implicitly, which is taken as falsey by the calling code, which promptly destroys the asteroid as if it had collided with a bullet.

    Also, if there are bullets to check and the first one happens to not collide, the loop breaks prematurely with else { return true; }, possibly missing collisions.

    Change it to:

    check(bullets) {
        for (let i = 0; i < bullets.length; i++) {
            if (dist(bullets[i].x, bullets[i].y, this.pos.x, this.pos.y) < this.r) {
                return false;
            }
        }
    
        return true; // no collisions (or bullets.length === 0)
    }
    

    Having said this, the function name check is pretty unclear. I'd rename it as collidesWith(bullets) and invert the boolean--it makes more sense to say "true, yes, we did collide with a bullet" than "false, yes, we did collide with a bullet". We can also make use of the for ... of loop construct. This gives us:

    collidesWith(bullets) {
        for (const bullet of bullets) {
            if (dist(bullet.x, bullet.y, this.pos.x, this.pos.y) < this.r) {
                return true;
            }
        }
    
        return false;
    }
    

    You could shorten this further to:

    collidesWith(bullets) {
      return bullets.some(e => dist(e.x, e.y, this.pos.x, this.pos.y) < this.r);
    }
    

    Similarly, I'd change bullet.check() to bullet.outOfBounds() or similar.

    Another tip: iterate in reverse over any arrays you plan to slice the current element from:

    for (let j = asteroids.length - 1; j >= 0; j--) {
       // code that could call `.splice(j, 1)`
    }
    

    Otherwise, after splicing, your loop will skip an element and you might miss a collision.

    A minor design point, but player.controls() seems strange--why should the player be responsible for listening for keypresses? I'd listen in the keyPressed() function and send the changes to update the player position from there. I'd also break up draw into smaller functions. But these are minor design decisions so the above is enough to get you rolling.

    Here's a first revision for the draw function, adjusted to match the modified booleans:

    function draw() {
      background(0);
      scene();
    
      if (tick <= 0) {
        if (asteroids.length < asteroidsLimit) {
          asteroids.push(new Asteroid());
        }
    
        tick = 60;
      }
    
      for (let i = asteroids.length - 1; i >= 0; i--) {
        if (asteroids[i].collidesWith(bullets)) {
          asteroids.splice(i, 1);
        } 
        else {
          asteroids[i].update();
          asteroids[i].display();
        }
      }
    
      for (let i = bullets.length - 1; i >= 0; i--) {
        if (bullets[i].outOfBounds()) {
          bullets.splice(i, 1);
        } 
        else {
          bullets[i].update();
          bullets[i].display();
        }
      }
    
      image(ship, player.x, player.y);
      player.controls();
      tick--;
    }