9
\$\begingroup\$

This is my attempt at a basic implementation of GOL. The user can add and remove cells with the mouse and some basic stats are printed to console. Any feedback is welcome, but I have a couple of specific questions:

  • In Cell::get_neighbours() it seems a bit inefficient to keep constructing vectors for each cell. In Python I would use yield to avoid this; is there anything comparable in C++?
  • In InputHandler::get_input I just call three 'subfunctions' which aren't reused anywhere else. Is this good practice or is one long function better?

A screenshot of the program running

Below is the code in rough order of importance; alternatively here is a github link.

main.cpp is the main loop for the program; including a framerate cap:

main.cpp

#include <cstdio>

#include <SDL2/SDL.h>

#include "graphics.h"
#include "input.h"
#include "logic.h"

const int TARGET_FRAMERATE = 60;
const int TARGET_TICKRATE = 2;
const int FRAMES_PER_TICK = TARGET_FRAMERATE / TARGET_TICKRATE;
const int TARGET_TIMESTEP = 1000 / TARGET_FRAMERATE;

int main() {
    Graphics graphics;
    InputHandler input_handler;
    Gamestate gamestate;

    unsigned int timestep, time0, time1 = 0;
    unsigned int frame_count = 0;
    while (!input_handler.quit) {
        // Get time
        time0 = time1;
        time1 = SDL_GetTicks();
        timestep = time1 - time0;
        frame_count++;
        // Main game loop
        if (frame_count % FRAMES_PER_TICK == 0) {
            gamestate.update();
        }
        input_handler.get_input(gamestate);
        graphics.render(gamestate, input_handler);
        // FPS cap
        if (timestep < TARGET_TIMESTEP) {
            SDL_Delay(TARGET_TIMESTEP - timestep);
            time1 += TARGET_TIMESTEP - timestep;
        }
    }
    printf("Exiting game\n");
}

The rules and implementation of the game are in logic.cpp and its header. I tried to keep this independent of SDL and the UI portions of the code:

logic.h

#ifndef _logic_h_included_
#define _logic_h_included_

#include <unordered_set>
#include <vector>

struct Cell {
    int x, y;

    bool operator==(const Cell&) const;
    std::vector<Cell> get_neighbours() const;
};

struct Hash {
    std::size_t operator()(const Cell&) const;
};

class Gamestate {
public:
    Gamestate();

    std::unordered_set<Cell, Hash> board;    // The set of live cells
    bool paused;
    int generation;

    void update();
};

#endif

logic.cpp

#include "logic.h"

#include <cstdio>
#include <unordered_map>

Gamestate::Gamestate() :
    paused(true),
    generation(0) {}

void Gamestate::update() {
    if (paused) {
        return;
    }
    std::unordered_map<Cell, int, Hash> num_neighbours;
    // Count neighbours of each cell
    for (Cell c : board) {
        for (Cell n : c.get_neighbours()) {
            if (!num_neighbours.count(n)) {
                num_neighbours.emplace(n, 0);
            }
            ++num_neighbours[n];    // Note each cell counts itself as a neighbour
        }
    }
    // Apply the iteration rule
    for (auto pair : num_neighbours) {
        Cell c;
        int n;
        std::tie(c, n) = pair;
        if (n == 3) {    // i.e. c is live and has 2 neighbours or is dead and has 3
            board.insert(c);
        } else if (board.count(c) and n != 4) {
            board.erase(c);
        }
    }
    ++generation;
    printf("Generation: %d, Total cells: %lu\n", generation, board.size());
}

bool Cell::operator==(const Cell& other) const {
    return (x == other.x) and (y == other.y);
}

std::vector<Cell> Cell::get_neighbours() const {
    std::vector<Cell> neighbours;
    for (int dx = -1; dx <= 1; ++dx) {
        for (int dy = -1; dy <= 1; ++dy) {
            neighbours.push_back({x + dx, y + dy});
        }
    }
    return neighbours;
}

std::size_t Hash::operator()(const Cell& c) const {
    return c.x * 31 + c.y;
}

Everything to do with getting user input is in input.cpp and its header:

input.h

#ifndef _input_h_included_
#define _input_h_included_

#include <SDL2/SDL.h>

#include "logic.h"

const int WINDOW_SIZE_X = 600;
const int WINDOW_SIZE_Y = 800;

const double CAMERA_PAN_SPEED = 50.0;
const double CAMERA_ZOOM_SPEED = 1.05;

class InputHandler {
private:
    const unsigned char* keyboard_state;
    unsigned int mouse_state;
    int mouse_pos_x;
    int mouse_pos_y;

    void get_input_poll(Gamestate&);
    void get_input_camera();
    void get_input_mouse(Gamestate&);

public:
    InputHandler();
    InputHandler(const InputHandler&) = delete;

    bool quit;
    double camera_x;
    double camera_y;
    double zoom;
    Cell highlighted_cell;

    void get_input(Gamestate&);
};

#endif

input.cpp

#include "input.h"

#include <cmath>

InputHandler::InputHandler() :
    quit(false),
    camera_x(0),
    camera_y(0),
    zoom(60) {
    keyboard_state = SDL_GetKeyboardState(NULL);
    mouse_state = SDL_GetMouseState(&mouse_pos_x, &mouse_pos_y);
}

void InputHandler::get_input(Gamestate& gamestate) {
    get_input_poll(gamestate);
    get_input_camera();
    get_input_mouse(gamestate);
}

void InputHandler::get_input_poll(Gamestate& gamestate) {
    SDL_Event event;
    while(SDL_PollEvent(&event) != 0) {
        if (event.type == SDL_QUIT) {
            quit = true;
            return;
        }
        if (event.type == SDL_KEYDOWN and event.key.keysym.sym == SDLK_SPACE) {
            gamestate.paused = not gamestate.paused;
        }
    }
}

void InputHandler::get_input_camera() {
    if (keyboard_state[SDL_SCANCODE_W]) {
        camera_y -= CAMERA_PAN_SPEED;
    }
    if (keyboard_state[SDL_SCANCODE_A]) {
        camera_x -= CAMERA_PAN_SPEED;
    }
    if (keyboard_state[SDL_SCANCODE_S]) {
        camera_y += CAMERA_PAN_SPEED;
    }
    if (keyboard_state[SDL_SCANCODE_D]) {
        camera_x += CAMERA_PAN_SPEED;
    }
    if (keyboard_state[SDL_SCANCODE_Q]) {
        zoom *= CAMERA_ZOOM_SPEED;
    }
    if (keyboard_state[SDL_SCANCODE_E]) {
        zoom /= CAMERA_ZOOM_SPEED;
    }
}

void InputHandler::get_input_mouse(Gamestate& gamestate) {
    mouse_state = SDL_GetMouseState(&mouse_pos_x, &mouse_pos_y);
    highlighted_cell = {(int)floor((mouse_pos_x + camera_x) / zoom),
                        (int)floor((mouse_pos_y + camera_y) / zoom)};
    if (mouse_state & SDL_BUTTON(SDL_BUTTON_LEFT)) {
        gamestate.board.insert(highlighted_cell);
    }
    if (mouse_state & SDL_BUTTON(SDL_BUTTON_RIGHT)) {
        gamestate.board.erase(highlighted_cell);
    }
}

Finally graphics.cpp and its header render the two previous classes to the screen:

graphics.h

#ifndef _graphics_h_included_
#define _graphics_h_included_

#include <SDL2/SDL.h>

#include "input.h"
#include "logic.h"

class Graphics {
private:
    SDL_Window* window;
    SDL_Renderer* renderer;

public:
    Graphics();
    Graphics(const Graphics&) = delete;
    ~Graphics();

    void render(const Gamestate&, const InputHandler&) const;
};

#endif

graphics.cpp

#include "graphics.h"

#include <cmath>

Graphics::Graphics() {
    SDL_Init(SDL_INIT_VIDEO);
    window = SDL_CreateWindow("dungeon", 0, 0, WINDOW_SIZE_X, WINDOW_SIZE_Y, SDL_WINDOW_SHOWN);
    renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED);
    SDL_SetRenderDrawBlendMode(renderer, SDL_BLENDMODE_BLEND);
}

Graphics::~Graphics() {
    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);
    SDL_Quit();
}

void Graphics::render(const Gamestate& gamestate, const InputHandler& i) const {
    SDL_SetRenderDrawColor(renderer, 0, 0, 0, 255);
    SDL_RenderClear(renderer);
    SDL_SetRenderDrawColor(renderer, 255, 255, 255, 255);
    SDL_Rect destination;
    // Draw board
    for (Cell c : gamestate.board) {
        destination = {(int)(c.x * i.zoom - i.camera_x), (int)(c.y * i.zoom - i.camera_y),
                       (int)ceil(i.zoom), (int)ceil(i.zoom)};
        SDL_RenderFillRect(renderer, &destination);
    }
    // Highlight cell under mouse
    SDL_SetRenderDrawColor(renderer, 255, 255, 255, 128);
    Cell c = i.highlighted_cell;
    destination = {(int)(c.x * i.zoom - i.camera_x), (int)(c.y * i.zoom - i.camera_y),
                   (int)ceil(i.zoom), (int)ceil(i.zoom)};
    SDL_RenderFillRect(renderer, &destination);

    SDL_RenderPresent(renderer);
}
\$\endgroup\$
2
  • 3
    \$\begingroup\$ In C++ generators are way more complex than in Python: stackoverflow.com/questions/9059187/… \$\endgroup\$ – Caridorc Jun 14 '16 at 13:06
  • 1
    \$\begingroup\$ Not a codereview but you should document the avaible commands more. See my pull request for example documentation \$\endgroup\$ – Caridorc Jun 14 '16 at 13:27
6
\$\begingroup\$

Here are some things that may help you improve your code.

Avoid object creation

The SDL library does a lot of memory allocation and deallocation which tends to slow things down a bit. You can avoid that problem by using references in several of your "range for" loops. For example, in Gamestate::update() you could use this:

// Count neighbours of each cell
for (const Cell &c : board) {
    for (Cell &n : c.get_neighbours()) {

By using references, you avoid the Cell construction/deletion that would otherwise occur.

Reuse objects where practical

Instead of creating a new Cell inside InputHandler::get_input_mouse each time, why not just update the one that already exists? For convenience, you could create this Cell member function:

void setPos(int xval, int yval) { x=xval; y=yval; }

And alter the line within get_input_mouse to use it instead of doing yet another Cell creation/destruction cycle:

highlighted_cell.setPos((int)floor((mouse_pos_x + camera_x) / zoom),
                    (int)floor((mouse_pos_y + camera_y) / zoom));

Rethink get_neighbours

You're right to question whether the current method is efficient. It's not particularly, because it requires the creation and destruction of many objects (the std::vector and each of the Cells within it) each time it's called. This is also true of the num_neighbors map.

A small improvement might be to define a neighbours array which can be constructed once and reused:

static const std::array<Cell, 9> neighbours{{ 
    {-1, -1}, { 0, -1}, {+1, -1},
    {-1,  0}, { 0,  0}, {+1,  0},
    {-1, +1}, { 0, +1}, {+1, +1} 
}};

Then the cell counting would look like this:

for (Cell n : neighbours) {
    n += c;
    if (!num_neighbours.count(n)) {
        num_neighbours.emplace(n, 0);
    }
    ++num_neighbours[n];    // Note each cell counts itself as a neighbour
}

In this version, it still creates a new Cell for each iteration (note that this version of the loop is not using a reference) but at least the std::vector is not being created/destroyed each call.

The map could also be made static and then simply clear() it each iteration instead of incurring the construct/destroy overhead.

Provide a "zoom extents" function

In the case of a prolific colony, it may not be easy to see the whole thing on screen. One feature enhancement might be to calculate a bounding box for all cells in the board and zoom to that if some particular key is pressed.

Change the name of the window

I was somewhat surprised to see this window titled "dungeon"! I assume that was a cut-and-paste from some other program, but it probably should be changed.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ I also meant to mention that yes, your three input routines are good design in my view. Nice clean separation making it easy to understand where things are done. \$\endgroup\$ – Edward Jun 14 '16 at 15:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.