HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

JavaScript Breakout Game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptgamebreakout

Problem

I'm a novice programmer who is very new to JavaScript, and animation/graphics, and I have this breakout game on my flask website.

I tried to do as much as I could from scratch, and I'm unsure if my physics system and especially collision detection are done reasonably well. I get little collision bugs sometimes (a block not being removed from the list although the ball bounces properly, or ball bouncing through a block instead of bouncing off it). It seems to have more trouble running in Firefox. Advice on general coding issues is of course welcome as well.

```




(function() { // from https://gist.github.com/paulirish/1579671
var lastTime = 0;
var vendors = ['webkit', 'moz'];
for(var x = 0; x 2) {timedif = 1}; // trying to prevent lag from killing the physics
}
requestAnimationFrame(animate);
drawGame(timedif);
}
function drawGame(time) {
updatePhysics(time);
updatePaddle();
ctx.clearRect(0, 0, canvas.width, canvas.height);
for (var i=0;i 0) {
var qball = queueballs[queueballs.length - 1]
qball.x = paddle.x + paddle.width / 2;
qball.y = paddle.y - qball.radius;
qball.vector = [0, 0];
qball.creator();
}
for (var i=0;i 0) {
if (!lastshot) {
lastshot = new Date().getTime();
fireBullet();
} else {
var now = new Date().getTime()
if (now - lastshot > 750) {
lastshot = now;
fireBullet();
}
}
}
ctx.font = "80px Arial";
ctx.fillStyle = "yellow";
ctx.fillText(queueballs.length + balls.length,25,680);
if (balls.length + queueballs.length == 0) {loseGame();}
else if (blocks.length == 0) {winGame();}
}
function winGame() {
currentlevel += 1;
if (currentlevel >= levels.length) {c

Solution

First, I wouldn't worry about the polyfill. Browsers that do support ` also support requestAnimationFrame. , being older, came earlier and there is a slight period where you may have but not requestAnimationFrame. But most users probably won't be using browsers that old anymore.

Your game could benefit using a game engine or even just a canvas drawing library. Canvas uses immediate-mode rendering. It's like paint. You apply, there's no turning back unless you spill white and start over. Libraries abstract this away from you and make it look like retained mode rendering. You define the state of an object, and the library takes care of everything else. It's much easier when all you know is you have a box at position and it just renders.

Now I'm not going going to review the entire code. Most likely you'll throw this code away in favor of a library or engine. I'll just nitpick a few notable things.

One thing I learned from game engines is that instead of using time elapsed between frames to compensate for framerate, what they do is provide a scaling factor instead. This can be calculated by
actual time elapsed / one frame at 60fps.

// At 60fps, your ball would have moved 60 units, but at 40fps, only 40.
// 60fps * 1unit = 60units
// 40fps * 1unit = 40units
ballPosition = ballPosition + 1;

// With a scaling factor, 40fps would have a 3/2 factor compensating the lag
// 25ms / 16.6ms ~ 1.5
// 40fps * (1unit * 1.5) = 60units
ballPosition = (ballPosition + 1) * scale;


The advantage of using a scaling factor is that the scale is unitless. Your equation can be written without anticipating a value that has a unit, like say time. You can simply write the equation as is, and multiply the scale.

I see that your code is clearing arrays by assigning new arrays. What you could do instead of creating another array is to set the
length of the existing array to 0. This will empty the array without making another array. Just make sure there's nothing else referencing the elements in the array for the GC to pick them up.

var foo = [1,2,3];
foo.length = 0;
foo // []


Another cause of performance issues in JS is when you keep altering the size of an array, like using
splice. What you could do is create a fixed-size array, and if you need to remove the item, have it marked as removed rather than removing it.

Constructor names usually have the convention of having the first letter of a word in capital letters. So
rect, circle and others should be Rect, Circle etc.

Consider using factories instead of constructors. Factories are just functions that return objects. Constructors are usually overkill in your case because they do all the work of hooking up a prototype object but your code doesn't appear to use inheritance or method sharing. Also, the only method you have for these objects is
creator` which is just like a factory with a closure around it.

function createCircle(x, y, radius, vector) {
  // The following is ES6 shorthand literal notation
  return {x, y, radius, vector};
}


I would also advise that you put the responsibility of pushing the objects into an array to the caller. Don't make it implicit that they get pushed to a queue. If necessary, create a function that creates circles and pushes them in a queue and named appropriately for the operation.

Code Snippets

// At 60fps, your ball would have moved 60 units, but at 40fps, only 40.
// 60fps * 1unit = 60units
// 40fps * 1unit = 40units
ballPosition = ballPosition + 1;

// With a scaling factor, 40fps would have a 3/2 factor compensating the lag
// 25ms / 16.6ms ~ 1.5
// 40fps * (1unit * 1.5) = 60units
ballPosition = (ballPosition + 1) * scale;
var foo = [1,2,3];
foo.length = 0;
foo // []
function createCircle(x, y, radius, vector) {
  // The following is ES6 shorthand literal notation
  return {x, y, radius, vector};
}

Context

StackExchange Code Review Q#117201, answer score: 3

Revisions (0)

No revisions yet.