Hello all!

I've been learning C++ for the past few months and lately I started messing around with SFML. I tried making some simple games but I always get to a point where I have 3 or 4 classes which always end up being a mess, and also I get header files compilation errors.

Question # 1 is, when dealing with OOP game programming, is there a general structure I should be aiming at? What I do is make a "game" class which creates and initializes the window, and it has a method with the game loop in it. in that method I create all instances of other classes, such as "eventHandler Event", and I pass a reference of the Window to a method in Event.. Same thing when rendering, I have a Graphics class which receives a reference of the Window(created in Game) and renders from there. Am I doing this right? I mean, it feels "right" to do but it gets messy very fast to the point that I'm struggling to understand my own code. Are there any recommended general guidelines for this?

Question # 2 - I seem to get header files errors quite a lot.. Say I have:

-Game class

-Graphics class

-Event handling class

-Sprite class

-Player class(inherited from sprite)

What I do is include all headers in Game.h, and include Game.h in all headers - does this means that if Graphics class needs to use something from Sprite class, Sprite.h won't be seen by Graphics.h? I get a lot of errors with this. I once was able to solve it with a forward declaration, "Class Sprite;" in Graphics.h but then other times this doesn't work. How do I avoid such errors?

Thanks so much!

I don't have any specific advice but try to reduce the amount of things each class does. Having a class that starts things up, initialises the window and runs the main loop is a good idea, it can then dispatch events to where ever they are needed. Below that you can have a Game class that is the actual game. The idea is to reduce the amount of work each class does, the Game class isn't also responsible for window management and so on.

The less each class does the less issues you will get with your headers too. You can use forward declaration in your headers such that you don't have to include things that are not needed (As you said you have tried). E.g. the Game has a Player object you can do this in the header:

class Player;

class Game
{
   Player* m_Player;
};

That way you don't need to have 'include "Player.h" at the top of Game.h and thus anything that includes Game.h no longer also includes Player.h. That can help a lot but it also means you have to create things with new. You'll have to be a bit mroe specific when you say your forward declaration didn't work, this won't work for example:

class Player

class Game
{
   Player m_Player; // note this is an object of type Player rather than a pointer
};

So smaller objects and forward declaration. If you think part of one of your objects could be neatly wrapped up in a small group then it is a good candidate to be a separate object.

-Sprite class
-Player class(inherited from sprite)

In addition to Nanoha's suggestions, something here seems wrong.

Use inheritance if the Player IS A Sprite. This seems unlikely to be the case.

You might want to look into composition, which is used when the Player HAS A Sprite.

Hi.

You're question is too big (for me) to cover with just 1 right answer.

So here's my share:

- when you have headers that you want to include in several places, 'ifdef' them

(so they're just compiled once)

#ifndef CD3DCAM_H
#define CD3DCAM_H

(top of your headers)

- in general a class should have 1 purpose only (like mentioned above)

- when you want to make a 'game engine', think of modules, for example:

-- audio

-- IO

-- rendering

-- scenes/ levels

-- objects

-- meshes ( --- submeshes etc.)

You could use a namespace for each 'module'

- when you have headers that you want to include in several places, 'ifdef' them
(so they're just compiled once)
#ifndef CD3DCAM_H
#define CD3DCAM_H
(top of your headers)

Just to clarify that a bit, At the top you could/should have:

#ifndef CD3DCAM_H // the name is chosen based on your header name

#define CD3DCAM_H

Then you would have all the code and finally at the bottom you would have

#endif

As Cozzie points out that will stop the headers being included more than once. You might have:

#pragma once

at the top of your headers instead which also solves the same issue. I do believe using both is ok but you should use at least one of the options.

Thanks so much everyone! I am indeed using #pragma once by the way. I noticed that when I'm creating a method which takes a reference of an object, I need a forward declaration for that object.

But why use a forward declaration and create a pointer for an object?

like here:

class Player;

class Game
{
Player* m_Player;
};

rather than:

#include "Player.h"

class Game
{
Player m_Player;
};

is this done just to avoid header file problems?

Imagine a game with elevators in it. It would usually have 2 classes

Person

Elevator

The Person class has a reference to the current elevator he is in. This is defined as:

class person
{
 private:
  elevator m_elevator;
};

However, this is saying that the person owns the elevator. Why should he own an elevator all to himself? That's why we do

class person
{
 private:
  elevator *m_elevator;
};

Now, you may be thinking, "why can't the elevator just have an array of all persons inside?"

There are a few good reasons. First off, creating objects is expensive. It is better to create them once, store them somewhere, and then reference them with pointers. Secondly, it makes the "person" class bigger because of unnecessary functions that only the "elevator" class will need; it is better to have the elevator class return needed variables (in some circumstances). Thirdly, does it make sense that the elevator owns a person?

And including unnecessary headers will also cause the file the header is included in to be recompiled each time you edit the included file. Example:

#include "player.hpp"

/* This file has to be recompiled each time player.hpp is edited. This can cause a long build time if this file is big enough */
class elevator
{
};

Thanks so much everyone! I am indeed using #pragma once by the way. I noticed that when I'm creating a method which takes a reference of an object, I need a forward declaration for that object.

But why use a forward declaration and create a pointer for an object?

like here:

class Player;

class Game
{
Player* m_Player;
};

rather than:

#include "Player.h"

class Game
{
Player m_Player;
};

is this done just to avoid header file problems?

Yes, that method is used to reduce the amount of dependencies on headers as TheCandadianVendingMachine describes it helps with recompile times and you can use it when you need an array of unknown size. It does have drawbacks, such as having to create the objects using new and remembering to delete them, having to implement non-trivial copy/assign constructors/operator.

Another method that might help you (and is good practice in general) is to use namespaces.

Game.h

#pragma once

namespace my_game
{
   class Player;

   class Game
   {
   public:
      Game();
      ~Game();
      // You would also need copy/assign constructors/operators since m_Player 
      // is created with new or just mark them deleted:
      Game(const Game&) = delete;
      Game& operator=(const Game&) = delete;
   private:
      Player* m_Player;
   };
}

Game.cpp

#include "Game.h"
#include "Player.h"

namespace my_game
{
   Game::Game()
   {
      m_Player = new Player;
   }

   Game::~Game()
   {
      delete m_Player;
   }
}

It doesn't reduce how headers are dragged in but it can help with some naming conflicts.