Skip to content

Commit f086130

Browse files
committed
component storage issue diagnosed and fixed
it was because in the handling of removing components, we would swap components within the storage. then we would update the entity's componentid list so that the removed component ref would be removed, and that the component ref pointing to the item that was swapped be updated to point at the new correct index within storage. this part was fine, but when updating the component id refs, we forgot to check that the componentid component storage type (base, updatable, renderable) when finding the componentid pointing to the moved component in storage.
1 parent 845ca91 commit f086130

File tree

1 file changed

+18
-12
lines changed

1 file changed

+18
-12
lines changed

source/re/ecs/storage.d

+18-12
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,9 @@ class ComponentStorage {
180180

181181
// - update storage
182182
auto storage = get_storage(id);
183-
writefln("REMOVING component_type: %s AT %d", to!string(id.type), id.index);
184-
writefln("storage[%d]: %s", storage.length, storage.array);
183+
auto storage_type = id.type;
184+
// writefln("REMOVING component_type: %s AT %d", to!string(id.type), id.index);
185+
// writefln("storage[%d]: %s", storage.length, storage.array);
185186

186187
// empty the slot, and swap it to the end
187188
assert(id.index < storage.length, format("id points to invalid position (%d) in %s storage",
@@ -197,18 +198,23 @@ class ComponentStorage {
197198
// handle swapping our nulled slot with the last slot
198199
auto tmp = storage[last_slot];
199200
assert(tmp, "storage tail slot is null");
200-
assert(tmp.entity, "entity in tail slot is null");
201+
assert(tmp.entity, "entity in tail slot is null. this means that"
202+
~ " the component doesn't have an assocated entity");
201203
storage[last_slot] = storage[id.index];
202204
storage[id.index] = tmp;
203-
writefln("swapped SLOT (%d) with TAIL (%d)", id.index, last_slot);
205+
// the entity we are removing has been swapped with the last slot
206+
// writefln("swapped SLOT (%d) with TAIL (%d)", id.index, last_slot);
204207
// find out who owns tmp, and tell them that their component has moved
205208
auto other = tmp.entity;
206209
// find the id that points to the old place
207-
auto other_id_pos = other.components.countUntil!(x => x.index == last_slot);
208-
writefln("working with OTHER, components %s", other.components);
209-
writefln("(%s) updating COMPREF from OLDSLOT (%d) to NEWSLOT (%d)", other.name, other
210-
.components[other_id_pos].index, id.index);
210+
// to find it, it has an index pointing to the last slot (because it was swapped, so the component was previously there)
211+
// we also need to be sure to be searching the correct storage type
212+
auto other_id_pos = other.components.countUntil!(x => x.type == storage_type && x.index == last_slot);
213+
// writefln("working with OTHER, components %s", other.components);
214+
// writefln("(%s) updating COMPREF from OLDSLOT (%d) to NEWSLOT (%d)", other.name, other
215+
// .components[other_id_pos].index, id.index);
211216
other.components[other_id_pos].index = id.index; // point to the new place
217+
// writefln("OTHER components result: %s", other.components);
212218
}
213219
// pop the last element off the array
214220
storage = storage.remove(last_slot);
@@ -280,23 +286,23 @@ unittest {
280286
writefln("all components: %s", storage.get_all(nt));
281287
auto thing1s = storage.get_all!Thing1(nt);
282288
assert(thing1s.length == 6, format("expected 6 thing1s, got %d", thing1s.length));
283-
writefln("match thing1s passed: %s", thing1s);
289+
// writefln("match thing1s passed: %s", thing1s);
284290

285291
auto thing2s = storage.get_all!Thing2(nt);
286292
assert(thing2s.length == 4, format("expected 4 thing2s, got %d", thing2s.length));
287-
writefln("match thing2s passed: %s", thing2s);
293+
// writefln("match thing2s passed: %s", thing2s);
288294

289295
// try removing random stuff
290296
manual_nt_remove(nt, thing13);
291297
manual_nt_remove(nt, thing11);
292298
thing1s = storage.get_all!Thing1(nt);
293299
assert(thing1s.length == 4, format("expected 4 thing1s, got %d", thing1s.length));
294-
writefln("expected-1 thing1s passed: %s", thing1s);
300+
// writefln("expected-1 thing1s passed: %s", thing1s);
295301

296302
manual_nt_remove(nt, thing22);
297303
thing2s = storage.get_all!Thing2(nt);
298304
assert(thing2s.length == 3, format("expected 3 thing2s, got %d", thing2s.length));
299-
writefln("expected-2 thing2s passed: %s", thing2s);
305+
// writefln("expected-2 thing2s passed: %s", thing2s);
300306
}
301307

302308
@("ecs-storage-types")

0 commit comments

Comments
 (0)