Search code examples
javascriptp5.js

P5.js code crashes after a while when removing objects from the screen


I've been following along with Dan Shiffman's videos, trying to brush up on my object orientated programing using classes.

I've written some code that generates bubbles with random diameters at random positions, using p5's noise function to give the bubbles some movement.

My intention is for the bubbles to pop (be removed from the array with splice()) every time a bubble reaches the edges of the canvas or when two or more bubbles intersect.

The code run as desired, but after a while it crashes throwing the error "Uncaught TypeError: Cannot read property 'x' of undefined (sketch: line 15)".

I've tried hacking around but no joy. If anyone could shed some light on why this error occurs, or general pointers on my approach I would be most grateful. Here's the code in question.

var balls = [];

function setup() {
  createCanvas(400, 400);
}

function draw() {
  background(220);

  for (var i = 0; i < balls.length; i++) {
    balls[i].showBall();
    balls[i].moveBall();

    for (var j = 0; j < balls.length; j++) {
      if (balls[i].x < 0 + balls[i].r ||
        balls[i].x > width - balls[i].r ||
        balls[i].y < 0 + balls[i].r ||
        balls[i].y > height - balls[i].r ||
        balls[i].life >= 220 ||
        i != j && balls[i].intersect(balls[j])) {
        balls.splice(i, 1);
      }
    }
  }
}

class Ball {
  constructor(x, y, r) {
    this.x = x;
    this.y = y;
    this.r = r;
    this.t = 0.0;
    this.t2 = 107.0;
    this.life = 0;
  }

  showBall() {
    noFill();
    stroke(this.life);
    strokeWeight(2);
    ellipse(this.x, this.y, this.r * 2);
  }

  moveBall() {
    this.x += map(noise(this.t), 0, 1, -1, 1) * 0.5;
    this.y += map(noise(this.t2), 0, 1, -1, 1) * 0.5;
    this.t += 0.02;
    this.life += 0.15;
  }
  intersect(other) {

    if (dist(this.x, this.y, other.x, other.y) < this.r + other.r) {
      return true;
    } else {
      return false;
    }
  }
}

function randBubbleGen() {
  let foo = floor(random(11));
  console.log(foo);

  if (foo >= 5) {
    let b = new Ball(random(41, 359),
      random(41, 359),
      random(5, 40));
    balls.push(b);
  }
}

setInterval(randBubbleGen, 1000);
<script src="https://cdn.jsdelivr.net/npm/[email protected]/lib/p5.js"></script>


Solution

  • The first part of the problem is a classic splice issue:

    const arr = [..."abcde"];
    
    for (let i = 0; i < arr.length; i++) {
      if (i === 2) { // some arbitrary condition
        arr.splice(i, 1);
      }
      else {
        console.log(i, arr[i]);
      }
    }

    What happened here? After splicing the element at index 2, "c", the length of arr becomes 4, yet i++ still happens, skipping an element "d" which is never printed or visited in the loop. The solution is to i-- after each splicing operation or iterate in reverse so that splice doesn't cause unvisited elements to be skipped.

    As for your error, the problem is that your inner loop over all j splices out an element, then continues on, acting as if balls[i] wasn't just removed. Based on the above demonstration, we know that after balls.splice(i, 1), balls[i] becomes the next element after the original i for the rest of that iteration of the outer loop body. This is a bug because some collisions will be skipped for i+1 after the spliced element, but won't cause errors unless i happens to be the last element in the balls array. In that case, balls[i+1] is undefined and you can't access properties on undefined.

    The solution is to break out of the inner j loop after splicing out an element. That's in addition to iterating in reverse or using i-- after each splice call to avoid skipping balls.


    From a time complexity standpoint, splice is a poor choice because it's O(n). If you have n collisions in the balls array, you'll need to loop over it a factor of n times, causing a triply-nested loop running in your update code.

    A better general approach is to create a new array of elements that survived a round, then resassign that new array to the old balls array after the frame. This involves some allocation overhead.


    Other tips:

    • Use const instead of let wherever possible.
    • Use let instead of var for loop counters and mutable variables. Ideally, never use var or let, although p5 promotes mutability due to its window-attached functions.
    • Prefer forEach and for ... of loops to classical C-style for loops.
    • You can return dist(this.x, this.y, other.x, other.y) < this.r + other.r since it's already a boolean, no need for if bool return true else return false verbosity.
    • Keep rendering and position updating separate as much as possible. It probably doesn't matter much for this animation, but as things get more complex, it can be odd when something dies but still gets rendered for a frame as is the case here.
    • Move the collision detection and edge detection to external functions -- the if (balls[i].x < 0 + balls[i].r ... condition is difficult to read.