4
\$\begingroup\$

I've been coding for about two months, and I recently attempted my first game. I'd appreciate any advice or feedback on how to improve my coding. For instance, I don't think the organisation is the best -- how should I have split this up into classes? Should I have made a Snake class and perhaps a Graphics class?

#include <SDL2/SDL.h>
#include <SDL2_image/SDL_image.h>
#include <SDL2_ttf/SDL_ttf.h>
#include <SDL2_mixer/SDL_mixer.h>
#include <iostream>
#include <random>
#include "Snake.h"
// Globals
//////////////////////
const int SPRITE_SIZE = 32;
const int SQUARES = 21;
const int PADDING_TOP = 32;
const int PADDING_SIDES = 8;
const int SCREEN_WIDTH = SPRITE_SIZE * SQUARES;
const int SCREEN_HEIGHT = SPRITE_SIZE * SQUARES;
const int SCREEN_CENTRE_X = PADDING_SIDES + (SCREEN_WIDTH/2)-(SPRITE_SIZE/2);
const int SCREEN_CENTRE_Y = PADDING_TOP + (SCREEN_HEIGHT/2)-(SPRITE_SIZE/2);
const int WINDOW_WIDTH = SCREEN_WIDTH + 2 * PADDING_SIDES;
const int WINDOW_HEIGHT = SCREEN_HEIGHT + PADDING_TOP + PADDING_SIDES;
const int FPS = 15;
const int FRAME_PERIOD = 1000.0f / FPS;
enum direction { stop, left, right, up, down };
direction dir { stop };
// For randomly generating position of item
std::random_device rd;
std::mt19937 rng(rd());
std::uniform_int_distribution<int> posX(0, SQUARES-1); // -1 to account for sprite size
std::uniform_int_distribution<int> posY(0,SQUARES-1);
// Snake class details
//////////////////////
Snake::Snake()
: headX {SCREEN_CENTRE_X}, headY {SCREEN_CENTRE_Y}, tailN {0}, score {0}, gameOver {false}, turnOver {false}, pause {false} {
 setup();
 loop();
}
void Snake::setup() {
 // Set up SDL, define sprite clips and default game values.
 // Needs error checking
 // Init Video & Fonts
 SDL_INIT_VIDEO;
 SDL_INIT_AUDIO;
 TTF_Init();
 // Splash Screen
 /////////////////
 SDL_Window* splashwindow = SDL_CreateWindow("Splash", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 400, 250, SDL_WINDOW_SHOWN);
 SDL_Surface* splashsurface = SDL_GetWindowSurface( splashwindow );
 SDL_Surface* splash = IMG_Load("snMEOWake.png");
 SDL_BlitSurface( splash, NULL, splashsurface, NULL );
 SDL_UpdateWindowSurface( splashwindow );
 SDL_Delay(2000);
 SDL_FreeSurface(splash);
 SDL_FreeSurface(splashsurface);
 SDL_DestroyWindow(splashwindow);
 // Main Game Window
 ////////////////////
 window = SDL_CreateWindow("sn-MEOW-ake", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, WINDOW_WIDTH, WINDOW_HEIGHT, SDL_WINDOW_SHOWN);
 if (window == nullptr)
 std::cerr << "Error: Create Window";
 // Create Renderer
 renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC);
 if (renderer == nullptr)
 std::cerr << "Error: Create renderer";
 // Load textures
 texture = IMG_LoadTexture(renderer, "sprites.png");
 if (texture == nullptr)
 std::cerr << "Error: Load texture";
 // Load scores
 updateScore();
 getHighScore();
 updateHighScore();
 //Audio
 Mix_OpenAudio(44100, MIX_DEFAULT_FORMAT, 2, 2048);
 meow = Mix_LoadWAV("cat_meow_x.wav");
 // Set clip for snake
 snakeClips[0].x = 0;
 snakeClips[0].y = 0;
 snakeClips[0].w = SPRITE_SIZE;
 snakeClips[0].h = SPRITE_SIZE;
 // Set clip for item ('fruit')
 snakeClips[1].x = 0;
 snakeClips[1].y = 4*SPRITE_SIZE;
 snakeClips[1].w = SPRITE_SIZE;
 snakeClips[1].h = SPRITE_SIZE;
 // Set random location for item
 itemX = posX(rng) * SPRITE_SIZE + PADDING_SIDES;
 itemY = posY(rng) * SPRITE_SIZE + PADDING_TOP;
}
void Snake::draw() {
 // darkest grey shade, background
 SDL_SetRenderDrawColor(renderer, 52, 51, 50, 0);
 SDL_RenderClear(renderer);
 // Fill screen rectangle
 SDL_SetRenderDrawColor(renderer, 172, 119, 120, 22);
 SDL_Rect screenfill { PADDING_SIDES, PADDING_TOP, SCREEN_WIDTH, SCREEN_HEIGHT };
 SDL_RenderFillRect(renderer, &screenfill);
 // Draw game elements
 //////////////////////
 renderTexture(printscore, renderer, PADDING_SIDES+3, 10, nullptr); // Score
 renderTexture(printhighscore, renderer, SCREEN_WIDTH/2+205, 10, nullptr); // Highscore
 renderTexture(texture, renderer, itemX, itemY, &snakeClips[1]); // item
 renderTexture(texture, renderer, headX, headY, &snakeClips[0]); // snake head
 // snake tail
 if (tailN > 0){
 for (int i = 0; i < tailN; ++i)
 renderTexture(texture, renderer, tailX[i], tailY[i], &snakeClips[1]);
 }
 // Game Over text rect - Messy. to do.
 SDL_Rect gameoverRect; //gameover
 SDL_Rect gameoverRect2; // press space
 void getRect(const SDL_Texture t);
 SDL_QueryTexture(printgameover, NULL, NULL, &gameoverRect.w, &gameoverRect.h);
 SDL_QueryTexture(printreplay, NULL, NULL, &gameoverRect2.w, &gameoverRect2.h);
 gameoverRect.x = PADDING_SIDES + (SCREEN_WIDTH/2)-(gameoverRect.w/2);
 gameoverRect.y = PADDING_TOP + (SCREEN_HEIGHT/2)-(gameoverRect.h); // higher than centre - personal preference
 gameoverRect2.x = (SCREEN_WIDTH/2)-(gameoverRect2.w/2);
 gameoverRect2.y = gameoverRect.y + gameoverRect.h + gameoverRect2.h;
 // Game Over text rendering
 renderTexture(printgameover, renderer, gameoverRect, nullptr); // Game Over
 renderTexture(printreplay, renderer, gameoverRect2, nullptr); // Space to continue
 SDL_RenderPresent(renderer);
}
void Snake::loop() {
 Uint32 frameStart, frameTime;
 SDL_Event event;
 while (gameOver == false){
 frameStart = SDL_GetTicks();
 while (SDL_PollEvent(&event)){
 if (event.type == SDL_QUIT)
 gameOver = true;
 if (event.type == SDL_KEYDOWN){
 if ((event.key.keysym.sym == SDLK_a ||
 event.key.keysym.sym == SDLK_LEFT) && dir != right)
 dir = left;
 if ((event.key.keysym.sym == SDLK_d ||
 event.key.keysym.sym == SDLK_RIGHT) && dir != left)
 dir = right;
 if ((event.key.keysym.sym == SDLK_w ||
 event.key.keysym.sym == SDLK_UP) && dir != down)
 dir = up;
 if ((event.key.keysym.sym == SDLK_s ||
 event.key.keysym.sym == SDLK_DOWN) && dir != up)
 dir = down;
 if (event.key.keysym.sym == SDLK_ESCAPE){
 gameOver = true;
 dir = stop;
 }
 if (event.key.keysym.sym == SDLK_SPACE){
 if (turnOver == true) // Play again when turn is over
 reset();
 else if (pause == true)
 pause = false;
 else if (pause == false)
 pause = true;
 }
 }
 }
 if (pause == false && tailCollision() == false){
 logic();
 draw();
 }
 frameTime = SDL_GetTicks() - frameStart;
 if (frameTime < FRAME_PERIOD){
 SDL_Delay((int)(FRAME_PERIOD - frameTime));
 }
 }
 SDL_DestroyTexture(texture);
 SDL_DestroyTexture(printscore);
 SDL_DestroyTexture(printgameover);
 SDL_DestroyTexture(printreplay);
 Mix_FreeChunk(meow);
 SDL_DestroyRenderer(renderer);
 SDL_DestroyWindow(window);
 IMG_Quit();
 Mix_Quit();
 SDL_Quit();
}
void Snake::logic() {
 // 'Pick up' item
 if (itemCollision()){
 score += 10;
 if (score > highScore) updateHighScore();
 ++tailN;
 Mix_PlayChannel(-1, meow, 0);
 updateScore(); // updates text texture
 itemX = posX(rng) * SPRITE_SIZE + PADDING_SIDES;
 itemY = posY(rng) * SPRITE_SIZE + PADDING_TOP;
 }
 // Snake movement
 if (dir == left)
 headX -= SPRITE_SIZE;
 else if (dir == right)
 headX += SPRITE_SIZE;
 else if (dir == up)
 headY -= SPRITE_SIZE;
 else if (dir == down)
 headY += SPRITE_SIZE;
 // Tail movement [1] to [tailN]
 for (int i = tailN-1; i > 0; --i){ // Tail starts counting at 1
 tailX[i] = tailX[i-1];
 tailY[i] = tailY[i-1];
 }
 // Tail[0] movement
 if (dir == left){
 tailX[0] = headX + SPRITE_SIZE;
 tailY[0] = headY;
 }
 else if (dir == right){
 tailX[0] = headX - SPRITE_SIZE;
 tailY[0] = headY;
 }
 else if (dir == up){
 tailX[0] = headX;
 tailY[0] = headY + SPRITE_SIZE;
 }
 else if (dir == down){
 tailX[0] = headX;
 tailY[0] = headY - SPRITE_SIZE;
 }
 // Tail collision check
 if(tailCollision()) {
 // Render GAME OVER message
 SDL_Color fontcolour = { 255, 255, 255, 255 };
 printgameover = renderText("GAME OVER", "GreenFlame.ttf", fontcolour, 60, renderer);
 printreplay = renderText("Press spacebar to continue", "GreenFlame.ttf", fontcolour, 13, renderer);
 turnOver = true;
 }
 // When snake leaves window loop to opposite side
 if (headX < PADDING_SIDES) headX = SCREEN_WIDTH + PADDING_SIDES - SPRITE_SIZE;
 if (headX > (PADDING_SIDES + SCREEN_WIDTH - SPRITE_SIZE) ) headX = PADDING_SIDES;
 if (headY < PADDING_TOP) headY = SCREEN_HEIGHT + PADDING_TOP - SPRITE_SIZE;
 if (headY > (PADDING_TOP + SCREEN_HEIGHT - SPRITE_SIZE) ) headY = PADDING_TOP;
}
bool Snake::itemCollision() {
 if ( headY + SPRITE_SIZE <= itemY ) // bottom_head <= top_item
 return false;
 if ( headY >= itemY+SPRITE_SIZE ) // top_head >= bottom_item
 return false;
 if ( headX + SPRITE_SIZE <= itemX ) // right_head <= left_item
 return false;
 if ( headX >= itemX+SPRITE_SIZE ) // left_head >= right_item
 return false;
 return true;
}
bool Snake::tailCollision() {
 for (int i = 0; i < tailN; ++i) {
 if (headX == tailX[i] && headY == tailY[i])
 return true;
 }
 return false;
}
void Snake::reset() {
 printgameover = nullptr;
 printreplay = nullptr;
 headX = SCREEN_CENTRE_X;
 headY = SCREEN_CENTRE_Y;
 itemX = posX(rng) * SPRITE_SIZE + PADDING_SIDES;
 itemY = posY(rng) * SPRITE_SIZE + PADDING_TOP;
 dir = stop;
 tailN = 0;
 score = 0;
 updateScore();
}
void Snake::updateScore() {
 SDL_Color fontcolour = { 240, 228, 228, 4 }; // light red / white
 std::string scorestring = std::to_string(score);
 std::string scoremsg = "score: " + scorestring;
 printscore = renderText(scoremsg, "GreenFlame.ttf", fontcolour, 16, renderer);
}
void Snake::updateHighScore() {
 if (score >= highScore) {
 highScore = score;
 // Update highscores file
 SDL_RWops* highscores = SDL_RWFromFile("highscores.txt", "w");
 if (highscores == NULL)
 printf( "Warning: Unable to open file! SDL Error: %s\n", SDL_GetError() );
 SDL_RWwrite(highscores, &highScore, sizeof(int), 1);
 SDL_RWclose(highscores);
 }
 // Render new score
 SDL_Color fontcolour = { 240, 228, 228, 4 }; // light red / white
 std::string scorestring = std::to_string(highScore);
 std::string scoremsg = "highscore: " + scorestring;
 printhighscore = renderText(scoremsg, "GreenFlame.ttf", fontcolour, 16, renderer);
}
void Snake::getHighScore() {
 SDL_RWops* highscores = SDL_RWFromFile("playground/highscores.txt", "r");
 if (highscores == NULL){
 std::cerr << "Error: Could not open highscores.txt or file does not exist" << std::endl;
 highScore = score;
 }
 else SDL_RWread(highscores, &highScore, sizeof(int), 1);
}
// Render texture with existing destination rect
void Snake::renderTexture(SDL_Texture *tex, SDL_Renderer *ren, SDL_Rect dst, SDL_Rect *clip = nullptr) {
 SDL_RenderCopy(ren, tex, clip, &dst);
}
// Create destination rect from x and y, render texture.
void Snake::renderTexture(SDL_Texture *tex, SDL_Renderer *ren, int x, int y, SDL_Rect *clip = nullptr) {
 SDL_Rect dst;
 dst.x = x;
 dst.y = y;
 if (clip != nullptr){
 dst.w = clip->w;
 dst.h = clip->h;
 }
 else {
 SDL_QueryTexture(tex, NULL, NULL, &dst.w, &dst.h);
 }
 SDL_RenderCopy(ren, tex, clip, &dst);
}
SDL_Texture* Snake::renderText(const std::string &message, const std::string &fontFile,
 SDL_Color color, int fontSize, SDL_Renderer *renderer)
{
 //Open the font
 TTF_Font *font = TTF_OpenFont(fontFile.c_str(), fontSize);
 if (font == nullptr){
 std::cout << "TTF_OpenFont";
 return nullptr;
 }
 //render to a surface as that's what TTF_RenderText returns
 SDL_Surface *surf = TTF_RenderText_Solid(font, message.c_str(), color);
 if (surf == nullptr){
 TTF_CloseFont(font);
 std::cout << "TTF_RenderText";
 return nullptr;
 }
 //then load that surface into a texture
 SDL_Texture *texture = SDL_CreateTextureFromSurface(renderer, surf);
 if (texture == nullptr){
 std::cout << "CreateTexture";
 }
 //Clean up the surface and font
 SDL_FreeSurface(surf);
 TTF_CloseFont(font);
 return texture;
}

The full source code can be found here.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 5, 2016 at 7:51
\$\endgroup\$
1
  • \$\begingroup\$ On your github, you should probably make a Makefile. \$\endgroup\$ Commented Oct 5, 2016 at 9:06

2 Answers 2

1
\$\begingroup\$

What i would recommend is that you put things together, that belong together.

So for example

int itemX;
int itemY;

should better be

std::pair<int, int> itemPosition;

then you can access them with first and second respectively. This applies basically for every coordinate you have.


Then i would urge you to define more classes that hold your respective data and apply some methods. Currently you have one class snake that does everything. It might make sense to encapsulate rendering into its own subclass and the snake itself too the same goes for the arena.

typedef std::pair<int, int> coordinate;
class snake {
public: 
 snake::snake()
 : tail({std::make_pair(SCREEN_CENTRE_X, SCREEN_CENTRE_>)}) {}
 void moveSnake(enum direction);
 bool checkTailCollision const (void)
 void checkItemCollision (const coordinate &itemPosition);
private:
 void growSnake (void);
 std::vector<coordinate> tail; 
};

For a function take your logic function, which does a lot of things. First it checks for item collisions, then it moves the snake and then it checks for further collisions, as well as border crossings. Everyone of those should be its own function.

BTW is there a reason you update the score every time and not just at the end of the game?


Why is the head separated from the tail? If you use a container you can utilize front() to get the first element or back() for the last one.


Your names should be more descriptive. The fact that the actual game is in the function loop is a bad wording. Same for logic. Generally a function does something which should be reflected in the name.


You should either use std::array or the more dynamic std::vector instead of plain c arrays like tailX[]. Again group the coordinates into pairs

std::vector<std::pair<int, int>> tail;

Be consistent in you code style. If you sprinkle your code randomly with stuff like this

if (score > highScore) updateHighScore();

It becomes much harder to read.

answered Oct 5, 2016 at 15:19
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Instead of std::pair<int, int> itemPosition; a struct with int x; int y; is clear on intent. Using first and second instead of x, y is counter intuitive. And drop the void in the params, it hurts my eyes ;-) \$\endgroup\$ Commented Oct 5, 2016 at 20:23
1
\$\begingroup\$

It is in most cases the best practice to use makefiles to compile your C/C++ code. It would also help the next person to test your code not to go through the struggle trying to figure out how to compile the code. In as much as this project is small, it would be a nice to use a makefile.

answered Jun 20, 2017 at 9:05
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.