Deprecate and remove insert_or_spawn_batch #15704
Labels
A-ECS
Entities, components, systems, and events
C-Usability
A targeted quality-of-life change that makes Bevy easier to use
D-Straightforward
Simple bug fixes and API improvements, docs, test and examples
M-Needs-Migration-Guide
A breaking change to Bevy's public API that needs to be noted in a migration guide
S-Ready-For-Implementation
This issue is ready for an implementation PR. Go for it!
X-Controversial
There is active debate or serious implications around merging this PR
Comments
alice-i-cecile
added
A-ECS
BenjaminBrienen
added
S-Ready-For-Implementation
|
#18054 showcases a serious performance footgun involved with these methods. |
alice-i-cecile
added
X-Controversial
github-merge-queue bot
pushed a commit
that referenced
this issue
Mar 6, 2025
## Objective `insert_or_spawn_batch` is due to be deprecated eventually (#15704), and removing uses internally will make that easier. ## Solution Replaced internal uses of `insert_or_spawn_batch` with `try_insert_batch` (non-panicking variant because `insert_or_spawn_batch` didn't panic). All of the internal uses are in rendering code. Since retained rendering was meant to get rid non-opaque entity IDs, I assume the code was just using `insert_or_spawn_batch` because `insert_batch` didn't exist and not because it actually wanted to spawn something. However, I am *not* confident in my ability to judge rendering code.
|
The deprecation notice could also point to this issue (maybe with a markdown link so people can click the issue number and come to the Github issue). |
|
Completed in #18148. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Just like in #15459 / #15652 for
get_or_spawn,insert_or_spawn_batchis an anti-pattern: it uses entity IDs in a non-opaque way, attempting to assign specific meaning to certain entities. With the retained rendering world in place, we should remove it to discourage this pattern and enforce the ECS invariants more strongly.#15702 should be merged first, to ensure that the migration path for the non-footgunny part of this API is smooth.
The text was updated successfully, but these errors were encountered: