So, I wrote a simple Pong game in C++ with SFML and Visual Studio 2013 and was wondering if somebody could tell me if there is anything I could improve upon in the code.

Noteworthy:

- The collision between the ball and the top and bottom of the player panel seems to be slightly bugged

- The full solution for visual studio can be found here: https://www.dropbox.com/s/558as04io6jr2b5/Pong.zip

- To quit the game, press escape

Game:

Files:

Code: (main.cpp)

#include <iostream>
#include <string>
#include <vector>
#include <SFML\Graphics.hpp>
#include <SFML\Audio.hpp>
#include <Windows.h>
#pragma warning(disable: 4244)

#define SCRW	1000	// Screen width
#define SCRH	600		// Screen height
#define BALLW	20		// Ball width
#define BALLH	20		// Ball height	
#define PADDLEW 20		// Paddle width
#define PADDLEH 100		// Paddle height
#define BSPEED	-0.1f	// Ball origin speed
#define ESPEED	0.08f	// Enemy origin speed (player is mouse controlled)

sf::RenderWindow window(sf::VideoMode(SCRW, SCRH), "Pong");
sf::RectangleShape ball(sf::Vector2f(BALLW, BALLH));
sf::RectangleShape player(sf::Vector2f(PADDLEW, PADDLEH));
sf::RectangleShape enemy(sf::Vector2f(PADDLEW, PADDLEH));
float ballSpeedX = BSPEED;
float ballSpeedY = BSPEED;
float enemySpeed = ESPEED;
bool gameOver = false;
int playerPoints = 0;
int enemyPoints = 0;
sf::Font font;
sf::Text plScoreTxt;
sf::Text enScoreTxt;

void draw();
void moveBall();
void enemyAI();
bool intersect(sf::Vector2f, sf::Vector2f, sf::Vector2f);
float intersectPos(float, sf::RectangleShape);
void reset();

int main(){
	// Font and text stuff
	std::string fontname = "segoeuil.ttf";
	if (!font.loadFromFile(fontname))
	{
		std::cerr << "Font " << "\"" << fontname << "\" couldn't be/wasn't loaded. Please try again.\n";
	}
	plScoreTxt.setFont(font);
	plScoreTxt.setCharacterSize(200);
	plScoreTxt.setPosition(sf::Vector2f(window.getSize().x / 2 - 250, window.getSize().y / 2 - 150));
	enScoreTxt.setFont(font);
	enScoreTxt.setColor(sf::Color(255, 255, 255, 32));
	enScoreTxt.setCharacterSize(200);
	enScoreTxt.setPosition(sf::Vector2f(window.getSize().x / 2 + 150, window.getSize().y / 2 - 150));

	// Setting up the window ant entities
	window.setMouseCursorVisible(false);
	ball.setPosition(sf::Vector2f(window.getSize().x / 2 - BALLW / 2, window.getSize().y / 2 - BALLH / 2));
	enemy.setPosition(sf::Vector2f(window.getSize().x - PADDLEW, window.getSize().y / 2 - PADDLEH / 2));

	// Mouse confinement stuff
	RECT rect;
	rect.bottom = window.getPosition().y + window.getSize().y + 32 - PADDLEH / 2;
	rect.top = window.getPosition().y + 31 + PADDLEH / 2;
	rect.left = window.getPosition().x + 50;
	rect.right = window.getPosition().x + window.getSize().x - 42;

	ClipCursor(&rect);

	// Game loop
	while (window.isOpen()){
		sf::Event event;
		while (window.pollEvent(event)){
			if (event.type == sf::Event::Closed){
				window.close();
			}
			if (event.type == sf::Event::KeyPressed){
				// Press escape to close the window
				if (event.key.code == sf::Keyboard::Escape)
					window.close();
			}
		}

		moveBall(); // Move the ball and test for collision 
		player.setPosition(sf::Vector2f(player.getPosition().x, sf::Mouse::getPosition(window).y - PADDLEH / 2)); // Move the player paddle to mouse position
		if (ballSpeedX > 0) // Perform enemy AI stuff when the ball is heading in it's direction
			enemyAI();
		if (gameOver){ // if someone scored, reset the game
			reset();
		}

		// Text stuff
		plScoreTxt.setString(std::to_string(playerPoints));
		enScoreTxt.setString(std::to_string(enemyPoints));
		plScoreTxt.setColor(sf::Color(ball.getFillColor().r, ball.getFillColor().g, ball.getFillColor().b, 32));
		enScoreTxt.setColor(sf::Color(ball.getFillColor().r, ball.getFillColor().g, ball.getFillColor().b, 32));
		player.setFillColor(ball.getFillColor());
		enemy.setFillColor(ball.getFillColor());

		draw(); // draw everything to the screen

		// If somebody wins, the window closes (the window can also be closed by pressing escape)
		if (playerPoints > 9 || enemyPoints > 9)
			window.close();
	}
	return 0;
}

void reset(){
	ball.setPosition(sf::Vector2f(window.getSize().x / 2 - BALLW / 2, window.getSize().y / 2 - BALLH / 2));
	enemy.setPosition(sf::Vector2f(window.getSize().x - PADDLEW, window.getSize().y / 2 - PADDLEH / 2));
	if (ballSpeedX < 0)
		ballSpeedX = BSPEED;
	else
		ballSpeedX = BSPEED * -1;
	ballSpeedY = BSPEED;
	enemySpeed = ESPEED;
	gameOver = false;
}

void draw(){
	window.clear();
	window.draw(plScoreTxt);
	window.draw(enScoreTxt);
	window.draw(player);
	window.draw(enemy);
	window.draw(ball);
	window.display();
}

void increaseSpeed(){
	if (ballSpeedY > 0)
		ballSpeedY += (rand() % 15 + 1) / 1000.0;
	else
		ballSpeedY -= (rand() % 15 + 1) / 1000.0;
	if (ballSpeedX > 0)
		ballSpeedX += (rand() % 20 + 1) / 1000.0;
	else
		ballSpeedX -= (rand() % 20 + 1) / 1000.0;
}

void moveBall(){
	ball.setPosition(sf::Vector2f(ball.getPosition().x + ballSpeedX, ball.getPosition().y + ballSpeedY));

	// Ball -> Wall
	if (ball.getPosition().x < 0){
		enemyPoints++;
		gameOver = true;
		return;
	}
	if (ball.getPosition().x + BALLW > window.getSize().x){
		playerPoints++;
		gameOver = true;
		return;
	}
	if (ball.getPosition().y < 0 || ball.getPosition().y + BALLH > window.getSize().y){
		ballSpeedY *= -1;
		return;
	}

	// Ball -> Both paddles (Side)
	if (intersect(sf::Vector2f(ballSpeedX, ballSpeedY), sf::Vector2f(player.getPosition().x + PADDLEW, player.getPosition().y),
		sf::Vector2f(player.getPosition().x + PADDLEW, player.getPosition().y + PADDLEH)) ||
		intersect(sf::Vector2f(ballSpeedX, ballSpeedY), enemy.getPosition(),
		sf::Vector2f(enemy.getPosition().x, enemy.getPosition().y + PADDLEH)))
	{
		ballSpeedX *= -1;
		ball.setFillColor(sf::Color(rand() % 156 + 100, rand() % 156 + 100, rand() % 156 + 100));
		increaseSpeed();
		return;
	}

	// Player - Top
	if (intersect(sf::Vector2f(ballSpeedX, ballSpeedY), player.getPosition(),
		sf::Vector2f(player.getPosition().x + PADDLEW, player.getPosition().y)))
	{
		ballSpeedY += 0.02f;
		if (ballSpeedX < 0)
			ballSpeedX *= -1;
		if (ballSpeedY > 0)
			ballSpeedY *= -1;
		return;
	}

	// Player - Bottom
	if (intersect(sf::Vector2f(ballSpeedX, ballSpeedY),
		sf::Vector2f(player.getPosition().x, player.getPosition().y + PADDLEH),
		sf::Vector2f(player.getPosition().x + PADDLEW, player.getPosition().y + PADDLEH)))
	{
		if (ballSpeedX < 0)
			ballSpeedX *= -1;
		if (ballSpeedY < 0)
			ballSpeedY *= -1;
		ballSpeedY += 0.02f;
	}
}

void enemyAI(){
	float Ypos = intersectPos(ballSpeedY, ball);
	// Move enemy to optimal location
	if (enemy.getPosition().y + PADDLEH / 2 > Ypos){
		enemy.setPosition(sf::Vector2f(enemy.getPosition().x, enemy.getPosition().y - enemySpeed));
	}
	else if (enemy.getPosition().y + PADDLEH / 2 < Ypos){
		enemy.setPosition(sf::Vector2f(enemy.getPosition().x, enemy.getPosition().y + enemySpeed));
	}
	// Check if enemy collided with the walls
	if (enemy.getPosition().y < 0)
		enemy.setPosition(enemy.getPosition().x, 0);
	else if (enemy.getPosition().y + PADDLEH > window.getSize().y)
		enemy.setPosition(enemy.getPosition().x, window.getSize().y - PADDLEH);
}

bool lineIntersect(sf::Vector2f LinePointA, sf::Vector2f LinePointB, sf::Vector2f pointMoving, float diam){
	if (LinePointA.x == LinePointB.x && pointMoving.x >= LinePointA.x - diam / 2 && pointMoving.x <= LinePointA.x + diam / 2 &&
		pointMoving.y >= LinePointA.y && pointMoving.y <= LinePointB.y)
		return true;
	if (LinePointA.y == LinePointB.y && pointMoving.y >= LinePointA.y - diam / 2 && pointMoving.y <= LinePointA.y + diam / 2 &&
		pointMoving.x >= LinePointA.x && pointMoving.x <= LinePointB.x)
		return true;
	return false;
}

bool intersect(sf::Vector2f vel, sf::Vector2f LinePointA, sf::Vector2f LinePointB){
	double m, b;
	m = ((ball.getPosition().y + ball.getSize().y / 2.0 + vel.y) - (ball.getPosition().y + ball.getSize().y / 2.0)) /
		((ball.getPosition().x + ball.getSize().x / 2.0 + vel.x) - (ball.getPosition().x + ball.getSize().x / 2.0));
	b = (ball.getPosition().y + ball.getSize().y / 2.0 + vel.y) - m * (ball.getPosition().x + ball.getSize().x / 2.0 + vel.x);

	// If the line is on the y axis, solve for y in order to have highest accuracy rate
	if (vel.x > 0 && LinePointA.x == LinePointB.x){
		for (double x = 0; x <= vel.x; x += 0.1){
			if (lineIntersect(LinePointA, LinePointB, sf::Vector2f(x + ball.getPosition().x + ball.getSize().x / 2, (int)(m * (x + ball.getPosition().x + ball.getSize().x / 2.0) + b)), ball.getSize().x))
				return true;
		}
	}
	else if (vel.x < 0 && LinePointA.x == LinePointB.x){
		for (double x = 0; x >= vel.x; x -= 0.1){
			if (lineIntersect(LinePointA, LinePointB, sf::Vector2f(x + ball.getPosition().x + ball.getSize().x / 2, (int)(m * (x + ball.getPosition().x + ball.getSize().x / 2.0) + b)), ball.getSize().x))
				return true;
		}
	}

	// if the line is on the x axis, solve for x in order to have highest accuracy rate
	else if (vel.y > 0 && LinePointA.y == LinePointB.y){
		for (double y = 0; y <= vel.y; y += 0.1){
			if (lineIntersect(LinePointA, LinePointB, sf::Vector2f((int)((y + ball.getPosition().y + ball.getSize().y / 2.0 - b) / m), y + ball.getPosition().y + ball.getSize().y / 2.0), ball.getSize().y))
				return true;
		}
	}
	else if (vel.y < 0 && LinePointA.y == LinePointB.y){
		for (double y = 0; y >= vel.y; y -= 0.1){
			if (lineIntersect(LinePointA, LinePointB, sf::Vector2f((int)((y + ball.getPosition().y + ball.getSize().y / 2.0 - b) / m), y + ball.getPosition().y + ball.getSize().y / 2.0), ball.getSize().y))
				return true;
		}
	}
	return false;
}

// This function is for the enemy to determine where it has to go
// It basically returns a 'y' coordinate of where the ball is going to end up for the enemy's 'x' coordinate
// If the ball hits either the ceiling or the floor before it reaches it's goal, the function is called recursively until the ball stays within bounds
float intersectPos(float tmpBallSpeedY, sf::RectangleShape tmpBall){
	float x = window.getSize().x - PADDLEW;
	float m = (((tmpBall.getPosition().y + BALLH / 2) + tmpBallSpeedY) - (tmpBall.getPosition().y + BALLH / 2)) /
		(((tmpBall.getPosition().x + BALLW / 2) + ballSpeedX) - (tmpBall.getPosition().x + BALLW / 2));
	float b = (tmpBall.getPosition().y) - m * (tmpBall.getPosition().x);
	float y = m * x + b;

	if (y <= 0){
		tmpBall.setPosition(sf::Vector2f((0 - b) / m, 0));
		y = intersectPos(tmpBallSpeedY*-1, tmpBall);
	}
	else if (y >= window.getSize().y - BALLH){
		tmpBall.setPosition(sf::Vector2f(((window.getSize().y - BALLH) - b) / m, window.getSize().y - BALLH));
		y = intersectPos(tmpBallSpeedY*-1, tmpBall);
	}

	return y;
}

Quick glance, here's what I noticed:

Lines 18 - 30: Lots of global variables here. In a small program with such limited scope as your project, this isn't a huge deal, but at the very least you should make sure to declare those variables as static (ie: static sf::RenderWindow window(etc);) so that those names aren't visible in other compilation units. Right now, if you declare a variable named 'window' at file scope in any other .cpp file, you'll run into a linker error for having 'window' defined multiple times. Using the 'static' keyword prevents that. Also, those variables should have some kind of prefix (g_ or s_ or something) to indicate that they're global or static, and not local or member variables. This will preserve your sanity.

Lines 224-226 (and elsewhere): Don't repeat yourself. It looks like you're trying to get the center of the ball by saying "ball.getPosition().y + ball.getSize().y / 2.0", which is fine, but you write that statement over and over, and the end result is a super long line of code that's hard to read. Declare a variable called center_y and set it to ball.getPosition().y + ball.getSize().y / 2.0, and then use center_y instead of constantly repeating the calculation. This advice can be applied to your code in a number of places, not just here.

@Samith

Thanks for the tips, I'll try to implement these.

1. window.clear();
2. window.draw(ball);
3. window.draw(player);
4. window.draw(enemy);
5. window.draw(plScoreTxt);
6. window.draw(enScoreTxt);
7. window.display();

Good job. game looks sound.

The above reorder of objects might look better.

... if you had color and things. LOL.

What i mean by reordering is that the ball would be under the pucks and inf the score was at top left and right, the pucks would be under them. Now that I took a good look at your screen shot I guess these improvements would not have an effect. Anyway maybe for you next project just remember last drawn will be the most visible and first drawn with be less. So, "the first shall be last".

-CROW

You have some magic numbers. One example is in increaseSpeed. You have % 15 and / 1000. These numbers don't specify what they mean. You should declare a constant and use that instead.