Skip to content

feat: add insert_if_new (#14397) #14646

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

Merged
merged 16 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,12 @@ impl ArchetypeId {
}
}

/// Used in [`AddBundle`] to track whether components in the bundle are newly
/// added or already existed in the entity's archetype.
#[derive(Copy, Clone, Eq, PartialEq)]
pub(crate) enum ComponentStatus {
Added,
Mutated,
Existing,
}

pub(crate) struct AddBundle {
Expand All @@ -122,7 +124,7 @@ pub(crate) struct AddBundle {
/// indicate if the component is newly added to the target archetype or if it already existed
pub bundle_status: Vec<ComponentStatus>,
pub added: Vec<ComponentId>,
pub mutated: Vec<ComponentId>,
pub existing: Vec<ComponentId>,
}

/// This trait is used to report the status of [`Bundle`](crate::bundle::Bundle) components
Expand Down Expand Up @@ -207,15 +209,15 @@ impl Edges {
archetype_id: ArchetypeId,
bundle_status: Vec<ComponentStatus>,
added: Vec<ComponentId>,
mutated: Vec<ComponentId>,
existing: Vec<ComponentId>,
) {
self.add_bundle.insert(
bundle_id,
AddBundle {
archetype_id,
bundle_status,
added,
mutated,
existing,
},
);
}
Expand Down
91 changes: 77 additions & 14 deletions crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,15 @@ impl SparseSetIndex for BundleId {
}
}

// What to do on insertion if component already exists
#[derive(Clone, Copy, Eq, PartialEq)]
pub(crate) enum InsertMode {
/// Any existing components of a matching type will be overwritten.
Replace,
/// Any existing components of a matching type will kept unchanged.
Keep,
}

/// Stores metadata associated with a specific type of [`Bundle`] for a given [`World`].
///
/// [`World`]: crate::world::World
Expand Down Expand Up @@ -410,6 +419,7 @@ impl BundleInfo {
table_row: TableRow,
change_tick: Tick,
bundle: T,
insert_mode: InsertMode,
#[cfg(feature = "track_change_detection")] caller: &'static Location<'static>,
) {
// NOTE: get_components calls this closure on each component in "bundle order".
Expand All @@ -425,8 +435,8 @@ impl BundleInfo {
unsafe { table.get_column_mut(component_id).debug_checked_unwrap() };
// SAFETY: bundle_component is a valid index for this bundle
let status = unsafe { bundle_component_status.get_status(bundle_component) };
match status {
ComponentStatus::Added => {
match (status, insert_mode) {
(ComponentStatus::Added, _) => {
column.initialize(
table_row,
component_ptr,
Expand All @@ -435,7 +445,7 @@ impl BundleInfo {
caller,
);
}
ComponentStatus::Mutated => {
(ComponentStatus::Existing, InsertMode::Replace) => {
column.replace(
table_row,
component_ptr,
Expand All @@ -444,6 +454,9 @@ impl BundleInfo {
caller,
);
}
(ComponentStatus::Existing, InsertMode::Keep) => {
column.drop(component_ptr);
}
}
}
StorageType::SparseSet => {
Expand Down Expand Up @@ -489,7 +502,7 @@ impl BundleInfo {
let current_archetype = &mut archetypes[archetype_id];
for component_id in self.component_ids.iter().cloned() {
if current_archetype.contains(component_id) {
bundle_status.push(ComponentStatus::Mutated);
bundle_status.push(ComponentStatus::Existing);
mutated.push(component_id);
} else {
bundle_status.push(ComponentStatus::Added);
Expand Down Expand Up @@ -692,6 +705,7 @@ impl<'w> BundleInserter<'w> {
entity: Entity,
location: EntityLocation,
bundle: T,
insert_mode: InsertMode,
#[cfg(feature = "track_change_detection")] caller: &'static core::panic::Location<'static>,
) -> EntityLocation {
let bundle_info = self.bundle_info.as_ref();
Expand All @@ -705,13 +719,15 @@ impl<'w> BundleInserter<'w> {
// SAFETY: Mutable references do not alias and will be dropped after this block
let mut deferred_world = self.world.into_deferred();

deferred_world.trigger_on_replace(
archetype,
entity,
add_bundle.mutated.iter().copied(),
);
if archetype.has_replace_observer() {
deferred_world.trigger_observers(ON_REPLACE, entity, &add_bundle.mutated);
if insert_mode == InsertMode::Replace {
deferred_world.trigger_on_replace(
archetype,
entity,
add_bundle.existing.iter().copied(),
);
if archetype.has_replace_observer() {
deferred_world.trigger_observers(ON_REPLACE, entity, &add_bundle.existing);
}
}
}

Expand All @@ -735,6 +751,7 @@ impl<'w> BundleInserter<'w> {
location.table_row,
self.change_tick,
bundle,
insert_mode,
#[cfg(feature = "track_change_detection")]
caller,
);
Expand Down Expand Up @@ -775,6 +792,7 @@ impl<'w> BundleInserter<'w> {
result.table_row,
self.change_tick,
bundle,
insert_mode,
#[cfg(feature = "track_change_detection")]
caller,
);
Expand Down Expand Up @@ -856,6 +874,7 @@ impl<'w> BundleInserter<'w> {
move_result.new_row,
self.change_tick,
bundle,
insert_mode,
#[cfg(feature = "track_change_detection")]
caller,
);
Expand All @@ -875,9 +894,34 @@ impl<'w> BundleInserter<'w> {
if new_archetype.has_add_observer() {
deferred_world.trigger_observers(ON_ADD, entity, &add_bundle.added);
}
deferred_world.trigger_on_insert(new_archetype, entity, bundle_info.iter_components());
if new_archetype.has_insert_observer() {
deferred_world.trigger_observers(ON_INSERT, entity, bundle_info.components());
match insert_mode {
InsertMode::Replace => {
// insert triggers for both new and existing components if we're replacing them
deferred_world.trigger_on_insert(
new_archetype,
entity,
bundle_info.iter_components(),
);
if new_archetype.has_insert_observer() {
deferred_world.trigger_observers(
ON_INSERT,
entity,
bundle_info.components(),
);
}
}
InsertMode::Keep => {
// insert triggers only for new components if we're not replacing them (since
// nothing is actually inserted).
deferred_world.trigger_on_insert(
new_archetype,
entity,
add_bundle.added.iter().cloned(),
);
if new_archetype.has_insert_observer() {
deferred_world.trigger_observers(ON_INSERT, entity, &add_bundle.added);
}
}
}
}

Expand Down Expand Up @@ -977,6 +1021,7 @@ impl<'w> BundleSpawner<'w> {
table_row,
self.change_tick,
bundle,
InsertMode::Replace,
#[cfg(feature = "track_change_detection")]
caller,
);
Expand Down Expand Up @@ -1230,6 +1275,9 @@ mod tests {
#[derive(Component)]
struct D;

#[derive(Component, Eq, PartialEq, Debug)]
struct V(&'static str); // component with a value

#[derive(Resource, Default)]
struct R(usize);

Expand Down Expand Up @@ -1302,6 +1350,7 @@ mod tests {
world.init_resource::<R>();
let mut entity = world.entity_mut(entity);
entity.insert(A);
entity.insert_if_new(A); // this will not trigger on_replace or on_insert
entity.flush();
assert_eq!(2, world.resource::<R>().0);
}
Expand Down Expand Up @@ -1371,4 +1420,18 @@ mod tests {
world.spawn(A).flush();
assert_eq!(4, world.resource::<R>().0);
}

#[test]
fn insert_if_new() {
let mut world = World::new();
let id = world.spawn(V("one")).id();
let mut entity = world.entity_mut(id);
entity.insert_if_new(V("two"));
entity.insert_if_new((A, V("three")));
entity.flush();
// should still contain "one"
let entity = world.entity(id);
assert!(entity.contains::<A>());
assert_eq!(entity.get(), Some(&V("one")));
}
}
8 changes: 8 additions & 0 deletions crates/bevy_ecs/src/storage/blob_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,14 @@ impl BlobVec {
}
}
}

/// Get the `drop` argument that was passed to `BlobVec::new`.
///
/// Callers can use this if they have a type-erased pointer of the correct
/// type to add to this [`BlobVec`], which they just want to drop instead.
pub fn get_drop(&self) -> Option<unsafe fn(OwningPtr<'_>)> {
self.drop
}
}

impl Drop for BlobVec {
Expand Down
14 changes: 14 additions & 0 deletions crates/bevy_ecs/src/storage/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,20 @@ impl Column {
}
}

/// Call [`drop`] on a value.
///
/// # Safety
/// `data` must point to the same type that this table stores, so the
/// correct drop function is called.
#[inline]
pub(crate) unsafe fn drop(&self, data: OwningPtr<'_>) {
if let Some(drop) = self.data.get_drop() {
// Safety: we're using the same drop fn that the BlobVec would
// if we inserted the data instead of dropping it.
unsafe { drop(data) }
}
}

/// Gets the current number of elements stored in the column.
#[inline]
pub fn len(&self) -> usize {
Expand Down
31 changes: 26 additions & 5 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use core::panic::Location;
use super::{Deferred, IntoObserverSystem, IntoSystem, RegisterSystem, Resource};
use crate::{
self as bevy_ecs,
bundle::Bundle,
bundle::{Bundle, InsertMode},
component::{ComponentId, ComponentInfo},
entity::{Entities, Entity},
event::Event,
Expand Down Expand Up @@ -895,6 +895,7 @@ impl EntityCommands<'_> {
/// Adds a [`Bundle`] of components to the entity.
///
/// This will overwrite any previous value(s) of the same component type.
/// See [`EntityCommands::insert_if_new`] to keep the old value instead.
///
/// # Panics
///
Expand Down Expand Up @@ -945,7 +946,24 @@ impl EntityCommands<'_> {
/// ```
#[track_caller]
pub fn insert(&mut self, bundle: impl Bundle) -> &mut Self {
self.add(insert(bundle))
self.add(insert(bundle, InsertMode::Replace))
}

/// Adds a [`Bundle`] of components to the entity without overwriting.
///
/// This is the same as [`EntityCommands::insert`], but in case of duplicate
/// components will leave the old values instead of replacing them with new
/// ones.
///
/// # 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`]
/// instead.
pub fn insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self {
self.add(insert(bundle, InsertMode::Keep))
}

/// Adds a dynamic component to an entity.
Expand Down Expand Up @@ -1044,7 +1062,7 @@ impl EntityCommands<'_> {
/// ```
#[track_caller]
pub fn try_insert(&mut self, bundle: impl Bundle) -> &mut Self {
self.add(try_insert(bundle))
self.add(try_insert(bundle, InsertMode::Replace))
}

/// Removes a [`Bundle`] of components from the entity.
Expand Down Expand Up @@ -1321,12 +1339,13 @@ fn despawn() -> impl EntityCommand {

/// An [`EntityCommand`] that adds the components in a [`Bundle`] to an entity.
#[track_caller]
fn insert<T: Bundle>(bundle: T) -> impl EntityCommand {
fn insert<T: Bundle>(bundle: T, mode: InsertMode) -> impl EntityCommand {
let caller = core::panic::Location::caller();
move |entity: Entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
entity.insert_with_caller(
bundle,
mode,
#[cfg(feature = "track_change_detection")]
caller,
);
Expand All @@ -1337,14 +1356,16 @@ fn insert<T: Bundle>(bundle: T) -> impl EntityCommand {
}

/// An [`EntityCommand`] that attempts to add the components in a [`Bundle`] to an entity.
/// Does nothing if the entity does not exist.
#[track_caller]
fn try_insert(bundle: impl Bundle) -> impl EntityCommand {
fn try_insert(bundle: impl Bundle, mode: InsertMode) -> impl EntityCommand {
#[cfg(feature = "track_change_detection")]
let caller = core::panic::Location::caller();
move |entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
entity.insert_with_caller(
bundle,
mode,
#[cfg(feature = "track_change_detection")]
caller,
);
Expand Down
Loading