patterncppMinor
C++ OpenGL Snake clone
Viewed 0 times
clonesnakeopengl
Problem
I have made a multi-platform Snake clone in C++ using OpenGL and GLUT. I am a beginner in graphic game development. I would like to add more features, but I have programmed the base game so far. I would love it if you could review my code and tell me how to make it better.
#include
#include
#include
#include
// A macro for unused variables (to bypass those pesky G++ warnings)
#define UNUSED(param) (void)(param)
// Snake direction macros
#define UP 1
#define DOWN 2
#define LEFT 3
#define RIGHT 4
char title[] = "OpenGL Snake";
float map_half_length = 30.0f;
int direction = DOWN;
int move_speed = 100;
bool moved = false;
std::deque > part_coords;
bool food_available = false;
int food_coords[2];
int growth_stage = 0;
int growth = 2;
void spawnFood(){
if(!food_available){
while(true){
bool collides = false;
// Produce a temporary random coordinate
int temp_food_coords[2] = { food_coords[0] = 2 * (rand() % ((int) map_half_length + 1)) - (int) map_half_length,
food_coords[1] = 2 * (rand() % ((int) map_half_length + 1)) - (int) map_half_length };
// Does it collide with the snake?
for(unsigned int a = 0; a new_head = part_coords[last_part];
if(direction == UP){
// Did we slither into ourself?
for(unsigned int a = 0; a row;
row.push_back(0.0f);
row.push_back((map_half_length + 2.0f + (initSize * 2)) - (a * 2));
part_coords.push_front(row);
}
srand(time(NULL));
initGL();
glutMainLoop();
return 0;
}Solution
This is a great way to learn OpenGL! This looks really straightforward, which is good. I can easily read it and understand what it's intended to do. Nice work.
Avoid Global Variables
In general, global state is a bad idea because it can be changed from any part of the code, and it then becomes difficult to figure who changed what, when. The beauty of Object Oriented programming is that you can encapsulate that data into classes which limits the scope of where changes to that data can occur. Most games have a class that keeps track of the game state. For example, in this case you could put 'direction
Avoid Deprecated Functionality
Both GLUT and OpenGL immediate mode are deprecated. What that means is that while the functionality will continue to ship for the time being, it isn't going to be supported forever, and won't work with newer versions of OpenGL (such as 3.3 or 4.x). That's somewhat unfortunate, as immediate mode makes learning 3D programming a lot easier. But moving forward you should learn about vertex buffer objects, vertex array objects, and shaders, as that's what you'll be using to draw geometry in the future.
Avoid Global Variables
In general, global state is a bad idea because it can be changed from any part of the code, and it then becomes difficult to figure who changed what, when. The beauty of Object Oriented programming is that you can encapsulate that data into classes which limits the scope of where changes to that data can occur. Most games have a class that keeps track of the game state. For example, in this case you could put 'direction
,move_speed,part_coords, etc. into a Game_State class.
In fact, the only C++ construct I see you using in this code is std::deque<>. You could probably improve your code with more use of object oriented programming.
Simplify
Most of your functions do a single task, and that's a very good thing. A few seem to be doing multiple things or doing the same thing repeatedly. I'd simplify them.
For example, your spawnFood() method is both spawning the food and drawing it. I'd break those into 2 separate functions - spawnFood() and drawFood() or something like that.
I also don't understand why you have an infinite loop in spawnFood(). Normally, an infinite loop like that is used in situations where you have a continuous stream of events that is likely to run as long as the app runs. It's used for things like dealing with incoming network packets, or user interface events. In your case, there's a very clear situation where the loop should end, and it would make sense to have that control your loop. I'd make it work something like this:
void spawnFood()
{
if (foodAvailable)
{
return;
}
bool collides;
do {
collides = false;
// Produce a temporary random coordinate
int temp_food_coords[2] = { food_coords[0] = 2 * (rand() % ((int) map_half_length + 1)) - (int) map_half_length,
food_coords[1] = 2 * (rand() % ((int) map_half_length + 1)) - (int) map_half_length };
// Does it collide with the snake?
for(unsigned int a = 0; a < part_coords.size(); a++){
if(temp_food_coords[0] == part_coords[a][0] &&
temp_food_coords[1] == part_coords[a][1]){
collides = true;
}
}
} while (collides);
food_coords[0] = temp_food_coords[0];
food_coords[1] = temp_food_coords[1];
food_available = true;
}
I think you could also simplify your moveSnake()` function, too. You're doing essentially the same check for each direction, with just minor differences. You can encode those differences to simplify your code by doing something like this:void moveSnake(int new_direction){
direction = new_direction;
int last_part = part_coords.size() - 1;
std::deque new_head = part_coords[last_part];
float deltaX = 0.0;
float deltaY = 0.0;
int snakePart = 0;
switch (direction)
{
case UP:
deltaY = 2.0;
snakePart = 1;
break;
case DOWN:
deltaY = -2.0;
snakePart = 1;
break;
case RIGHT:
deltaX = 2.0;
snakePart = 0;
break;
case LEFT:
deltaX = -2.0;
snakePart = 0;
break;
}
// Did we slither into ourself?
for(unsigned int a = 0; a < part_coords.size(); a++){
if(part_coords[0][0] + deltaX == part_coords[a][0] &&
part_coords[0][1] + deltaY == part_coords[a][1]){
exit(0);
}
}
// Did we slither into a wall?
if(part_coords[0][snakePart] == map_half_length){
exit(0);
}
// Did we get food?
if(part_coords[0][0] + deltaX == food_coords[0] &&
part_coords[0][1] + deltaY == food_coords[1]){
growth_stage++;
food_available = false;
}
new_head[0] = part_coords[0][0] + deltaX;
new_head[1] = part_coords[0][1] + deltaY;
part_coords.push_front(new_head);
if(!growth_stage){
part_coords.pop_back();
} else if(growth_stage == growth){
growth_stage = 0;
} else {
growth_stage++;
}
glutPostRedisplay(); // <- Really this should be outside this function
}Avoid Deprecated Functionality
Both GLUT and OpenGL immediate mode are deprecated. What that means is that while the functionality will continue to ship for the time being, it isn't going to be supported forever, and won't work with newer versions of OpenGL (such as 3.3 or 4.x). That's somewhat unfortunate, as immediate mode makes learning 3D programming a lot easier. But moving forward you should learn about vertex buffer objects, vertex array objects, and shaders, as that's what you'll be using to draw geometry in the future.
Code Snippets
void spawnFood()
{
if (foodAvailable)
{
return;
}
bool collides;
do {
collides = false;
// Produce a temporary random coordinate
int temp_food_coords[2] = { food_coords[0] = 2 * (rand() % ((int) map_half_length + 1)) - (int) map_half_length,
food_coords[1] = 2 * (rand() % ((int) map_half_length + 1)) - (int) map_half_length };
// Does it collide with the snake?
for(unsigned int a = 0; a < part_coords.size(); a++){
if(temp_food_coords[0] == part_coords[a][0] &&
temp_food_coords[1] == part_coords[a][1]){
collides = true;
}
}
} while (collides);
food_coords[0] = temp_food_coords[0];
food_coords[1] = temp_food_coords[1];
food_available = true;
}void moveSnake(int new_direction){
direction = new_direction;
int last_part = part_coords.size() - 1;
std::deque<float> new_head = part_coords[last_part];
float deltaX = 0.0;
float deltaY = 0.0;
int snakePart = 0;
switch (direction)
{
case UP:
deltaY = 2.0;
snakePart = 1;
break;
case DOWN:
deltaY = -2.0;
snakePart = 1;
break;
case RIGHT:
deltaX = 2.0;
snakePart = 0;
break;
case LEFT:
deltaX = -2.0;
snakePart = 0;
break;
}
// Did we slither into ourself?
for(unsigned int a = 0; a < part_coords.size(); a++){
if(part_coords[0][0] + deltaX == part_coords[a][0] &&
part_coords[0][1] + deltaY == part_coords[a][1]){
exit(0);
}
}
// Did we slither into a wall?
if(part_coords[0][snakePart] == map_half_length){
exit(0);
}
// Did we get food?
if(part_coords[0][0] + deltaX == food_coords[0] &&
part_coords[0][1] + deltaY == food_coords[1]){
growth_stage++;
food_available = false;
}
new_head[0] = part_coords[0][0] + deltaX;
new_head[1] = part_coords[0][1] + deltaY;
part_coords.push_front(new_head);
if(!growth_stage){
part_coords.pop_back();
} else if(growth_stage == growth){
growth_stage = 0;
} else {
growth_stage++;
}
glutPostRedisplay(); // <- Really this should be outside this function
}Context
StackExchange Code Review Q#134601, answer score: 4
Revisions (0)
No revisions yet.