Sign in to follow this  
rpiller

Component design question

Recommended Posts

rpiller    839

Currently I store a string name along with each component inside a map in my GameObject. This is how I identify each component when I call GetComponent<ComponentType>(componentName).

 

I think I'm going to go the route of only allowing 1 component type per GameObject (make it so you can't add 2 of the same components). To me that sort of makes having to pass a name in the GetComponent function because I'm already defining the type via the template param. So I'm wondering how can I structure my C++ code to make my get call more like:

 

GetComponent<ComponentType>()

 

 

If this is how I want t get my components, how would the storage of these components look in GameObject? The type itself is basically identifying it. Would I still need to ID this type another way? Possibly have each component have a static function that returns a unique ID for that component and inside GetComponent I use Type::GetID() and I still store them in a map where the key is the ID? Or is there another way? I can't think of another way, but curious if I'm missing some ideas around this.

 

Thanks

Share this post


Link to post
Share on other sites
DrEvil    1148

There's no reason to use naming to get components. You have all you need just by the template parameter.

 

Rather than looping through the components looking for a name match, you can do a dynamic_cast instead, or some other sort of manual rtti, like a ComponentType::GetComponentType() which returns a unique identifier per component type to compare against and you could then static cast. 

Share this post


Link to post
Share on other sites
rpiller    839

So this is what I currently have. The thing I like about this is that GetComponent() doesn't need anything but the type (since it can call the static method based on that). The thing I don't like is that in AddComponent() I have to specify the type because that Type() function is static. I'd prefer AddComponent to not need a type.

 

I guess I could make some kind of virtual ToString() on Component, and in each of my components return that components static Type() inside the virtual ToString(). Seems a little hooky and requires extra steps to remember when making a component but it should work.

#include <map>
#include <string>
 
using namespace std;
 
class Component
{
};
 
class HUD : public Component
{
private:
   int health; // just for testing
public:
   HUD()
   {
      health = 100;
   }
   int GetHealth() { return health; }
   static string Type() { return "HUD"; }
};
 
class Object
{
private:
   map<string, Component*> components;
public:
   template<class T>
   void AddComponent(Component* comp)
   {
      if(components.find(T::Type()) == components.end())
         components[T::Type()] = comp;
   }
 
   template<class T>
   T* GetComponent()
   {
      if(components.find(T::Type()) == components.end())
         return NULL;
 
      return (T*)components[T::Type()];
   }
};
 
 
 
int main()
{
   Object o;
 
   o.AddComponent<HUD>(new HUD());
 
   HUD* h = o.GetComponent<HUD>();
 
   int health = h->GetHealth();
 
   return 0;
}
Edited by rpiller

Share this post


Link to post
Share on other sites
rpiller    839

That doesn't seem to work given I don't want to pass the type in the AddComponent(). So if I do typeid() around the component passed into AddComponent() it returns "class Component*", but since I do need the type in GetComponent() to both ID and cast doing typeid() on the type gives "HUD". So given the syntax I'm looking at this doesn't look like it would work. The benefit of making AddComponent() require the type template would be that the creators or the components don't have to worry about ID's. The downside is just visually looking at AddComponent() where I'm already passing in an instance of a component it would seem redundant to have to pass the type also. I guess that's the lesser of 2 evils though as the less rules component creators have to remember the better.

 

I guess I could make a version of AddComponent() that doesn't take any parameters and inside it'll just make a new object of 'T'. This would work for any components that don't need arguments in the ctor or I could fill the arguments out after the fact.

Edited by rpiller

Share this post


Link to post
Share on other sites
databurn    154

I agree with not liking the idea of passing the type when you have already passed an instance of the component into the object.

 

The way I have my AddComponent() method implemented is to use a template for the type, as you have, however having the method do the allocation of the component and adding it to its internal list for management and then returning a pointer to the object.

template<typename T>
T* AddComponent()
{
    T* newT = new T();
    // add to lists
    // do whatever else
    return newT;
}

This way the component is managed from the start and the caller is just a consumer.

Share this post


Link to post
Share on other sites
rpiller    839

My issue with that method is that I don't know if I like not being able to use the ctor with parameters though. Seems there won't be a "perfect" way to do it though and I'll just have to pick the one that I dislike the least.

 

I also think I'll look at returning references from GetComponent() instead of pointers too (because the Object class owns the component memory management), but I'm not sure how I would do that if I need to check if the component exists in the container and if not what to return.

Edited by rpiller

Share this post


Link to post
Share on other sites
wintertime    4108

These problems you have are the reason why I think its the wrong approach to put the components into the entity.

I would create some template storage class, where each instantiation holds all components of one type, and then retrieve it by giving the entity id to the appropriate one. Its automatically determined then which type it is and you avoid all RTTI/emulated RTTI. Meanwhile the storage could be a nice cache-friendly array internally, where it allows updating all components of a type with one iteration through it by the corresponding system, without repeatedly asking for each single component.

Share this post


Link to post
Share on other sites
rpiller    839

Do you mean something like what I have below? This does take away the problems I was having and should be faster because of less cache misses right? This is for sure a different way to look at it. I don't like explicitly making containers for each one, but there must be a way to mask that in another templated class or something. A way I can sort/group like component types so they get looped over together all in 1 container. I know I have "property" code that allows storage of any data type in one container so I'll see if I can't merge the ideas.

#include <map>
 
using namespace std;
 
class Component
{
private:
    int entityID;
public:
    Component() {}
    Component(int id)
    {
        entityID = id;
    }
 
    virtual void Update()
    {
    }
};
 
//--------------------------------
 
class Object
{
private:
    static int ID;
    int id;
public:
    Object()
    {
        id = ID;
        ID++;
    }
 
    int GetID() { return id; }
};
 
int Object::ID = 1;
 
//--------------------------------
 
class AI : public Component
{
public:
    AI() {}
    AI(int id) : Component(id) {}
    virtual void Update()
    {
    }
};
 
//--------------------------------
 
template<class T>
class Storage
{
private:
    map<int, T> data;
public:
    void Add(Object& obj)
    {
        int id = obj.GetID();
 
        // make sure this object can only have 1 of this type
        if(data.find(id) == data.end())
        { 
            data[id] = T(obj.GetID());
        }
    }
 
    T& Get(Object& obj)
    {
        return data[obj.GetID()];   // this is dangerous because it could cause an exception, but I don't want to return a pointer because the caller shouldn't be able to mistakenly delete it. a reference explicitly tells them they don't have to worry about the memory
    }
 
    void Update()
    {
        map<int, T>::iterator iter;
 
        for(iter = data.begin(); iter != data.end(); ++iter)
            (*iter).second.Update();
    }
};
 
//--------------------------------
 
int main()
{
    Storage<AI> ai;
    //etc. for all component types
 
    Object o1;
    Object o2;
 
    // make a new AI component and assign it to o1 object
    ai.Add(o1);
 
    // make another AI component and assign it to o2 object
    ai.Add(o2);
 
    AI a1 = ai.Get(o1);
    AI a2 = ai.Get(o2);
 
    while(true)
    {
        // update each component type together
        ai.Update();
 
        // if we had other component type containers we'd update all of them as well
    }
 
    return 0;
}
Edited by rpiller

Share this post


Link to post
Share on other sites
wintertime    4108

Yes, though you have it a bit different than I have.

It is kind of flexible so you can adapt it as you like. Some examples of what could be done different:

- I did not care for handling different component types the same, so I dont have any virtual method there. I just add on whats needed and let the systems handle that.

- I have the storage class (I called it ComponentTable in my code) be only responsible for storing the components so it can be a single templated class mostly independent of the component types. I let it keep a vector sorted by entity id currently, but I imagine many different data structures have the potential to be useful for that (can be easily changed and profiled later on).

- I'm making system classes that get a reference to the corresponding storage (to get iterators) and provide the update method, but can internally do what they like with all components of the corresponding type (it could sometimes simplify the code if putting most logic into a system that knows the exact component type). Its good to try for a 1:1 relationship at first, but there may be some system reading one component type and writing a second, which some other system could read later or it may be needed even more sometimes, which can be done if the system and storage are not combined.

- I dont even have an Entity/Object class, only an EntityTable to avoid the static variable and not disallow the future possibility of having more than one ECS running at once (although thats not very likely). I also put a sorted vector with all entity ids of currently alive entities there, but I may remove that eventually (but then there would be only the way of asking all component storages if any got a component with that entity id left).

- It may be useful to collect one instance of all storage and system classes into one class that handles completely deleting an entity with all components or creating or calling all system updates in the right sequence, but it would need to be modified depending on which systems/component types the game needs.

Share this post


Link to post
Share on other sites
bmsq    134
I use a simple template class to translate the template type into an id that I can use in a map. I'd don't remember where I first saw this technique but I've found that it's quite handy :)

[source lang="cpp"]
class ComponentType
{
public:
// Public functions
template<T>
static unsigned char Id()
{
static unsigned char id = GetNextId();
return id;
}

private:
// Private functions
static unsigned char GetNextId()
{
static unsigned char id = 0;
return id++;
}
};
[/source]

Now you just need to use ComponentType::Id<HealthComponent>() to get a unique id for any component types.

I hope this helps.

Share this post


Link to post
Share on other sites
rpiller    839

I think I found a way to do it the old way I was thinking.

 

class Component
{
public:
    virtual string ToString()=0;
};
 
class HUD : public Component
{
public:
    virtual string ToString() { return "HUD"; }
};
 
class Object
{
private:
    map<string, Component*> components;
public:
    void AddComponent(Component* c)
    {
        string id = c->ToString();
 
        if(components.find(id) == components.end())
            components[id] = c;
    }
 
    template<class T>
    T* GetComponent()
    {
        T temp;
        string id = temp.ToString();
 
        if(components.find(id) == components.end())
            return NULL;
        else
            return (T*)components[id];
    }

};

 

So the trick would be making a temp object of the type in GetComponent() so that we can access it's ToString() to find this unique component in our list. This is a little slower because you have to make the object but it gives the simple API usage that I was looking for.

Share this post


Link to post
Share on other sites
phil_t    8084


This is a little slower because you have to make the object but it gives the simple API usage that I was looking for.

 

The slow part would be the heap memory allocations you are now requiring (in string) just for calling GetComponent. (Also, does ToString need to be virtual?). So it's not a little slower, it's a lot slower.

 

Why not just use an integer identifier instead of a string?

Share this post


Link to post
Share on other sites
rpiller    839

In my component design you are only allowed to attach 1 of the same component class. Which means the identification of this needs to be by component class. So I could have a GetID() function that returns a number, but it's much less likely for any sort of collisions between components by using a string (this is like a library so anyone, not just me, can create components and share them between each other).

 

Also, it's pure virtual because it's required for this identification purpose. All components need to derive from the Component class and so this enforces that they override this very critical function. It doesn't technically need to be, but this way if they don't override it, they'll get a compiler error vs a run-time error.

Share this post


Link to post
Share on other sites
phil_t    8084

In my component design you are only allowed to attach 1 of the same component class. Which means the identification of this needs to be by component class. So I could have a GetID() function that returns a number, but it's much less likely for any sort of collisions between components by using a string (this is like a library so anyone, not just me, can create components and share them between each other).

 

I don't understand how using an int would result in any greater chance of collisions. Someone could just as easily make a component with the same string. Instead of a set of "reserved strings" that you can't use, you just have a set of reserved ids.

 

In fact, you could probably find a way to enforce unique integer ids with some compiler tricks, or requiring component registration of some sort.

 

 

 


Also, it's pure virtual because it's required for this identification purpose. All components need to derive from the Component class and so this enforces that they override this very critical function. It doesn't technically need to be, but this way if they don't override it, they'll get a compiler error vs a run-time error.

 

Wouldn't they get a compiler error (template instantiation) any place the code calls GetComponent<Foo>, where Foo doesn't have ToString implemented? (I agree the compiler error might be less obvious in this case)

Edited by phil_t

Share this post


Link to post
Share on other sites
rpiller    839


I don't understand how using an int would result in any greater chance of collisions.

 

My thinking was because you can create namespace like strings. "MyNamespace.MyComponent" I feel will be less likely to collide with randomly picking a number or something like that. There is no reserved strings really, but if you run into a collision then you'd have to just change the value of one of them, but again I think that's more unlikely then a number.

 

Would love to know of a compiler trick if one exists for this functionality. I don't like the idea of any kind of registration. 

 

 

Wouldn't they get a compiler error (template instantiation) any place the code calls GetComponent<Foo>, where Foo doesn't have ToString implemented? 

 

You are correct. I didn't think about that. This is probably my own mind I have to get out of my way but I always felt like it's cleaner when looking at the doc of the base class and you see pure virtual functions that you know you have to override. Also a component creator would get the compiler error on their component class when they create it vs someone calling GetComponent<T>() function which might not happen at all or happen some time down the line after everything is in place already and they change something to add this call. I know there is a penalty on virtual functions, but what kind of hit are we really talking about with this? I'm not a speed junky, and more favor understandable/clear API usage.

Share this post


Link to post
Share on other sites
phil_t    8084


Would love to know of a compiler trick if one exists for this functionality. I don't like the idea of any kind of registration. 

 

There a bunch of options here, none of them are perfect (I googled for "c++ generate unique id for class")

http://stackoverflow.com/questions/922442/unique-class-type-id-that-is-safe-and-holds-across-library-boundaries

 

 

Virtual function call is probably a minor hit in your usage case, and if you're going for API ease of use then the way you're doing it makes sense.

Share this post


Link to post
Share on other sites
Sova    356

My issue with that method is that I don't know if I like not being able to use the ctor with parameters though. Seems there won't be a "perfect" way to do it though and I'll just have to pick the one that I dislike the least.

 

I also think I'll look at returning references from GetComponent() instead of pointers too (because the Object class owns the component memory management), but I'm not sure how I would do that if I need to check if the component exists in the container and if not what to return.

 

Behold, the magic of variadic templates and forwarding! In Windows-land, this will work in VS2013 and, I think, latest version of MinGW.

template<typename T, typename... Args> T* createComponent(Args&&... args)
{
        /* Check that `T` is indeed of expected type. */
        static_assert(std::is_base_of<Component, T>::value,
                      "`Component` is not a base class of `T`");

        auto t = new T(std::forward<Args>(args)...);
        /* handwave */
        return t;
}

With that, you can make calls like this and it will "just work" as you'd expect.

auto comp = createComponent<YourComponentType>(arg1, arg2, arg3, ..., argN);
Edited by Mnemotic

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this