Header file list so ugly

When loading external map files the tilemap class loads each object from a string in a ObjectFactory map I set up. Even though they all inherit from 1 class it looks really ugly to see so many header files just so I can create these objects. http://puu.sh/bLxYS/1f143e728d.png

Am I just stuck with this or is there a nicer way to handle it? Forward declarations wont work as I am creating the objects.
I really hate C++ for its header file system (yes, yes I know I should switch or stop complaining) but just asking.
Last edited on
There are tricks you can do.

You can "register" each class type with the factory by passing it a callback which will generate the object. The factory can keep a container of all the callbacks and use them to create the objects, rather than creating the objects itself. That way the factory doesn't have to include any extra headers.




Of course... I also want to question whether inheritance+factory is the right approach here. From the looks of it... it seems like it might not be. But uprooting/rewriting your entire system probably isn't worth it at this stage.
Last edited on
You could always make giant preprocessor files that handle all of the #includes, then just #include that.
I have already considered storing them all in 1 place then just including that file, but that's still "ugly". They are all included only in one place at the moment anyway (Tilemap class).

Disch could you show me a small example of what you mean with callback functions? I can imagine certain situations but sooner or later you would have to include all the files in order to store everything in a vector etc.
Also if you have any better ways I can handle this kind of thing I'm open for any ideas.
Consider you're using a factory to generate objects, you can register the object creators into the factory.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25

class Object;

class Factory
{
public:
    
    // singleton
    static Factory& sharedFactory();
    
    typedef Object* (*ObjectMaker)();

    // register the type with an object maker
    void registerObject(const std::string &typeName, ObjectMaker maker);

    // create an instance of the object from type name
    Object* createObject(const std::string &typeName);
    
protected:
    
    typedef std::map<std::string, ObjectMaker> MakerMap;
    MakerMap _makers;
    
};


So that each of your Object class should have:

 
static Object* createObject()


which returns an instance of that type.

And you need a macro and a helper class to help register the object types automatically.

1
2
3
4
5
6
7
8
9
10
11
12

class AutoRegisterObject
{
public:
    AutoRegisterObject(const std::string &typeName, Factory::ObjectMaker maker)
    {
        Factory::sharedFactory().registerObject(typeName, maker);
    }
};

#define AUTO_REGISTER_OBJECT(type) AutoRegisterObject __auto_register_##type(#type, type::createObject)


And now, when ever you define a new object class, you add the macro after the class declaration:

 
AUTO_REGISTER_OBJECT(MyGameObject);


Done. You can create the object from it's name now.

Last edited on
but sooner or later you would have to include all the files in order to store everything in a vector etc


You actually wouldn't... because you only store the callback in the vector.. and the callback doesn't need to know about the class.

It's actually more work to do it this way... so if you're looking for less typing then this isn't the way to go. The main advantage to doing it this way is that it's easier to expand. You don't have to keep updating the factory every time you add a new type -- each type can add itself to the factory automatically.


Assuming you have a 'Parent' base class... and one or more 'Child' derived classes. The factory will generate the appropriate Child object depending on the ID it is given.

Right now you are probably doing something like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
#include "spike.h"
#include "block.h"
// etc

Parent* factory(int id)
{
    switch(id)
    {
        case id_spike:  return new Spike;
        case id_block:  return new Block;
        // etc
    }
}



While this is direct... it requires adding a new #include and a new 'case' each time you create a new child class.


Instead... you can have the factory maintain a list of callbacks, where each callback constructs a different object. Since a callback is just a function, the factory doesn't need to actually generate the object itself (so it doesn't need to include the header), and instead it can just call the callback.

I'm going to use globals for the example since that's quick-n-dirty and I don't feel like coming up with an elaborate design for this. Also note this is pseudo-code and won't work as-is. Some of this will have to be placed in headers and stuff.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
// factory.cpp

#include <map>
#include <functional>

class Parent;

typedef std::function<Parent*()> callback;  // our callback typedef

typedef std::map<int,callback> registry;    // our 'registry'
                // IE, each child class will add themselves to this and provide
                //   and provide a callback to generate their object
                
// this bit is important... to ensure 'theReg' is always constructed, make it static
//  in a function -- do not make it global.  If it is global, it is possible for other global
//  objects to be created first ... which means some Child classes may add themselves
//  to a map that has not been constructed yet!
registry& getFactoryRegistry()
{
    static registry theReg;
    return theReg;
}

// child classes will call this to register themselves
void addFactoryCallback(int id, callback clbk)
{
    getFactoryRegistry()[id] = clbk;
}

// then, to actually generate the objects, just call the callback:
Parent* factory(int id)
{
    callback c = getFactoryRegistry()[id];  // get the callback from the registry
    return c();  // call the callback
}



Then... the child would be responsible for adding itself to the registry. This can be "automatic" by using a global object which registers itself in its constructor. To avoid copy/pasting this for every Child class, you can template it:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
// in parent.h?

template <int ID, typename ChildType>
class AutoRegisterer  // not the best name
{
public:
    // the callback we will give to the factory:
    Parent* generate()
    {
        return new ChildType;
    }
    
    // the constructor to automatically register this type
    AutoRegisterer()
    {
        addFactoryCallback( ID, generate );
    }
};


Once that is templated.... each child class needs only to add a single line to their cpp file:

1
2
// globally in spike.cpp
static AutoRegisterer< id_spike, Spike >  burpfart;

1
2
// globally in block.cpp
static AutoRegisterer< id_block, Block >  burpfart;


The name of the object doesn't matter because you'll never use it. I just picked "burpfart" because I'm tired and I thought it was funny.

By making this global object, it calls that object's ctor... which calls addFactoryCallback... which gives the factory a way to generate our child class.





So yeah. More complicated.



Also if you have any better ways I can handle this kind of thing I'm open for any ideas.


Well I can't really hit the point exactly without seeing what the difference is between all these classes... but usually minor differences in tiles can be handled with simple attributes rather than polymorphism.

IE, you shouldn't need to create a separate 'Spike' and 'Ladder' class. Maybe just have a 'Tile' class that has spike/ladder attributes. Then you don't need a factory... and then you don't need to include a bunch of headers, because you'll only have 1.

But again -- hard to say for sure without seeing the classes in question.


EDIT: ninja'd by liuyang. We had the same idea.
Last edited on
Funny, I have a feeling I knew about this way to do it a long time ago and it somehow slipped my mind, so thank you for those ideas. Even though it is more typing I think it is a much nicer system than #include Item1000001.h.

I suppose this could all be done in 1 master Tile class but wouldn't it get bloated pretty fast? I can imagine ending up with something like http://puu.sh/bLIg5/819f7338b8.png. However I have gotten pretty stuck in relying on inheritance/polymorphism almost completely so I'm sure there is a better way to do it than what I posted.

You asked for seeing a class in question (well I'm going to pretend you asked :P), I do something similar to this for all of my custom objects on the tilemap.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
#include "Spike.h"
#include "Collision.h"
#include "Player.h"

//Inherited from MapObject, always gets a player object to work on 
void Spike::Run(Player &p)
{ 
        //Runs anything else needed from here

	if (CheckCollision(getRect(), p.getRect()))
	{
		p.Die();
	}
}

//Inherited from MapObject
void Spike::Show()
{
	Render(x, y, Image, &Clip);
}


EDIT: Guess I just have to look into more composition type designs, will think about resigning a lot of this soon.
Last edited on
Sorry for double post but I have a question regarding component based systems.

Right now I have a Door class and a Key class. In the door class it checks for any collisions with a key (by looping over the mapobjects and checking each name string) and does something if true.

With an Entity/Component based engine there are no longer "Key" and "Door" classes from what I understand, they are just game objects with specific components added. But then how do I handle custom collisions like this? I could have a separate component for every collision between certain objects but that doesn't exactly seem like a good idea.

I know you can implement messaging systems and all of that to communicate between objects but I still don't quite see how to handle very specific events with this.
Last edited on
closed account (10X9216C)
Just have an enum that specifies the type of entity it is. The point of the component system is to be able to separate logic to specific components, just use the enum for grouping.
I understand you need some sort of ID to say what each entity is. I always seem to end up with strings/maps over enums. But I see myself ending up with a lot of code looking like this.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
void CollisionSystem()
{
	vector<GameObject*> Doors = GetObjects("Door");
	vector<GameObject*> Keys = GetObjects("Key");

	for(auto i : Doors)
	{
		for(auto j : Keys)
		{
			if(Collides(i,j)
			{
				//Do something 
			}
		}
	}
}


Is this ok?
Last edited on
Bump.

Even after reading many articles on Composition/Entity design I am still missing some key point. I understand each system works on GameObjects with certain components, but I see no way for each object to have custom events after that as they are all 1 class.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
class CollisionSystem : System 
{
	vector<GameObject*> objects = GetObjects(Collider);

	void update()
	{
		for(auto i : objects)
		{
			for(auto j : objects)
			{
				// Where to put all these checks?  
                                // They can't be in the Gameobject class as everything is sharing it.  

				if(i.Name() == "Player" && j.Name() == "Powerup")
				{
					//Powerup Info
				}

				if(i.Name() == "Enemy" && j.Name() == "Bomb")
				{
					//Kill Info
				}

				if(i.Name() == "Player" && j .Name()== "Door")
				{
					//Load next map
				}
			}
		}
	}
};


I suppose you could use 1000 systems for every collision that could happen, but I don't even see that solving it.
Topic archived. No new replies allowed.