I'm not sure just what was going on other than *other* components were
getting double-removed when the hierarchy reference component was
removed when the entity was being deleted. This might even prevent
issues with removing the hierarchy from an entity that's not being
deleted as the pre-invalidation prevents the removal from deleting the
entity.
It turns out that the fixes for other problems related to removing
hierarchy reference components fixed the problem moving the entity
invalidation fixed, and invalidating the entity late somehow broke the
sprite renderer (at least in glsl).
The hierarchy leak was particularly troublesome to fix, but now the
hierarchies get updated (and freed) automatically just by removing the
hierarchy reference component from the entity. I suspect there will be
issues with entities that are on multiple hierarchies, but I'll sort
that out later.
It turns out that the bsearch bug was hiding incorrect handling of
indices in the subpool beyond the last tracked subpool. In which case, a
correctly working bsearch correctly fails to find the range, but the
search can be skipped entirely.
When bubbling a component past an empty range, there's no need for any
actual movement other than adjusting the range itself, and doing so
corrupts the sparse/dense array relationship. Fixes a segfault when
hiding the deathmatch overlay (that resulted from the change to using
canvases).
I found I needed the subrange start as well as the end, but I liked that
the subpools themselves used only the end of the range, so switching to
just a unint32_t for the value and adding a function to return a tuple
made sense. I had kept the struct because I thought I might want to
store additional information (eg, the entity "owning" the subpool), but
found that I didn't need such information as the systems using subpools
that way would have access to the entity by other means.
Interestingly, the change found a bug in subpool creation: I really
don't know why things worked before, but they work better now :)
Subpools are for grouping components by some criterion. Any component
that has a rangeid callback will be grouped with other components that
return the same render id. Note that the ordering of components within a
group will be affected by adding a component into a group that comes
before that group (or removing a component).
Component pools can have multiple groups, added and removed dynamically,
but removing a group should (currently) be done only when empty.
While this does require an extra call after registering components, it
allows for multiple component sets (ie, sub-systems) to be registered
before the component pools are created. The base id for the registered
component set is returned so it can be passed to the subsystem as
needed.
There's now a main ecs.h file that includes the sub-system headers,
removing the need to explicitly include several header files, but the
sub-systems are a less cluttered.
This means that the component id used for hierarchy references must be
passed to Hierarchy_New and Hierarchy_Copy, but does all an entity to
have more than one hierarchy, which is useful for canvases (hierarchies
of views) in the 3d world (the canvas root would have a 3d hierarchy
reference and a 2d (view) hierarchy reference).
While simple component pools can be cleared simply by zeroing their
counts, ones that have a delete function need that function to be called
for all the components in the pool otherwise leaks can happen.
In the end, it was removal of the old entries that corrupted the parent
indices. Very nicely, most of the fixes involved removing code. Taking
advantage of the ECS to debug the hierarchies was fun, and the resulting
colorized entity names helped no end.
Even 37 objects is a lot, but it's a whole lot better than 180. Most
importantly, it reproduces the problem, which seems to be not all parent
indices getting updated. The child indices seem to be working nice, as
do the reference object indices (ie, the entity components). I suspect
its the parent indices getting corrupted that cause problems on the
second switch of the hud/sbar cvar as the parent indices are used to
find the child indices that need to be updated.
This improves the behavior of hierarchies when self-inserting, but nq's
sbar still crashes when trying to do so. However, its tree is a fair bit
more complex than the test case (that does pass now), so I need to try
to replicate the important parts of the tree with fewer objects (180 is
too many to work with).
As expected, reparenting a sub-hierarchy such that it (and possibly its
children) move up the arrays fails (this is why sbar needs to first
remove the sub-hierarchy then insert it).
Since test_build_hierarchy2 already tested removal of a sub-hierarchy
(once fixed), it seems test_build_hierarchy3 testing parenting within
the same hierarchy would be a good idea. Reparenting such that
everything moves to later in the arrays works nicely (not very
surprising).
Its updates to the various indices were out, but this was missed due to
the tests being wrong. I wonder if I got interrupted while working on
them last and just assumed the removals were correct. This improves
sbar's behavior, but it's still wrong when pulling the armory view out
of the inventory. Very unsure what's going on, but the various indices
look ok, as do the view positions.
Ugh, things were quite bad, it turns out. It seems a lot of trouble
would have been saved if these tests had worked (however, something is
still not quite right as views are out of place).
This is the bug that sbar found when pulling a sub-hierarchy out of a
larger hierarchy: child indices not getting updated correctly for later
siblings and any niece objects.
The hierarchy-specific tests from the transform tests have been moved
into the ecs tests and the transform tests renamed appropriately. As
part of the process, hierarchies can now have a null type (ie, no
additional components maintained by the hierarchy). This should make
sorting out the issues highlighted by sbar a bit easier.
It should have always been here, but when I first created the hierarchy
and transform objects, I didn't know where things would go. Having two
chunks of code for setting an entity's parent was too already too much,
and I expect to have other hierarchy types. Doesn't fix the issues
encountered with sbar, of course.
As the bookkeeping data is spread between three arrays, sorting a
component pool is not trivial and thus not something to duplicate around
the codebase.
As an implementation detail, inserting null hierarchies (src hierarchy
is null) is supported for ease of hierarchy construction from data. No
component data is copied, only the child and parent indices and counts
are updated.
While the libraries are probably getting a little out of hand, the
separation into its own directory is probably a good thing as an ECS
should not be tied to scenes. This should make the ECS more generally
useful.
