1
\$\begingroup\$

I have been playing with JavaScript for some time now and made a simple canvas animation. I tried to make my code clean but I would like to hear your opinions about it.

GitHub

'use strict';

var Ball = function (options = {}) {
	var self = {};
	
	var x = options.x || 0;
	var y = options.y || 0;
	var width = options.width || 20;
	var height = options.height || 20;
	var color = options.color || '#aaaaaa';
	var verticalSpeed = options.verticalSpeed || 1;
	var horizontalSpeed = options.horizontalSpeed || 1;
	
	self.move = function () {
		x += horizontalSpeed;
		y += verticalSpeed;
	};
	
	self.verticalBounce = function () {
		verticalSpeed = -verticalSpeed;
	};
	
	self.horizontalBounce = function () {
		horizontalSpeed = -horizontalSpeed;
	};
	
	self.getX = function () {
		return x;
	};
	
	self.getY = function () {
		return y;
	};
	
	self.getWidth = function () {
		return width;
	};
	
	self.getHeight = function () {
		return height;
	};
	
	self.getColor = function () {
		return color;
	};
	
	self.getVerticalSpeed = function () {
		return verticalSpeed;
	};
	
	self.getHorizontalSpeed = function () {
		return horizontalSpeed;
	};
		
	return Object.seal(self);
};

var Board = function (options = {}) {
	var self = {};
	
	var objects = [];
	var canvas = options.canvas || document.getElementsByTagName('canvas')[0];
	var ctx = canvas.getContext('2d');
	
	var moveObjects = function () {
		objects.forEach(function (object) {
			object.move();
		});
	};
	
	var detectCollisions = function () {
		
		var wallCollision = function (object) {
			if (object.getX() <= 0 || (object.getX() + object.getWidth()) > canvas.width) object.horizontalBounce();
			if (object.getY() <= 0 || (object.getY() + object.getHeight()) > canvas.height) object.verticalBounce();
		};
		
		var areColliding = function (objectA, objectB) {
			return (
				objectA.getX() < objectB.getX() + objectB.getWidth() &&
				objectA.getX() + objectA.getWidth() > objectB.getX() &&
				objectA.getY() < objectB.getY() + objectB.getHeight() &&
				objectA.getHeight() + objectA.getY() > objectB.getY()
			);
		}
		
		objects.forEach(function (currentObject, index, objects) {
			wallCollision(currentObject);
			for (var i = index + 1; i < objects.length; i++) {
				var nextObject = objects[i];
				if (areColliding(currentObject, nextObject)) {
					if (Math.sign(currentObject.getHorizontalSpeed()) !== Math.sign(nextObject.getHorizontalSpeed())) {
						currentObject.horizontalBounce();
						nextObject.horizontalBounce();
					}
					if (Math.sign(currentObject.getVerticalSpeed()) !== Math.sign(nextObject.getVerticalSpeed())) {
						currentObject.verticalBounce();
						nextObject.verticalBounce();
					}
				}
			}
		});
	};
	
	var draw = function () {
		ctx.clearRect(0, 0, canvas.width, canvas.height);
		objects.forEach(function (object) {
			ctx.fillStyle = object.getColor();
			ctx.fillRect(object.getX(), object.getY(), object.getWidth(), object.getHeight());
		});
	};
	
	self.frame = function () {
		moveObjects();
		draw();
		detectCollisions();
	};
	
	self.addObject = function (object) {
		objects.push(object);
	};
	
	return Object.seal(self);
};

var ball1 = Ball();
var ball2 = Ball({horizontalSpeed: -1, x: 638, y: 123, color: '#ff0000', width: 25, height: 60, verticalSpeed: 2, horizontalSpeed: 2});
var ball3 = Ball({verticalSpeed: -1, x: 235, y: 453, color: '#00ff00', width: 75, height: 50});
var ball4 = Ball({horizontalSpeed: 1, x: 300, y: 300, color: '#0000ff', width: 50, height: 30});
var board = Board();

board.addObject(ball1);
board.addObject(ball2);
board.addObject(ball3);
board.addObject(ball4);

setInterval(board.frame, 1000/60);
body {
	background: #000;
	text-align: center;
}
	
canvas {
	background: #434343;
	border: 5px solid #323232;
}
<!doctype html>
<html>
<head>
	<link rel="stylesheet" href="css/main.css">
</head>
<body>
	<canvas width="800" height="600"></canvas>
</body>
</html>

\$\endgroup\$
0

2 Answers 2

Reset to default
1
\$\begingroup\$

Some observations:

  • Try not to define methods inside the Ball constructor, it uses more memory since a new function is created for each ball instance. Rather create them using ES2016 classes (if available) or defining them on the prototype.

  • Rather than defining getX, getY, etc, methods rather create properties (again using ES2016 syntax if possible or otherwise defineProperty)

  • I would add draw and checkCollision methods to the Ball class and move the code there.

  • I would add a class and objects for the walls. This has the advantage that they can draw themselves in the same way as a Ball (The main program just becomes a loop asking every object to draw itself) and that you can generalize collision detection as just being between two objects without worrying about their types. As you add different objects to your code this keeps things much simpler.

\$\endgroup\$
0
\$\begingroup\$

Options and defaults:

I'd like to suggest 1. an improvement and 2. an alternative to how you construct a Ball object from user supplied options and defaults:

  1. JavaScript class syntax with destructuring default arguments:

class Ball {
  constructor({x = 0, y = 0, vx = 1, vy = 1, width = 20, height = 20, color = "#aaa"} = {}) {
    this.x = x;
    this.y = y;
    this.vx = vx;
    this.vy = vy;
    this.width = width;
    this.height = height;
    this.color = color;
  }
}

// Example:
console.log(new Ball({color: "#000"}));

  1. JavaScript class syntax with Ball.from method similar to Array.from:

class Ball {
  constructor(x = 0, y = 0, vx = 1, vy = 1, width = 20, height = 20, color = "#aaa") {
    this.x = x;
    this.y = y;
    this.vx = vx;
    this.vy = vy;
    this.width = width;
    this.height = height;
    this.color = color;
  }
  static from(ball) {
    return new Ball(ball.x, ball.y, ball.vx, ball.vy, ball.width, ball.height, ball.color);
  }
}

// Example:
console.log(Ball.from({color: "#000"}));

Rigid body physics:

Simulating the kinematics of rigid bodies over time requires integrating over their velocities to find their positions. As you don't have any additional forces such as gravity or springs acting on those bodies, this step could be as simple as object.x += object.vx * dt where dt denotes the time passed between two frames which you might want to supply via Board.frame(dt).

However, when collisions occur, the velocities have discontinuities. Therefore, you might want to update object positions as above only until a collision occurs. Then you compute the new velocities and continue with the integration. Even though your bodies are just balls and therefore pretty simple, this can easily become tedious and problematic when the time between collisions becomes very small. An alternative and more generalizable approach is to update object positions without regard for collisions and then resolve collisions in a dedicated step.

Contrary to @MarcRohloff's suggestion, I'd recommend updating the ball movements by a dedicated physics "engine" which can be part of the Board as knowledge about the complete board state is necessary in order to update a ball's position.

Graphics:

I also recommend keeping your drawing routines separate from your models. A dedicated graphics "engine" can perform optimizations such as minimizing the number of context state changes or apply global effects such as z-sorting which requires knowledge about the complete board state.

Apart from that, I recommend following the advice given by @MarcRohloff.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.