From 443fa3d8ca9c04aaac12c166810e0a6bef745dbe Mon Sep 17 00:00:00 2001 From: lisyarus Date: Fri, 24 May 2024 21:42:17 +0300 Subject: [PATCH] ECS object lifetime issues fixes --- libs/ecs/include/psemek/ecs/container.hpp | 65 ++++++++++--------- .../psemek/ecs/detail/apply_helper.hpp | 2 - libs/ecs/include/psemek/ecs/dispatcher.hpp | 2 - libs/ecs/source/container.cpp | 23 +++---- libs/ecs/source/detail/entity_list.cpp | 15 ++++- 5 files changed, 58 insertions(+), 49 deletions(-) diff --git a/libs/ecs/include/psemek/ecs/container.hpp b/libs/ecs/include/psemek/ecs/container.hpp index 08309d72..65a965f0 100644 --- a/libs/ecs/include/psemek/ecs/container.hpp +++ b/libs/ecs/include/psemek/ecs/container.hpp @@ -68,7 +68,7 @@ namespace psemek::ecs * @return True if the entity is still alive, false otherwise * @pre The entity was previously obtained by a `create()` call */ - bool alive(handle const & entity) const; + bool alive(handle entity) const; /** Destroy an entity specified by a handle. After the call, `alive()` returns false for this handle. * @@ -79,7 +79,7 @@ namespace psemek::ecs * @param entity A handle to the entity to be destroyed * @pre The entity was previously obtained by a `create()` call and is `alive()` */ - void destroy(handle const & entity); + void destroy(handle entity); /** Get an accessor for an entity, which provides access to the entity's components. * It is designed to be a single-use object; it cannot be stored as a reference to @@ -91,7 +91,7 @@ namespace psemek::ecs * @warning Creating or destroying entities, as well as attaching or detaching components, * invalidates all previously created accessors */ - accessor get(handle const & entity); + accessor get(handle entity); /** Check if the entity can be cloned. * @@ -101,7 +101,7 @@ namespace psemek::ecs * @return True if the entity can be cloned, false otherwise * @pre The entity was previously obtained by a `create()` call and is `alive()` */ - bool can_clone(handle const & entity) const; + bool can_clone(handle entity) const; /** Clone the entity into a new entity by copy-constructing all components, * as if by calling `create()` with all the original entity's components. @@ -113,7 +113,7 @@ namespace psemek::ecs * @pre The entity was previously obtained by a `create()` call and is `alive()` * @throw entity_not_cloneable if the entity is not cloneable */ - handle clone(handle const & entity); + handle clone(handle entity); /** Try to clone the entity into a new entity by copy-constructing all components, * as if by calling `create()` with all the original entity's components. @@ -124,7 +124,7 @@ namespace psemek::ecs * @return A handle to cloned entity if the original entity was cloneable, and std::nullopt otherwise * @pre The entity was previously obtained by a `create()` call and is `alive()` */ - std::optional try_clone(handle const & entity); + std::optional try_clone(handle entity); /** Attach new components to an existing entity, or update existing * components with new values. Other components of this entity @@ -143,7 +143,7 @@ namespace psemek::ecs * @warning Attaching components invalidates all previously created accessors */ template - void attach(handle const & entity, Components && ... components); + void attach(handle entity, Components && ... components); /** Detach (remove) components from an existing entity. Other components of this * entity are left untouched. Detaching a component that doesn't exist is not @@ -160,7 +160,7 @@ namespace psemek::ecs * @warning Detaching components invalidates all previously created accessors */ template - void detach(handle const & entity); + void detach(handle entity); /** Create a query cache that can be used to speed up `apply()` calls. * @@ -394,7 +394,7 @@ namespace psemek::ecs query_cache cache_impl(Constructor && constructor, Destructor && destructor); detail::table * insert_table(std::vector> columns); - void do_destroy(handle const & entity, bool trigger_destructors); + void do_destroy(handle entity); void remove_row(detail::table & table, std::uint32_t row, util::span entities); void finilize_iteration(detail::table & table); }; @@ -441,7 +441,7 @@ namespace psemek::ecs } template - void container::attach(handle const & entity, Components && ... components) + void container::attach(handle entity, Components && ... components) { static_assert(detail::all_different_types_v...>, "all component types must be different"); @@ -450,12 +450,12 @@ namespace psemek::ecs auto uuids = uuid_list_pool_.get(); auto attached_uuid_set = uuid_set_pool_.get(); - auto & data = entity_list_.get_entities()[entity.id]; - for (auto const & column : data.table->columns()) + auto * data = entity_list_.get_entities().begin() + entity.id; + for (auto const & column : data->table->columns()) uuids.push_back(column->uuid()); bool archetype_changed = false; - ((data.table->column(std::remove_cvref_t::uuid()) ? 0 : (archetype_changed = true, attached_uuid_set.insert(std::remove_cvref_t::uuid()), 0)), ...); + ((data->table->column(std::remove_cvref_t::uuid()) ? 0 : (archetype_changed = true, attached_uuid_set.insert(std::remove_cvref_t::uuid()), 0)), ...); for (auto const & uuid : attached_uuid_set) uuids.push_back(uuid); @@ -465,31 +465,31 @@ namespace psemek::ecs if (!table) { std::vector> columns; - for (auto const & column : data.table->columns()) + for (auto const & column : data->table->columns()) columns.push_back(column->clone()); - ((data.table->column(std::remove_cvref_t::uuid()) ? 0 : (columns.push_back(std::make_unique>>()), 0)), ...); + ((data->table->column(std::remove_cvref_t::uuid()) ? 0 : (columns.push_back(std::make_unique>>()), 0)), ...); table = insert_table(std::move(columns)); } - if (table != data.table) + if (table != data->table) { if (table->get_iteration_data()) table = table->get_delayed_table(); - auto new_row = table->move_row(entity, data.table, data.row); - do_destroy(entity, false); + auto new_row = table->move_row(entity, data->table, data->row); + do_destroy(entity); - data.table = table; - data.row = new_row; + data->table = table; + data->row = new_row; } auto accessor = get(entity); ((accessor.get>() = std::forward(components)), ...); if (archetype_changed) - table->trigger_constructors(*this, data.row, attached_uuid_set); + table->trigger_constructors(*this, data->row, attached_uuid_set); attached_uuid_set.clear(); uuids.clear(); @@ -499,7 +499,7 @@ namespace psemek::ecs } template - void container::detach(handle const & entity) + void container::detach(handle entity) { static_assert(detail::all_different_types_v...>, "all component types must be different"); @@ -508,8 +508,8 @@ namespace psemek::ecs auto uuids = uuid_list_pool_.get(); - auto & data = entity_list_.get_entities()[entity.id]; - for (auto const & column : data.table->columns()) + auto * data = entity_list_.get_entities().begin() + entity.id; + for (auto const & column : data->table->columns()) { if (detached_uuid_set.contains(column->uuid())) detached_uuid_set.insert(column->uuid()); @@ -522,28 +522,31 @@ namespace psemek::ecs auto table = table_container_.get(uuids); if (archetype_changed) - data.table->trigger_destructors(*this, data.row, detached_uuid_set); + data->table->trigger_destructors(*this, data->row, detached_uuid_set); + + // Destructors could lead to reallocation of entity list + data = entity_list_.get_entities().begin() + entity.id; if (!table) { std::vector> columns; - for (auto const & column : data.table->columns()) + for (auto const & column : data->table->columns()) if (!detached_uuid_set.contains(column->uuid())) columns.push_back(column->clone()); table = insert_table(std::move(columns)); } - if (table != data.table) + if (table != data->table) { if (table->get_iteration_data()) table = table->get_delayed_table(); - auto new_row = table->move_row(entity, data.table, data.row); - do_destroy(entity, false); + auto new_row = table->move_row(entity, data->table, data->row); + do_destroy(entity); - data.table = table; - data.row = new_row; + data->table = table; + data->row = new_row; } detached_uuid_set.clear(); diff --git a/libs/ecs/include/psemek/ecs/detail/apply_helper.hpp b/libs/ecs/include/psemek/ecs/detail/apply_helper.hpp index 00301fdd..3241357e 100644 --- a/libs/ecs/include/psemek/ecs/detail/apply_helper.hpp +++ b/libs/ecs/include/psemek/ecs/detail/apply_helper.hpp @@ -3,8 +3,6 @@ #include #include -#include - namespace psemek::ecs { diff --git a/libs/ecs/include/psemek/ecs/dispatcher.hpp b/libs/ecs/include/psemek/ecs/dispatcher.hpp index 782dcde2..496fdbfd 100644 --- a/libs/ecs/include/psemek/ecs/dispatcher.hpp +++ b/libs/ecs/include/psemek/ecs/dispatcher.hpp @@ -4,8 +4,6 @@ #include #include -#include - namespace psemek::ecs { diff --git a/libs/ecs/source/container.cpp b/libs/ecs/source/container.cpp index 41d37d75..4e524bb8 100644 --- a/libs/ecs/source/container.cpp +++ b/libs/ecs/source/container.cpp @@ -4,31 +4,35 @@ namespace psemek::ecs { - bool container::alive(handle const & entity) const + bool container::alive(handle entity) const { return entity_list_.get_entities()[entity.id].epoch == entity.epoch; } - void container::destroy(handle const & entity) + void container::destroy(handle entity) { assert(alive(entity)); - do_destroy(entity, true); + + auto const data = entity_list_.get_entities()[entity.id]; + data.table->trigger_destructors(*this, data.row); + + do_destroy(entity); entity_list_.destroy(entity.id); } - accessor container::get(handle const & entity) + accessor container::get(handle entity) { assert(alive(entity)); auto const data = entity_list_.get_entities()[entity.id]; return {data.table, data.row}; } - bool container::can_clone(handle const & entity) const + bool container::can_clone(handle entity) const { return entity_list_.get_entities()[entity.id].table->non_copyable_components().empty(); } - handle container::clone(handle const & entity) + handle container::clone(handle entity) { if (auto result = try_clone(entity)) return *result; @@ -37,7 +41,7 @@ namespace psemek::ecs throw entity_not_cloneable(entity, data.table->non_copyable_components()); } - std::optional container::try_clone(handle const & entity) + std::optional container::try_clone(handle entity) { auto const data = entity_list_.get_entities()[entity.id]; if (!data.table->non_copyable_components().empty()) @@ -69,15 +73,12 @@ namespace psemek::ecs return table; } - void container::do_destroy(handle const & entity, bool trigger_destructors) + void container::do_destroy(handle entity) { auto entities = entity_list_.get_entities(); auto & data = entities[entity.id]; auto & iteration_data = data.table->get_iteration_data(); - if (trigger_destructors) - data.table->trigger_destructors(*this, data.row); - if (!iteration_data || iteration_data->current_row < data.row) remove_row(*data.table, data.row, entities); else diff --git a/libs/ecs/source/detail/entity_list.cpp b/libs/ecs/source/detail/entity_list.cpp index 803200bf..4bcd9af3 100644 --- a/libs/ecs/source/detail/entity_list.cpp +++ b/libs/ecs/source/detail/entity_list.cpp @@ -1,4 +1,5 @@ #include +#include namespace psemek::ecs::detail { @@ -11,15 +12,23 @@ namespace psemek::ecs::detail auto id = free_ids_.back(); free_ids_.pop_back(); - entities_[id].table = table; - entities_[id].row = row; + auto & data = entities_[id]; + assert(data.table == nullptr); + + data.table = table; + data.row = row; return id; } void entity_list::destroy(entity_id id) { - entities_[id].epoch += 1; + auto & data = entities_[id]; + assert(data.table != nullptr); + + data.table = nullptr; + data.epoch += 1; + free_ids_.push_back(id); }