14
\$\begingroup\$

I decided to write a little snake game in c++ to practice and as an opportunity to learn ncurses. It turned out to be much bigger than I anticipated but since I've never really written anything big, I'd be really thankful if you could point out more efficient ways or better practices.

main.cpp

#include "ui.hpp" #include "settings.hpp" Point Settings::field_size = {18, 35}; bool Settings::enable_walls = false; int main() { MainMenu main_menu; main_menu.show(); return 0; }

ui.hpp

#pragma once #include <ncurses.h> #include <string> #include <vector> #include "point.hpp" class Field; enum class Facing; using menu_item_t = int; struct MenuItem { std::string label; Point pos; }; class MainMenu { private: template<typename Functor> void display_menu(std::vector<MenuItem> &p_menu_items, Functor p_selected_item_handler, bool p_quit_with_q, std::string p_title = "Snake"); void new_game(); void show_settings(); public: MainMenu(); ~MainMenu(); void show(); }; class GameUI { private: WINDOW *m_border_win, *m_field_win; const Field *m_field; void update_field(); public: GameUI(WINDOW *p_border_win, WINDOW *p_field_win); void set_field(Field *p_field) { m_field = p_field; }; void draw_border(); void draw_static_elements(); void update(int score); Facing get_input(); }; class UIUtils { private: UIUtils() {}; public: static menu_item_t dialogbox(std::string p_text, std::vector<std::string> p_buttons); };

ui.cpp

#include <stdexcept> #include "ui.hpp" #include "field.hpp" #include "game.hpp" #include "player.hpp" #include "settings.hpp" struct GameExit : std::exception {}; const char* const bool_to_str(bool b) { return b ? "enabled" : "disabled"; } template<typename Functor> void MainMenu::display_menu(std::vector<MenuItem> &p_menu_items, Functor p_selected_item_handler, bool p_quit_with_q, std::string p_title) { for(std::size_t i = 0; i < p_menu_items.size(); ++i) { p_menu_items[i].pos = {LINES / 2 + (int) i, (COLS - (int) p_menu_items[i].label.length()) / 2}; } try { erase(); menu_item_t selected_item = 0; bool is_selected = false; while(true) { mvprintw(LINES / 4, (COLS - p_title.length()) / 2, p_title.c_str()); for(std::size_t i = 0; i < p_menu_items.size(); ++i) { mvprintw(p_menu_items[i].pos.y, p_menu_items[i].pos.x, p_menu_items[i].label.c_str()); } // make the currently selected item standout mvchgat(p_menu_items[selected_item].pos.y, p_menu_items[selected_item].pos.x, p_menu_items[selected_item].label.length(), A_STANDOUT, 0, NULL); refresh(); switch(getch()) { case KEY_UP: selected_item = selected_item != 0 ? selected_item - 1 : p_menu_items.size() - 1; break; case KEY_DOWN: selected_item = selected_item != (int) p_menu_items.size() - 1 ? selected_item + 1 : 0; break; case '\n': is_selected = true; break; case 'q': case 27: if(p_quit_with_q) throw GameExit(); break; } if(is_selected) { p_selected_item_handler(selected_item); is_selected = false; erase(); } } } // exit the game, if it's called for an exit catch(const GameExit &) {} } void MainMenu::new_game() { erase(); refresh(); WINDOW *game_win = newwin(Settings::field_size.y + 2, Settings::field_size.x + 2, (LINES - Settings::field_size.y) / 2 - 1, (COLS - Settings::field_size.x) / 2 - 1); WINDOW *game_field_win = newwin(Settings::field_size.y, Settings::field_size.x, (LINES - Settings::field_size.y) / 2, (COLS - Settings::field_size.x) / 2); GameUI *game_ui = new GameUI(game_win, game_field_win); Game game(game_ui); game.start(); delwin(game_field_win); delwin(game_win); delete game_ui; } void MainMenu::show_settings() { std::vector<MenuItem> settings_menu_items = {{ {std::string("Field size: ") + std::to_string(Settings::field_size.y) + " rows, " + std::to_string(Settings::field_size.x) + " cols"}, {std::string("Walls: ") + bool_to_str(Settings::enable_walls), {} }, }}; display_menu(settings_menu_items, [&settings_menu_items](menu_item_t p_selected_item) { switch (p_selected_item) { case 0: switch(Settings::field_size.y) { case 18: Settings::field_size = {25, 50}; break; case 25: Settings::field_size = {10, 20}; break; default: Settings::field_size = {18, 35}; break; } settings_menu_items[0].label = std::string("Field size: ") + std::to_string(Settings::field_size.y) + " rows, " + std::to_string(Settings::field_size.x) + " cols"; break; case 1: Settings::enable_walls = !Settings::enable_walls; settings_menu_items[1].label = std::string("Walls: ") + bool_to_str(Settings::enable_walls); break; default: break; } }, true, "Settings"); } MainMenu::MainMenu() { initscr(); cbreak(); noecho(); curs_set(0); keypad(stdscr, true); } MainMenu::~MainMenu() { endwin(); } void MainMenu::show() { std::vector<MenuItem> main_menu_items = {{ {"New Game", {} }, {"Settings", {} }, {"Exit", {} } }}; display_menu(main_menu_items, [this](menu_item_t p_selected_item) { switch(p_selected_item) { // New Game case 0: new_game(); break; // Settings case 1: show_settings(); break; case 2: throw GameExit(); } }, false); } GameUI::GameUI(WINDOW *p_border_win, WINDOW *p_field_win) : m_border_win(p_border_win), m_field_win(p_field_win) { draw_border(); nodelay(m_field_win, true); keypad(m_field_win, true); } void GameUI::draw_border() { box(m_border_win, 0, 0); wrefresh(m_border_win); } void GameUI::draw_static_elements() { for(int row = 0; row < m_field->m_field_size.y; ++row) { for(int col = 0; col < m_field->m_field_size.x; ++col) { if(m_field->get({row, col}) == Object::wall) mvwaddch(m_field_win, row , col, '#'); } } wrefresh(m_field_win); } void GameUI::update(int score) { mvwprintw(m_border_win, 0, 2, "Score: %d", score); wrefresh(m_border_win); update_field(); wrefresh(m_field_win); } void GameUI::update_field() { for(int row = 0; row < m_field->m_field_size.y; ++row) { for(int col = 0; col < m_field->m_field_size.x; ++col) { switch(m_field->get({row, col})) { case Object::empty: mvwaddch(m_field_win, row , col, ' '); break; case Object::player: mvwaddch(m_field_win, row , col, '*'); break; case Object::food: mvwaddch(m_field_win, row , col, '$'); break; default: break; } } } } Facing GameUI::get_input() { int input = wgetch(m_field_win); switch (input) { case KEY_UP: return Facing::up; case KEY_RIGHT: return Facing::right; case KEY_DOWN: return Facing::down; case KEY_LEFT: return Facing::left; case 'q': case 27: throw GameEndQuit(); break; } return Facing::null; } menu_item_t UIUtils::dialogbox(std::string p_text, std::vector<std::string> p_buttons) { // if COLS / 4 < min_width(the width so that all elements would fit) -> width = COLS - 4, else width = COLS / 4 int width = COLS / 4 < [&p_text, &p_buttons]() -> int { int min_width = 0; for(std::string button : p_buttons) { min_width += button.length() + 2; } min_width = min_width > (int) p_text.length() ? min_width : p_text.length(); return min_width + 10; } () ? COLS - 10 : COLS / 4; WINDOW *win = newwin(7, width, (LINES - 7) / 2, (COLS - (width)) / 2); keypad(win, true); box(win, 0, 0); mvwprintw(win, 2, (win->_maxx - p_text.length()) / 2, p_text.c_str()); wrefresh(win); menu_item_t selected_item = 0; while(true) { for(std::size_t i = 0; i < p_buttons.size(); ++i) { // x = (total width of the window / (amount of buttons + 1)) * (current button + 1) - (length of the text of the button / 2) mvwprintw(win, 5, (win->_maxx / (p_buttons.size() + 1)) * (i + 1) - (p_buttons[i].length() / 2), p_buttons[i].c_str()); } mvwchgat(win, 5, (win->_maxx / (p_buttons.size() + 1)) * (selected_item + 1) - (p_buttons[selected_item].length() / 2), p_buttons[selected_item].length(), A_STANDOUT, 0, NULL); switch(wgetch(win)) { case KEY_LEFT: selected_item = selected_item != 0 ? selected_item - 1 : p_buttons.size() - 1; break; case KEY_RIGHT: selected_item = selected_item != (int) p_buttons.size() - 1 ? selected_item + 1 : 0; break; // Enter case '\n': werase(win); wrefresh(win); delwin(win); return selected_item; } } throw std::logic_error("Out of the infinite while loop"); }

point.hpp

#pragma once struct Point { int y; int x; }; inline bool operator==(const Point& left, const Point& right) { return left.y == right.y && left.x == right.x; }

field.hpp

#pragma once #include "point.hpp" class Player; enum class Object { empty, player, food, wall }; class Field { private: Object **m_field; public: Field(); ~Field(); const Point m_field_size; Object get(Point p_point) const { return m_field[p_point.y][p_point.x]; } void set(Point p_point, Object p_object) { m_field[p_point.y][p_point.x] = p_object; } void place_food(); void add_walls(); void update_player(Player *p_player); };

field.cpp

#include <random> #include "field.hpp" #include "player.hpp" #include "settings.hpp" Field::Field() : m_field_size(Settings::field_size) { m_field = new Object*[m_field_size.y]; for(int row = 0; row < m_field_size.y; ++row) { m_field[row] = new Object[m_field_size.x]; } for(int y = 0; y < m_field_size.y; ++y) { for(int x = 0; x < m_field_size.x; ++x) { m_field[y][x] = Object::empty; } } } Field::~Field() { for(int row = 0; row < m_field_size.y; ++row) delete [] m_field[row]; delete [] m_field; } void Field::place_food() { while(true) { static std::mt19937 rng; rng.seed(std::random_device()()); std::uniform_int_distribution<std::mt19937::result_type> disty(0, m_field_size.y - 1); std::uniform_int_distribution<std::mt19937::result_type> distx(0, m_field_size.x - 1); Point new_food = {(int) disty(rng), (int) distx(rng)}; if(m_field[new_food.y][new_food.x] == Object::empty) { m_field[new_food.y][new_food.x] = Object::food; break; } } } void Field::add_walls() { for(int y = 0; y < m_field_size.y; ++y) { m_field[y][0] = Object::wall; m_field[y][m_field_size.x - 1] = Object::wall; } for(int x = 0; x < m_field_size.x; ++x) { m_field[0][x] = Object::wall; m_field[m_field_size.y - 1][x] = Object::wall; } } void Field::update_player(Player *p_player) { for(int row = 0; row < m_field_size.y; ++row) { for(int col = 0; col < m_field_size.x; ++col) { if (m_field[row][col] == Object::player) { m_field[row][col] = Object::empty; } } } for(int i = 0; i < p_player->size(); ++i) { Point player_point = p_player->get(i); m_field[player_point.y][player_point.x] = Object::player; } }

player.hpp

#pragma once #include <vector> #include "point.hpp" enum class Facing { right, down, left, up, null }; class Player { private: std::vector<Point> m_position {{5, 5}}; unsigned int m_length = 1; Facing m_facing = Facing::right; public: void move(Point p_field_size); void lengthen() { ++m_length; }; Point get(unsigned int p_at = 0) { return m_position.at(p_at); } Facing get_facing() { return m_facing; } void set_facing(Facing p_facing); // returns the amount of Points the player occupies (costly!) int size() { return m_position.size(); } // returns the player's length. size() may have not been updated to it yet unsigned int length() { return m_length; } };

player.cpp

#include <stdexcept> #include "player.hpp" void Player::move(Point p_field_size) { switch (m_facing) { case Facing::right: { if(m_position[0].x + 1 == p_field_size.x) m_position.insert(m_position.begin(), { m_position.front().y, 0 }); else m_position.insert(m_position.begin(), { m_position.front().y, m_position.front().x + 1 }); break; } case Facing::down: { if(m_position[0].y + 1 == p_field_size.y) m_position.insert(m_position.begin(), { 0, m_position.front().x }); else m_position.insert(m_position.begin(), { m_position.front().y + 1, m_position.front().x }); break; } case Facing::left: { if(m_position[0].x - 1 == -1) m_position.insert(m_position.begin(), { m_position.front().y, p_field_size.x - 1 }); else m_position.insert(m_position.begin(), { m_position.front().y, m_position.front().x - 1 }); break; } case Facing::up: { if(m_position[0].y - 1 == -1) m_position.insert(m_position.begin(), { p_field_size.y - 1, m_position.front().x }); else m_position.insert(m_position.begin(), { m_position.front().y - 1, m_position.front().x }); break; } default: { throw std::invalid_argument("Player has wrong Facing"); } } if(m_position.size() > m_length) m_position.pop_back(); } void Player::set_facing(Facing p_facing) { switch (p_facing) { case Facing::right: if(m_facing != Facing::left) m_facing = p_facing; break; case Facing::left: if(m_facing != Facing::right) m_facing = p_facing; break; case Facing::down: if(m_facing != Facing::up) m_facing = p_facing; break; case Facing::up: if(m_facing != Facing::down) m_facing = p_facing; break; default: break; } }

game.hpp

#pragma once #include <exception> class Field; class GameUI; class Player; struct GameEndDeath : std::exception {}; struct GameEndQuit : std::exception {}; class Game { private: GameUI *m_ui; Field *m_field; Player *m_player; void tick(); void update(); public: Game(GameUI *p_ui); ~Game(); void start(); };

game.cpp

#include <chrono> #include <unistd.h> #include "game.hpp" #include "field.hpp" #include "player.hpp" #include "settings.hpp" #include "ui.hpp" void Game::tick() { const static std::chrono::milliseconds TICK_DURATION(145); auto last_tick = std::chrono::high_resolution_clock::now(); while(true) { m_player->set_facing(m_ui->get_input()); // true if the time of the next tick(last tick + tick duration) is in the past while((last_tick + TICK_DURATION) < std::chrono::high_resolution_clock::now()) { update(); last_tick += TICK_DURATION; } // sleep for 25 ms usleep(25 * 1000); } } void Game::update() { Point player_head = m_player->get(); switch(m_field->get(player_head)) { case Object::food: { m_field->set(player_head, Object::player); m_field->place_food(); m_player->lengthen(); break; } case Object::wall: case Object::player: { throw GameEndDeath(); break; } default: break; } m_field->update_player(m_player); m_player->move(m_field->m_field_size); m_ui->update(m_player->length() - 1); } Game::Game(GameUI *p_ui) : m_ui(p_ui) { m_field = new Field(); m_ui->set_field(m_field); m_player = new Player(); } Game::~Game() { delete m_field; delete m_player; } void Game::start() { if(Settings::enable_walls) m_field->add_walls(); m_field->place_food(); m_ui->draw_static_elements(); while(true) { try { tick(); } catch(const GameEndQuit &) { // TODO: redraw the field when "No" is clicked if(UIUtils::dialogbox(std::string("Quit?"), std::vector<std::string> {std::string("No"), std::string("Yes")}) == 1) return; m_ui->draw_border(); m_ui->draw_static_elements(); } catch(const GameEndDeath &) { UIUtils::dialogbox(std::string("You died"), std::vector<std::string> {std::string("OK")}); return; } } }

settings.hpp

#pragma once struct Point; class Settings { private: Settings() {}; public: static Point field_size; static bool enable_walls; };
\$\endgroup\$
2
  • 5
    \$\begingroup\$ Don't worry, we can handle big projects at Code Review. Welcome! \$\endgroup\$ Commented Mar 13, 2019 at 18:06
  • \$\begingroup\$ The more I look at the code, the more I notice ways I can improve it (e.g. use a namespace instead of a class with a private constructor). But too late now, I guess. The question is, why didn't I notice it earlier since I've been looking for ways to improve the code for some time now? Well... \$\endgroup\$ Commented Mar 13, 2019 at 20:34

1 Answer 1

Reset to default
6
\$\begingroup\$

This is a pretty nice effort for a C++ beginner. Well done! I see a number of things that may help you improve your code.

Don't reseed the random number generator more than once

In the Field::place_food() routine, the loop is written like this:

while(true) { static std::mt19937 rng; rng.seed(std::random_device()()); std::uniform_int_distribution<std::mt19937::result_type> disty(0, m_field_size.y - 1); std::uniform_int_distribution<std::mt19937::result_type> distx(0, m_field_size.x - 1); Point new_food = {(int) disty(rng), (int) distx(rng)}; if(m_field[new_food.y][new_food.x] == Object::empty) { m_field[new_food.y][new_food.x] = Object::food; break; } }

There are a few problems with this. First, it reseeds the rng each time which is neither necessary nor advisable. Second, it uses std::mt19937::result_type as the distribution type but then casts to an int. Fourth, it hides the loop exit function. Here's how I'd write it instead:

void Field::place_food() { static std::mt19937 rng(std::random_device{}()); std::uniform_int_distribution<int> disty(0, m_field_size.y - 1); std::uniform_int_distribution<int> distx(0, m_field_size.x - 1); Point location{disty(rng), distx(rng)}; while(get(location) != Object::empty) { location = Point{disty(rng), distx(rng)}; } set(location, Object::food); }

Note also that I've named the point location which seemed more appropriate to me, and used the get and set functions already defined. Which leads us to the next suggestion...

Pass references where appropriate

The Point parameter to the Field::get and Field::set functions should probably be a const Point& and Point&, respectively.

Don't specify type qualifiers on return types

The ui.cpp file contains this function:

const char* const bool_to_str(bool b) { return b ? "enabled" : "disabled"; }

The problem with it is that it's claiming to not allow the caller to modify the returned pointer. What's intended is for the caller not to be able to modify the strings to which they're pointing -- the other const is just ignored. So the way to write this would actually be:

static const char* bool_to_str(bool b) { return b ? "enabled" : "disabled"; }

Note also that I've made it static because it's not used anywhere else.

Prefer standard functions to platform-specific ones

The Game::tick() routine is much more complex than needed and uses usleep from <unistd.h> which is not standard C++. I'd use <thread> instead and write the function like this:

void Game::tick() { m_player->set_facing(m_ui->get_input()); update(); std::this_thread::sleep_for(std::chrono::milliseconds(145)); }

Eliminate raw new and delete where practical

The MainWindow::new_game() has these lines:

GameUI *game_ui = new GameUI(game_win, game_field_win); Game game(game_ui); game.start(); delwin(game_field_win); delwin(game_win); delete game_ui;

But is there really any reason to use new there? I'd suggest it would be better to write it like this:

GameUI game_ui{game_win, game_field_win}; Game game(&game_ui); game.start(); delwin(game_field_win); delwin(game_win);

Now there's no chance of forgetting to call the destructor for game_ui.

Initialize all members

In MainMenu::show_settings() the settings_menu_items vector initialization fails to initialize the pos member for the first item. However, rather than simply fixing that, have a look at the next suggestion instead.

Rethink class interfaces

There are a number of peculiarities in the class interface. For instance, as you've already noted in the comments, the use of a Settings singleton is probably not ideal. Instead, it would probably make sense to associate the Settings with a Game instance. The MainMenu class is also strange. First, it doesn't just have a main menu, but functions as a generic menu class. Second, the MenuItem class doesn't seem to do much. I'd expect that instead a Menu might be a collection of MenuItem and that its function would be solely to display the menu and get a valid choice back from the user. Instead, this MainMenu class also contains all of the processing for the user choices. I think it would make more sense and be much more reusable to separate responsibilities that way.
The other somewhat awkward interface is the relationship among the Game, GameUI, Player and Field objects. This might benefit from the Model-View-Controller design pattern. The model would comprise the Field and Player objects, the view would contain the display portion of the Game::update() function and the controller would comprise all of the portions of GameUI that manage player input. I think you'll find that it would result in a much cleaner interface that's easier to understand and maintain. One way that often helps when reasoning about this design pattern is to ask yourself if the component (model, view or controller) could be replaced with an alternative without affecting the other two components.

Don't define enum values you don't want

The player.hpp file has this enum class:

enum class Facing { right, down, left, up, null };

It seems that null is not particularly meaningful here. The only place it's used is in the case of non-input by the user. Again, this suggests that a single enum class omitting null would make more sense. Then the UI would sort out by itself whether to tell the Player object to change direction (or not) and the ambiguous null direction would no longer exist.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.