-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Deprecate insert_or_spawn
function family
#18147
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
Deprecate insert_or_spawn
function family
#18147
Conversation
Mark this as deprecated so it's easier to find and remove later please :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you link the issue number in the deprecation notice please? I'd like to funnel affected users to it, and it's a much more compelling explanation than a bare assertion that this is slow.
Great idea! I've added the issue number. If you like, I'd be happy to change it to a full link too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Merging the prerequisite PR, then this. |
# Objective Based on and closes #18054, this PR builds on #18035 and #18147 to remove: - `Commands::insert_or_spawn_batch` - `Entities::alloc_at_without_replacement` - `Entities::alloc_at` - `entity::AllocAtWithoutReplacement` - `World::insert_or_spawn_batch` - `World::insert_or_spawn_batch_with_caller` ## Testing Just removing unused, deprecated code, so no new tests. Note that as of writing, #18035 is still under testing and review. ## Future Work Per [this](#18054 (comment)) comment on #18054, there may be additional performance improvements possible to the entity allocator now that `alloc_at` no longer is supported. At a glance, I don't see anything obvious to improve, but it may be worth further investigation in the future. --------- Co-authored-by: JaySpruce <[email protected]>
# Objective Based on and closes bevyengine#18054, this PR builds on bevyengine#18035 and bevyengine#18147 to remove: - `Commands::insert_or_spawn_batch` - `Entities::alloc_at_without_replacement` - `Entities::alloc_at` - `entity::AllocAtWithoutReplacement` - `World::insert_or_spawn_batch` - `World::insert_or_spawn_batch_with_caller` ## Testing Just removing unused, deprecated code, so no new tests. Note that as of writing, bevyengine#18035 is still under testing and review. ## Future Work Per [this](bevyengine#18054 (comment)) comment on bevyengine#18054, there may be additional performance improvements possible to the entity allocator now that `alloc_at` no longer is supported. At a glance, I don't see anything obvious to improve, but it may be worth further investigation in the future. --------- Co-authored-by: JaySpruce <[email protected]>
Objective
Based on #18054, this PR builds on #18035 to deprecate:
Commands::insert_or_spawn_batch
Entities::alloc_at_without_replacement
Entities::alloc_at
World::insert_or_spawn_batch
World::insert_or_spawn_batch_with_caller
Testing
Just deprecation, so no new tests. Note that as of writing #18035 is still under testing and review.
Open Questions
entity::AllocAtWithoutReplacement
be deprecated? It is internal and only used inEntities::alloc_at_without_replacement
. EDIT: Now deprecated.Migration Guide
The following functions have been deprecated:
Commands::insert_or_spawn_batch
World::insert_or_spawn_batch
World::insert_or_spawn_batch_with_caller
These functions, when used incorrectly, can cause major performance problems and are generally viewed as anti-patterns and foot guns. These are planned to be removed altogether in 0.17.
Instead of these functions consider doing one of the following:
Option A) Instead of despawing entities and re-spawning them at a particular id, insert the new
Disabled
component without despawning the entity, and usetry_insert_batch
orinsert_batch
and removeDisabled
instead of re-spawning it.Option B) Instead of giving special meaning to an entity id, simply use
spawn_batch
and ensure entity references are valid when despawning.