I started to code in c++ recently and my goal is to develop games using c++. After learning basics I tried to implement my own version of snake console based game in c++ with the help of some online tutorials. I used OOP approach. I would like to hear ideas about this code and what mistakes i have made or ways to improve/optimize this code. I really value your opinions. Thank you!.
#include <iostream>
#include <Windows.h>
#include <sstream>
#include <thread>
#include <list>
#include <chrono>
#include "main.h"
using namespace std::chrono_literals;
//initialize console/window variables
const int SCREEN_WIDITH = 120;
const int SCREEN_HEIGHT = 30;
const int HORIZONTAL_OFFSET = 20;
const int VERTICAL_OFFSET = 5;
static wchar_t* screen = new wchar_t[SCREEN_WIDITH * SCREEN_HEIGHT];
//enum to set snake move direction
enum EDirection
{
UP,
DOWN,
LEFT,
RIGHT
};
//point objects defines x,y cordinates in the screen buffer
struct Point
{
int m_X{};
int m_Y{};
Point(int x, int y) :m_X(x),m_Y(y)
{
}
Point()
{
}
//copy contructer to determine two points are equals/unequals
bool operator==(const Point& other)
{
return (m_X == other.m_X) && (m_Y == other.m_Y) ? true : false;
}
};
//food class creates an object which can be consumed by snake
class Food
{
private:
Point m_CurrentPosiiton; //gives currrent position of the spawned food
public:
Food()
{
MoveFood(); //initial position update for food
}
void MoveFood()
{
//determining a random location within boundries to spawn food
//rand()%(max-min+1)+min;
m_CurrentPosiiton.m_X = rand() % (SCREEN_WIDITH - 2 * HORIZONTAL_OFFSET) + HORIZONTAL_OFFSET+1;
m_CurrentPosiiton.m_Y = rand() % (SCREEN_HEIGHT- 3*VERTICAL_OFFSET +1) + VERTICAL_OFFSET;
//if the determined positon is already have a character then determine again
if (screen[m_CurrentPosiiton.m_X + m_CurrentPosiiton.m_Y * SCREEN_WIDITH] != L' ') { MoveFood(); }
}
//draws food to screen
void DrawFood()
{
screen[m_CurrentPosiiton.m_X+ m_CurrentPosiiton.m_Y*SCREEN_WIDITH] = L'%';
}
//getter to get current postion of food
Point GetCurrenPos()
{
return m_CurrentPosiiton;
}
};
//snake class creates an snake object which user can control
class Snake
{
private:
unsigned char m_Size = 5; //size of the snake
Point m_DefaultPosition{ 60,12 }; //initial start positon of snake
std::list<Point> m_SnakeBody; //snake body represented as a list of points
wchar_t snakeArt = L'O'; //snake art for drawing snake
public:
Snake(unsigned char size) : m_Size(size)
{
//constrcuter automatically determines snake body positions
for (int i = 0; i < m_Size; i++)
{
m_SnakeBody.push_back({ m_DefaultPosition.m_X+i,m_DefaultPosition.m_Y});
}
}
//used to update snake art
void ChangeSnakeArt(const wchar_t& art)
{
snakeArt = art;
}
//draws snake body in to screen
void DrawSnake() const
{
for (const Point &point : m_SnakeBody)
{
screen[point.m_X + SCREEN_WIDITH * point.m_Y ] = snakeArt;
}
}
//Updates snakes body after eating food
void IncreaseSize()
{
m_Size++;
m_SnakeBody.push_back({ GeTailPos().m_X+1,GeTailPos().m_Y });
}
//Handles movement of snake based on player inputs
void MoveSnake(const EDirection& direction)
{
switch (direction)
{
case UP:
m_SnakeBody.push_front({ m_SnakeBody.front().m_X, m_SnakeBody.front().m_Y - 1 });
m_SnakeBody.pop_back();
break;
case DOWN:
m_SnakeBody.push_front({ m_SnakeBody.front().m_X, m_SnakeBody.front().m_Y + 1 });
m_SnakeBody.pop_back();
break;
case LEFT:
m_SnakeBody.push_front({ m_SnakeBody.front().m_X - 1, m_SnakeBody.front().m_Y });
m_SnakeBody.pop_back();
break;
case RIGHT:
m_SnakeBody.push_front({ m_SnakeBody.front().m_X + 1, m_SnakeBody.front().m_Y });
m_SnakeBody.pop_back();
break;
}
}
//check if snake hits its own body
bool HitSelf()
{
for(auto i= m_SnakeBody.begin();i!=m_SnakeBody.end();i++)
{
if(m_SnakeBody.begin()!=i)
{
if(GetHeadPos()==*i)
{
return true;
}
}
}
return false;
}
//helper to get snake head coordinates
Point GetHeadPos()
{
return m_SnakeBody.front();
}
//helper to get snake tail coordinates
Point GeTailPos()
{
return m_SnakeBody.back();
}
};
//to draw level borders
void DrawLevel(wchar_t* screen)
{
//Draw top & bottom horizontal line
for (int i = 0; i < (SCREEN_WIDITH - HORIZONTAL_OFFSET * 2); i++)
{
screen[SCREEN_WIDITH * 4 + HORIZONTAL_OFFSET + i] = L'_';
screen[SCREEN_WIDITH * 20 + HORIZONTAL_OFFSET + i] = L'_';
}
//Draw vertical left & right line
for (int i = VERTICAL_OFFSET - 1; i <= SCREEN_HEIGHT - VERTICAL_OFFSET * 2; i++)
{
screen[SCREEN_WIDITH * i + HORIZONTAL_OFFSET] = L'|';
screen[SCREEN_WIDITH * i + HORIZONTAL_OFFSET * 5] = L'|';
}
}
void ClearScreen()
{
//Clear screen
for (int i = 0; i < SCREEN_HEIGHT * SCREEN_WIDITH; i++)
{
screen[i] = L' ';
}
}
void DrawInfo(const int& score)
{
//Draw Stats & Border
for (int i = 0; i < SCREEN_WIDITH; i++)
{
screen[i] = L'=';
screen[SCREEN_WIDITH * 2 + i] = L'=';
}
wsprintf(&screen[SCREEN_WIDITH + 3], L"Verison:1 Saki Games - SNAKE!! SCORE: %d",score);
}
void DrawEndScreen()
{
wsprintf(&screen[23*SCREEN_WIDITH + 45], L"GAME OVER - PRESS SPACE TO RESTART");
}
int main()
{
// Create Screen Buffer
for (int i = 0; i < SCREEN_WIDITH * SCREEN_HEIGHT; i++) screen[i] = L' ';
HANDLE hConsole = CreateConsoleScreenBuffer(GENERIC_READ | GENERIC_WRITE, 0, NULL, CONSOLE_TEXTMODE_BUFFER, NULL);
SetConsoleActiveScreenBuffer(hConsole);
DWORD dwBytesWritten = 0;
while (1) {
Snake snake = Snake(5);
Food food = Food();
bool isDead{};
int score{};
EDirection snakeDirection = EDirection::LEFT;
while (!isDead)
{
//Timing & input
auto t1 = std::chrono::system_clock::now();
while ((std::chrono::system_clock::now() - t1)<200ms)
{
if (GetAsyncKeyState(VK_LEFT) && snakeDirection != EDirection::RIGHT)
{
snakeDirection = EDirection::LEFT;
}
else if (GetAsyncKeyState(VK_RIGHT) && snakeDirection != EDirection::LEFT)
{
snakeDirection = EDirection::RIGHT;
}
else if (GetAsyncKeyState(VK_UP) && snakeDirection != EDirection::DOWN)
{
snakeDirection = EDirection::UP;
}
else if (GetAsyncKeyState(VK_DOWN) && snakeDirection != EDirection::UP)
{
snakeDirection = EDirection::DOWN;
}
}
//Game Logic
snake.MoveSnake(snakeDirection);
//Colision detection
if (snake.GetHeadPos() == food.GetCurrenPos())
{
score++;
food.MoveFood();
snake.IncreaseSize();
}
//Colision detection with self
isDead = snake.HitSelf();
//Coliision detection with boundry
for (int i = 0; i < (SCREEN_WIDITH - HORIZONTAL_OFFSET * 2); i++)
{
int snakeCor = snake.GetHeadPos().m_X + SCREEN_WIDITH * snake.GetHeadPos().m_Y;
if (((SCREEN_WIDITH * 4 + HORIZONTAL_OFFSET + i) == (snakeCor)) ||
((SCREEN_WIDITH * 20 + HORIZONTAL_OFFSET + i) == (snakeCor)))
{
isDead = true;
}
}
for (int i = VERTICAL_OFFSET - 1; i <= SCREEN_HEIGHT - VERTICAL_OFFSET * 2; i++)
{
int snakeCor = snake.GetHeadPos().m_X + SCREEN_WIDITH * snake.GetHeadPos().m_Y;
if (((SCREEN_WIDITH * i + HORIZONTAL_OFFSET) == (snakeCor)) ||
((SCREEN_WIDITH * i + HORIZONTAL_OFFSET * 5) == (snakeCor)))
{
isDead = true;
}
}
//Draw stuff to screen
ClearScreen();
DrawInfo(score);
DrawLevel(screen);
//check for dead condition
if (isDead)
{
DrawEndScreen();
snake.ChangeSnakeArt(L'X');
}
//draws snake and food to screen
snake.DrawSnake();
food.DrawFood();
//Display Frame
WriteConsoleOutputCharacter(hConsole, screen, SCREEN_WIDITH * SCREEN_HEIGHT, { 0,0 }, &dwBytesWritten);
}
//wait till space bar input to restart game
while (GetAsyncKeyState(VK_SPACE) == 0);
}
return 0;
}
2 Answers 2
To start, I think you should be proud of your work so far! There are still many ways to improve (I'm not going to cover all of them) but now you can say you have created a fun game in C++ and show it to your friends and family and the internet. Many people cannot say that.
Grouping and Naming
Regardless of which paradigm you're aiming for, this is a fundamental concept of programming that just takes different forms based on which paradigm and language you're working in. It is clear that you have this concept in mind but it's so fundamental that I will expand on it further.
Naming Meaningful Operations
Within your code you represent the screen as a one dimensional array and frequently access it like so screen[x + y * screen_width]
. This isn't a meaningless fragment of some strange formula, this formula is how you access an (x, y) coordinate of your screen representation. In the context of OOP, you could create a screen class containing a member function that serves this purpose, so instead of writing screen[x + y * SCREEN_WIDTH] you would write screen.at(x, y). Notice that now you only have to make sure the calculation is correct on one line of code instead of like 8++.
Grouping and Naming Meaningful Data
Within your code the variables SCREEN_WIDITH, SCREEN_HEIGHT, and screen appear together frequently. These values work together to describe the visual state of your application. In the context of OOP classes are employed so you could create a class called Screen to hold these three variables. Notice now that if you have to pass this information on to another function, class, thread, etc... you only have to worry about one variable of type Screen instead of three of types (wchar_t*, int, int).
Grouping Meaningful Data and Operations
Having code that is conceptually related grouped together means it is easier to find, consume and understand. (Whether through a plain header file, a class, or any other grouping method). The advantages of this become clearer in larger projects when you are either searching for the definition of data that a function works on, searching for functionality related to some data definition, or trying to figure out the concepts behind some code.
Un-grouping Meaningless Data and Operations
Within your main function you have the variable dwBytesWritten which holds how many bytes have been written to the window. main() is an important function because it (usually) communicates every single thing our application is doing, and so it is essential to understanding any application. dwBytesWritten could not be less important to understanding how this snake game works, so we should un-group them. Now I personally don't think it has much meaning anywhere else at the moment but, since I'm assuming it is required for WriteConsoleOutputCharacter, the most logical place to put it is the Screen class.
So we apply these concepts to the screen representation and we arrive at this
class Screen
{
private:
const int WIDTH;
const int HEIGHT;
wchar_t *screen;
HANDLE hConsole;
DWORD dwBytesWritten;
public:
Screen(int width, int height) : WIDTH(width),
HEIGHT(height),
dwBytesWritten(0)
{
this->screen = new wchar_t[this->WIDTH * this->HEIGHT];
this->clear();
this->hConsole = CreateConsoleScreenBuffer(GENERIC_READ | GENERIC_WRITE, 0, NULL, CONSOLE_TEXTMODE_BUFFER, NULL);
SetConsoleActiveScreenBuffer(this->hConsole);
}
~Screen()
{
CloseHandle(this->hConsole);
delete[] this->screen;
}
void clear()
{
for (int i = 0; i < this->WIDTH * this->HEIGHT; ++i)
this->screen[i] = L' ';
}
wchar_t &at(int x, int y)
{
return this->screen[x + y * this->WIDTH];
}
const wchar_t &at(int x, int y) const
{
return this->at(x, y);
}
void display()
{
WriteConsoleOutputCharacter(this->hConsole, this->screen, this->WIDTH * this->HEIGHT, {0, 0}, &this->dwBytesWritten);
}
int getWidth() const
{
return this->WIDTH;
}
int getHeight() const
{
return this->HEIGHT;
}
};
Now the start of main would look like
int main()
{
Screen screen(120, 30);
while (1)
{
Snake snake = ...
and your Food::DrawFood member function would look like
void DrawFood(Screen& screen)
{
screen.at(m_CurrentPosiiton.m_X, m_CurrentPosiiton.m_Y) = L'%';
}
It's important to not be blind to the fact that the class itself generates more lines of code than if we hadn't grouped anything. This is why it is important not to apply the concepts without thought: we must always try to know that the benefits of the decisions we are making right now outweigh the drawbacks. This is not easy, but to get you started consider how sooo many classes are using the horizontal and vertical offset. Why should Food have to know it's absolute position in the console, rather than just where it is within the arena. Wouldn't it simplify many calculations if the top left square of the snake arena could be called (0, 0) instead of (horizontalOffset, verticalOffset)?
-
\$\begingroup\$ First of all thank you for your valuable input. You have shown me many areas which can improve and most importantly you have given me confidence to do more these stuff. So thank you you for that. I clearly understand every thing you are suggesting and i'm gonna use it on my next console game. Again thank you for taking your valuable time to write this awesome review. \$\endgroup\$Sakitha Navod– Sakitha Navod2020年08月10日 04:46:58 +00:00Commented Aug 10, 2020 at 4:46
According to S from SOLID objects should have only one responsibility. Therefore I would move draw and input logic from current objects into a separate one. It could be something like UI and InputController classes. The idea here is to hide all I/O related stuff in a way that allows changing I/O without changing game logic. It is a very common problem and popular solution is called MVC
Another thing that I would improve is the code in the main function - it could be moved into Game class. Game class may contain UI, InputController, and GameLogic (the place were all game rules live)
-
\$\begingroup\$ Cool suggestion. I'll use these techniques in my next console game. Thank you for the answer. \$\endgroup\$Sakitha Navod– Sakitha Navod2020年08月10日 04:47:52 +00:00Commented Aug 10, 2020 at 4:47
Explore related questions
See similar questions with these tags.