Generic 2D engine for simulation

Question

I'm using SFML to manage my windows and I'm writing this engine to visualize some PSO work. I'm trying to make it generic enough that I can extend it for other simulations later on and keep it as minimal as possible. (I'm going to copy this design pattern to a freeglut initialized openGL 3D engine w/CUDA later on as well, so going for performance here.)

It would be a great help to be given some technique and design tips before it before I continue and get too deep to make it an easy rework. I suppose the relevant knowledge in this area is mostly gameloop design and class structuring, but this is mostly skeleton code anyways. Please be pedantic, but not mean!

I'm going to implement exception catching everywhere and I want to make sure I'm compartmentalizing as much as possible, but not too much! Maybe there are ways to store my variables or implement handlers differently. Basically, let me know of general good coding practices that you think I may be over or underdoing.

main.cpp

#include "App.h"

int main()
{
    class GE gol;
    gol.run();

    return 0;
}

App.h

#include <SFML/Graphics.hpp>
#include <iostream>
#include <string>

class App {
public:
    App(sf::VideoMode mode, std::string title,  _Uint32t style)
        : Window(mode, title, style)
    {
        quit_s = false, vs_s = true;

        logic_ticks_sec = 60; // make ctor to handle these, default ctor will always init like this
        frame_time = sf::milliseconds(1000 / logic_ticks_sec);
        max_frameskip = 5;

        Window.setVerticalSyncEnabled(vs_s);
    }

    ~App()
    {

    }

    int run();

protected:
    sf::RenderWindow Window;
    sf::Event Event;

    //State Variables

    //Timing 
    sf::Clock clock;

    int logic_ticks_sec, max_frameskip;
    sf::Time frame_time, elapsed;

    // Resources

    //Functions
        //Event Handlers
    inline void toggleVsync() { vs_s = !vs_s;  Window.setVerticalSyncEnabled(vs_s); };
    inline void quit() { quit_s = true; };
    virtual void handleMouse();

        //Resource Management
    virtual int initResources();
    virtual int cleanResources();

        //Game Execution
    virtual int gameLoop(); //not usually overridden, maybe not virtual and private?
    virtual int eventLoop(); //handle events
    virtual int renderLoop(); //virtual


private:
    //State Variables
    bool quit_s, vs_s;
    sf::Vector2i mouse_pos;
};

class GE : public App // GOL example engine derivation, move to own files when App project is finished
{
public:
    GE() 
        : App(sf::VideoMode::getDesktopMode(), std::string("Game of Life"), sf::Style::None) 
    {};
    ~GE()
    {};

private:
};

App.cpp

#include "App.h"

//App
int App::run()
{
    initResources();
    gameLoop();
    cleanResources();

    Window.close();
    return 0;
}

    //Event Handlers
void App::handleMouse()
{
    mouse_pos = sf::Mouse::getPosition(Window);
}

    //Resource Management
int App::initResources()
{

    return 0;
}

int App::cleanResources()
{

    return 0;
}
    //Main Loops
int App::gameLoop()
{
    while (!quit_s)
    {
        clock.restart();

        eventLoop();
        renderLoop();

        elapsed = clock.getElapsedTime();
    }

    return 0;
}

int App::eventLoop()
{
    while (Window.pollEvent(Event))
    {
        switch (Event.type)
        {
        case sf::Event::Closed:
            quit();
            break;
        case sf::Event::MouseMoved:
            handleMouse();
            break;
        case sf::Event::KeyPressed:
            if (sf::Keyboard::isKeyPressed(sf::Keyboard::Escape))
            {
                quit();
            } else
            if (sf::Keyboard::isKeyPressed(sf::Keyboard::F5))
            {
                toggleVsync();
            } 
            break;
        case sf::Event::MouseButtonPressed:
            if (sf::Mouse::isButtonPressed(sf::Mouse::Button::Left))
            {
            }
            else
            if (sf::Mouse::isButtonPressed(sf::Mouse::Button::Right))
            {
            }
            break;
        default:
            break;
        }
    }

    //Physics
    //AI

    return 0;
}

int App::renderLoop()
{
    Window.clear();

    Window.display();
    return 0;
}

Answer 1

Just as it was nicely put by @David in his answer, you will want to split responsibilities into separate classes, if your intention is to build more on top of this. A good thing to keep in mind when designing is the Single Responsibility Principle. Design each class to do just one thing, but do this one thing very well.

Code Review:

• I didn't see an Include Guard in your header file. If you don't have one, you'll definitely have to add it.

• App is meant to be inherited from, so it should have a virtual destructor. If you are targeting C++11, you can declare a default virtual destructor, since you currently are not performing any manual cleanup:

  virtual ~App() = default;
  

• _Uint32t, where is that type coming from? It is not standard and the _ prefix tells me that it is probably an internal detail of one of your libraries or the compiler. Use the standard sized types from <cstdint> instead.

• inline is implicit when you declare and define a class method directly inside the class body. It should be removed to avoid redundancy.

• Use booleans instead of return codes. Your methods return 0 on success, which can be quite error prone. A bool with true on success and false on failure is a lot clearer.

• I didn't get the point behind the _s suffix on the booleans quit_s and vs_s. Also, vs_s is impossible to guess what it means. Please give it a better name. (Okay, after looking around you figure out that it stands for V-Sync, so just name it as such: vsyncEnabled or similar).

• Explicitly constructing a string from a char *, such as in here:

  : App(sf::VideoMode::getDesktopMode(), std::string("Game of Life"), sf::Style::None) 
  

  Is not necessary, since std::string implicitly constructs from a char array. Doing this is potentially worse since it might create an extra copy of the string that gets thrown away on the next line. Some compilers will not optimize this tiny mistake. These little things are a good reminder that we should stop trying to help the compiler ;)

Answer 2

The first thing I am worried about is that the App class (or subclass) will get responsible for all the logic for event handling, rendering, resources, physics, etc. That are a lot of responsibilities for one class. It would be more maintainable to have several classes for this, for instance:

• App is responsible for instantiating and couple all other classes and perhaps the basic gameloop
• ResourceManager is responsible for getting and freeing resources
• EventHandler or Controler is responsible for events or user input
• PhysicsEngine will handle the physics

This way each class has it's own responsibility, but it would be slightly more difficult to interact with each other.

Another thing I noticed was this:

class GE gol;

The class keyword is not needed in this case.