So I created a simple pong game but with a paddle on the bottom of the canvas and on the top. But I was wondering how I could clean the code up to polish the game. One thing I would like to do is stop the ball from sinking into the paddle when it hits it. And also, I know I should use requestAnimationFrame()
rather than setInterval()
, but I just made this game to practice so I don't really care. So basically I'm just asking for advice on ways I could clean my code up. Thanks in advance!
My Code
var canvas = document.getElementById("myCanvas");
var ctx = canvas.getContext("2d");
var ballRadius = 10;
var x = (canvas.width/2)-5;
var y = canvas.height-30;
var dx = 2;
var dy = -2;
var paddleWidth = 75;
var paddleHeight = 10;
var paddleX = (canvas.width/2)-paddleWidth/2;
var leftPressed = false;
var rightPressed = false;
function ball() {
ctx.beginPath();
ctx.arc(x, y, ballRadius, 0, Math.PI*2);
ctx.fillStyle = "#0095DD";
ctx.fill();
ctx.closePath();
}
function paddleOne() {
ctx.beginPath();
ctx.rect(paddleX, canvas.height-paddleHeight, paddleWidth, paddleHeight);
ctx.fillStyle = "green";
ctx.fill();
ctx.closePath();
if(leftPressed && paddleX > 0) {
paddleX -= 7;
}
if(rightPressed && paddleX < canvas.width-paddleWidth) {
paddleX += 7;
}
}
function paddleTwo() {
ctx.beginPath();
ctx.rect(paddleX, 0, paddleWidth, paddleHeight);
ctx.fillStyle = "green";
ctx.fill();
ctx.closePath();
}
function ballBounce() {
if(x + dx > canvas.width-ballRadius || x + dx < ballRadius) {
dx = -dx;
}
else if(y + dy < 0 + ballRadius) {
if(x > paddleX && x < paddleX + paddleWidth) {
dy = -dy;
}
else {
alert("Game Over");
document.location.reload();
}
}
else if(y + dy > canvas.height - ballRadius) {
if(x > paddleX && x < paddleX + paddleWidth) {
dy = -dy;
}
else {
alert("Game Over");
document.location.reload();
}
}
}
function animate() {
ctx.clearRect(0, 0, canvas.width, canvas.height);
ball();
paddleOne();
paddleTwo();
ballBounce();
x += dx;
y += dy;
}
setInterval(animate, 10);
document.addEventListener("keydown", keyDownHandler, false);
document.addEventListener("keyup", keyUpHandler, false);
function keyDownHandler(e) {
if(e.keyCode === 37) {
leftPressed = true;
}
if(e.keyCode === 39) {
rightPressed = true;
}
}
function keyUpHandler(e) {
if(e.keyCode === 37) {
leftPressed = false;
}
if(e.keyCode === 39) {
rightPressed = false;
}
}
* { padding: 0; margin: 0; }
canvas { background: #eee; display: block; margin: 0 auto; }
<canvas id="myCanvas" width="480" height="320"></canvas>
First, you can do some refactoring.
Exemple : PaddleTwo
and PaddleOne
function are very similar, there you can use a function with parameter something like : DrawPaddle(positionX, positionY)
The 2 conditions in PaddleOne
don't have to be there. Create a dedicated function for the movement.
Second : add some variables (it's close to refactoring)
There you have multiple value directly in the code. Exemple : If I ask you to change the speed of the paddle, you should find all the values "speed" in the script.
Replace all the direct value in your script by variable (keycode, speed, time for animate, color..ect..) . You will have a better control on the script.
Third : Use OOP.
Use class to clear the code and have a code more modular. Here I see some class you can already create : - Ball (with the move function, the drawing function, the keyboard listener function) - Paddle (with the drawing function and the bounce function) - Point (with and X and a Y properties, and then avoid the variable like somethingX/somethingY)
One more advice for me : check and double check the validity of your naming. If you got wrong name in your code, you will be unable to understand it, your collegues will not understand too. And some blackzone in your code will appear. There I can see dx and dy, I guess you mean directionX and directionY but who knows ? It have an infinite number of reason than in a few time you will ask what is this ?