Jump to content
  • Advertisement
Sign in to follow this  
username@gamedevnet

Feedback wanted on Javascript PONG clone

This topic is 1898 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I finished my PONG clone today; finished enough anyway, it has a start, a middle, and a win condition. No audio at the moment. Any feedback would be appreciated. I should probably add that this is the first game or programming project I've gotten to this state of 'finished', let alone shown anyone. I've only included the .js file as the .html and .css files are pretty bare. It's a hot mess, but here it is...

var canvas = document.getElementById('canvas');
var context = canvas.getContext('2d');

function Paddle() {
	this.x = 0;
	this.y = 0;
	this.speed = 7;
	this.width = 16;
	this.height = 64;
	this.score = 0;

	this.draw = function() {
		context.fillStyle = 'white';
		context.fillRect(this.x, this.y, this.width, this.height);
	}
}

function Ball() {
	this.x = 0;
	this.y =0;
	this.xv = 1;
	this.yv = 1;
	this.speed = 4;
	this.width = 16;
	this.height = 16;

	this.draw = function() {
		context.fillStyle = 'white';
		context.fillRect(this.x, this.y, this.width, this.height);
	}
}

// Keep track of keypresses...
// Using an array this way ensures smooth movement,
// without the jerky motion at the start
var keystates = [];
// Some "constants" to track specific keys
var LEFT_PADDLE_UP = 87;
var LEFT_PADDLE_DOWN = 83;
var RIGHT_PADDLE_UP =79;
var RIGHT_PADDLE_DOWN =75;
var SPACE = 32;

// Flag a key as active on keydown by adding it to
// the keystates array
window.addEventListener('keydown', function(e) {
	keystates[e.keyCode] = true;
});
// Remove a key on keyup...I think delete sets it to undefined...
window.addEventListener('keyup', function(e) {
	delete keystates[e.keyCode];
});

// Do something with all the keypresses stored in keystates
function updatePaddles() {
	// Move the left paddle
	if (keystates[LEFT_PADDLE_UP] && leftPaddle.y > 0) {
		leftPaddle.y -= leftPaddle.speed;
	}
	if (keystates[LEFT_PADDLE_DOWN] && leftPaddle.y < canvas.height - leftPaddle.height) {
		leftPaddle.y += leftPaddle.speed;
	}
	// Move the right paddle
	if (keystates[RIGHT_PADDLE_UP] && rightPaddle.y > 0) {
		rightPaddle.y -= rightPaddle.speed;
	}
	if (keystates[RIGHT_PADDLE_DOWN] && rightPaddle.y < canvas.height - rightPaddle.height) {
		rightPaddle.y += rightPaddle.speed;
	}
}

function updateBall() {
	if (ball.y < 0 || ball.y > canvas.height - ball.height) {
		ball.yv *= -1;
	}
	if (ball.x <= 0) {
		rightPaddle.score++;
		resetBall();
	}
	if (ball.x >= canvas.width - ball.width) {
		leftPaddle.score++;
		resetBall();
	}
	// As is, if the ball hits the paddle from the top/bottom
	// then it can get stuck on the x-axis behind the paddle
	if (ball.x < leftPaddle.x + leftPaddle.width &&
		ball.x + ball.width > leftPaddle.x &&
		ball.y < leftPaddle.y + leftPaddle.height &&
		ball.y > leftPaddle.y) {
		ball.xv *= -1;
	}
	if (ball.x < rightPaddle.x + rightPaddle.width &&
		ball.x + ball.width > rightPaddle.x &&
		ball.y < rightPaddle.y + rightPaddle.height &&
		ball.y > rightPaddle.y) {
		ball.xv *= -1;
	}

	ball.x += ball.xv * ball.speed;
	ball.y += ball.yv * ball.speed;
}

// Centers the ball on screen
function resetBall() {
	ball.x = (canvas.width - ball.width) / 2;
	ball.y = (canvas.height - ball.height) / 2;
	ball.xv *= -1;
}

function resetPaddles() {
	leftPaddle.x = 0;
	leftPaddle.y = (canvas.height - leftPaddle.height) / 2;
	rightPaddle.x = canvas.width - rightPaddle.width;
	rightPaddle.y = (canvas.height - rightPaddle.height) / 2;

	leftPaddle.score = 0;
	rightPaddle.score = 0;
}

// Set up game objects
var leftPaddle = new Paddle();
var rightPaddle = new Paddle();
var ball = new Ball();
var isPlaying = false;
var startScreen = document.getElementById('start');

resetPaddles();
resetBall();

context.fillStyle = 'white';
context.font = '64px Arial';
context.textBaseline = 'top';

// Run the game
function loop(timestamp) {
	window.requestAnimationFrame(loop);
	if (keystates[SPACE]) {
		isPlaying = !isPlaying;
		delete keystates[SPACE];
	}

	if (isPlaying) {
		startScreen.style.display = 'none';
		updatePaddles();
		updateBall();

		context.clearRect(0, 0, canvas.width, canvas.height);
		context.fillRect((canvas.width - 16) / 2, 0, 16, canvas.height);
		context.fillText(leftPaddle.score, (canvas.width / 2) - (context.measureText(leftPaddle.score).width + 32), 0);
		context.fillText(rightPaddle.score, (canvas.width / 2) + 32, 0);
		ball.draw();
		leftPaddle.draw();
		rightPaddle.draw();
	}

	if (leftPaddle.score === 3 || rightPaddle.score === 3) {
		isPlaying = !isPlaying;
		startScreen.style.display = 'block';
		resetPaddles();
		resetBall();
	}
}
window.requestAnimationFrame(loop);

Share this post


Link to post
Share on other sites
Advertisement

I'll keep adding comments as I review your code throughout the next day:

 

1) Great indentation and consistent style. biggrin.png!

2) Oh, those objects are more like glorified variable containers. sad.png! You should try:

  • Adding constructors
  • Allowing those constructors to accept parameters which determine their speed / width etc.

3) This:

this.score = 0;

being in the paddle class. sad.png! Think about what the paddle class is responsible for. Why is the paddle keeping track of the score? That doesn't make much sense. Maybe you should make a function / class which manages your overall game.

 

4) Woah:

function updatePaddles() {
// Move the left paddle
if (keystates[LEFT_PADDLE_UP] && leftPaddle.y > 0) {
leftPaddle.y -= leftPaddle.speed;
}
if (keystates[LEFT_PADDLE_DOWN] && leftPaddle.y < canvas.height - leftPaddle.height) {
leftPaddle.y += leftPaddle.speed;
}
// Move the right paddle
if (keystates[RIGHT_PADDLE_UP] && rightPaddle.y > 0) {
rightPaddle.y -= rightPaddle.speed;
}
if (keystates[RIGHT_PADDLE_DOWN] && rightPaddle.y < canvas.height - rightPaddle.height) {
rightPaddle.y += rightPaddle.speed;
}
}

This *really* should be in the Paddle class sad.png! You're changing all of the paddles variables here, and this piece of code solely affects the paddles behavior! Why is it in a global function?

 

5) Same as above:

function updateBall() {
if (ball.y < 0 || ball.y > canvas.height - ball.height) {
ball.yv *= -1;
}
if (ball.x <= 0) {
rightPaddle.score++;
resetBall();
}
if (ball.x >= canvas.width - ball.width) {
leftPaddle.score++;
resetBall();
}
// As is, if the ball hits the paddle from the top/bottom
// then it can get stuck on the x-axis behind the paddle
if (ball.x < leftPaddle.x + leftPaddle.width &&
ball.x + ball.width > leftPaddle.x &&
ball.y < leftPaddle.y + leftPaddle.height &&
ball.y > leftPaddle.y) {
ball.xv *= -1;
}
if (ball.x < rightPaddle.x + rightPaddle.width &&
ball.x + ball.width > rightPaddle.x &&
ball.y < rightPaddle.y + rightPaddle.height &&
ball.y > rightPaddle.y) {
ball.xv *= -1;
}
 
ball.x += ball.xv * ball.speed;
ball.y += ball.yv * ball.speed;
}

sad.png!

 

6) You seriously should read a short book about Object-Oriented Design:

function resetBall() {
ball.x = (canvas.width - ball.width) / 2;
ball.y = (canvas.height - ball.height) / 2;
ball.xv *= -1;
}
 
function resetPaddles() {
leftPaddle.x = 0;
leftPaddle.y = (canvas.height - leftPaddle.height) / 2;
rightPaddle.x = canvas.width - rightPaddle.width;
rightPaddle.y = (canvas.height - rightPaddle.height) / 2;
 
leftPaddle.score = 0;
rightPaddle.score = 0;
}

sad.png!

 

So far, it seems like you should read more about Object-Oriented Design and refactor your Paddle and Ball classes.

Share this post


Link to post
Share on other sites

Thanks for the input. I'm not done reworking it yet, but I thought I would share what I have to make sure I'm going in the right direction. I think part of the problem I'm having is determining what goes in each class/object. Like the player controls, it seems like the input should be a game responsibility, and the actual updating of the paddle should be a paddle responsibility...or maybe I'm over thinking it?

// Utility object inherited by Paddle and Ball, provides
// everything needed to draw the object to the canvas.
// These methods may need to be empty...
function GameObj() {
	this.x;
	this.y;
	this.width;
	this.height;
	// Initialize the 'x' and 'y'...not sure this should be here
	this.init = function(x, y) {
		this.x = x;
		this.y = y;
	}
	// Draw 'this' to the canvas
	this.draw = function() {
		this.context.fillRect(this.x, this.y, this.width, this.height);
	}
}

// Define the paddles used to hit the ball. Inherits from GameObj().
function Paddle() {
	this.width = 16;
	this.height = 64;

	this.moveUp = function() {
		// this.y -= this.speed;
	}

	this.moveDown = function() {
		// this.y += this.speed;
	}
}
Paddle.prototype = new GameObj();

// Define the bouncy ball. Inherits from GameObj().
function Ball() {
	this.width = 16;
	this.height = 16;
}
Ball.prototype = new GameObj();

// All the game stuff
function Game() {
	this.canvas = document.getElementById('canvas');
	this.context = canvas.getContext('2d');
	this.canvasHeight = this.canvas.height;
	this.canvasWidth = this.canvas.width;

	this.init = function() {
		// Not sure if this is the best way, but it does eliminate the need
		// for an extra parameter to make the draw() method work
		GameObj.prototype.context = this.context;
		// Initialize game objects
		this.ball = new Ball();
		this.playerOne = new Paddle();
		this.playerTwo = new Paddle();
		// This seems like the easiest way to do this...probably not the best.
		this.ball.init((this.canvasWidth - this.ball.width) / 2, (this.canvasHeight - this.ball.height) / 2);
		this.playerOne.init(0, (this.canvasHeight - this.playerOne.height) / 2);
		this.playerTwo.init((this.canvasWidth - this.playerTwo.width), (this.canvasHeight - this.playerTwo.height) / 2);
	}

	// Update everything
	this.update = function() {
		// do some stuff...
	}

	// Draw everything
	this.draw = function(timestamp) {
		window.requestAnimationFrame(this.draw.bind(this));

		// Update game stuff
		this.update();

		// Draw game objects to the canvas
		this.ball.draw();
		this.playerOne.draw();
		this.playerTwo.draw();
	}
}

// Play the game
var game = new Game();
game.init();
game.draw();

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!