Skip to content

Commit 8513a6d

Browse files
authored
relax return value of World::resource_scope (#22686)
# Objective Follow-up to #22290 Currently, when calling `resource_scope` or `try_resource_scope`, the function will panic or return `None`, respectively, if `World::clear_resources` is called from within the scope. This behavior was intentionally preserved in #22290 to reduce user-facing changes, however the behavior is not necessary, as, at the point where it becomes relevant, the user-provided scope closure has already completed successfully. Since we explicitly support methods such as `clear_resources`, we should treat them as valid to call, and avoid causing unnecessary knock-on errors. ## Testing A test added in #22290 was updated to reflect the new behavior.
1 parent d734e7f commit 8513a6d

File tree

2 files changed

+32
-36
lines changed

2 files changed

+32
-36
lines changed

crates/bevy_ecs/src/lib.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,8 +1604,6 @@ mod tests {
16041604
assert!(world.contains_resource::<ResA>());
16051605
}
16061606

1607-
// NOTE: this test is meant to validate the current behavior of `{try_}resource_scope` when resource metadata is cleared
1608-
// within the scope. future contributors who wish to change this behavior should feel free to delete this test.
16091607
#[test]
16101608
fn resource_scope_resources_cleared() {
16111609
let mut world = World::default();
@@ -1615,7 +1613,7 @@ mod tests {
16151613
assert!(!world.contains_resource::<ResA>());
16161614
world.clear_resources();
16171615
});
1618-
assert_eq!(r, None);
1616+
assert_eq!(r, Some(()));
16191617
assert!(!world.contains_resource::<ResA>());
16201618
}
16211619

crates/bevy_ecs/src/world/mod.rs

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2705,6 +2705,12 @@ impl World {
27052705
/// });
27062706
/// assert_eq!(world.get_resource::<A>().unwrap().0, 2);
27072707
/// ```
2708+
///
2709+
/// # Note
2710+
///
2711+
/// If the world's resource metadata is cleared within the scope, such as by calling
2712+
/// [`World::clear_resources`] or [`World::clear_all`], the resource will *not* be re-inserted
2713+
/// at the end of the scope.
27082714
#[track_caller]
27092715
pub fn resource_scope<R: Resource, U>(&mut self, f: impl FnOnce(&mut World, Mut<R>) -> U) -> U {
27102716
self.try_resource_scope(f)
@@ -2718,6 +2724,12 @@ impl World {
27182724
/// For more complex access patterns, consider using [`SystemState`](crate::system::SystemState).
27192725
///
27202726
/// See also [`resource_scope`](Self::resource_scope).
2727+
///
2728+
/// # Note
2729+
///
2730+
/// If the world's resource metadata is cleared within the scope, such as by calling
2731+
/// [`World::clear_resources`] or [`World::clear_all`], the resource will *not* be re-inserted
2732+
/// at the end of the scope.
27212733
pub fn try_resource_scope<R: Resource, U>(
27222734
&mut self,
27232735
f: impl FnOnce(&mut World, Mut<R>) -> U,
@@ -2742,7 +2754,6 @@ impl World {
27422754
value: ManuallyDrop<R>,
27432755
ticks: ComponentTicks,
27442756
caller: MaybeLocation,
2745-
was_successful: &'a mut bool,
27462757
}
27472758
impl<R> Drop for ReinsertGuard<'_, R> {
27482759
fn drop(&mut self) {
@@ -2796,44 +2807,31 @@ impl World {
27962807
resource_data.insert_with_ticks(ptr, self.ticks, self.caller);
27972808
}
27982809
});
2799-
2800-
*self.was_successful = true;
28012810
}
28022811
}
28032812

2804-
// used to track whether the guard's drop impl was able to successfully reinsert the value into the world.
2805-
// an alternative way to track success would be to have a separate `guard.apply()` method used
2806-
// in the happy path -- however this would require two code paths for panicking vs regular control flow
2807-
// which would have suboptimal codegen. `resource_scope` is a widely used primitive, both throughout the
2808-
// engine and in user code, so this measure is likely worth it.
2809-
let mut was_successful = false;
2810-
let result = {
2811-
let mut guard = ReinsertGuard {
2812-
world: self,
2813-
component_id,
2814-
value: ManuallyDrop::new(value),
2815-
ticks,
2816-
caller,
2817-
was_successful: &mut was_successful,
2818-
};
2819-
2820-
let value_mut = Mut {
2821-
value: &mut *guard.value,
2822-
ticks: ComponentTicksMut {
2823-
added: &mut guard.ticks.added,
2824-
changed: &mut guard.ticks.changed,
2825-
changed_by: guard.caller.as_mut(),
2826-
last_run: last_change_tick,
2827-
this_run: change_tick,
2828-
},
2829-
};
2830-
2831-
f(guard.world, value_mut)
2813+
let mut guard = ReinsertGuard {
2814+
world: self,
2815+
component_id,
2816+
value: ManuallyDrop::new(value),
2817+
ticks,
2818+
caller,
2819+
};
28322820

2833-
// guard's drop impl runs here
2821+
let value_mut = Mut {
2822+
value: &mut *guard.value,
2823+
ticks: ComponentTicksMut {
2824+
added: &mut guard.ticks.added,
2825+
changed: &mut guard.ticks.changed,
2826+
changed_by: guard.caller.as_mut(),
2827+
last_run: last_change_tick,
2828+
this_run: change_tick,
2829+
},
28342830
};
28352831

2836-
was_successful.then_some(result)
2832+
let result = f(guard.world, value_mut);
2833+
2834+
Some(result)
28372835
}
28382836

28392837
/// Writes a [`Message`].

0 commit comments

Comments
 (0)