Basic game engine setup question

[icemuppet]

I'm putting together a small game engine setup. I have the following code and I have some beginner questions about it.

1. For a start, is this a stable setup. Are there objects I would need to instantiate differently or destroy and etc. Question is mostly about the RenderWindow.

2. You will notice, that I used the .create method to create the window. I ran into a problem trying to use the constructor there.
I'm a little inexperienced with this, so my question is - Does it have anything to do with RenderWindow being NonCopyable?
The error messages I got were extreme at the least.

window = sf::RenderWindow(sf::VideoMode(m_iWidth, m_iHeight), m_czTitle, sf::Style::Titlebar | sf::Style::Close);

This is what I tried originally and I'm just making sure I understand how I fixed the problem. Now it works of course.

3. Other than that, any suggestions or improvements are welcome.

#include "engine.h"

int main()
{
        CEngine myGame;
       
        myGame.Init();
        myGame.Start();
               
    return 0;
}
 

#ifndef ENGINE_H
#define ENGINE_H

#include <SFML/Graphics.hpp>

class CEngine
{
private:
        sf::RenderWindow window;

        int m_iWidth;   // Screen width
        int m_iHeight;  // Screen height
       
        const char* m_czTitle;
       
protected:
        void HandleInput();
        void DoRender();
       
public:
        CEngine();
        ~CEngine();
       
        void Init();
        void Start();
};

#endif
 

#include "engine.h"

CEngine::CEngine()
{
        m_iWidth = 1280;
        m_iHeight = 720;

        m_czTitle = "My Game";
}

CEngine::~CEngine()
{

}

void CEngine::Init()
{
        window.create(sf::VideoMode(m_iWidth, m_iHeight), m_czTitle, sf::Style::Titlebar | sf::Style::Close);
}

void CEngine::Start()
{
        while(window.isOpen())
        {
                HandleInput();
               
                DoRender();
        }
}

void CEngine::HandleInput()
{
        sf::Event event;
       
        while(window.pollEvent(event))
        {
                if(event.type == sf::Event::Closed)
                {
                        window.close();
                }
        }
}

void CEngine::DoRender()
{
        window.clear(sf::Color::Blue);
       
        window.display();
}

 

[Ixrec]

1. What you have there looks fine to me.  Though you don't have very much yet so there aren't many big mistakes you could have made with this, other than putting the render window at global scope or something.

2. It sounds like you're confused about how initialization works in C++.  If you haven't already, find and read a good C++ book that covers this kind of stuff.

If your "window = ..." attempt was in the class declaration, then the C++ language simply didn't allow initializing class members in the declaration, at least until recently.  Details here: https://stackoverflow.com/questions/15451840/why-cant-we-initialize-class-members-at-their-declaration

If it was in the constructor, which would explain why you saw NonCopyable, then that's because "window" was already constructed and "window = sf::RenderWindow(...)" was copy-assigning to it.  Which you can't do because copying RenderWindows is not allowed (inheriting from a NonCopyable class is a common idiom for implementing this prohibition).  What you probably wanted to do was construct "window" itself, rather than assign to it after it had already been constructed.  This can and should be done in the initializer list of your class' constructor.  However, you appear to be using two-phase initialization for your CEngine object, in which case it should not be a surprise that you have to use two-phase initialization (in this case, create()) for its RenderWindow member.

3. I am obligated to mention that you should "build games, not engines," just in case.

The small amount of code you have so far has no obvious style problems.  But:
- Start() doesn't sound like a method that would loop indefinitely, so perhaps a name like StartGameLoop() might be better.
- CEngine::Init() seems pointless to me.  The window.create() call could just as easily go in CEngine's constructor, or in CEngine::Start().
- If you don't have any "real" code to put in a destructor, it's best to not write one at all.  That means slightly less code to understand, and slightly more freedom for your compiler.

[icemuppet]

Great, thank you for the detailed explanation. I'm looking into initialization asap.

You mentioned two-phase initialization for the CEngine object? I'm not sure how to identify that.

[Ixrec]

Basically, an object is said to have "two-phase initialization" if it has a constructor *and* an Init() or Setup() or whatever method that completes the initialization work.  Generally speaking, this means the object is unusable or invalid until this Init/Setup/whatever method is called.

Most of the time, two-phase initialization is frowned upon in C++, because it's one extra function call the user has to write and can potentially forget to do, and in most cases there's no clear benefit.  Init() methods are very common in C because that language has no constructors and destructors, but in C++ it's usually better to just let the constructor (and nothing else) handle the construction.  However, sf::RenderWindow happens to be an exception to that rule.

For sf::RenderWindow, actually creating the visible window on the user's screen counts as part of initialization (since all the other functions like pollEvent() and draw() are invalid until then).  But the class allows either one-phase initialization (use the non-default constructor) or two-phase initialization (use create() sometime after construction).  The main reason is that it's common and reasonable to write a class with a RenderWindow member that wants to do things in its constructor before actually creating the visible window, eg loading the user's config data and checking if it should open in fullscreen mode or a 1080p window or something else.  Doing this would be a bit awkward if sf::RenderWindow always created a visible window during construction.

CEngine has a constructor and an Init() method, which is why I claimed it had two-phase initialization.  At the moment this separation serves no real purpose, but if you later end up doing something like reading from a config file to decide the video mode, it may become a good thing.