From 8ac9fa5dbd9afebf6addbd50debd816fd91d0c8d Mon Sep 17 00:00:00 2001 From: Mergul Date: Mon, 24 Apr 2023 23:46:16 +0200 Subject: [PATCH] Fix crash in commit() when all components were removed from entity Fix crash from issue #2 and add unittest case for it. Also implements general "empty" entity support --- source/bubel/ecs/manager.d | 98 ++++++++++++++++++++---------------- tests/basic.d | 100 +++++++++++++++++++++++++++++++++++++ tests/bugs.d | 29 +++++++++++ 3 files changed, 185 insertions(+), 42 deletions(-) diff --git a/source/bubel/ecs/manager.d b/source/bubel/ecs/manager.d index 801068d..d6fae94 100644 --- a/source/bubel/ecs/manager.d +++ b/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,46 +1871,66 @@ 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); - uint components_size = EntityID.sizeof; - - foreach (i, id; ids) + if(ids is null) { - info.alignment = max(info.alignment, components[id].alignment); - alignNum(info.size, components[id].alignment); - info.tmpl_deltas[id] = info.size; - info.size += components[id].size; - components_size += components[id].size; + 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; } - alignNum(info.size, info.alignment); - - uint block_memory = cast(uint)( - m_page_size - EntitiesBlock.sizeof - (info.size - components_size)); - //uint entity_comps_size = EntityID.sizeof; - uint mem_begin = EntitiesBlock.sizeof; - - uint entites_in_block = block_memory / info.size; //entity_comps_size; - info.max_entities = cast(ushort) entites_in_block; - ushort current_delta = cast(ushort)(mem_begin + entites_in_block * EntityID.sizeof); - - foreach (i, id; ids) + else { - if (current_delta == 0) - current_delta = ushort.max; - alignNum(current_delta, components[id].alignment); - info.deltas[id] = cast(ushort) current_delta; - current_delta += entites_in_block * components[id].size; + 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); + alignNum(info.size, components[id].alignment); + info.tmpl_deltas[id] = info.size; + info.size += components[id].size; + components_size += components[id].size; + } + alignNum(info.size, info.alignment); + + uint block_memory = cast(uint)( + m_page_size - EntitiesBlock.sizeof - (info.size - components_size)); + //uint entity_comps_size = EntityID.sizeof; + uint mem_begin = EntitiesBlock.sizeof; + + uint entites_in_block = block_memory / info.size; //entity_comps_size; + info.max_entities = cast(ushort) entites_in_block; + ushort current_delta = cast(ushort)(mem_begin + entites_in_block * EntityID.sizeof); + + foreach (i, id; ids) + { + if (current_delta == 0) + current_delta = ushort.max; + alignNum(current_delta, components[id].alignment); + info.deltas[id] = cast(ushort) current_delta; + 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); @@ -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); diff --git a/tests/basic.d b/tests/basic.d index 084500a..ea54382 100644 --- a/tests/basic.d +++ b/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 diff --git a/tests/bugs.d b/tests/bugs.d index 1a3968c..10a3c62 100644 --- a/tests/bugs.d +++ b/tests/bugs.d @@ -142,5 +142,34 @@ unittest gEntityManager.update(); gEntityManager.end(); + 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(); } \ No newline at end of file