How well does this ECS implementation follow these principles?

Question

I'm currently learning about Entity-Component-System architecture Data-Driven design as a way to counter bad OOP design and optimize data fetching as a CPU friendly operation.

I'm trying to write a simple ECS system that tries to follows these principals:

• CPU friendly by storing entities/components in contiguous chunk of memory and minimize cache miss
• Not set an upper bound to number of entities/components/systems in the system

My basic implementation so far is as follows:

Entity class with a counting m_ID member being incremented for each newly created entity.

An std::vector<Entity*> for containing children entities.

A std::vector<bool> for containing the signature of what components the entity uses. I've chosen std::vector<bool> over std::bitset<S> since I wish to be able as a requirement to add/remove components in runtime and I do not wish to set an upper-bound to the size of components an entity have so I cannot know size in compile time, so I've decided to use the std::vector<bool> specialization that stores each bool as 1 bit which save some space and allows for growth.

An std::unordered_map<unsigned int, unsigned int> where the key is the component's ID and the value is an index into a std::vector<unsigned int, std::vector<ComponentBase>> map of component's ID to components vector residing in ECSManager class.

Component class with a unique ID per component type using templates to generate a unique Component<T>::ID for each component type.

System class containing std::vector<bool> responsible for holding the signature of what entities the system wants to operate on depends on if they have a sufficient components signature.

Now I'm sure my implementation is not nearly close to what my requirements are, for example I'm not really sure if it's CPU friendly as an ECS should be since I rely heavily on std::unordered_map which is implemented by a linked list. But to be honest I'm not sure at all so this is why I'm posting my implementation here hoping for a feedback on what's good and what's not and how I can maybe make it better.

ComponentBase.h:

#pragma once
#include <atomic>
static std::atomic<unsigned int> s_CurrentId;
class ComponentBase
{
public:
 inline static unsigned GetID() { return ++s_CurrentId; }
};

Component.h:

#pragma once
#include "ComponentBase.h"
template <typename T>
class Component : public ComponentBase
{
public:
 static const unsigned int ID = ComponentBase::GetID();
};

Entity.h:

#pragma once
#include <vector>
#include <unordered_map>
class Entity
{
private:
 unsigned int m_Id;
 std::vector<Entity*> m_Children;
 std::vector<bool> m_Signature;
 std::unordered_map<unsigned int, unsigned int> m_ComponentsIndex;
protected:
 Entity();
 friend class ComponentManager;
 friend class Scene;
public:
 Entity(const Entity& other) = delete;
 virtual ~Entity();
 Entity& operator=(const Entity& other) = delete;
 inline unsigned int GetID() { return m_Id; }
 void AddChild(Entity* entity);
 inline const std::vector<Entity*>& GetChildren() const { return m_Children; }
 void AddComponent(unsigned int componentID, unsigned int index);
 void RemoveComponent(unsigned int componentID);
 inline const std::vector<bool>& GetSignature() const { return m_Signature; }
 inline const std::unordered_map<unsigned int, unsigned int>& GetComponentsIndex() const { return m_ComponentsIndex; }
};

Entity.cpp:

#include "Entity.h"
#include <atomic>
static std::atomic<unsigned int> s_CurrentId;
Entity::Entity()
 : m_Id(s_CurrentId++)
{
}
Entity::~Entity()
{
 for (const auto& child : m_Children)
 delete child;
}
void Entity::AddChild(Entity* entity)
{
 m_Children.push_back(entity);
}
void Entity::AddComponent(unsigned int componentID, unsigned int index)
{
 m_Signature[componentID] = true;
 m_ComponentsIndex[componentID] = index;
}
void Entity::RemoveComponent(unsigned int componentID)
{
 m_Signature[componentID] = false;
 m_ComponentsIndex.erase(componentID);
}

System.h:

#pragma once
#include <vector>
#include "Entity.h"
class System
{
private:
 std::vector<bool> m_ComponentIDs;
protected:
 System(std::vector<unsigned int>& componentIDs);
public:
 virtual ~System();
 virtual void Update() = 0;
};

System.cpp:

#include "System.h"
System::System(std::vector<unsigned int>& componentIDs)
{
 for (const auto& componentID : componentIDs)
 m_ComponentIDs[componentID] = true;
}
System::~System()
{
}

ECSManager.h:

#pragma once
#include <vector>
#include <unordered_map>
#include "../Entity.h"
#include "ComponentBase.h"
class ECSManager
{
private:
 std::unordered_map<unsigned int, std::vector<ComponentBase>> m_Components;
 std::unordered_map<unsigned int, Entity> m_Entities;
private:
 ECSManager();
public:
 const Entity& CreateEntity()
 {
 Entity e;
 m_Entities.emplace(e.GetID(), std::move(e));
 return m_Entities[e.GetID()];
 }
 template <typename TComponent, typename... Args>
 void AddComponent(unsigned int entityId, Args&&... args)
 {
 unsigned int componentID = TComponent::ID;
 if (m_Components[componentID] == m_Components.end())
 m_Components[componentID] = std::vector<TComponent>();
 m_Components[componentID].push_back(T(std::forward<Args>(args)...));
 m_Entities[entityId].AddComponent(componentID, m_Components[componentID].size() - 1);
 if (m_Signatures[m_Entities[entityId].GetSignature()] == m_Signatures.end())
 m_Signatures[m_Entities[entityId].GetSignature()] = std::vector<unsigned int>();
 m_Signatures[m_Entities[entityId].GetSignature()].push_back(entityId);
 }
 template <typename TComponent>
 TComponent& GetComponent(Entity& entity)
 {
 return m_Components[TComponent::ID][entity.GetComponentsIndex()[TComponent::ID]];
 }
 template <typename TComponent>
 std::vector<TComponent>& GetComponents()
 {
 unsigned int componentID = TComponent::ID;
 return m_Components[componentID];
 }
 std::vector<Entity&> GetEntities(std::vector<bool> componentIDs)
 {
 std::vector<Entity&> entities;
 for (auto& entity : m_Entities)
 {
 const auto& entitySignature = entity.second.GetSignature();
 if (componentIDs.size() > entitySignature.size())
 continue;
 bool equal = true;
 for (std::size_t i = 0; i != componentIDs.size(); i++)
 {
 if (componentIDs[i] & entitySignature[i] == 0)
 {
 equal = false;
 break;
 }
 }
 if (!equal)
 continue;
 entities.push_back(entity.second);
 }
 return entities;
 }
};
```

• nwp: "CPU friendly by storing entities/components in contiguous chunk of memory and minimize cache miss" and "An std::vector<Entity*> for containing children entities" do not go together. It doesn't help that your Entity*s are neatly organized in contiguous memory when you must dereference them and the Entitys are all over the place because you used dynamic memory. If you do it "correctly" the ECS will have no inheritance at all. You do not need ComponentBase. template <class Component> std::vector<Component> components; is your friend.

Answer 1

1. Since class Entity deletes its children in the destructor - it has ownership over the data. According to modern C++ guidelines you don't pass ownership via raw pointers nor class should own any data it has only a raw pointer to (except for most basic classes I suppose). Use std::unique_ptr<Entity> instead of Entity* - besides safety it will automatically implement your custom trivial destructor. It will also automatically mark your class Entity to be non-copyable.
2. Just write virtual ~System() = default; instead of making a custom empty constructor in .cpp.
3. System(std::vector<unsigned int>& componentIDs); replace with System(const std::vector<unsigned int>& componentIDs); since it doesn't modify componentIDs.
4. Template function AddComponent doesn't compile or work. Your compiler might not scream since you don't use it at any point in your code but it is wrong in many places. Core review is for reviewing working code not debugging non-working code.
5. There is something terribly wrong with your approach to the ECSManager::m_Components. It won't do what you want it to do.
6. std::vector<Entity&> doesn't compile... std::vector cannot hold native references. Use std::reference_wrapper for it.

I assume there many other errors. Please test your code prior to posting it on code review. If you fail to make something work, you can post it on StackOverflow first.

• Jorayen: Thanks for the feedback, about the AddComponent why wouldn't it work, and what so terrible about ECSManager::m_Components is terribly wrong? Also about std::vector<Entity&> this you're right. But besides that you I'm looking for more feedback regarding to the requirements of the ECS system less so much about the nitty gritty things of cpp like default destructors and what not.
• ALX23z: @Jorayen AddComponent simply doesn't compile and I assume you want m_Components to reference some other type but it stores only base type. You lose all the components' info. You ought to store them via a pointers.
• Jorayen: Oh should've made it std::unordered_map<unsigned int, std::vector<ComponentBase*>> m_Components; my bad