Skip to content

Commit e7c14bd

Browse files
authored
Add EntityWorldMut::(try_)resource_scope (#20162)
# Objective Fixes #20139 ## Solution Implement the methods, and leverage them where applicable ## Testing Added unit tests --- ## Showcase ```rust let id = entity_world_mut.id(); let world = entity_world_mut.into_world_mut(); world.resource_scope::<_, _>(|world, res| { let entity_world_mut = world.entity_mut(id); /* ... */ }); ``` becomes ```rust entity_world_mut.resource_scope::<_, _>(|entity, res| { /* ... */ }); ```
1 parent d28d18e commit e7c14bd

File tree

5 files changed

+174
-111
lines changed

5 files changed

+174
-111
lines changed

crates/bevy_ecs/src/event/update.rs

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,16 @@ pub fn signal_event_update_system(signal: Option<ResMut<EventRegistry>>) {
3131

3232
/// A system that calls [`Events::update`](super::Events::update) on all registered [`Events`][super::Events] in the world.
3333
pub fn event_update_system(world: &mut World, mut last_change_tick: Local<Tick>) {
34-
if world.contains_resource::<EventRegistry>() {
35-
world.resource_scope(|world, mut registry: Mut<EventRegistry>| {
36-
registry.run_updates(world, *last_change_tick);
34+
world.try_resource_scope(|world, mut registry: Mut<EventRegistry>| {
35+
registry.run_updates(world, *last_change_tick);
3736

38-
registry.should_update = match registry.should_update {
39-
// If we're always updating, keep doing so.
40-
ShouldUpdateEvents::Always => ShouldUpdateEvents::Always,
41-
// Disable the system until signal_event_update_system runs again.
42-
ShouldUpdateEvents::Waiting | ShouldUpdateEvents::Ready => {
43-
ShouldUpdateEvents::Waiting
44-
}
45-
};
46-
});
47-
}
37+
registry.should_update = match registry.should_update {
38+
// If we're always updating, keep doing so.
39+
ShouldUpdateEvents::Always => ShouldUpdateEvents::Always,
40+
// Disable the system until signal_event_update_system runs again.
41+
ShouldUpdateEvents::Waiting | ShouldUpdateEvents::Ready => ShouldUpdateEvents::Waiting,
42+
};
43+
});
4844
*last_change_tick = world.change_tick();
4945
}
5046

crates/bevy_ecs/src/reflect/entity_commands.rs

Lines changed: 26 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
use crate::{
2-
entity::Entity,
32
prelude::Mut,
43
reflect::{AppTypeRegistry, ReflectBundle, ReflectComponent},
54
resource::Resource,
65
system::EntityCommands,
7-
world::{EntityWorldMut, World},
6+
world::EntityWorldMut,
87
};
98
use alloc::{borrow::Cow, boxed::Box};
109
use bevy_reflect::{PartialReflect, TypeRegistry};
@@ -22,7 +21,7 @@ pub trait ReflectCommandExt {
2221
/// - If [`AppTypeRegistry`] does not have the reflection data for the given
2322
/// [`Component`](crate::component::Component) or [`Bundle`](crate::bundle::Bundle).
2423
/// - If the component or bundle data is invalid. See [`PartialReflect::apply`] for further details.
25-
/// - If [`AppTypeRegistry`] is not present in the [`World`].
24+
/// - If [`AppTypeRegistry`] is not present in the [`World`](crate::world::World).
2625
///
2726
/// # Note
2827
///
@@ -92,11 +91,11 @@ pub trait ReflectCommandExt {
9291
///
9392
/// # Panics
9493
///
95-
/// - If the given [`Resource`] is not present in the [`World`].
94+
/// - If the given [`Resource`] is not present in the [`World`](crate::world::World).
9695
///
9796
/// # Note
9897
///
99-
/// - The given [`Resource`] is removed from the [`World`] before the command is applied.
98+
/// - The given [`Resource`] is removed from the [`World`](crate::world::World) before the command is applied.
10099
fn insert_reflect_with_registry<T: Resource + AsRef<TypeRegistry>>(
101100
&mut self,
102101
component: Box<dyn PartialReflect>,
@@ -214,23 +213,18 @@ impl<'w> EntityWorldMut<'w> {
214213
/// - If [`AppTypeRegistry`] does not have the reflection data for the given
215214
/// [`Component`](crate::component::Component) or [`Bundle`](crate::bundle::Bundle).
216215
/// - If the component or bundle data is invalid. See [`PartialReflect::apply`] for further details.
217-
/// - If [`AppTypeRegistry`] is not present in the [`World`].
216+
/// - If [`AppTypeRegistry`] is not present in the [`World`](crate::world::World).
218217
///
219218
/// # Note
220219
///
221220
/// Prefer to use the typed [`EntityWorldMut::insert`] if possible. Adding a reflected component
222221
/// is much slower.
223222
pub fn insert_reflect(&mut self, component: Box<dyn PartialReflect>) -> &mut Self {
224223
self.assert_not_despawned();
225-
let entity_id = self.id();
226-
self.world_scope(|world| {
227-
world.resource_scope(|world, registry: Mut<AppTypeRegistry>| {
228-
let type_registry = &registry.as_ref().read();
229-
insert_reflect_with_registry_ref(world, entity_id, type_registry, component);
230-
});
231-
world.flush();
224+
self.resource_scope(|entity, registry: Mut<AppTypeRegistry>| {
225+
let type_registry = &registry.as_ref().read();
226+
insert_reflect_with_registry_ref(entity, type_registry, component);
232227
});
233-
self.update_location();
234228
self
235229
}
236230

@@ -245,21 +239,16 @@ impl<'w> EntityWorldMut<'w> {
245239
/// - If the given [`Resource`] does not have the reflection data for the given
246240
/// [`Component`](crate::component::Component) or [`Bundle`](crate::bundle::Bundle).
247241
/// - If the component or bundle data is invalid. See [`PartialReflect::apply`] for further details.
248-
/// - If the given [`Resource`] is not present in the [`World`].
242+
/// - If the given [`Resource`] is not present in the [`World`](crate::world::World).
249243
pub fn insert_reflect_with_registry<T: Resource + AsRef<TypeRegistry>>(
250244
&mut self,
251245
component: Box<dyn PartialReflect>,
252246
) -> &mut Self {
253247
self.assert_not_despawned();
254-
let entity_id = self.id();
255-
self.world_scope(|world| {
256-
world.resource_scope(|world, registry: Mut<T>| {
257-
let type_registry = registry.as_ref().as_ref();
258-
insert_reflect_with_registry_ref(world, entity_id, type_registry, component);
259-
});
260-
world.flush();
248+
self.resource_scope(|entity, registry: Mut<T>| {
249+
let type_registry = registry.as_ref().as_ref();
250+
insert_reflect_with_registry_ref(entity, type_registry, component);
261251
});
262-
self.update_location();
263252
self
264253
}
265254

@@ -275,28 +264,18 @@ impl<'w> EntityWorldMut<'w> {
275264
/// # Panics
276265
///
277266
/// - If the entity has been despawned while this `EntityWorldMut` is still alive.
278-
/// - If [`AppTypeRegistry`] is not present in the [`World`].
267+
/// - If [`AppTypeRegistry`] is not present in the [`World`](crate::world::World).
279268
///
280269
/// # Note
281270
///
282271
/// Prefer to use the typed [`EntityCommands::remove`] if possible. Removing a reflected component
283272
/// is much slower.
284273
pub fn remove_reflect(&mut self, component_type_path: Cow<'static, str>) -> &mut Self {
285274
self.assert_not_despawned();
286-
let entity_id = self.id();
287-
self.world_scope(|world| {
288-
world.resource_scope(|world, registry: Mut<AppTypeRegistry>| {
289-
let type_registry = &registry.as_ref().read();
290-
remove_reflect_with_registry_ref(
291-
world,
292-
entity_id,
293-
type_registry,
294-
component_type_path,
295-
);
296-
});
297-
world.flush();
275+
self.resource_scope(|entity, registry: Mut<AppTypeRegistry>| {
276+
let type_registry = &registry.as_ref().read();
277+
remove_reflect_with_registry_ref(entity, type_registry, component_type_path);
298278
});
299-
self.update_location();
300279
self
301280
}
302281

@@ -313,75 +292,56 @@ impl<'w> EntityWorldMut<'w> {
313292
/// # Panics
314293
///
315294
/// - If the entity has been despawned while this `EntityWorldMut` is still alive.
316-
/// - If [`AppTypeRegistry`] is not present in the [`World`].
295+
/// - If [`AppTypeRegistry`] is not present in the [`World`](crate::world::World).
317296
pub fn remove_reflect_with_registry<T: Resource + AsRef<TypeRegistry>>(
318297
&mut self,
319298
component_type_path: Cow<'static, str>,
320299
) -> &mut Self {
321300
self.assert_not_despawned();
322-
let entity_id = self.id();
323-
self.world_scope(|world| {
324-
world.resource_scope(|world, registry: Mut<T>| {
325-
let type_registry = registry.as_ref().as_ref();
326-
remove_reflect_with_registry_ref(
327-
world,
328-
entity_id,
329-
type_registry,
330-
component_type_path,
331-
);
332-
});
333-
world.flush();
301+
self.resource_scope(|entity, registry: Mut<T>| {
302+
let type_registry = registry.as_ref().as_ref();
303+
remove_reflect_with_registry_ref(entity, type_registry, component_type_path);
334304
});
335-
self.update_location();
336305
self
337306
}
338307
}
339308

340309
/// Helper function to add a reflect component or bundle to a given entity
341310
fn insert_reflect_with_registry_ref(
342-
world: &mut World,
343-
entity: Entity,
311+
entity: &mut EntityWorldMut,
344312
type_registry: &TypeRegistry,
345313
component: Box<dyn PartialReflect>,
346314
) {
347315
let type_info = component
348316
.get_represented_type_info()
349317
.expect("component should represent a type.");
350318
let type_path = type_info.type_path();
351-
let Ok(mut entity) = world.get_entity_mut(entity) else {
352-
panic!("error[B0003]: Could not insert a reflected component (of type {type_path}) for entity {entity}, which {}. See: https://bevy.org/learn/errors/b0003",
353-
world.entities().entity_does_not_exist_error_details(entity));
354-
};
355319
let Some(type_registration) = type_registry.get(type_info.type_id()) else {
356320
panic!("`{type_path}` should be registered in type registry via `App::register_type<{type_path}>`");
357321
};
358322

359323
if let Some(reflect_component) = type_registration.data::<ReflectComponent>() {
360-
reflect_component.insert(&mut entity, component.as_partial_reflect(), type_registry);
324+
reflect_component.insert(entity, component.as_partial_reflect(), type_registry);
361325
} else if let Some(reflect_bundle) = type_registration.data::<ReflectBundle>() {
362-
reflect_bundle.insert(&mut entity, component.as_partial_reflect(), type_registry);
326+
reflect_bundle.insert(entity, component.as_partial_reflect(), type_registry);
363327
} else {
364328
panic!("`{type_path}` should have #[reflect(Component)] or #[reflect(Bundle)]");
365329
}
366330
}
367331

368332
/// Helper function to remove a reflect component or bundle from a given entity
369333
fn remove_reflect_with_registry_ref(
370-
world: &mut World,
371-
entity: Entity,
334+
entity: &mut EntityWorldMut,
372335
type_registry: &TypeRegistry,
373336
component_type_path: Cow<'static, str>,
374337
) {
375-
let Ok(mut entity) = world.get_entity_mut(entity) else {
376-
return;
377-
};
378338
let Some(type_registration) = type_registry.get_with_type_path(&component_type_path) else {
379339
return;
380340
};
381341
if let Some(reflect_component) = type_registration.data::<ReflectComponent>() {
382-
reflect_component.remove(&mut entity);
342+
reflect_component.remove(entity);
383343
} else if let Some(reflect_bundle) = type_registration.data::<ReflectBundle>() {
384-
reflect_bundle.remove(&mut entity);
344+
reflect_bundle.remove(entity);
385345
}
386346
}
387347

crates/bevy_ecs/src/world/entity_ref.rs

Lines changed: 106 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1547,6 +1547,50 @@ impl<'w> EntityWorldMut<'w> {
15471547
self.world.get_resource_mut()
15481548
}
15491549

1550+
/// Temporarily removes the requested resource from the [`World`], runs custom user code,
1551+
/// then re-adds the resource before returning.
1552+
///
1553+
/// # Panics
1554+
///
1555+
/// Panics if the resource does not exist.
1556+
/// Use [`try_resource_scope`](Self::try_resource_scope) instead if you want to handle this case.
1557+
///
1558+
/// See [`World::resource_scope`] for further details.
1559+
#[track_caller]
1560+
pub fn resource_scope<R: Resource, U>(
1561+
&mut self,
1562+
f: impl FnOnce(&mut EntityWorldMut, Mut<R>) -> U,
1563+
) -> U {
1564+
let id = self.id();
1565+
self.world_scope(|world| {
1566+
world.resource_scope(|world, res| {
1567+
// Acquiring a new EntityWorldMut here and using that instead of `self` is fine because
1568+
// the outer `world_scope` will handle updating our location if it gets changed by the user code
1569+
let mut this = world.entity_mut(id);
1570+
f(&mut this, res)
1571+
})
1572+
})
1573+
}
1574+
1575+
/// Temporarily removes the requested resource from the [`World`] if it exists, runs custom user code,
1576+
/// then re-adds the resource before returning. Returns `None` if the resource does not exist in the [`World`].
1577+
///
1578+
/// See [`World::try_resource_scope`] for further details.
1579+
pub fn try_resource_scope<R: Resource, U>(
1580+
&mut self,
1581+
f: impl FnOnce(&mut EntityWorldMut, Mut<R>) -> U,
1582+
) -> Option<U> {
1583+
let id = self.id();
1584+
self.world_scope(|world| {
1585+
world.try_resource_scope(|world, res| {
1586+
// Acquiring a new EntityWorldMut here and using that instead of `self` is fine because
1587+
// the outer `world_scope` will handle updating our location if it gets changed by the user code
1588+
let mut this = world.entity_mut(id);
1589+
f(&mut this, res)
1590+
})
1591+
})
1592+
}
1593+
15501594
/// Retrieves the change ticks for the given component. This can be useful for implementing change
15511595
/// detection in custom runtimes.
15521596
///
@@ -4959,6 +5003,46 @@ mod tests {
49595003
assert!(entity.get_mut_by_id(invalid_component_id).is_err());
49605004
}
49615005

5006+
#[derive(Resource)]
5007+
struct R(usize);
5008+
5009+
#[test]
5010+
fn entity_mut_resource_scope() {
5011+
// Keep in sync with the `resource_scope` test in lib.rs
5012+
let mut world = World::new();
5013+
let mut entity = world.spawn_empty();
5014+
5015+
assert!(entity.try_resource_scope::<R, _>(|_, _| {}).is_none());
5016+
entity.world_scope(|world| world.insert_resource(R(0)));
5017+
entity.resource_scope(|entity: &mut EntityWorldMut, mut value: Mut<R>| {
5018+
value.0 += 1;
5019+
assert!(!entity.world().contains_resource::<R>());
5020+
});
5021+
assert_eq!(entity.resource::<R>().0, 1);
5022+
}
5023+
5024+
#[test]
5025+
fn entity_mut_resource_scope_panic() {
5026+
let mut world = World::new();
5027+
world.insert_resource(R(0));
5028+
5029+
let mut entity = world.spawn_empty();
5030+
let old_location = entity.location();
5031+
let result = std::panic::catch_unwind(AssertUnwindSafe(|| {
5032+
entity.resource_scope(|entity: &mut EntityWorldMut, _: Mut<R>| {
5033+
// Change the entity's `EntityLocation`.
5034+
entity.insert(TestComponent(0));
5035+
5036+
// Ensure that the entity location still gets updated even in case of a panic.
5037+
panic!("this should get caught by the outer scope")
5038+
});
5039+
}));
5040+
assert!(result.is_err());
5041+
5042+
// Ensure that the location has been properly updated.
5043+
assert_ne!(entity.location(), old_location);
5044+
}
5045+
49625046
// regression test for https://github.com/bevyengine/bevy/pull/7387
49635047
#[test]
49645048
fn entity_mut_world_scope_panic() {
@@ -4983,6 +5067,28 @@ mod tests {
49835067
assert_ne!(entity.location(), old_location);
49845068
}
49855069

5070+
#[test]
5071+
fn entity_mut_reborrow_scope_panic() {
5072+
let mut world = World::new();
5073+
5074+
let mut entity = world.spawn_empty();
5075+
let old_location = entity.location();
5076+
let res = std::panic::catch_unwind(AssertUnwindSafe(|| {
5077+
entity.reborrow_scope(|mut entity| {
5078+
// Change the entity's `EntityLocation`, which invalidates the original `EntityWorldMut`.
5079+
// This will get updated at the end of the scope.
5080+
entity.insert(TestComponent(0));
5081+
5082+
// Ensure that the entity location still gets updated even in case of a panic.
5083+
panic!("this should get caught by the outer scope")
5084+
});
5085+
}));
5086+
assert!(res.is_err());
5087+
5088+
// Ensure that the location has been properly updated.
5089+
assert_ne!(entity.location(), old_location);
5090+
}
5091+
49865092
// regression test for https://github.com/bevyengine/bevy/pull/7805
49875093
#[test]
49885094
fn removing_sparse_updates_archetype_row() {
@@ -5420,9 +5526,6 @@ mod tests {
54205526
#[derive(Component)]
54215527
struct A;
54225528

5423-
#[derive(Resource)]
5424-
struct R;
5425-
54265529
#[test]
54275530
fn disjoint_access() {
54285531
fn disjoint_readonly(_: Query<EntityMut, With<A>>, _: Query<EntityRef, Without<A>>) {}

crates/bevy_ecs/src/world/mod.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2551,6 +2551,11 @@ impl World {
25512551
/// This enables safe simultaneous mutable access to both a resource and the rest of the [`World`].
25522552
/// For more complex access patterns, consider using [`SystemState`](crate::system::SystemState).
25532553
///
2554+
/// # Panics
2555+
///
2556+
/// Panics if the resource does not exist.
2557+
/// Use [`try_resource_scope`](Self::try_resource_scope) instead if you want to handle this case.
2558+
///
25542559
/// # Example
25552560
/// ```
25562561
/// use bevy_ecs::prelude::*;
@@ -2568,8 +2573,6 @@ impl World {
25682573
/// });
25692574
/// assert_eq!(world.get_resource::<A>().unwrap().0, 2);
25702575
/// ```
2571-
///
2572-
/// See also [`try_resource_scope`](Self::try_resource_scope).
25732576
#[track_caller]
25742577
pub fn resource_scope<R: Resource, U>(&mut self, f: impl FnOnce(&mut World, Mut<R>) -> U) -> U {
25752578
self.try_resource_scope(f)

0 commit comments

Comments
 (0)