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

Reserve Entity ID Range #11602

Open
DrewRidley opened this issue Jan 29, 2024 · 4 comments
Open

Reserve Entity ID Range #11602

DrewRidley opened this issue Jan 29, 2024 · 4 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible X-Controversial There is active debate or serious implications around merging this PR

Comments

@DrewRidley
Copy link

In networking, It's fairly common to synchronize some pieces of game state. Currently, almost all networking solutions use their own HashMap or other indirection to map entity IDs between a client and server. While this solution generally works, it adds an unnecessary layer of complexity and makes it harder to reason about networked logic.

I am proposing extending the bevy API to include a reserve_entities() method, or something to this effect. This would allow you to reserve a contiguous interval of entity IDs, and they would not be used by bevy. This would reduce indirection and complexity in writing networked games, and will likely have other non-networked benefits in other circumstances. The ability to have deterministic IDs is generally a useful pattern, and I think it has a lot of value in Bevy.

In the coming days I will begin working on a naive approach just to get some feedback, but I would love to hear other thoughts in the meantime.

@DrewRidley DrewRidley added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 29, 2024
@MrGVSV
Copy link
Member

MrGVSV commented Jan 29, 2024

Possibly related: #7335

@SkiFire13
Copy link
Contributor

A problem I see with this is that it would make it very easy to increase the amount of memory being used.

Currently there are a bunch of sparse sets indexed by entities' indexes, and their memory usage depends on the maximum index being used. So if for example you reserve a range of 1M entities when someone else reserves another entity they get an index bigger than 1M and those sparse sets will have to allocate space for 1M entities, even though there may exist a lot less.

This could be fixed by reserving many small ranges alternated with unreserved ranges, this way both normal entities and networked entities get to use small indexes, but this doesn't mean the footgun is not there.

@DrewRidley
Copy link
Author

Thanks for your input! I agree there is a lot of nuance to this issue, but I still think having some form of reservation capability would be useful. Perhaps the API can be constructed with this in mind.

It may sound unorthodox, but it might make sense to have the API reserve a fragmented 'slice' of the full range of indices like you suggested. ie, instead of reserving 0..10_000 you might reserve every odd ID, or maybe every 10 ids. The only downside of this I would see is that a potential server might only use ID 10,20,30,etc, and waste the entire range between each of these values.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events X-Controversial There is active debate or serious implications around merging this PR and removed S-Needs-Triage This issue needs to be labelled labels Jan 30, 2024
@james7132
Copy link
Member

james7132 commented Mar 5, 2024

You can technically already do this via the function World::entities and Entities::reserve_entities. This is explicitly made unsafe for a reasons. Should be 100% safe.

Nevermind, I think this might be unsound since external users do not have access to World::flush_entities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

No branches or pull requests

5 participants