patterncppModerate
Pong game using SDL 2.0
Viewed 0 times
usinggamepongsdl
Problem
I've just finished writing my first game: a clone of the classic Pong game for Linux and Mac only. You will need SDL 2.0, SDL_ttf 2.0 and SDL_Mixer 2.0 to compile it. The complete project can be found here.
My experience with C++ and game development in general is fairly limited and I would like to know what could be improved in my code.
Some questions I have:
```
/*
* Pong game
* Author: Chafic Najjar
* Note: Origin of the coordinate system is the upper left corner of the screen
*/
#include // SDL library
#include // SDL font library
#include // SDL sound library
#include // abs()
#include // rand()
#include
using namespace std;
SDL_Window* window; // holds window properties
SDL_Renderer* renderer; // holds rendering surface properties
SDL_Texture* font_image_score1; // holds text indicating player 1 score (left)
SDL_Texture* font_image_score2; // holds text indicating palyer 2 score (right)
SDL_Texture* font_image_winner; // holds text indicating winner
SDL_Texture* font_image_restart; // holds text suggesting to restart the game
SDL_Texture* font_image_launch1; // holds first part of text suggesting to launch the ball
SDL_Texture* font_image_launch2; // holds second part of text suggesting to launch the ball
Mix_Chunk *paddle_sound; // holds sound produced after ball collides with paddle
Mix_Chunk *wall_sound; // holds sound produced after ball collides with wall
Mix_Chunk *score_sound; // holds sound produced when updating score
SDL_Color dark_font = {67, 68, 69}; // dark grey
SDL_Color light_font = {187, 191, 194}; // light grey
bool done = false; // true when player exits game
// Scree
My experience with C++ and game development in general is fairly limited and I would like to know what could be improved in my code.
Some questions I have:
- Would it be better to use OOP?
- Should I be using headers?
- Should I divide the code into multiple files?
```
/*
* Pong game
* Author: Chafic Najjar
* Note: Origin of the coordinate system is the upper left corner of the screen
*/
#include // SDL library
#include // SDL font library
#include // SDL sound library
#include // abs()
#include // rand()
#include
using namespace std;
SDL_Window* window; // holds window properties
SDL_Renderer* renderer; // holds rendering surface properties
SDL_Texture* font_image_score1; // holds text indicating player 1 score (left)
SDL_Texture* font_image_score2; // holds text indicating palyer 2 score (right)
SDL_Texture* font_image_winner; // holds text indicating winner
SDL_Texture* font_image_restart; // holds text suggesting to restart the game
SDL_Texture* font_image_launch1; // holds first part of text suggesting to launch the ball
SDL_Texture* font_image_launch2; // holds second part of text suggesting to launch the ball
Mix_Chunk *paddle_sound; // holds sound produced after ball collides with paddle
Mix_Chunk *wall_sound; // holds sound produced after ball collides with wall
Mix_Chunk *score_sound; // holds sound produced when updating score
SDL_Color dark_font = {67, 68, 69}; // dark grey
SDL_Color light_font = {187, 191, 194}; // light grey
bool done = false; // true when player exits game
// Scree
Solution
Global variables
You have many global variables, which is very bad. In general, global variables should be avoided as they can be modified anywhere in the program and at any time. This especially makes debugging difficult as you won't be able to keep track of where these variables could have been modified, as it can happen anywhere in the program.
I also cannot tell which of these should be constants (minus the two that are already constants). If any of them should be a constant, add
For the rest, you should be passing them to functions as needed, or just keep them in one function if they're only used in one. They can be passed separately, or encapsulated in a container class, depending on the situation. This will give you a much better idea of how each variable is used, making debugging and maintainability easier.
Classes
Yes, you should consider using OOP or at least just classes. I'm not too familiar with OOP, so I won't go more into that. But I can say that you should at least consider having a
With this, yes, you'll need to have multiple files. The driver file will just have
You could also have, say, a
Misc.
-
I'd recommend against using
-
I see that you use
-
As you are using C++11, you should no longer use
You have many global variables, which is very bad. In general, global variables should be avoided as they can be modified anywhere in the program and at any time. This especially makes debugging difficult as you won't be able to keep track of where these variables could have been modified, as it can happen anywhere in the program.
I also cannot tell which of these should be constants (minus the two that are already constants). If any of them should be a constant, add
const. Those can then stay there as they'll no longer be mutable.For the rest, you should be passing them to functions as needed, or just keep them in one function if they're only used in one. They can be passed separately, or encapsulated in a container class, depending on the situation. This will give you a much better idea of how each variable is used, making debugging and maintainability easier.
Classes
Yes, you should consider using OOP or at least just classes. I'm not too familiar with OOP, so I won't go more into that. But I can say that you should at least consider having a
Game class and probably additional classes if needed.With this, yes, you'll need to have multiple files. The driver file will just have
main() and will include any needed headers. A separate header and implementation file will be for Game. In the driver, you create a Game object, which will essentially run the game.You could also have, say, a
Ball class and a Paddle class. Basically, if you have some "object" that holds data and has functions to modify that data, it could have its own class. This will help encapsulate your program even more, allowing you to manage these separate classes instead of just straight functions and variables that can correspond with anything.Misc.
-
I'd recommend against using
using namespace std.-
I see that you use
nullptr, yet your std::srand() call uses NULL. If you have access to nullptr, you should use it everywhere as needed and replace NULL.-
As you are using C++11, you should no longer use
std::srand() and std::rand(). You should instead consider any of the new functionality found in ``.Context
StackExchange Code Review Q#42823, answer score: 13
Revisions (0)
No revisions yet.