5
\$\begingroup\$

I'm a self-taught programmer and I'm in the process of developing a simple 2D Isometric game. There is next to no game content yet and I've been focusing on creating solid "foundation" first if you will, and I'm here to get some code reviews on the structure of the source code.

  • Is it clean
  • Am I using proper coding conventions?
  • What can I do better? (For this project)

Summary

  • initialize game(.h) from main.cpp
  • from game.h I instantiate Engine class with title, window size, fps variable and fullscreen boolean.
  • After engine is instantiated I instantiate the GUI class.
  • Once Engine and GUI class are instantiated, instantiate game loop that updates game events and handles game states.
  • The event system is inside the engine class, and I use forward declaration for engine class so that different game systems can access it, I heard it speeds up compilation time. Not sure if I did forward declaration correctly though, or if I got the right idea about this. Would especially like to have this reviewed*
  • GUI class uses graphics class, again using forward declaration and same as before, is this correct usage? Do I have the right idea about this concept?
  • Timer and Input class are the same as Engine and Graphics class, forward declaration, and same as before, this is what I would especially like reviewed.

Other than that, in general how does this "design"/"structure" look to you? Ugly? Pretty? etc.

The code is also on GitHub.

File Structure

**Header files:**
engine/fwd/
 -engine.h
 -graphics.h
 -input.h
 -timer.h
engine/
 -engine.h
 -graphics.h
 -input.h
 -timer.h
/
 -game.h
 -gui.h
 -spritesheet.h
 -stdafx.h

**Source files:**
engine/
 -engine.cpp
 -graphics.cpp
 -input.cpp
 -timer.cpp
/
 -main.cpp
 -game.cpp
 -gui.cpp
 -spritesheet.cpp

stdafx.h

#ifndef STDAFX_H
#define STDAFX_H

#include <iostream>
#include <string>
#include <array>
#include <vector>
#include <chrono>
#include <ctime> 
#include <iomanip>
#include <allegro5/allegro.h>
#include <allegro5/allegro_primitives.h>
#include <allegro5/allegro_image.h>
#include <allegro5/allegro_audio.h>
#include <allegro5/allegro_acodec.h>
#include <allegro5/allegro_font.h>
#include <allegro5/allegro_ttf.h>

#include "engine/fwd/engine.h"
#include "engine/fwd/graphics.h"
#include "engine/fwd/input.h"
#include "engine/fwd/timer.h"
#include "engine/fwd/map.h"

#include "engine/engine.h"
#include "engine/graphics.h"
#include "engine/input.h"
#include "engine/timer.h"
#include "engine/map.h"

#include "game.h"
#include "gui.h"
#include "spritesheet.h"

#endif

Main.cpp

#include "game.h"

int main() {
    game_initialize();

    game_loop();

    game_cleanup();

    return 0;
}

Game.h

#ifndef GAME_H
#define GAME_H

#include "stdafx.h"

typedef enum {
    GAME_STATE_MENU,
    GAME_STATE_PLAY,
} GAME_STATE;

bool game_initialize();
void game_loop();
void game_cleanup();
void handle_game_state();
void do_game_events();

extern Engine* engine;

#endif

Game.cpp

#include "game.h"

Engine* engine;
GAME_STATE game_state;

GUI* gui;

Map* map;

std::string keyboard_string = "";

bool game_initialize() {
    engine = new Engine("Engine", 800, 600, 60.0f, false);

    game_state = GAME_STATE_MENU;

    //bitmap.load_resource("Data/Images/Login/Background.png");
    //bitmap.load_resource("Data/Images/Login/UI.png");
    //bitmap.load_resource("Data/Images/Login/LoginBox.png");

    gui = new GUI();
    map = new Map();

    return true;
}

void game_loop() {
    while (engine->engine_running()) {
        do_game_events();

        if (engine->has_ticked()) {
            engine->clear(0, 0, 0);

            handle_game_state();

            engine->flip();
        }
    }
}

void game_cleanup() {
    delete(engine);
}

void handle_game_state() {
    switch (game_state) {
    case GAME_STATE_MENU:
        gui->draw(GAME_STATE_MENU);
        gui->handle_input(GAME_STATE_MENU, *engine->input);
        gui->update(GAME_STATE_MENU);
        break;
    case GAME_STATE_PLAY:
        gui->draw(1);
        break;
    }
}

void do_game_events() {
    switch (game_state) {
    case GAME_STATE_MENU:
        engine->update_event_system(GAME_STATE_MENU, gui->textbox_active, gui->textbox2_active);
        break;
    }
}

gui.h

#ifndef GUI_H
#define GUI_H

#include "stdafx.h"

enum class MAIN_ELEMENTS {
    BACKGROUND,
    GRADIENT_LEFT,
    GRADIENT_RIGHT,
    BUTTONS,
    LOGINBOX,
    TEXTBOX,
    TEXTBAR,
    MAX,
};

class GUI
{
public:
    GUI();
    ~GUI();

    void draw(int state);
    void handle_input(int state, Input &input);
    void update(int state);

    int xoffset;
    int yoffset;

    std::time_t ct = std::time(0);
    char* cc = ctime(&ct);

    bool textbox_hover, textbox_active, textbox2_hover, textbox2_active;

    std::string keyboard_string;

    bool textboxflag;

protected:
    int gfx[(unsigned int)MAIN_ELEMENTS::MAX];
};

#endif

gui.cpp

#include "gui.h"

GUI::GUI() {
    /*this->gfx[1] = engine->gfx->load_from_file("Data/Images/Login/Background.png");
    this->gfx[2] = engine->gfx->load_from_file("Data/Images/Login/Overlay.png");
    this->gfx[3] = engine->gfx->load_from_file("Data/Images/Login/LoginBox.png");*/

    this->xoffset = 0;
    this->yoffset = 0;

    this->gfx[(unsigned int)MAIN_ELEMENTS::BACKGROUND] = engine->gfx->load_from_file("gfx/gui/title/scrolling_background_tile.png");
    this->gfx[(unsigned int)MAIN_ELEMENTS::GRADIENT_LEFT] = engine->gfx->load_from_file("gfx/gui/title/gradient_left.png");
    this->gfx[(unsigned int)MAIN_ELEMENTS::GRADIENT_RIGHT] = engine->gfx->load_from_file("gfx/gui/title/gradient_right.png");
    this->gfx[(unsigned int)MAIN_ELEMENTS::BUTTONS] = engine->gfx->load_from_file("gfx/gui/title/title_buttons.png");
    this->gfx[(unsigned int)MAIN_ELEMENTS::LOGINBOX] = engine->gfx->load_from_file("gfx/gui/window.png");
    this->gfx[(unsigned int)MAIN_ELEMENTS::TEXTBOX] = engine->gfx->load_from_file("gfx/gui/title/textfield.png");
    this->gfx[(unsigned int)MAIN_ELEMENTS::TEXTBAR] = engine->gfx->load_from_file("gfx/gui/title/textbar_bottom.png");

    engine->usefultimer->Reset(al_get_time());
}

GUI::~GUI() {
    delete cc;
}

void GUI::draw(int state)
{
    if (state == 0) { //Menu
        //Background
        for (int i = 0; i <= al_get_display_width(engine->get_display()) / 48; i++) {
            for (int y = 0; y <= al_get_display_height(engine->get_display()) / 48; y++) {
                engine->gfx->blit(this->gfx[(unsigned int)MAIN_ELEMENTS::BACKGROUND], (i * 48) + xoffset, (y * 48) + yoffset, 0);
                engine->gfx->blit(this->gfx[(unsigned int)MAIN_ELEMENTS::BACKGROUND], -816 + (i * 48) + xoffset, (y * 48) + yoffset, 0);
                engine->gfx->blit(this->gfx[(unsigned int)MAIN_ELEMENTS::BACKGROUND], (i * 48) + xoffset, -624 + (y * 48) + yoffset, 0);
                engine->gfx->blit(this->gfx[(unsigned int)MAIN_ELEMENTS::BACKGROUND], -816 + (i * 48) + xoffset, -624 + (y * 48) + yoffset, 0);
            }
        }

        //Side gradients
        for (int i = 0; i < al_get_display_height(engine->get_display()); i++) {
            engine->gfx->blit(this->gfx[(unsigned int)MAIN_ELEMENTS::GRADIENT_LEFT], 0, i, 0);
            engine->gfx->blit(this->gfx[(unsigned int)MAIN_ELEMENTS::GRADIENT_RIGHT], al_get_display_width(engine->get_display()) - engine->gfx->get_width(this->gfx[(unsigned int)MAIN_ELEMENTS::GRADIENT_LEFT]), i, 0);
        }

        //Login box
        engine->gfx->blit_region(this->gfx[(unsigned int)MAIN_ELEMENTS::LOGINBOX], 0, 7, al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 - 20, al_get_display_height(engine->get_display()) / 2 + 110, 6, 15, 0);
        for (int i = 0; i < 215; i++) {
            engine->gfx->blit_region(this->gfx[(unsigned int)MAIN_ELEMENTS::LOGINBOX], 7, 7, 6+i+al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 - 20, al_get_display_height(engine->get_display()) / 2 + 110, 1, 15, 0);
        }

        //Username label
        al_draw_text(engine->gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, al_get_display_height(engine->get_display()) / 2 + 130, 0, "Username: ");
        //Password label
        al_draw_text(engine->gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), -70 + al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, al_get_display_height(engine->get_display()) / 2 + 180, 0, "Password: ");


        //Textbox not hovered
        if (!this->textbox_hover) {
            engine->gfx->blit_region(this->gfx[(unsigned int)MAIN_ELEMENTS::TEXTBOX], 0, 0, al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, al_get_display_height(engine->get_display()) / 2 + 150, engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX), engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, 0);
        } else { //Assume textbox is hovered
            engine->gfx->blit_region(this->gfx[(unsigned int)MAIN_ELEMENTS::TEXTBOX], 0, engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, al_get_display_height(engine->get_display()) / 2 + 150, engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX), engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, 0);
        }

        //Textbox2 not hovered
        if (!this->textbox2_hover)
            engine->gfx->blit_region(this->gfx[(unsigned int)MAIN_ELEMENTS::TEXTBOX], 0, 0, al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, al_get_display_height(engine->get_display()) / 2 + 180, engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX), engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, 0);
        //Textbox2 hovered
        else
            engine->gfx->blit_region(this->gfx[(unsigned int)MAIN_ELEMENTS::TEXTBOX], 0, engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, al_get_display_height(engine->get_display()) / 2 + 180, engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX), engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2, 0);

        //Textbox active
        if (!this->textbox_active) {
            al_draw_textf(engine->gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 + 4, al_get_display_height(engine->get_display()) / 2 + 146, 0, "%s", engine->input->username_keyboard_string.c_str());
        }
        else {
            al_draw_textf(engine->gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 + 4, al_get_display_height(engine->get_display()) / 2 + 146, 0, "%s", engine->input->username_keyboard_string.c_str());
            if (textboxflag) {
                al_draw_textf(engine->gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 + 4, al_get_display_height(engine->get_display()) / 2 + 146, 0, "%s|", engine->input->username_keyboard_string.c_str());
            }
        }

        //Hide password *******
        std::string temp_pass = "";
        for (unsigned int i = 0; i < engine->input->password_keyboard_string.size(); i++) {
            temp_pass.append("*");
        }
        //Textbox active
        if (!this->textbox2_active) {
            al_draw_textf(engine->gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 + 4, al_get_display_height(engine->get_display()) / 2 + 176, 0, "%s", temp_pass.c_str());
        }
        else {
            al_draw_textf(engine->gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 + 4, al_get_display_height(engine->get_display()) / 2 + 176, 0, "%s", temp_pass.c_str());
            if (textboxflag) {
                al_draw_textf(engine->gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 + 4, al_get_display_height(engine->get_display()) / 2 + 176, 0, "%s|", temp_pass.c_str());
            }
        }

        //Bottom Textbar
        for (int i = 0; i < al_get_display_width(engine->get_display()) / engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBAR); i++) {
            engine->gfx->blit(this->gfx[(unsigned int)MAIN_ELEMENTS::TEXTBAR], i * engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBAR), al_get_display_height(engine->get_display()) - engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBAR), 0);
            engine->gfx->blit(this->gfx[(unsigned int)MAIN_ELEMENTS::TEXTBAR], i * engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBAR), al_get_display_height(engine->get_display()) - engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBAR) * 2 + 1, 0);
        }
        //Bottom text for textbar
        al_draw_textf(engine->gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), 0, al_get_display_height(engine->get_display()) - 24, 0, "version %s %s", engine->get_version().c_str(), engine->get_date().c_str());
    }
    else if (state == 1) {
        //Game GUI
    }
}

void GUI::handle_input(int state, Input &input) {
    if (state == 0) {

        if (input.mouseX >= al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 && input.mouseX <= al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 + engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX)
            && input.mouseY >= al_get_display_height(engine->get_display()) / 2 + 150 && input.mouseY <= al_get_display_height(engine->get_display()) / 2 + 150 + engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2) {
            textbox_hover = true;
            if (engine->input->mouseB) {
                engine->input->mouseB = false;
                textbox_active = true;
                textbox2_active = false;
            }
        }
        else {
            textbox_hover = false;
        }

        if (input.mouseX >= al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 && input.mouseX <= al_get_display_width(engine->get_display()) / 2 - engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2 + engine->gfx->get_width((unsigned int)MAIN_ELEMENTS::TEXTBOX)
            && input.mouseY >= al_get_display_height(engine->get_display()) / 2 + 180 && input.mouseY <= al_get_display_height(engine->get_display()) / 2 + 180 + engine->gfx->get_height((unsigned int)MAIN_ELEMENTS::TEXTBOX) / 2) {
            textbox2_hover = true;
            if (engine->input->mouseB) {
                engine->input->mouseB = false;
                textbox2_active = true;
                textbox_active = false;
            }
        }
        else {
            textbox2_hover = false;
        }
    }
    else if (state == 1) {

    }
}

void GUI::update(int state) {

    if (state == 0) {
        this->xoffset += 1;
        this->yoffset += 1;

        if (this->xoffset > 480) this->xoffset = 48;
        if (this->yoffset > 480) this->yoffset = 48;

        engine->usefultimer->Update(al_get_time());
        if (engine->usefultimer->hasPassed(1)) {
            textboxflag = !textboxflag;
            engine->usefultimer->Reset(al_get_time());
        }
    }
    else if (state == 1) {

    }
}

spritesheet.h

#ifndef SPRITESHEET_H
#define SPRITESHEET_H

#include "stdafx.h"

enum class Player_Spritesheet {
    STANDING,
    WALKING,
    ATTACKING,
    RESTING,
    MAX
};

class Spritesheet
{
public:
    Spritesheet();
    ~Spritesheet();

    void draw(int state);

protected:
    int gfx[(unsigned int)Player_Spritesheet::MAX];
};

#endif

spritesheet.cpp

#include "spritesheet.h"

Spritesheet::Spritesheet()
{
    this->gfx[(unsigned int)Player_Spritesheet::STANDING] = engine->gfx->load_from_file("gfx/character/101.png");

    for (int i = 0; i < (unsigned int)Player_Spritesheet::MAX; i++) {
        this->gfx[i] = 0;
    }
}

Spritesheet::~Spritesheet()
{

}

void Spritesheet::draw(int state)
{
    if (state == 0) {
        //Menu GUI
        engine->gfx->blit(this->gfx[0], 0, 0, 0);
    }
    else if (state == 1) {
        //Game GUI
    }
}

fwd/engine.h

#ifndef FWD_ENGINE
#define FWD_ENGINE

class Engine;

#endif

engine/engine.h

#ifndef ENGINE_H
#define ENGINE_H

#include "../stdafx.h"

class Engine
{
public:
    Engine(std::string title, int width, int height, const float fps, bool fullscreen);
    ~Engine();

    bool engine_running() { return this->is_running; }
    void stop() { this->is_running = false; }
    void update_event_system(int state, int textbox_active, int textbox_active2);
    ALLEGRO_EVENT get_event() { return this->event; };
    ALLEGRO_DISPLAY* get_display() { return this->display; };
    void clear(int r, int g, int b);
    void flip();
    bool has_ticked();
    void capture_screen(std::string filename);
    void error(std::string message);
    void error(std::string message, std::string filepath);
    std::string get_version() { return this->version; }
    std::string get_date() {
        now = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());

        char buf[100] = { 0 };
        std::strftime(buf, sizeof(buf), "%Y-%m-%d %H:%M:%S", std::localtime(&now));
        return buf;
    }

    Graphics* gfx;
    Input* input;
    Timer* usefultimer;

private:
    ALLEGRO_DISPLAY* display;
    ALLEGRO_TIMER* timer;
    ALLEGRO_EVENT_QUEUE* queue;
    ALLEGRO_EVENT event;

    bool is_running;
    bool ticked;
    std::string version;

    std::time_t now{ 0 };
};

#endif

engine/Engine.cpp

#include "engine.h"

Engine::Engine(std::string title, int width, int height, const float fps, bool fullscreen) : is_running(false), ticked(false), version("0.01") {
    this->display = nullptr;

    if (!al_init())
        this->error("Failed to initialize program!");

    this->queue = al_create_event_queue();
    if (!this->queue)
        this->error("Failed to initialize queue system!");

    this->gfx = new Graphics();
    this->input = new Input(this->queue);
    this->usefultimer = new Timer();

    int flags = ALLEGRO_RESIZABLE;

    if (fullscreen)
        flags = ALLEGRO_FULLSCREEN;

    al_set_new_display_flags(flags);

    this->display = al_create_display(width, height);
    if (!this->display)
        this->error("Failed to create display!");

    flags = al_get_display_flags(this->display);

    al_set_window_title(this->display, title.c_str());

    this->timer = al_create_timer(ALLEGRO_BPS_TO_SECS(fps));
    if (!this->timer)
        this->error("Failed to initialize timer system!");

    al_register_event_source(this->queue, al_get_display_event_source(this->display));
    al_register_event_source(this->queue, al_get_timer_event_source(this->timer));

    al_start_timer(this->timer);

    this->is_running = true;
}


Engine::~Engine() {
    if (this->display) al_destroy_display(this->display);
    if (this->timer) al_destroy_timer(this->timer);
    if (this->queue) al_destroy_event_queue(this->queue);

    delete(this->gfx);
    delete(this->input);
}

void Engine::update_event_system(int state, int textbox_active, int textbox_active2) {
    al_wait_for_event(this->queue, &event);

    switch (this->event.type) {
    case ALLEGRO_EVENT_DISPLAY_CLOSE:
        this->stop();
        break;
    case ALLEGRO_EVENT_TIMER:
        this->ticked = true;
        break;
    case ALLEGRO_EVENT_KEY_DOWN:
        this->input->set_key_state(this->event.keyboard.keycode, true);
        break;
    case ALLEGRO_EVENT_KEY_UP: {
        this->input->set_key_state(this->event.keyboard.keycode, false);
        if (engine->get_event().keyboard.keycode == ALLEGRO_KEY_PRINTSCREEN) {
            FILE* fh;
            char buf[30];
            int num = 0;

            while (true) {
                num++;
                sprintf(buf, "screen/screen_%i.png", num);

                fh = fopen(buf, "r");

                if (!fh)
                    break;

                fclose(fh);
            }

            printf("print Screen! id=%i\n", num);
            engine->capture_screen(buf);
        }
        break;
    }
    case ALLEGRO_EVENT_KEY_CHAR:
    {
        const int inputChar = engine->get_event().keyboard.unichar;
        if (engine->get_event().keyboard.keycode == ALLEGRO_KEY_BACKSPACE) {

            if (state == GAME_STATE_MENU && textbox_active == 1) {
                if (this->input->username_keyboard_string.length() > 0) {
                    input->username_keyboard_string = this->input->username_keyboard_string.substr(0, this->input->username_keyboard_string.length() - 1);
                }
            }
            if (state == GAME_STATE_MENU && textbox_active2 == 1) {
                if (this->input->password_keyboard_string.length() > 0) {
                    input->password_keyboard_string = this->input->password_keyboard_string.substr(0, this->input->password_keyboard_string.length() - 1);
                }
            }
        }
        else if
            (
            (inputChar >= 48 && inputChar <= 57) //is a number
                || (inputChar >= 65 && inputChar <= 90) //is a capital letter
                || (inputChar >= 97 && inputChar <= 122) //is a lower-case letter
                || (inputChar == 95) //is an underscore
                ) {
            if (state == GAME_STATE_MENU && textbox_active == 1)
                this->input->username_keyboard_string += inputChar;

            if (state == GAME_STATE_MENU && textbox_active2 == 1)
                this->input->password_keyboard_string += inputChar;

            std::cout << this->input->keyboard_string << std::endl;

        }
    }
    break;
    case ALLEGRO_EVENT_MOUSE_AXES:
        this->input->mouseX = event.mouse.x;
        this->input->mouseY = event.mouse.y;
        break;
    case ALLEGRO_EVENT_MOUSE_BUTTON_DOWN:
        if (event.mouse.button & 1)
            this->input->set_mouse_state(this->event.mouse.button, true);
        break;
    case ALLEGRO_EVENT_MOUSE_BUTTON_UP:
        if (event.mouse.button & 1)
            this->input->set_mouse_state(this->event.mouse.button, false);
        break;

    }

}

void Engine::clear(int r, int g, int b) {
    al_set_target_backbuffer(this->display);
    al_clear_to_color(al_map_rgb(r, g, b));
}

void Engine::flip() {
    al_flip_display();
}

bool Engine::has_ticked() {
    bool temp = this->ticked;
    this->ticked = false;

    return temp;
}

void Engine::capture_screen(std::string filename) {
    al_draw_textf(gfx->get_font(FONT_TINYUNICODE), al_map_rgb(255, 255, 255), 0, 0, 0, "Game V%s", version.c_str());
    al_save_bitmap(filename.c_str(), al_get_backbuffer(this->display));
}

void Engine::error(std::string message) {
    printf(message.c_str());
    exit(-1);
}

void Engine::error(std::string message, std::string filepath) {
    printf(message.c_str(), filepath.c_str());
    exit(-1);
}

fwd/graphics.h

#ifndef FWD_GRAPHICS_H
#define FWD_GRAPHICS_H

class Graphics;

#endif

engine/graphics.h

#ifndef GRAPHICS_H
#define GRAPHICS_H

#include "../stdafx.h"

enum fonts {
    FONT_COMICI,
    FONT_TINYUNICODE,
    FONT_MAX
};

class Graphics
{
public:
    Graphics();
    ~Graphics();

    int load_from_file(std::string filename);
    void blit(int id, int x, int y, int flags, int alpha = 255);
    void blit_region(int id, int sx, int sy, int x, int y, int sw, int sh, int flags, int alpha = 255);
    int get_width(int id);
    int get_height(int id);
    ALLEGRO_BITMAP* get_bitmap(int id);
    int count();
    ALLEGRO_FONT* get_font(int id);

protected:
    std::vector<ALLEGRO_BITMAP*> bitmap;
    ALLEGRO_FONT* comici;
    ALLEGRO_FONT* tinyunicode;
    bool is_valid_id(unsigned int id);
};

#endif

engine/graphics.cpp

#include "graphics.h"

Graphics::Graphics()
{
    if (!al_init_image_addon())
        engine->error("Failed to initialize image addon!");
    if (!al_init_font_addon())
        engine->error("Failed to initialize font addon!");
    if (!al_init_ttf_addon())
        engine->error("Failed to initialize ttf addon!");

    this->comici = al_load_ttf_font("data/font/comici.ttf", 12, 0);
    if (!this->comici)
        engine->error("Failed to initialize font: \"data/font/comici.ttf\"");
    this->tinyunicode = al_load_ttf_font("data/font/TinyUnicode.ttf", 24, 0);
    if (!this->tinyunicode)
        engine->error("Failed to initialize font: \"data/font/TinyUnicode.ttf\"");
}

Graphics::~Graphics()
{
    for (unsigned int i = 0; i<this->bitmap.size(); i++)
    {
        if (this->bitmap[i])
            al_destroy_bitmap(this->bitmap[i]);
    }

    if (this->comici) al_destroy_font(this->comici);
    if (this->tinyunicode) al_destroy_font(this->tinyunicode);

    al_shutdown_image_addon();
    al_shutdown_font_addon();
    al_shutdown_ttf_addon();
}

int Graphics::load_from_file(std::string filename)
{
    ALLEGRO_BITMAP* temp_bmp = al_load_bitmap(filename.c_str());
    if (!temp_bmp)
        engine->error("Error loading bitmap: gfx/updatethis/%s", filename.c_str());

    if (temp_bmp)
    {
        this->bitmap.push_back(temp_bmp);
        return this->bitmap.size() - 1;
    }

    return -1;
}

void Graphics::blit(int id, int x, int y, int flags, int alpha)
{
    if (this->is_valid_id(id))
    {
        if (alpha != 255)
        {
            al_set_blender(ALLEGRO_ADD, ALLEGRO_ALPHA, ALLEGRO_INVERSE_ALPHA);
            al_draw_tinted_bitmap(this->bitmap[id], al_map_rgba(255, 255, 255, alpha), x, y, flags);
        }
        else
            al_draw_bitmap(this->bitmap[id], x, y, flags);
    }
}

void Graphics::blit_region(int id, int sx, int sy, int x, int y, int sw, int sh, int flags, int alpha)
{
    if (this->is_valid_id(id))
    {
        if (alpha != 255)
        {
            al_set_blender(ALLEGRO_ADD, ALLEGRO_ALPHA, ALLEGRO_INVERSE_ALPHA);
            al_draw_tinted_bitmap_region(this->bitmap[id], al_map_rgba(255, 255, 255, alpha), sx, sy, sw, sh, x, y, flags);
        }
        else
            al_draw_bitmap_region(this->bitmap[id], sx, sy, sw, sh, x, y, flags);
    }
}

int Graphics::get_width(int id)
{
    if (this->is_valid_id(id))
    {
        return al_get_bitmap_width(this->bitmap[id]);
    }

    return 0;
}

int Graphics::get_height(int id)
{
    if (this->is_valid_id(id))
    {
        return al_get_bitmap_height(this->bitmap[id]);
    }

    return 0;
}

ALLEGRO_BITMAP* Graphics::get_bitmap(int id)
{
    if (this->is_valid_id(id))
    {
        return this->bitmap[id];
    }

    return NULL;
}

bool Graphics::is_valid_id(unsigned int id)
{
    if (id < 0 || id >= this->bitmap.size())
        return false;

    if (!this->bitmap[id])
        return false;

    return true;
}

int Graphics::count()
{
    return this->bitmap.size();
}

ALLEGRO_FONT * Graphics::get_font(int id)
{
    switch (id) {
    case FONT_COMICI:
        return this->comici;
        break;
    case FONT_TINYUNICODE:
        return this->tinyunicode;
    default:
        return nullptr;
        break;
    }
}

fwd/input.h

#ifndef FWD_INPUT_H
#define FWD_INPUT_H

class Input;

#endif

engine/input.h

#ifndef INPUT_H
#define INPUT_H

#include "../stdafx.h"

class Input
{
public:
    Input(ALLEGRO_EVENT_QUEUE* queue);
    ~Input();

    //void processInput(int state, GUI* gui);

    int mouseX, mouseY, mouseB;
    std::string username_keyboard_string = "";
    std::string password_keyboard_string = "";
    std::string keyboard_string = "";

private:
    bool keys[ALLEGRO_KEY_MAX];
    bool mouse[2];
    int prevMouseState;
    int curMouseState;

    void set_key_state(int keycode, bool is_down) { this->keys[keycode] = is_down; }
    void set_mouse_state(int button, bool is_down) { this->mouseB = is_down; }

    friend class Engine;
};

#endif

engine/input.cpp

#include "input.h"

Input::Input(ALLEGRO_EVENT_QUEUE* queue) {
    if (!al_install_keyboard()) {
        engine->error("Failed to install keyboard!");
    }

    if (!al_install_mouse()) {
        engine->error("Failed to install mouse!");
    }

    al_register_event_source(queue, al_get_keyboard_event_source());
    al_register_event_source(queue, al_get_mouse_event_source());

    for (int i = 0; i < ALLEGRO_KEY_MAX; i++) {
        this->keys[i] = false;
    }
    for (int i = 0; i < 2; i++) {
        this->mouse[i] = false;
    }

    this->mouseX = 0;
    this->mouseY = 0;
    this->mouseB = 0;
}

Input::~Input() {
    al_uninstall_keyboard();
    al_uninstall_mouse();
}

//void Input::processInput(int state, GUI* gui) {
//  switch (engine->get_event().type) {
//  case ALLEGRO_EVENT_KEY_CHAR:
//  {
//      const int inputChar = engine->get_event().keyboard.unichar;
//      if (engine->get_event().keyboard.keycode == ALLEGRO_KEY_BACKSPACE) {
//          if (gui->keyboard_string.length() > 0) {
//              gui->keyboard_string = gui->keyboard_string.substr(0, gui->keyboard_string.length() - 1);
//          }
//      }
//      else if
//          (
//          (inputChar >= 48 && inputChar <= 57) //is a number
//              || (inputChar >= 65 && inputChar <= 90) //is a capital letter
//              || (inputChar >= 97 && inputChar <= 122) //is a lower-case letter
//              || (inputChar == 95) //is an underscore
//              ) {
//          gui->keyboard_string += inputChar;
//          std::cout << gui->keyboard_string << std::endl;
//          std::cout << inputChar << std::endl;
//      }
//  }
//  break;
//  case ALLEGRO_EVENT_MOUSE_BUTTON_DOWN:
//      if (engine->get_event().mouse.button & 1)
//          std::cout << "Hello";
//      break;
//  case ALLEGRO_EVENT_KEY_DOWN: {
//      if (engine->get_event().keyboard.keycode == ALLEGRO_KEY_RIGHT) {
//
//      }
//  }
//  case ALLEGRO_EVENT_KEY_UP: {
//      if (engine->get_event().keyboard.keycode == ALLEGRO_KEY_PRINTSCREEN) {
//          FILE* fh;
//          char buf[30];
//          int num = 0;
//
//          while (true) {
//              num++;
//              sprintf(buf, "screen/screen_%i.png", num);
//
//              fh = fopen(buf, "r");
//
//              if (!fh)
//                  break;
//
//              fclose(fh);
//          }
//
//          printf("print Screen! id=%i\n", num);
//          engine->capture_screen(buf);
//      }
//      break;
//  }
//  }
//}

fwd/timer.h

#ifndef FWD_TIMER_H
#define FWD_TIMER_H

class Timer;

#endif

engine/timer.h

#ifndef TIMER_H
#define TIMER_H

#include "../stdafx.h"

/* SunOS 4.1.* does not define CLOCKS_PER_SEC, so include <sys/param.h> */
/* to get the HZ macro which is the equivalent.                         */
#if defined(__sun__) && !defined(SVR4) && !defined(__SVR4)
#include <sys/param.h>
#define CLOCKS_PER_SEC HZ
#endif


class Timer
{
public:
    /* Default c'tor */
    Timer();
    ~Timer();

    time_t GetTime();
    void Update(const time_t& time);
    bool hasPassed(double seconds) {
        m_elapsedTime = al_get_time() - m_startTime;
        if (m_elapsedTime >= seconds)
            return true;
        else
            return false;
        return false;
    }
    bool Paused();
    void Pause();

    void Reset(const time_t& time);

private:
    bool    m_paused;    // Whether system timer is currently paused
    double m_startTime;  // Previous global time, compares against current global time
    double m_elapsedTime;      // Local amount of elapsed time (pause-aware)

};


#endif  //  Timer_h

engine/timer.cpp

#include "engine/timer.h"


/* Default c'tor */
Timer::Timer() :
    m_paused(false),
    m_startTime(0),
    m_elapsedTime(0)
{
    // Default constructor
}

Timer::~Timer() {

}

time_t Timer::GetTime() {
    return m_elapsedTime;
}

void Timer::Update(const time_t& time) {
    double delta = m_elapsedTime - m_startTime;
    if (!m_paused)
        m_elapsedTime += delta;
}

bool Timer::Paused() {
    return m_paused;
}

void Timer::Pause() {
    m_paused = !m_paused;
}

void Timer::Reset(const time_t& time) {
    m_elapsedTime = 0;
    m_startTime = time;
}
\$\endgroup\$
0

1 Answer 1

5
\$\begingroup\$

There's a lot here that I believe you can improve. Here are some suggestions that may help you to do that.

Isolate platform-specific code

If you must have stdafx.h, consider wrapping it so that the code is portable:

#ifdef WINDOWS
#include "stdafx.h"
#endif

In this case, it is being seriously abused, so I'd recommend omitting it entirely as in the next few suggestions.

Avoid relative paths in #includes

Generally it's better to omit relative path names from #include files and instead point the compiler to the appropriate location. So don't write this:

#include "engine/timer.h"

Write this:

#include "timer.h"

For gcc, you'd use -I. This makes the code less dependent on the actual directory structure, and leaves such details in a single location: a Makefile or compiler configuration file.

Separate interface from implementation

In C++, this is usually done by putting the interface into separate .h files and the corresponding implementation into .cpp files. It helps users (or reviewers) of the code see and understand the interface and hides implementation details. So instead of having this in Engine.h:

#include "../stdafx.h"

I would highly recommend something more like this:

#include "graphics.h"
#include "input.h"
#include "timer.h"
#include <string>
#include <chrono>
#include <ctime>

This way, it's much easier to identify the real dependencies for the interface. For implementation files such as Engine.cpp, one can hide the details of the particular implementation. For example, I'd recommend moving the implementation of get_date() into the .cpp file and moving the <chrono> and <ctime> includes there as well. Those files are not essential details that a user of the class needs to know.

Don't hardcode file names

The font files, bitmaps, etc. might be something that a user of this program has in different locations than you do on your machine. Because all of these settings are both hardcoded and embedded in the .cpp files, it would be quite tedious to fix all of those paths so that your game works on anyone else's computer. Use a configuration file instead, or at the very least, isolate the file names within the .h files so that the dependencies are obvious and easy to find and update.

Don't write this->

Within member functions this->data is redundant. It add visual clutter and does not usually aid in understanding. So for example, we have the existing Engine destructor:

Engine::~Engine() {
    if (this->display) al_destroy_display(this->display);
    if (this->timer) al_destroy_timer(this->timer);
    if (this->queue) al_destroy_event_queue(this->queue);

    delete(this->gfx);
    delete(this->input);
}

It's much less cluttered and easier to read if it's written like this instead:

Engine::~Engine() {
    if (display) {
        al_destroy_display(display);
    }
    if (timer) {
        al_destroy_timer(timer);
    }
    if (queue) {
        al_destroy_event_queue(queue);
    }
    delete(gfx);
    delete(input);
}

Note also that I've eliminated the single-line if statements.

Be wary of signed versus unsigned

The code currently contains this code:

bool Graphics::is_valid_id(unsigned int id)
{
    if (id < 0 || id >= this->bitmap.size())
            return false;

    if (!this->bitmap[id])
            return false;

    return true;
}

However, that first condition id < 0 can never be true because it's an unsigned number. I would simplify this considerably and write it like this:

bool Graphics::is_valid_id(unsigned int id) const
{
    return id < bitmap.size() && bitmap[id];
}

Because of short-circuit evaluation, the second clause will only be executed if the first one is true.

Avoid global variables

There are some very dubious design decisions embedded within the current code. One is the use of a global Engine object. Another is the fact that three important data members of Engine are public. Yet another is the fact those members are all three raw pointers. All of these make code that is brittle and hard to understand and maintain.

Rethink your class design

It seems that Game should be a class instead of a collection of global variables and some free functions. It also appears that the GUI class is inextricably bound to the Engine class (and to a global instance) and the Graphics class, but it's unclear which responsibilities belong to which class or why there are three different kinds of objects. Another peculiar decision is that the Input class appears to only have a pointer to a queue which is actually a member of the Engine class instead of being a member of the Input class.

Avoid magic numbers

One of the lines of code here is this:

engine->gfx->blit(this->gfx[(unsigned int)MAIN_ELEMENTS::BACKGROUND], -816 + (i * 48) + xoffset, (y * 48) + yoffset, 0);

All of those numbers, such as -816 and 48 no doubt mean something, but what? I'm sure I could figure it out with enough study, but it would be better to use well-named constants to avoid the need to guess. It also makes the code easier to maintain because if, for example, you wanted to change all instances of 48 to some other value, you wouldn't have to figure out, for each instance, whether this particular 48 is refers to the particular thing you're trying to change or not.

Use const where possible

The engine_running, get_event, get_display and get_version functions do not (and should not) alter the underlying Engine object and should therefore be declared const.

Use const references where practical

The code currently defines one of its error functions like so:

void Engine::error(std::string message, std::string filepath) {
    printf(message.c_str(), filepath.c_str());
    exit(-1);
}

Both message and filepath could be passed by const std::string & instead.

Eliminate unused variables

In the following code:

void set_mouse_state(int button, bool is_down) { this->mouseB = is_down; }

the button parameter is never used and could be eliminated.

\$\endgroup\$
2
  • \$\begingroup\$ stdafx.h is just a precompiled header - it's not Windows specific. (Though it probably isn't necessary / shouldn't have that much in it). \$\endgroup\$
    – user673679
    Feb 17, 2019 at 22:05
  • 1
    \$\begingroup\$ Yes, I know what it is, but it seems only to be used by programmers using Windows compilers. I'd suggest that it is attempting to solve a problem that probably doesn't exist (slow compile times) and that you're better off with a clean class design and forgetting about fwd and stdafx.h until you actually encounter a problem. \$\endgroup\$
    – Edward
    Feb 17, 2019 at 22:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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