From ce7c2183d38e5d3c272c41de71729e47f0c0c5d6 Mon Sep 17 00:00:00 2001 From: Bill Currie Date: Sat, 6 Jan 2024 02:11:31 +0900 Subject: [PATCH] [ecs] Delay component destruction Copy the component out of the pool so that it can be overwritten now, thus removing it from the pool, before actually destroying the component so that any recursive removals of the same component don't mess up the indices, and also don't try to remove the component from the same entity. This fixes a rather sneaky component leak caused by said recursion. --- libs/ecs/entity.c | 17 ++++++++++++----- libs/ui/imui.c | 12 ++++-------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/libs/ecs/entity.c b/libs/ecs/entity.c index e9815f3f0..6f58c7945 100644 --- a/libs/ecs/entity.c +++ b/libs/ecs/entity.c @@ -116,11 +116,14 @@ Ent_RemoveComponent (uint32_t ent, uint32_t comp, ecs_registry_t *registry) uint32_t ind = pool->sparse[id]; component_t *c = ®istry->components.a[comp]; if (ind < pool->count && pool->dense[ind] == ent) { - // invalidate the entity for this component to prevent the component - // being double-removed due to deletion of the component resulting - // in the entity being deleted (happens with hierarchies) - pool->dense[ind] = -1; - Component_DestroyElements (c, pool->data, ind, 1, registry); + // copy the component out of the pool so that it can be overwritten + // now, thus removing it from the pool, before actually destroying + // the component so that any recursive removals of the same component + // don't mess up the indices, and also don't try to remove the + // component from the same entity + byte tmp_comp[c->size]; + Component_CopyElements (c, tmp_comp, 0, pool->data, ind, 1); + uint32_t range_count = subpool->num_ranges - subpool->available; // if ind >= the last range, then it is outside the subpools if (range_count && ind < subpool->ranges[range_count - 1]) { @@ -144,5 +147,9 @@ Ent_RemoveComponent (uint32_t ent, uint32_t comp, ecs_registry_t *registry) } pool->count--; pool->sparse[id] = nullent; + + // the component has been fully removed from the pool so it is now + // safe to destroy it + Component_DestroyElements (c, tmp_comp, 0, 1, registry); } } diff --git a/libs/ui/imui.c b/libs/ui/imui.c index 12e849437..e036890dd 100644 --- a/libs/ui/imui.c +++ b/libs/ui/imui.c @@ -1213,14 +1213,10 @@ IMUI_Passage (imui_ctx_t *ctx, const char *name, struct passage_s *passage) Canvas_SetReference (ctx->csys, psg_view.id, Canvas_Entity (ctx->csys, View_GetRoot (anchor_view).id)); - if (Ent_HasComponent (psg_view.id, c_passage_glyphs, reg)) { - // FIXME this shouldn't be necessary and is a sign of bigger problems - Ent_RemoveComponent (psg_view.id, c_passage_glyphs, reg); - } - if (Ent_HasComponent (psg_view.id, c_updateonce, reg)) { - // FIXME this shouldn't be necessary and is a sign of bigger problems - Ent_RemoveComponent (psg_view.id, c_updateonce, reg); - } + // FIXME this shouldn't be necessary and is a sign of bigger problems + Ent_RemoveComponent (psg_view.id, c_passage_glyphs, reg); + // FIXME this shouldn't be necessary and is a sign of bigger problems + Ent_RemoveComponent (psg_view.id, c_updateonce, reg); Ent_SetComponent (psg_view.id, c_passage_glyphs, reg, Ent_GetComponent (psg_view.id, t_passage_glyphs, reg)); void *update = passage_update;