Proper string handling without nasty memory leaks. PHEW.

This commit is contained in:
2016-02-19 14:37:14 +01:00
parent ca907b912e
commit 881ce7b02f
8 changed files with 144 additions and 24 deletions
+2 -1
View File
@@ -2,6 +2,7 @@
#define ComponentInfo_h__ #define ComponentInfo_h__
#include "../Common.h" #include "../Common.h"
#include <boost/shared_array.hpp>
struct ComponentInfo struct ComponentInfo
{ {
@@ -29,7 +30,7 @@ struct ComponentInfo
std::vector<std::string> FieldsInOrder; std::vector<std::string> FieldsInOrder;
std::vector<std::string> StringFields; std::vector<std::string> StringFields;
unsigned int Stride = 0; unsigned int Stride = 0;
std::shared_ptr<char> Defaults = nullptr; boost::shared_array<char> Defaults = nullptr;
std::shared_ptr<Meta_t> Meta = nullptr; std::shared_ptr<Meta_t> Meta = nullptr;
}; };
+65 -1
View File
@@ -2,6 +2,7 @@
#define ComponentWrapper_h__ #define ComponentWrapper_h__
#include <boost/shared_array.hpp> #include <boost/shared_array.hpp>
#include <boost/any.hpp>
#include "../Common.h" #include "../Common.h"
#include "Entity.h" #include "Entity.h"
#include "ComponentInfo.h" #include "ComponentInfo.h"
@@ -75,11 +76,22 @@ struct ComponentWrapper
{ {
for (auto& name : component.Info.StringFields) { for (auto& name : component.Info.StringFields) {
std::size_t offset = component.Info.Fields.at(name).Offset; std::size_t offset = component.Info.Fields.at(name).Offset;
auto& value = *reinterpret_cast<const std::string*>(component.Data + offset); std::string value = *reinterpret_cast<const std::string*>(component.Data + offset);
new (component.Data + offset) std::string(value); new (component.Data + offset) std::string(value);
} }
} }
// This needs to be called to properly free component data, because strings.
static void Destroy(ComponentInfo info, char* data)
{
// Call std::string destructors
for (auto& name : info.StringFields) {
std::size_t offset = info.Fields.at(name).Offset;
auto field = reinterpret_cast<std::string*>(data + offset);
field->~basic_string();
}
}
struct SubscriptProxy struct SubscriptProxy
{ {
friend struct ComponentWrapper; friend struct ComponentWrapper;
@@ -124,4 +136,56 @@ private:
boost::shared_array<char> m_DataReference; boost::shared_array<char> m_DataReference;
}; };
// TODO: Move this to Tests once entity importing is finished
class ComponentWrapperFactory
{
public:
ComponentWrapperFactory() = default;
ComponentWrapperFactory(std::string componentTypeName, unsigned int allocation = 0)
{
m_ComponentInfo.Name = componentTypeName;
m_ComponentInfo.Meta = std::make_shared<ComponentInfo::Meta_t>();
m_ComponentInfo.Meta->Allocation = allocation;
}
template <typename T>
void AddProperty(std::string fieldName, T defaultValue)
{
auto& field = m_ComponentInfo.Fields[fieldName];
field.Name = fieldName;
field.Type = typeid(T).name();
field.Offset = m_ComponentInfo.Stride;
field.Stride = sizeof(T);
m_ComponentInfo.FieldsInOrder.push_back(field.Name);
if (field.Type == typeid(std::string).name()) {
field.Type = "string";
m_ComponentInfo.StringFields.push_back(field.Name);
}
m_ComponentInfo.Stride += sizeof(T);
m_DefaultValues.push_back(std::make_pair(field, defaultValue));
}
ComponentInfo& Finalize()
{
m_ComponentInfo.Defaults = boost::shared_array<char>(new char[m_ComponentInfo.Stride]);
std::size_t offset = 0;
for (auto& pair : m_DefaultValues) {
if (pair.first.Type == "string") {
new (m_ComponentInfo.Defaults.get() + offset) std::string(*reinterpret_cast<std::string*>(pair.second.Data.get()));
} else {
memcpy(m_ComponentInfo.Defaults.get() + offset, pair.second.Data.get(), pair.second.Size);
}
offset += pair.second.Size;
}
return m_ComponentInfo;
}
operator ComponentInfo&() { return Finalize(); }
private:
ComponentInfo m_ComponentInfo;
std::vector<std::pair<ComponentInfo::Field_t, Any>> m_DefaultValues;
};
#endif #endif
+8 -6
View File
@@ -68,12 +68,13 @@ public:
MemoryPool(const MemoryPool<T>& other) MemoryPool(const MemoryPool<T>& other)
: m_StartAddress(new char[other.m_NumSlots*other.m_Stride]) : m_StartAddress(new char[other.m_NumSlots*other.m_Stride])
, m_SlotIsAllocated(other.m_NumSlots, false) , m_SlotIsAllocated(other.m_SlotIsAllocated)
, m_ExtraMemory()
, m_NumSlots(other.m_NumSlots) , m_NumSlots(other.m_NumSlots)
, m_LowestAllocatedSlot(other.m_LowestAllocatedSlot)
, m_NumAllocatedSlots(other.m_NumAllocatedSlots)
, m_Stride(other.m_Stride) , m_Stride(other.m_Stride)
, m_NumAllocatedSlots(0) , m_CurrentAllocSlot(other.m_CurrentAllocSlot)
, m_CurrentAllocSlot(0)
, m_LowestAllocatedSlot(m_NumSlots)
{ {
// Copy statically allocated pool // Copy statically allocated pool
memcpy(m_StartAddress, other.m_StartAddress, m_NumSlots*m_Stride); memcpy(m_StartAddress, other.m_StartAddress, m_NumSlots*m_Stride);
@@ -93,8 +94,9 @@ public:
delete[] m_StartAddress; delete[] m_StartAddress;
m_StartAddress = nullptr; m_StartAddress = nullptr;
} }
for (char* addr : m_ExtraMemory) for (char* addr : m_ExtraMemory) {
free(addr); free(addr);
}
m_ExtraMemory.clear(); m_ExtraMemory.clear();
} }
+4 -3
View File
@@ -2,6 +2,7 @@
#define Util_Any_h__ #define Util_Any_h__
#include <memory> #include <memory>
#include <boost/shared_array.hpp>
struct Any struct Any
{ {
@@ -10,7 +11,7 @@ struct Any
template <typename T> template <typename T>
Any(const T& value) Any(const T& value)
{ {
Data = std::shared_ptr<char>(new char[sizeof(T)]); Data = boost::shared_array<char>(new char[sizeof(T)]);
Size = sizeof(T); Size = sizeof(T);
memcpy(Data.get(), &value, Size); memcpy(Data.get(), &value, Size);
} }
@@ -18,7 +19,7 @@ struct Any
template <typename T> template <typename T>
Any(T&& value) Any(T&& value)
{ {
Data = std::shared_ptr<char>(new char[sizeof(T)]); Data = boost::shared_array<char>(new char[sizeof(T)]);
Size = sizeof(T); Size = sizeof(T);
memcpy(Data.get(), &value, Size); memcpy(Data.get(), &value, Size);
} }
@@ -35,7 +36,7 @@ struct Any
return Any(value); return Any(value);
} }
std::shared_ptr<char> Data = nullptr; boost::shared_array<char> Data = nullptr;
std::size_t Size = 0; std::size_t Size = 0;
}; };
+12 -5
View File
@@ -33,10 +33,18 @@ ComponentPoolForwardIterator& ComponentPoolForwardIterator::operator++()
ComponentPool::ComponentPool(const ComponentPool& other) ComponentPool::ComponentPool(const ComponentPool& other)
: m_ComponentInfo(other.m_ComponentInfo) : m_ComponentInfo(other.m_ComponentInfo)
, m_Pool(other.m_Pool) , m_Pool(other.m_Pool)
, m_EntityToComponent()
{ {
// Update EntityToComponent pointers
for (char& ptr : m_Pool) {
EntityID entity = *reinterpret_cast<EntityID*>(&ptr);
m_EntityToComponent[entity] = &ptr;
}
// Duplicate strings // Duplicate strings
for (auto& name : m_ComponentInfo.StringFields) { for (auto& name : m_ComponentInfo.StringFields) {
for (auto& c : *this) { for (auto& c : *this) {
std::string& val = c[name];
ComponentWrapper::SolidifyStrings(c); ComponentWrapper::SolidifyStrings(c);
} }
} }
@@ -44,11 +52,9 @@ ComponentPool::ComponentPool(const ComponentPool& other)
ComponentPool::~ComponentPool() ComponentPool::~ComponentPool()
{ {
// Call std::string destructors // Destroy component data
for (auto& name : m_ComponentInfo.StringFields) { for (auto& c : *this) {
for (auto& c : *this) { ComponentWrapper::Destroy(c.Info, c.Data);
c.Field<std::string>(name).~basic_string();
}
} }
} }
@@ -86,6 +92,7 @@ bool ComponentPool::KnowsEntity(EntityID ent)
void ComponentPool::Delete(ComponentWrapper& wrapper) void ComponentPool::Delete(ComponentWrapper& wrapper)
{ {
ComponentWrapper::Destroy(wrapper.Info, wrapper.Data);
m_EntityToComponent.erase(wrapper.EntityID); m_EntityToComponent.erase(wrapper.EntityID);
m_Pool.Free(wrapper.Data - sizeof(EntityID)); m_Pool.Free(wrapper.Data - sizeof(EntityID));
} }
+1 -1
View File
@@ -204,7 +204,7 @@ void EntityFilePreprocessor::parseDefaults()
for (auto& ci : m_ComponentInfo) { for (auto& ci : m_ComponentInfo) {
// Allocate memory for default values // Allocate memory for default values
ci.second.Defaults = std::shared_ptr<char>(new char[ci.second.Stride]); ci.second.Defaults = boost::shared_array<char>(new char[ci.second.Stride], std::bind(&ComponentWrapper::Destroy, ci.second, std::placeholders::_1));
memset(ci.second.Defaults.get(), 0, ci.second.Stride); memset(ci.second.Defaults.get(), 0, ci.second.Stride);
std::string componentName = ci.first; std::string componentName = ci.first;
+7 -6
View File
@@ -11,14 +11,18 @@ World::~World()
World::World(const World& other) World::World(const World& other)
: m_EventBroker(other.m_EventBroker) : m_EventBroker(other.m_EventBroker)
, m_CurrentEntityID(other.m_CurrentEntityID)
, m_EntityParents(other.m_EntityParents)
, m_EntityChildren(other.m_EntityChildren)
, m_EntityNames(other.m_EntityNames)
{ {
// Deep copy component pools // Deep copy component pools
for (auto& kv : m_ComponentPools) { for (auto& kv : other.m_ComponentPools) {
m_ComponentPools[kv.first] = new ComponentPool(*kv.second); m_ComponentPools[kv.first] = new ComponentPool(*kv.second);
} }
} }
EntityID World::CreateEntity(EntityID parent /*= 0*/) EntityID World::CreateEntity(EntityID parent /*= EntityID_Invalid*/)
{ {
EntityID newEntity = generateEntityID(); EntityID newEntity = generateEntityID();
if (newEntity == parent) { if (newEntity == parent) {
@@ -53,11 +57,8 @@ ComponentWrapper World::AttachComponent(EntityID entity, const std::string& comp
ComponentPool* pool = m_ComponentPools.at(componentType); ComponentPool* pool = m_ComponentPools.at(componentType);
const ComponentInfo& ci = pool->ComponentInfo(); const ComponentInfo& ci = pool->ComponentInfo();
// Allocate space for the component // Allocate component with default values
ComponentWrapper c = pool->Allocate(entity); ComponentWrapper c = pool->Allocate(entity);
// Write default values
memcpy(c.Data, ci.Defaults.get(), ci.Stride);
ComponentWrapper::SolidifyStrings(c);
return c; return c;
} }
+44
View File
@@ -69,3 +69,47 @@ BOOST_AUTO_TEST_CASE(WorldTestMultipleAllocations, * utf::tolerance(0.00001))
i++; i++;
} }
} }
BOOST_AUTO_TEST_CASE(WorldCopy, *utf::tolerance(0.00001))
{
World w1;
// Create a test component
auto testComponent = ComponentWrapperFactory("Test", 2);
testComponent.AddProperty("TestInteger", 1337);
testComponent.AddProperty("TestDouble", 13.37);
testComponent.AddProperty("TestString", std::string("DefaultString"));
testComponent.AddProperty("TestVec3", glm::vec3(1.f, 2.f, 3.f));
w1.RegisterComponent(testComponent);
// Create a test entity
EntityID w1_e1 = w1.CreateEntity();
auto w1_c1 = w1.AttachComponent(w1_e1, "Test");
// Create a child
EntityID w1_e2 = w1.CreateEntity(w1_e1);
auto w1_c2 = w1.AttachComponent(w1_e2, "Test");
w1_c2["TestString"] = "NonDefaultString";
// Copy the world!
World w2 = w1;
// Fetch the components
auto w2_c1 = w2.GetComponent(w1_e1, "Test");
auto w2_c2 = w2.GetComponent(w1_e2, "Test");
// Check that built-in types are copied but don't reside in the same memory
BOOST_CHECK((int)w1_c1["TestInteger"] == (int)w2_c1["TestInteger"]);
BOOST_CHECK(&(int&)w1_c1["TestInteger"] != &(int&)w2_c1["TestInteger"]);
BOOST_CHECK((double)w1_c1["TestDouble"] == (double)w2_c1["TestDouble"]);
BOOST_CHECK(&(int&)w1_c1["TestDouble"] != &(int&)w2_c1["TestDouble"]);
BOOST_CHECK((int)w1_c2["TestInteger"] == (int)w2_c2["TestInteger"]);
BOOST_CHECK(&(int&)w1_c2["TestInteger"] != &(int&)w2_c2["TestInteger"]);
BOOST_CHECK((double)w1_c2["TestDouble"] == (double)w2_c2["TestDouble"]);
BOOST_CHECK(&(int&)w1_c2["TestDouble"] != &(int&)w2_c2["TestDouble"]);
// Check that specially handled strings are fine
BOOST_CHECK((std::string)w1_c1["TestString"] == (std::string)w2_c1["TestString"]);
BOOST_CHECK(&(std::string&)w1_c1["TestString"] != &(std::string&)w2_c1["TestString"]);
BOOST_CHECK((std::string)w1_c2["TestString"] == (std::string)w2_c2["TestString"]);
BOOST_CHECK(&(std::string&)w1_c2["TestString"] != &(std::string&)w2_c2["TestString"]);
}