bevy
bevy copied to clipboard
Add insert_by_id and try_insert_by_id to EntityCommands
Objective
- Allow queuing insertion of dynamic components to an existing entity
Solution
- Add
insert_by_id<T: Send + 'static>(commands: &mut EntityCommands, component_id: ComponentId, value: T)and thetry_insert_by_idcounterpart
Testing
TODO
- Did you test these changes? If so, how?
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything specific they need to know?
- If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
Alternatives
This PR is not feature-complete for dynamic components. In particular, it
- only supports one component
- only supports adding components with a known, sized type
These were not implemented because doing so would require enhancing CommandQueue to support pushing unsized commands (or equivalently, pushing commands with a buffer of data). Even so, the cost would not be transparent compared to the implementation in this PR, which simply captures the ComponentId and value: T into the command closure and can be easily memcpy'ed to the stack during execution. For example, to efficiently pass &[ComponentId] from the caller to the world, we would need to:
- Update
CommandQueue.bytesfromVec<MaybeUninit<u8>>toVec<MaybeUninit<usize>>so that it has the same alignment asComponentId(which probably needs to be made#[repr(transparent)]too) - After pushing the Command metadata, push padding bytes until the vec len is a multiple of
size_of::<usize>() - Store
components.len()in the data - memcpy the user-provided
&[ComponentId]toCommandQueue.bytes - During execution, round up the data pointer behind the
Commandto skip padding, then cast the pointer and consume it as a&[ComponentId]
The effort here seems unnecessarily high, unless someone else has such a requirement. At least for the use case I am working with, I only need a single known type, and if we need multiple components, we could always enhance this function to accept a [ComponentId; N].
I recommend enhancing the Bundle API in the long term to achieve this goal more elegantly.
Welcome, new contributor!
Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨
Not sure what would be appropriate for testing this PR. Any thoughts on adding an example for dynamic systems that include inserting dynamic components?
Any thoughts on adding an example for dynamic systems that include inserting dynamic components?
I would be very happy to see this. Keep it simple, but try and demonstrate a whole lifecycle: spawning, querying...
Regarding the comment on ComponentId world identity, any thoughts on using something like ghost-cell brand lifetime to identify the world at compile time? Checking the world at runtime might not be heavy, but it is just very unlikely to be useful considering that most applications shouldn't have more than one world at a time [citation needed].