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>
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:
const
instead of let
wherever possible.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.forEach
and for ... of
loops to classical C-style for
loops.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.if (balls[i].x < 0 + balls[i].r ...
condition is difficult to read.