2
\$\begingroup\$

I am currently working on my own little game engine and thought about implementing a simplified version of Unitys component system where you can attach objects of derived Components to an GameObject. Later you can request specific Components to use their functionality. Below is a stripped down (but still working) version of what I think about implementing.

#include <iostream>
#include <vector>

class GameObject;

class Component
{
private:
    GameObject * parent;

public:
    Component(GameObject * parent = nullptr)
    {
        this->parent = parent;
    }

    virtual ~Component() { parent = nullptr; }

    GameObject* GetParent() { return parent; }

    void SetParent(GameObject* parent) { this->parent = parent; }

    virtual std::string What() { return "Basic Component"; }
};

class SoundSource : public Component
{
private:
    // some sound specific members

public:
    SoundSource(GameObject * parent = nullptr) : Component(parent) {}

    virtual ~SoundSource() { }

    std::string What() { return "SoundSource Component"; }
};

class GameObject
{
private:
    std::vector<Component*> m_Components;

public:
    GameObject() :
        m_Components()
    {}

    ~GameObject()
    {
        for (size_t i = 0; i < m_Components.size(); ++i)
            if (m_Components[i]->GetParent() == this) delete m_Components[i];
        m_Components.clear();
    }


    void AddComponent(Component* comp)
    {
        if (comp->GetParent() == nullptr)
        {
            comp->SetParent(this);
        }
        m_Components.push_back(comp);
    }

    template <class component_type>
    component_type* GetComponent()
    {
        for (auto comp : m_Components)
        {
            component_type* c =  dynamic_cast<component_type*>(comp);
            if (c) return c;
        }
        return nullptr;
    }
};

int main()
{
    GameObject go;
    go.AddComponent(new SoundSource());
    go.AddComponent(new Component());

    SoundSource *s = go.GetComponent<SoundSource>();
    if (s) std::cout << s->What() << std::endl;
}

My initial plan for GetComponent<>() was to use the name of typeid to compare the component types, however since all components need to derive from the same base I think that using a dynamic_cast is the better option in this case. However I have never worked with dynamic casting before and I am not sure whether the way I use it is safe, so please tell me is it?

Also if you see any points for improvements I would appreciate your opinions.

\$\endgroup\$
3
\$\begingroup\$

I'm not a C++ programmer, but there are a few things that I notice that I feel are important in any language.

Be Consistent:

While you'll find many people that are passionate about different naming conventions and coding styles, one thing that they'll all agree on is to at least be consistent in your choice. There are a few inconsistencies in your code:

  • You declare a private member of your Component class named parent, and go on to introduce Hungarian notation in your GameObject class with a member named m_Components.

  • You declare a pointer with a space on either side of the asterisk: Component * parent; and go on with this style in your constructors, but you also connect the asterisk to the type in some other cases: GameObject* GetParent() and SetParent(GameObject* parent).

  • In your GameObject destructor, you have a for with a single-line body and choose to omit curly braces, but in it's AddComponent member function, you have an if with a single-line body but keep the curly-braces. If you're going to omit curly-braces in the former, you may as well omit them in the latter.

  • If you insist on omitting the curly-braces, the former could also be written in a more readable manner:

    for (size_t i = 0; i < m_Components.size(); ++i)
        if (m_Components[i]->GetParent() == this) 
            delete m_Components[i];   
    

Also:

  • virtual std::string What() { return "Basic Component"; } "What" is not a very descriptive identifier. Consider replacing it with something along the lines of "GetName()"

  • The dynamic cast:

    component_type* c = dynamic_cast<component_type*>(comp);
        if (c) return c;
    

    I've often seen dynamic_cast in this form, which I find more readable:

    if (component_type* c = dynamic_cast<component_type*>(comp))
        return c;
    
  • You use std::vector for your collection of Components. Any time where you loop through the elements of an std::vector in order to find a single element, you should consider using std::map. Using data structures like these to locate a Value object using a Key object will be much faster than looping through a vector, because it will take the same amount of time regardless of how many items are in the collection. This approach would result in a much faster GetComponent() method

I'm not sure if this is the approach used to implement GetComponent in Unity, but I know that it's advised against using GetComponent in the Update method in Unity because of how slow it is. If you used an std::map to implement your GetComponent method, it would be a marginal difference compared to doing the lookup in advance and storing the result (which is currently the recommended approach in Unity).

That's the only real feedback I have on your object-oriented design. For the most part, it seems like a pretty accurate C++ representation of Unity's component system.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.