forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Change World::try_despawn and World::try_insert_batch to return Result (#17376)

Browse Source 
## Objective

Most `try` methods on `World` return a `Result`, but `try_despawn` and
`try_insert_batch` don't. Since Bevy's error handling is advancing,
these should be brought in line.

## Solution

- Added `TryDespawnError` and `TryInsertBatchError`.
- `try_despawn`, `try_insert_batch`, and `try_insert_batch_if_new` now
return their respective errors.
- Fixed slightly incorrect behavior in `try_insert_batch_with_caller`.
- The method was always meant to continue with the rest of the batch if
an entity was missing, but that only worked after the first entity; if
the first entity was missing, the method would exit early. This has been
resolved.

## Migration Guide
- `World::try_despawn` now returns a `Result` rather than a `bool`.
- `World::try_insert_batch` and `World::try_insert_batch_if_new` now
return a `Result` where they previously returned nothing.
This commit is contained in:
JaySpruce 2025-01-21 17:21:32 -06:00 committed by GitHub
parent 44ad3bf62b
commit fe24652cc0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 201 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
crates/bevy_ecs/src/lib.rs
View File

@ -1878,7 +1878,9 @@ mod tests {
let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))];
world.try_insert_batch(values);
let error = world.try_insert_batch(values).unwrap_err();
assert_eq!(e1, error.entities[0]);
assert_eq!(
world.get::<A>(e0),
@ -1900,7 +1902,9 @@ mod tests {
let values = vec![(e0, (A(1), B(0))), (e1, (A(0), B(1)))];
world.try_insert_batch_if_new(values);
let error = world.try_insert_batch_if_new(values).unwrap_err();
assert_eq!(e1, error.entities[0]);
assert_eq!(
world.get::<A>(e0),

35
crates/bevy_ecs/src/system/commands/command.rs
View File

@ -126,46 +126,27 @@ where
}
/// A [`Command`] that consumes an iterator to add a series of [`Bundles`](Bundle) to a set of entities.
/// If any entities do not exist in the world, this command will panic.
///
/// If any entities do not exist in the world, this command will return a
/// [`TryInsertBatchError`](crate::world::error::TryInsertBatchError).
///
/// This is more efficient than inserting the bundles individually.
#[track_caller]
pub fn insert_batch<I, B>(batch: I, mode: InsertMode) -> impl Command
pub fn insert_batch<I, B>(batch: I, insert_mode: InsertMode) -> impl Command<Result>
where
I: IntoIterator<Item = (Entity, B)> + Send + Sync + 'static,
B: Bundle,
{
#[cfg(feature = "track_location")]
let caller = Location::caller();
move |world: &mut World| {
world.insert_batch_with_caller(
batch,
mode,
#[cfg(feature = "track_location")]
caller,
);
}
}
/// A [`Command`] that consumes an iterator to add a series of [`Bundles`](Bundle) to a set of entities.
/// If any entities do not exist in the world, this command will ignore them.
///
/// This is more efficient than inserting the bundles individually.
#[track_caller]
pub fn try_insert_batch<I, B>(batch: I, mode: InsertMode) -> impl Command
where
I: IntoIterator<Item = (Entity, B)> + Send + Sync + 'static,
B: Bundle,
{
#[cfg(feature = "track_location")]
let caller = Location::caller();
move |world: &mut World| {
move |world: &mut World| -> Result {
world.try_insert_batch_with_caller(
batch,
mode,
insert_mode,
#[cfg(feature = "track_location")]
caller,
);
)?;
Ok(())
}
}

21
crates/bevy_ecs/src/system/commands/mod.rs
View File

@ -705,7 +705,7 @@ impl<'w, 's> Commands<'w, 's> {
/// Pushes a [`Command`] to the queue for adding a [`Bundle`] type to a batch of [`Entities`](Entity).
///
/// A batch can be any type that implements [`IntoIterator`] containing `(Entity, Bundle)` tuples,
/// such as a [`Vec<(Entity, Bundle)>`] or an array `[(Entity, Bundle); N]`.
/// such as a [`Vec<(Entity, Bundle)>`](alloc::vec::Vec) or an array `[(Entity, Bundle); N]`.
///
/// When the command is applied, for each `(Entity, Bundle)` pair in the given batch,
/// the `Bundle` is added to the `Entity`, overwriting any existing components shared by the `Bundle`.
@ -732,7 +732,7 @@ impl<'w, 's> Commands<'w, 's> {
/// Pushes a [`Command`] to the queue for adding a [`Bundle`] type to a batch of [`Entities`](Entity).
///
/// A batch can be any type that implements [`IntoIterator`] containing `(Entity, Bundle)` tuples,
/// such as a [`Vec<(Entity, Bundle)>`] or an array `[(Entity, Bundle); N]`.
/// such as a [`Vec<(Entity, Bundle)>`](alloc::vec::Vec) or an array `[(Entity, Bundle); N]`.
///
/// When the command is applied, for each `(Entity, Bundle)` pair in the given batch,
/// the `Bundle` is added to the `Entity`, except for any components already present on the `Entity`.
@ -759,7 +759,7 @@ impl<'w, 's> Commands<'w, 's> {
/// Pushes a [`Command`] to the queue for adding a [`Bundle`] type to a batch of [`Entities`](Entity).
///
/// A batch can be any type that implements [`IntoIterator`] containing `(Entity, Bundle)` tuples,
/// such as a [`Vec<(Entity, Bundle)>`] or an array `[(Entity, Bundle); N]`.
/// such as a [`Vec<(Entity, Bundle)>`](alloc::vec::Vec) or an array `[(Entity, Bundle); N]`.
///
/// When the command is applied, for each `(Entity, Bundle)` pair in the given batch,
/// the `Bundle` is added to the `Entity`, overwriting any existing components shared by the `Bundle`.
@ -769,7 +769,7 @@ impl<'w, 's> Commands<'w, 's> {
/// and passing the bundle to [`insert`](EntityCommands::insert),
/// but it is faster due to memory pre-allocation.
///
/// This command silently fails by ignoring any entities that do not exist.
/// This command will send a warning if any of the given entities do not exist.
///
/// For the panicking version, see [`insert_batch`](Self::insert_batch).
#[track_caller]
@ -778,13 +778,16 @@ impl<'w, 's> Commands<'w, 's> {
I: IntoIterator<Item = (Entity, B)> + Send + Sync + 'static,
B: Bundle,
{
self.queue(command::try_insert_batch(batch, InsertMode::Replace));
self.queue(
command::insert_batch(batch, InsertMode::Replace)
.handle_error_with(error_handler::warn()),
);
}
/// Pushes a [`Command`] to the queue for adding a [`Bundle`] type to a batch of [`Entities`](Entity).
///
/// A batch can be any type that implements [`IntoIterator`] containing `(Entity, Bundle)` tuples,
/// such as a [`Vec<(Entity, Bundle)>`] or an array `[(Entity, Bundle); N]`.
/// such as a [`Vec<(Entity, Bundle)>`](alloc::vec::Vec) or an array `[(Entity, Bundle); N]`.
///
/// When the command is applied, for each `(Entity, Bundle)` pair in the given batch,
/// the `Bundle` is added to the `Entity`, except for any components already present on the `Entity`.
@ -794,7 +797,7 @@ impl<'w, 's> Commands<'w, 's> {
/// and passing the bundle to [`insert_if_new`](EntityCommands::insert_if_new),
/// but it is faster due to memory pre-allocation.
///
/// This command silently fails by ignoring any entities that do not exist.
/// This command will send a warning if any of the given entities do not exist.
///
/// For the panicking version, see [`insert_batch_if_new`](Self::insert_batch_if_new).
#[track_caller]
@ -803,7 +806,9 @@ impl<'w, 's> Commands<'w, 's> {
I: IntoIterator<Item = (Entity, B)> + Send + Sync + 'static,
B: Bundle,
{
self.queue(command::try_insert_batch(batch, InsertMode::Keep));
self.queue(
command::insert_batch(batch, InsertMode::Keep).handle_error_with(error_handler::warn()),
);
}
/// Pushes a [`Command`] to the queue for inserting a [`Resource`] in the [`World`] with an inferred value.

27
crates/bevy_ecs/src/world/error.rs
View File

@ -1,5 +1,6 @@
//! Contains error types returned by bevy's schedule.
use alloc::vec::Vec;
use thiserror::Error;
use crate::{
@ -15,6 +16,32 @@ use crate::{
#[error("The schedule with the label {0:?} was not found.")]
pub struct TryRunScheduleError(pub InternedScheduleLabel);
/// The error type returned by [`World::try_despawn`] if the provided entity does not exist.
///
/// [`World::try_despawn`]: crate::world::World::try_despawn
#[derive(Error, Debug, Clone, Copy)]
#[error("Could not despawn the entity with ID {entity} because it {details}")]
pub struct TryDespawnError {
/// The entity's ID.
pub entity: Entity,
/// Details on why the entity does not exist, if available.
pub details: EntityDoesNotExistDetails,
}
/// The error type returned by [`World::try_insert_batch`] and [`World::try_insert_batch_if_new`]
/// if any of the provided entities do not exist.
///
/// [`World::try_insert_batch`]: crate::world::World::try_insert_batch
/// [`World::try_insert_batch_if_new`]: crate::world::World::try_insert_batch_if_new
#[derive(Error, Debug, Clone)]
#[error("Could not insert bundles of type {bundle_type} into the entities with the following IDs because they do not exist: {entities:?}")]
pub struct TryInsertBatchError {
/// The bundles' type name.
pub bundle_type: &'static str,
/// The IDs of the provided entities that do not exist.
pub entities: Vec<Entity>,
}
/// An error that occurs when dynamically retrieving components from an entity.
#[derive(Error, Debug, Clone, Copy, PartialEq, Eq)]
pub enum EntityComponentError {

249
crates/bevy_ecs/src/world/mod.rs
View File

@ -51,19 +51,22 @@ use crate::{
system::Commands,
world::{
command_queue::RawCommandQueue,
error::{EntityFetchError, TryRunScheduleError},
error::{EntityFetchError, TryDespawnError, TryInsertBatchError, TryRunScheduleError},
},
};
use alloc::{boxed::Box, vec::Vec};
use bevy_platform_support::sync::atomic::{AtomicU32, Ordering};
use bevy_ptr::{OwningPtr, Ptr};
use core::{any::TypeId, fmt, panic::Location};
use core::{any::TypeId, fmt};
use log::warn;
use unsafe_world_cell::{UnsafeEntityCell, UnsafeWorldCell};
#[cfg(feature = "track_location")]
use bevy_ptr::UnsafeCellDeref;
#[cfg(feature = "track_location")]
use core::panic::Location;
/// Stores and exposes operations on [entities](Entity), [components](Component), resources,
/// and their associated metadata.
///
@ -1264,9 +1267,11 @@ impl World {
Ok(result)
}
/// Despawns the given `entity`, if it exists. This will also remove all of the entity's
/// [`Component`]s. Returns `true` if the `entity` is successfully despawned and `false` if
/// the `entity` does not exist.
/// Despawns the given [`Entity`], if it exists. This will also remove all of the entity's
/// [`Components`](Component).
///
/// Returns `true` if the entity is successfully despawned and `false` if
/// the entity does not exist.
///
/// # Note
///
@ -1291,36 +1296,55 @@ impl World {
#[track_caller]
#[inline]
pub fn despawn(&mut self, entity: Entity) -> bool {
self.despawn_with_caller(entity, Location::caller(), true)
if let Err(error) = self.despawn_with_caller(
entity,
#[cfg(feature = "track_location")]
Location::caller(),
) {
warn!("{error}");
false
} else {
true
}
}
/// Performs the same function as [`Self::despawn`] but does not emit a warning if
/// the entity does not exist.
/// Despawns the given `entity`, if it exists. This will also remove all of the entity's
/// [`Components`](Component).
///
/// Returns a [`TryDespawnError`] if the entity does not exist.
///
/// # Note
///
/// This will also despawn the entities in any [`RelationshipTarget`](crate::relationship::RelationshipTarget) that is configured
/// to despawn descendants. For example, this will recursively despawn [`Children`](crate::hierarchy::Children).
#[track_caller]
#[inline]
pub fn try_despawn(&mut self, entity: Entity) -> bool {
self.despawn_with_caller(entity, Location::caller(), false)
pub fn try_despawn(&mut self, entity: Entity) -> Result<(), TryDespawnError> {
self.despawn_with_caller(
entity,
#[cfg(feature = "track_location")]
Location::caller(),
)
}
#[inline]
pub(crate) fn despawn_with_caller(
&mut self,
entity: Entity,
caller: &'static Location,
log_warning: bool,
) -> bool {
#[cfg(feature = "track_location")] caller: &'static Location,
) -> Result<(), TryDespawnError> {
self.flush();
if let Ok(entity) = self.get_entity_mut(entity) {
entity.despawn_with_caller(
#[cfg(feature = "track_location")]
caller,
);
true
Ok(())
} else {
if log_warning {
warn!("error[B0003]: {caller}: Could not despawn entity {entity}, which {}. See: https://bevyengine.org/learn/errors/b0003", self.entities.entity_does_not_exist_error_details(entity));
}
false
Err(TryDespawnError {
entity,
details: self.entities().entity_does_not_exist_error_details(entity),
})
}
}
@ -2344,7 +2368,7 @@ impl World {
///
/// This function will panic if any of the associated entities do not exist.
///
/// For the non-panicking version, see [`World::try_insert_batch`].
/// For the fallible version, see [`World::try_insert_batch`].
#[track_caller]
pub fn insert_batch<I, B>(&mut self, batch: I)
where
@ -2374,7 +2398,7 @@ impl World {
///
/// This function will panic if any of the associated entities do not exist.
///
/// For the non-panicking version, see [`World::try_insert_batch_if_new`].
/// For the fallible version, see [`World::try_insert_batch_if_new`].
#[track_caller]
pub fn insert_batch_if_new<I, B>(&mut self, batch: I)
where
@ -2395,12 +2419,10 @@ impl World {
/// This can be called by:
/// - [`World::insert_batch`]
/// - [`World::insert_batch_if_new`]
/// - [`Commands::insert_batch`]
/// - [`Commands::insert_batch_if_new`]
#[inline]
pub(crate) fn insert_batch_with_caller<I, B>(
&mut self,
iter: I,
batch: I,
insert_mode: InsertMode,
#[cfg(feature = "track_location")] caller: &'static Location,
) where
@ -2408,22 +2430,20 @@ impl World {
I::IntoIter: Iterator<Item = (Entity, B)>,
B: Bundle,
{
self.flush();
let change_tick = self.change_tick();
let bundle_id = self
.bundles
.register_info::<B>(&mut self.components, &mut self.storages);
struct InserterArchetypeCache<'w> {
inserter: BundleInserter<'w>,
archetype_id: ArchetypeId,
}
let mut batch = iter.into_iter();
self.flush();
let change_tick = self.change_tick();
let bundle_id = self
.bundles
.register_info::<B>(&mut self.components, &mut self.storages);
if let Some((first_entity, first_bundle)) = batch.next() {
let mut batch_iter = batch.into_iter();
if let Some((first_entity, first_bundle)) = batch_iter.next() {
if let Some(first_location) = self.entities().get(first_entity) {
let mut cache = InserterArchetypeCache {
// SAFETY: we initialized this bundle_id in `register_info`
@ -2449,7 +2469,7 @@ impl World {
)
};
for (entity, bundle) in batch {
for (entity, bundle) in batch_iter {
if let Some(location) = cache.inserter.entities().get(entity) {
if location.archetype_id != cache.archetype_id {
cache = InserterArchetypeCache {
@ -2496,11 +2516,11 @@ impl World {
/// This will overwrite any previous values of components shared by the `Bundle`.
/// See [`World::try_insert_batch_if_new`] to keep the old values instead.
///
/// This function silently fails by ignoring any entities that do not exist.
/// Returns a [`TryInsertBatchError`] if any of the provided entities do not exist.
///
/// For the panicking version, see [`World::insert_batch`].
#[track_caller]
pub fn try_insert_batch<I, B>(&mut self, batch: I)
pub fn try_insert_batch<I, B>(&mut self, batch: I) -> Result<(), TryInsertBatchError>
where
I: IntoIterator,
I::IntoIter: Iterator<Item = (Entity, B)>,
@ -2511,7 +2531,7 @@ impl World {
InsertMode::Replace,
#[cfg(feature = "track_location")]
Location::caller(),
);
)
}
/// For a given batch of ([`Entity`], [`Bundle`]) pairs,
/// adds the `Bundle` of components to each `Entity` without overwriting.
@ -2523,11 +2543,11 @@ impl World {
/// This is the same as [`World::try_insert_batch`], but in case of duplicate
/// components it will leave the old values instead of replacing them with new ones.
///
/// This function silently fails by ignoring any entities that do not exist.
/// Returns a [`TryInsertBatchError`] if any of the provided entities do not exist.
///
/// For the panicking version, see [`World::insert_batch_if_new`].
#[track_caller]
pub fn try_insert_batch_if_new<I, B>(&mut self, batch: I)
pub fn try_insert_batch_if_new<I, B>(&mut self, batch: I) -> Result<(), TryInsertBatchError>
where
I: IntoIterator,
I::IntoIter: Iterator<Item = (Entity, B)>,
@ -2538,7 +2558,7 @@ impl World {
InsertMode::Keep,
#[cfg(feature = "track_location")]
Location::caller(),
);
)
}
/// Split into a new function so we can differentiate the calling location.
@ -2546,91 +2566,116 @@ impl World {
/// This can be called by:
/// - [`World::try_insert_batch`]
/// - [`World::try_insert_batch_if_new`]
/// - [`Commands::insert_batch`]
/// - [`Commands::insert_batch_if_new`]
/// - [`Commands::try_insert_batch`]
/// - [`Commands::try_insert_batch_if_new`]
#[inline]
pub(crate) fn try_insert_batch_with_caller<I, B>(
&mut self,
iter: I,
batch: I,
insert_mode: InsertMode,
#[cfg(feature = "track_location")] caller: &'static Location,
) where
) -> Result<(), TryInsertBatchError>
where
I: IntoIterator,
I::IntoIter: Iterator<Item = (Entity, B)>,
B: Bundle,
{
self.flush();
let change_tick = self.change_tick();
let bundle_id = self
.bundles
.register_info::<B>(&mut self.components, &mut self.storages);
struct InserterArchetypeCache<'w> {
inserter: BundleInserter<'w>,
archetype_id: ArchetypeId,
}
let mut batch = iter.into_iter();
self.flush();
let change_tick = self.change_tick();
let bundle_id = self
.bundles
.register_info::<B>(&mut self.components, &mut self.storages);
if let Some((first_entity, first_bundle)) = batch.next() {
if let Some(first_location) = self.entities().get(first_entity) {
let mut cache = InserterArchetypeCache {
// SAFETY: we initialized this bundle_id in `register_info`
inserter: unsafe {
BundleInserter::new_with_id(
self,
first_location.archetype_id,
bundle_id,
change_tick,
)
},
archetype_id: first_location.archetype_id,
};
// SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter
unsafe {
cache.inserter.insert(
first_entity,
first_location,
first_bundle,
insert_mode,
#[cfg(feature = "track_location")]
caller,
)
};
let mut invalid_entities = Vec::<Entity>::new();
let mut batch_iter = batch.into_iter();
for (entity, bundle) in batch {
if let Some(location) = cache.inserter.entities().get(entity) {
if location.archetype_id != cache.archetype_id {
cache = InserterArchetypeCache {
// SAFETY: we initialized this bundle_id in `register_info`
inserter: unsafe {
BundleInserter::new_with_id(
self,
location.archetype_id,
bundle_id,
change_tick,
)
},
archetype_id: location.archetype_id,
}
}
// SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter
unsafe {
cache.inserter.insert(
entity,
location,
bundle,
insert_mode,
#[cfg(feature = "track_location")]
caller,
// We need to find the first valid entity so we can initialize the bundle inserter.
// This differs from `insert_batch_with_caller` because that method can just panic
// if the first entity is invalid, whereas this method needs to keep going.
let cache = loop {
if let Some((first_entity, first_bundle)) = batch_iter.next() {
if let Some(first_location) = self.entities().get(first_entity) {
let mut cache = InserterArchetypeCache {
// SAFETY: we initialized this bundle_id in `register_info`
inserter: unsafe {
BundleInserter::new_with_id(
self,
first_location.archetype_id,
bundle_id,
change_tick,
)
};
},
archetype_id: first_location.archetype_id,
};
// SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter
unsafe {
cache.inserter.insert(
first_entity,
first_location,
first_bundle,
insert_mode,
#[cfg(feature = "track_location")]
caller,
)
};
break Some(cache);
}
invalid_entities.push(first_entity);
} else {
// We reached the end of the entities the caller provided and none were valid.
break None;
}
};
if let Some(mut cache) = cache {
for (entity, bundle) in batch_iter {
if let Some(location) = cache.inserter.entities().get(entity) {
if location.archetype_id != cache.archetype_id {
cache = InserterArchetypeCache {
// SAFETY: we initialized this bundle_id in `register_info`
inserter: unsafe {
BundleInserter::new_with_id(
self,
location.archetype_id,
bundle_id,
change_tick,
)
},
archetype_id: location.archetype_id,
}
}
// SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter
unsafe {
cache.inserter.insert(
entity,
location,
bundle,
insert_mode,
#[cfg(feature = "track_location")]
caller,
)
};
} else {
invalid_entities.push(entity);
}
}
}
if invalid_entities.is_empty() {
Ok(())
} else {
Err(TryInsertBatchError {
bundle_type: core::any::type_name::<B>(),
entities: invalid_entities,
})
}
}
/// Temporarily removes the requested resource from this [`World`], runs custom user code,