Merge branch '2-empty-entity-crash' into 'master'

Fix crash in commit() when all components were removed from entity

Closes #2

See merge request Mergul/bubel-ecs!27

source/bubel/ecs/manager.d

@@ -1856,6 +1856,7 @@ export struct EntityManager
     */
     export EntityTemplate* allocateTemplate(EntityTemplate* copy_tmpl)
     {
+        assert(copy_tmpl, "copy_tmpl can't be null");
         EntityTemplate* tmpl = Mallocator.make!EntityTemplate;
         tmpl.info = copy_tmpl.info;
         tmpl.entity_data = Mallocator.makeArray(copy_tmpl.entity_data);
@@ -1870,20 +1871,30 @@ export struct EntityManager
     */
     export EntityInfo* getEntityInfo(ushort[] ids)
     {
+        if(ids.length == 0)ids = null;
         EntityInfo* info = entities_infos.get(ids, null);
         if (info is null)
         {
             info = Mallocator.make!EntityInfo;
 
-            info.components = Mallocator.makeArray(ids);
-            info.deltas = Mallocator.makeArray!ushort(ids[$ - 1] + 1);
-
             info.size = EntityID.sizeof;
             info.alignment = EntityID.alignof;
 
-            info.tmpl_deltas = Mallocator.makeArray!ushort(ids[$ - 1] + 1, ushort.max);
+            if(ids is null)
+            {
+                uint block_memory = cast(uint)(
+                    m_page_size - EntitiesBlock.sizeof - info.size);
+                uint entites_in_block = block_memory / info.size;
+                info.max_entities = cast(ushort) entites_in_block;
+            }
+            else 
+            {
                 uint components_size = EntityID.sizeof;
 
+                info.components = Mallocator.makeArray(ids);
+                info.deltas = Mallocator.makeArray!ushort(ids[$ - 1] + 1);
+                info.tmpl_deltas = Mallocator.makeArray!ushort(ids[$ - 1] + 1, ushort.max);
+
                 foreach (i, id; ids)
                 {
                     info.alignment = max(info.alignment, components[id].alignment);
@@ -1912,6 +1923,16 @@ export struct EntityManager
                     current_delta += entites_in_block * components[id].size;
                 }
 
+                info.comp_add_info = Mallocator.makeArray!(EntityInfo*)(info.deltas.length);
+                info.comp_rem_info = Mallocator.makeArray!(EntityInfo*)(info.deltas.length);
+
+                foreach (comp; info.components)
+                {
+                    info.comp_add_info[comp] = info;
+                    info.comp_rem_info[comp] = null;
+                }
+            }
+
             info.systems = Mallocator.makeArray!bool(systems.length);
 
             foreach (i, ref system; systems)
@@ -1930,15 +1951,6 @@ export struct EntityManager
                 addSystemCaller(*info, cast(uint) i);
             }
 
-            info.comp_add_info = Mallocator.makeArray!(EntityInfo*)(info.deltas.length);
-            info.comp_rem_info = Mallocator.makeArray!(EntityInfo*)(info.deltas.length);
-
-            foreach (comp; info.components)
-            {
-                info.comp_add_info[comp] = info;
-                info.comp_rem_info[comp] = null;
-            }
-
             entities_infos.add(info.components, info);
 
             generateListeners(info);
@@ -2552,7 +2564,7 @@ export struct EntityManager
     use you should save ID instead of pointer.
     
     Params:
-    tmpl = pointer entity template allocated by EntityManager.
+    tmpl = pointer entity template allocated by EntityManager. Can be null in which case empty entity would be added (entity without components)
     */
     export Entity* addEntity(EntityTemplate* tmpl)
     {
@@ -2564,12 +2576,14 @@ export struct EntityManager
     use you should save ID instead of pointer.
     
     Params:
-    tmpl = pointer entity template allocated by EntityManager.
+    tmpl = pointer entity template allocated by EntityManager. Can be null in which case empty entity would be added (entity without components)
     replacement = list of components references to used. Memory form list replace data from template inside new entity. Should be used only for data which vary between most entities (like 3D position etc.)
     */
     export Entity* addEntity(EntityTemplate* tmpl, ComponentRef[] replacement)
     {
-        EntityInfo* info = tmpl.info;
+        EntityInfo* info = void;
+        if(tmpl)info = tmpl.info;
+        else info = getEntityInfo(null);
 
         ushort index = 0;
         EntitiesBlock* block;
@@ -3446,7 +3460,7 @@ export struct EntityManager
                     break;
                 }
             }
-            if (id > components[$ - 1])
+            if (components.length == 0 || id > components[$ - 1])
                 ids[len++] = id;
 
             assert(len == components.length + 1);

tests/basic.d

@@ -113,6 +113,31 @@ struct EmptySystem
     int count = 0;
 }
 
+struct EntityCounterSystem
+{
+    mixin ECS.System!1;
+
+    struct EntitiesData
+    {
+        int thread_id;
+        uint length;
+        Entity[] entity;
+    }
+
+    bool onBegin()
+    {
+        count = 0;
+        return true;
+    }
+
+    void onUpdate(EntitiesData data)
+    {
+        count += data.length;
+    }
+
+    int count = 0;
+}
+
 void beforeEveryTest()
 {
     becsID!CUnregistered = ushort.max;
@@ -243,7 +268,82 @@ unittest
     gEntityManager.commit();
     entity3 = gEntityManager.getEntity(id);
     assert(!entity3.getComponent!CUnregistered);
+}
 
+@("AddEmptyEntity")
+unittest
+{
+    struct OnAddRemoveChangeCounter
+    {
+        mixin ECS.System!1;
+
+        struct EntitiesData
+        {
+            int thread_id;
+            uint length;
+            Entity[] entity;
+        }
+
+        void onAddEntity(EntitiesData data)
+        {
+            add += data.length;
+        }
+        
+        void onRemoveEntity(EntitiesData data)
+        {
+            assert(0, "It's impossible to remove entity from being updated by system which accept empty entity");
+        }
+
+        int add = 0;
+    }
+
+    gEntityManager.beginRegister();
+
+    gEntityManager.registerSystem!EntityCounterSystem(0);
+    gEntityManager.registerSystem!OnAddRemoveChangeCounter(1);
+
+    gEntityManager.endRegister();
+
+    CLong long_component = CLong(3);
+
+    Entity* entity = null;
+    EntityID entity_id = gEntityManager.addEntity(null).id;
+
+    EntityCounterSystem* system = gEntityManager.getSystem!EntityCounterSystem;
+    assert(system !is null);
+    assert(system.count == 0);
+
+    OnAddRemoveChangeCounter* add_remove_change_system = gEntityManager.getSystem!OnAddRemoveChangeCounter;
+    assert(add_remove_change_system !is null);
+    assert(add_remove_change_system.add == 0);
+
+    gEntityManager.commit();
+    assert(add_remove_change_system.add == 1);
+
+    entity = gEntityManager.getEntity(entity_id);
+    assert(!entity.hasComponent(becsID!CLong));
+    assert(entity.getComponent(becsID!CLong) is null);
+    
+
+    gEntityManager.begin();
+    gEntityManager.update();
+    assert(system.count == 1);
+    gEntityManager.end();
+
+    gEntityManager.addEntityCopy(entity_id);
+    gEntityManager.addEntityCopy(entity_id);
+    gEntityManager.addComponents(entity_id, [ComponentRef(&long_component, becsID(long_component))].staticArray);
+    gEntityManager.commit();
+    assert(add_remove_change_system.add == 3, "onAddEntity missed");
+
+    entity = gEntityManager.getEntity(entity_id);
+    assert(entity.hasComponent(becsID!CLong));
+    assert(*entity.getComponent!CLong == 3);
+
+    gEntityManager.begin();
+    gEntityManager.update();
+    assert(system.count == 3);
+    gEntityManager.end();
 }
 
 //allocate templates

tests/bugs.d

@@ -144,3 +144,32 @@ unittest
 
     gEntityManager.destroy();
 }
+
+@("2-empty-entity-crash")
+unittest
+{
+    
+    gEntityManager.initialize(0);
+
+    gEntityManager.beginRegister();
+
+    gEntityManager.registerComponent!CInt;
+    gEntityManager.registerComponent!CFloat;
+
+    gEntityManager.endRegister();
+
+
+    EntityTemplate* tmpl = gEntityManager.allocateTemplate([becsID!CInt, becsID!CFloat].staticArray);
+    scope(exit) gEntityManager.freeTemplate(tmpl);
+
+    EntityID id = gEntityManager.addEntity(tmpl).id;
+    // EntityID id2 = gEntityManager.addEntity().id;
+
+    gEntityManager.commit();
+
+    gEntityManager.removeComponents(id, [becsID!CInt, becsID!CFloat].staticArray);
+
+    gEntityManager.commit();
+
+    gEntityManager.destroy();
+}