cute_framework icon indicating copy to clipboard operation
cute_framework copied to clipboard

ECS - older entity handles become invalid after handle table grows

Open ogam opened this issue 2 years ago • 3 comments

Without overriding any of the memory allocators, after creating a bunch of entities and ensure_capacity() is called, it seems to invalidate all older entities by CF_HandleEntry::data.generation.

Example

struct E_Test;
struct C_Test {};

void system_handle_test(CF_ComponentList components, int entity_count, void* udata)
{
    CF_Entity *entities = cf_get_entities(components);
    for (int index = 0; index < entity_count; ++index)
    {
        CF_ASSERT(cf_entity_is_valid(entities[index]));
    }
} 

void handle_test()
{
    cf_component_begin();
    cf_component_set_name(CF_STRINGIZE(C_Test));
    cf_component_set_size(sizeof(C_Test));
    cf_component_end();
    cf_entity_begin();
    cf_entity_set_name(CF_STRINGIZE(E_Test));
    cf_entity_add_component(CF_STRINGIZE(C_Test));
    cf_entity_end();

    cf_system_begin();
    cf_system_set_name(CF_STRINGIZE(system_handle_test));
    cf_system_set_update(system_handle_test);
    cf_system_require_component(CF_STRINGIZE(C_Test));
    cf_system_end();

    int counter = 0;
    while (counter++ < 16) cf_make_entity(CF_STRINGIZE(E_Test));
}

cute_handle_table.cpp:99 CF_Handle cf_handle_allocator_alloc(CF_HandleTable* table, uint32_t index, uint16_t type)

table->m_handles.ensure_capacity(first_index * 2);

goes to this array macro in cute_array.h

#define CF_ARRAY_ENSURE_CAPACITY(capacity)                     \
	int num_elements = capacity;                               \
	if (num_elements > m_capacity) {                           \
		if (m_capacity == 0) {                                 \
			m_capacity = 8;                                    \
		}                                                      \
		while (m_capacity < num_elements) {                    \
			m_capacity *= 2;                                   \
		}                                                      \
		T* new_ptr = (T*)cf_alloc(sizeof(T) * m_capacity);     \
		for (int i = 0; i < m_count; ++i) {                    \
			CF_PLACEMENT_NEW(new_ptr + i) T(move(m_ptr[i]));   \
		}                                                      \
		cf_free(m_ptr);                                        \
		m_ptr = new_ptr;                                       \
	}                                                          \

Switching out cf_alloc with cf_calloc older handles seems to be fine.

ogam avatar Nov 12 '23 23:11 ogam

m_count doesn't get updated on handle allocations so it doesn't seem to get moved?

ogam avatar Nov 13 '23 06:11 ogam

Was this related to your PR about ECS I just merged in?

RandyGaul avatar Nov 17 '23 20:11 RandyGaul

Not related. This one happens when entity collection gets reallocated and any older CF_HandleEntry::data.generation gets stomped on.

int counter = 8;
while (counter--) cf_make_entity("thing");
// [0-7] entities will have generation of 0

counter = 8;
while (counter--) cf_make_entity("thing");
// [0-7] entities will have generation of 12345
// [8-15] entities will have generation of 0

edit: Issue isn't causing any blockers, just raising for awareness.

ogam avatar Nov 17 '23 22:11 ogam

Did you have a suggested fix for this? I can take a look soon, but if you had a local fix in mind let me know.

RandyGaul avatar Jul 05 '24 11:07 RandyGaul

I haven't looked at this in a while. Currently I'm pre-allocating 8k entities at startup to grow world->handles (HandleTable) to work around this.

ogam avatar Jul 05 '24 14:07 ogam

I think I figured out the problem. The Array<T> is used for memory storage, and that's it. You're right, m_count in the array is never updated. Solution should be to swap to ensure_count instead of ensure_capacity.

RandyGaul avatar Jul 07 '24 22:07 RandyGaul