Ocena kodu SFML

Question

Prosze o ocene kodu gry napisanej w C++ przy użyciu SFML 

#include <SFML/Graphics.hpp>
#include <SFML/Audio.hpp>
#include <SFML/Network.hpp>
#include <random>
#include <string>
#include <chrono>
#include <thread>

#define EXIT_FAILURE 1
#define EXIT_SUCCESS 0

int randmoNumber (int intervalMin, int intervalMax);
void wait (unsigned int millisecondsTime);

int main (int argc, char * argv[])
{
	const int WIDTH = 700, HEIGHT = 400, DEPTH_OF_COLOR = 32, MAX_FPS = 60;
	const float PADDLE_WIDTH = 20, PADDLE_HEIGHT = 90;
	float ballSpeedX = 5.0, ballSpeedY = 0.0;
	const std::string SEPARATION = "|";
	int scorePlayerFirst = 0, scorePlayerSecond = 0;
	
	float ballRadius = 10.f;
	sf::Vector2f paddleSize (PADDLE_WIDTH, PADDLE_HEIGHT);

	//Load font
	sf::Font mainFont;
	if (!mainFont.loadFromFile ("resources/fontComic.ttf"))
		return EXIT_FAILURE;

	//Load music
	sf::Music mainMusic;
	if (!mainMusic.openFromFile ("resources/soundtrack.wav"))
		return EXIT_FAILURE;
	mainMusic.setLoop (true);
	mainMusic.setVolume (1);
	//mainMusic.play ();

	//Create the left paddle
	sf::RectangleShape leftPaddle;
	leftPaddle.setSize (paddleSize - sf::Vector2f (3, 3));
	leftPaddle.setOutlineThickness (3);
	leftPaddle.setOutlineColor (sf::Color::White);
	leftPaddle.setFillColor (sf::Color::Black);
	leftPaddle.setOrigin (paddleSize / 2.f);
	leftPaddle.setPosition (20, 200);

	//Create the right paddle
	sf::RectangleShape rightPaddle;
	rightPaddle.setSize (paddleSize - sf::Vector2f (3, 3));
	rightPaddle.setOutlineThickness (3);
	rightPaddle.setOutlineColor (sf::Color::White);
	rightPaddle.setFillColor (sf::Color::Black);
	rightPaddle.setOrigin (paddleSize / 2.f);
	rightPaddle.setPosition (680, 200);

	//Create the ball
	sf::CircleShape ball;
	ball.setRadius (ballRadius - 3);
	ball.setOutlineThickness (3);
	ball.setOutlineColor (sf::Color::White);
	ball.setFillColor (sf::Color::Black);
	ball.setOrigin (ballRadius / 2, ballRadius / 2);
	ball.setPosition (350, 200);

	

	//Main settings of windows
	sf::RenderWindow mainWindow (sf::VideoMode (WIDTH, HEIGHT, DEPTH_OF_COLOR), "PING-PONG", sf::Style::Titlebar | sf::Style::Close);
	mainWindow.setFramerateLimit (MAX_FPS);

	//Main loop game
	while (mainWindow.isOpen ())
	{
		sf::Event eventCloseMainWindow;
		while (mainWindow.pollEvent (eventCloseMainWindow))
		{
			if (eventCloseMainWindow.type == sf::Event::Closed)
				mainWindow.close ();
		}
		
		std::string scorePlayerFirstConversionToString = std::to_string (scorePlayerFirst);
		std::string scorePlayerSecondConversionToString = std::to_string (scorePlayerSecond);
		
		//Limit move right paddle
		if (rightPaddle.getPosition ().y < 0)
		{
			rightPaddle.setPosition (680, 0);
		}
		else if (rightPaddle.getPosition ().y > 400)
		{
			rightPaddle.setPosition (680, 400);
		}
		//Limit move left paddle
		if (leftPaddle.getPosition ().y < 0)
		{
			leftPaddle.setPosition (20, 0);
		}
		else if (leftPaddle.getPosition ().y > 400)
		{
			leftPaddle.setPosition (20, 400);
		}

		//Move left paddles
		if (sf::Keyboard::isKeyPressed (sf::Keyboard::W))
		{
			leftPaddle.move (0, -3);
		}
		else if (sf::Keyboard::isKeyPressed (sf::Keyboard::S))
		{
			leftPaddle.move (0, 3);
		}
		//Move right paddles
		if (sf::Keyboard::isKeyPressed (sf::Keyboard::Up))
		{
			rightPaddle.move (0, -3);
		}
		else if (sf::Keyboard::isKeyPressed (sf::Keyboard::Down))
		{
			rightPaddle.move (0, 3);
		}

		//Colision bounds
		sf::FloatRect leftPaddleCollision = leftPaddle.getGlobalBounds ();
		sf::FloatRect rightPaddleCollision = rightPaddle.getGlobalBounds ();
		sf::FloatRect ballCollision = ball.getGlobalBounds ();

		//TODO: Collision action
		ball.move (ballSpeedX, ballSpeedY);
		if (ballCollision.intersects (rightPaddleCollision))
		{
			ballSpeedX = -5;
			ballSpeedY = randmoNumber (0, 5);
			ball.move (ballSpeedX, ballSpeedY);
		}
		else if (ballCollision.intersects (leftPaddleCollision))
		{
			ballSpeedX = 5;
			ballSpeedY = randmoNumber (-5, 0);
			ball.move (ballSpeedX, ballSpeedY);
		}
		else if (ball.getPosition ().x > WIDTH)
		{
			scorePlayerFirst++;
			
			ball.setPosition (350, 200);
			rightPaddle.setPosition (680, 200);
			leftPaddle.setPosition (20, 200);
			ballSpeedX = 5.0; 
			ballSpeedY = 0.0;
			ball.move (ballSpeedX, ballSpeedY);
			wait (500);
		}
		else if (ball.getPosition ().x < 0)
		{
			scorePlayerSecond++;
			ball.setPosition (350, 200);
			rightPaddle.setPosition (680, 200);
			leftPaddle.setPosition (20, 200);
			ballSpeedX = 5.0;
			ballSpeedY = 0.0;
			ball.move (ballSpeedX, ballSpeedY);
			wait (500);
		}
		else if (ball.getPosition ().y > (HEIGHT - 5))
		{
			ballSpeedX = -5;
			ballSpeedY = randmoNumber (-5, 5);
			if (ballSpeedY == -1 || ballSpeedY == 0 || ballSpeedY == 1)
				ballSpeedY = randmoNumber (-5, 5);
			ball.move (ballSpeedX, ballSpeedY);
		}
		else if (ball.getPosition ().y < 5)
		{
			ballSpeedX = 5;
			ballSpeedY = randmoNumber (-5, 5);
			if (ballSpeedY == -1 || ballSpeedY == 0 || ballSpeedY == 1)
				ballSpeedY = randmoNumber (-5, 5);
			ball.move (ballSpeedX, ballSpeedY);
		}

		std::string scoreGame = scorePlayerFirstConversionToString + SEPARATION + scorePlayerSecondConversionToString;

		//Score
		sf::Text scoreText (scoreGame, mainFont);
		scoreText.setCharacterSize (30);
		scoreText.setStyle (sf::Text::Bold);
		scoreText.setPosition (325, 10);

		mainWindow.clear (sf::Color::Black);
		mainWindow.draw (scoreText);
		mainWindow.draw (leftPaddle);
		mainWindow.draw (rightPaddle);
		mainWindow.draw (ball);
		mainWindow.display ();
	}

	return 0;
}

void wait (unsigned int millisecondsTime)
{
	std::this_thread::sleep_for (std::chrono::milliseconds (millisecondsTime));
}


int randmoNumber (int intervalMin, int intervalMax)
{
	std::random_device random;
	std::mt19937 gen (random ());
	std::uniform_int<> randomNumber (intervalMin, intervalMax);
	return randomNumber (gen);
}

 

Comments

https://drive.google.com/drive/folders/0B-PC2_mw4FrRMVc0LWgtN2IzVGs?usp=sharing link do gry

Answer 1

Rzeczy z którymi "mam problem"

• literówka int randmoNumber
• nietrzymanie się zasady "1 instrukcja = 1 linia" (inie: 17-21) const int WIDTH = 700, HEIGHT = 400, DEPTH_OF_COLOR = 32, MAX_FPS = 60;
• po co string? Nie lepiej char? const std::string SEPARATION = "|";
• brak informacji o rodzaju błędu, masz error-checki ale nie wiesz co się posypie: if (!mainFont.loadFromFile ("resources/fontComic.ttf"))
• używasz wielokrotnie wartości sf::Vector2f (3, 3) lepiej byłoby stworzyć jeden taki obiekt i go używać. a nie tworzyć go co chwila i niszczyć
• j.w. co chwilę liczysz to samo paddleSize / 2.f
• hardcoded value leftPaddle.setOutlineThickness (3); (podobnie z kolorami itd)
• (w myśl niektórych technik programowania proces inicjalizacji powinien być porozbijany na mniejsze funkcje)
• pętla nie jest stałokrokowa

To tak z grubsza, część błędów się powtarzała i ich już nie wypisałem ponownie

Comments

sf::Vector2f (3, 3) jak moge to przypisac do jakiegos obiektu/zmiennej?

"hardcoded value leftPaddle.setOutlineThickness (3); (podobnie z kolorami itd)", nie wiem o co chodzi

dodatkowo mogl bys mi cos podsunac o tych error-checkach

---

1.

sf::Vector size = sf::Vector2f(3, 3);

2.

const float thickness = 3.0f;
...
leftPaddle.setOutlineThickness (thickness);

// kolory

sf::Color outlineColor = sf::Color::White;
...
ball.setOutlineColor (outlineColor);

3.

err-checki, np:

const std::string fontPath = "resources/fontComic.ttf";
if (!mainFont.loadFromFile(fontPath))
{
  std::cerr << "Cannot load font from path: " << fontPath << "\n";
  return EXIT_FAILURE;
}


// alternatywnie z kodami błędów:

#define EXIT_ERR_NOFONT 1
#define EXIT_ERR_NOAUDIO 2
...

const std::string fontPath = "resources/fontComic.ttf";
if (!mainFont.loadFromFile(fontPath))
{
  return EXIT_ERR_NOFONT;
}

 

---

dzieki

---

mogl bys mi wytlumaczyc zasade dzialania  std::cerr?

---

Strumień std::cerr jest niebuforowany. Oznacza to natychmiastowe wydrukowanie każdego komunikatu który się na nim znajdzie. Dodatkowo strumień ten jest odseparowany od typowego wyjścia. W systemach GNU/Linux lub innych pochodnych Unix oraz MS WIndows, możesz przekierować ten strumień tak:

towja_fajna_gra 2> ścieżka_do_pliku_zbierajacego_błędy.txt

Dodatkowo, bardzo łatwo przekierować go do pliku:

auto file = std::ofstream("log.txt");
std::cerr.rdbuf(file.rdbuf());

Innym istotnym strumieniem o którym się zapomina jest std::clog. Ten domyślnie jest połączony z std::cerr ale służy do wyprowadzania informacji diagnostycznych. Jak przekierować w programie widzisz wyżej (zamień cerr na clog). Także jest niebuforowany.

Answer 2

shead już trochę wypisał, ale jeszcze dodam:

1. Nie ustawiaj czcionki, rozmiaru, stylu dla tesktu co klatkę. Raz wystarczy.

2. Chyba niektóre obiekty w klasie randomNumber możesz zrobić statyczne(nie tworzą się za każdym razem), ale nie jestem pewien

3. Kilka razy nadajesz wartość dla ballSpeed 5 i -5 może wystarczy abs(liczba) dla liczby bezwzględnej i -liczba żeby dostać liczbę minusową. Albo tworzysz  2 zmienne po prostu

Comments

co do 3 to jest to randomowa liczba z przedzialu -5 lub 5

---

Mam na myśli ballSpeedX