Add insert_by_id and try_insert_by_id to EntityCommands (#14283)

# 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 the `try_insert_by_id`
counterpart

## 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:

1. Update `CommandQueue.bytes` from `Vec<MaybeUninit<u8>>` to
`Vec<MaybeUninit<usize>>` so that it has the same alignment as
`ComponentId` (which probably needs to be made `#[repr(transparent)]`
too)
2. After pushing the Command metadata, push padding bytes until the vec
len is a multiple of `size_of::<usize>()`
3. Store `components.len()` in the data
4. memcpy the user-provided `&[ComponentId]` to `CommandQueue.bytes`
5. During execution, round up the data pointer behind the `Command` to
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.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Felix Rath <felixm.rath@gmail.com>
This commit is contained in:
Jonathan Chan Kwan Yin 2024-07-16 07:29:13 +08:00 committed by GitHub
parent 73d7e89a18
commit e66cd484a7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -12,6 +12,7 @@ use crate::{
world::command_queue::RawCommandQueue,
world::{Command, CommandQueue, EntityWorldMut, FromWorld, World},
};
use bevy_ptr::OwningPtr;
use bevy_utils::tracing::{error, info};
pub use parallel_scope::*;
@ -937,6 +938,50 @@ impl EntityCommands<'_> {
self.add(insert(bundle))
}
/// Adds a dynamic component to an entity.
///
/// See [`EntityWorldMut::insert_by_id`] for more information.
///
/// # Panics
///
/// The command will panic when applied if the associated entity does not exist.
///
/// To avoid a panic in this case, use the command [`Self::try_insert_by_id`] instead.
///
/// # Safety
///
/// - [`ComponentId`] must be from the same world as `self`.
/// - `T` must have the same layout as the one passed during `component_id` creation.
pub unsafe fn insert_by_id<T: Send + 'static>(
&mut self,
component_id: ComponentId,
value: T,
) -> &mut Self {
// SAFETY: same invariants as parent call
self.add(unsafe {insert_by_id(component_id, value, move |entity| {
panic!("error[B0003]: Could not insert a component {component_id:?} (with type {}) for entity {entity:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/#b0003", std::any::type_name::<T>());
})});
self
}
/// Attempts to add a dynamic component to an entity.
///
/// See [`EntityWorldMut::insert_by_id`] for more information.
///
/// # Safety
///
/// - [`ComponentId`] must be from the same world as `self`.
/// - `T` must have the same layout as the one passed during `component_id` creation.
pub unsafe fn try_insert_by_id<T: Send + 'static>(
&mut self,
component_id: ComponentId,
value: T,
) -> &mut Self {
// SAFETY: same invariants as parent call
self.add(unsafe { insert_by_id(component_id, value, |_| {}) });
self
}
/// Tries to add a [`Bundle`] of components to the entity.
///
/// This will overwrite any previous value(s) of the same component type.
@ -1252,6 +1297,31 @@ fn try_insert(bundle: impl Bundle) -> impl EntityCommand {
}
}
/// An [`EntityCommand`] that attempts to add the dynamic component to an entity.
///
/// # Safety
///
/// - The returned `EntityCommand` must be queued for the world where `component_id` was created.
/// - `T` must be the type represented by `component_id`.
unsafe fn insert_by_id<T: Send + 'static>(
component_id: ComponentId,
value: T,
on_none_entity: impl FnOnce(Entity) + Send + 'static,
) -> impl EntityCommand {
move |entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
// SAFETY:
// - `component_id` safety is ensured by the caller
// - `ptr` is valid within the `make` block;
OwningPtr::make(value, |ptr| unsafe {
entity.insert_by_id(component_id, ptr);
});
} else {
on_none_entity(entity);
}
}
}
/// An [`EntityCommand`] that removes components from an entity.
/// For a [`Bundle`] type `T`, this will remove any components in the bundle.
/// Any components in the bundle that aren't found on the entity will be ignored.