Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose entity locations #303

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Expose entity locations #303

wants to merge 2 commits into from

Conversation

Ralith
Copy link
Owner

@Ralith Ralith commented Mar 12, 2023

Column-major serialization is desirable for compressibility and efficient processing. hecs provides APIs sufficient to allow column-major serialization of the entire world by walking its archetypes, but is less helpful when only a small subset of entities should be serialized at a time. This requirement arises e.g. in large streaming worlds such as hypermine, where entities will be serialized when their enclosing volume is unloaded to conserve memory.

I'm interested in feedback on the approach taken here, since it constrains future evolution somewhat. One less constraining (and less breaking) alternative would be to expose a fn get_archetype(&self, index: u32) -> &Archetype on World rather than exposing the entire slice, for example.

@Ralith Ralith force-pushed the entity-archetype branch from 1ccf805 to 3c862ac Compare March 12, 2023 20:58
Enables efficient random access of archetypes. Commits us to a dense,
contiguous table of archetypes, but gives downstream more flexibility.
@Ralith Ralith force-pushed the entity-archetype branch from 3c862ac to 46998ed Compare March 12, 2023 20:59
@Ralith Ralith force-pushed the entity-archetype branch from 46998ed to 7386c5b Compare March 13, 2023 03:11
@adamreichold
Copy link
Collaborator

I'm interested in feedback on the approach taken here, since it constrains future evolution somewhat. One less constraining (and less breaking) alternative would be to expose a fn get_archetype(&self, index: u32) -> &Archetype on World rather than exposing the entire slice, for example.

I think exposing that entities are organized into archetypes is already done so that part (pub struct Location) does not really change anything. (I guess it would also be a different ECS if that ever changed, even if the API stayed the same.)

As for exposing the slice, I suspect that storing the archetypes in a slice will always remain the sweet spot for fast component access even if additional index structures are added on top. Maybe you could add some language as to how long the returned Location values are guaranteed to stay valid though, so that you could in principle reorganize the archetypes vector in any call that modifies the components.

@Ralith
Copy link
Owner Author

Ralith commented Mar 13, 2023

I think exposing that entities are organized into archetypes is already done so that part (pub struct Location) does not really change anything.

Yeah; only new leak there is that u32 is sufficient to identify one, but I think that's fine.

I suspect that storing the archetypes in a slice will always remain the sweet spot for fast component access even if additional index structures are added on top

Yeah, that makes sense. There is still the possibility of interposing some structure like slab that's morally a slice but which doesn't expose a &[T], though. We could always hack around this be reimplementing said structure ourselves, but maybe that's not the best use of maintenance budget? Or maybe I'm overthinking this.

@adamreichold
Copy link
Collaborator

Yeah, that makes sense. There is still the possibility of interposing some structure like slab that's morally a slice but which doesn't expose a &[T], though. We could always hack around this be reimplementing said structure ourselves, but maybe that's not the best use of maintenance budget? Or maybe I'm overthinking this.

I suspect that if you have that many archetypes that you want to slab-allocate them, you are probably better off with an ECS that is not archetype-based. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants