Skip to content

Commit

Permalink
Make get_resource (and friends) infallible (#4047)
Browse files Browse the repository at this point in the history
# Objective

- In the large majority of cases, users were calling `.unwrap()` immediately after `.get_resource`.
- Attempting to add more helpful error messages here resulted in endless manual boilerplate (see #3899 and the linked PRs).

## Solution

- Add an infallible variant named `.resource` and so on.
- Use these infallible variants over `.get_resource().unwrap()` across the code base.

## Notes

I did not provide equivalent methods on `WorldCell`, in favor of removing it entirely in #3939.

## Migration Guide

Infallible variants of `.get_resource` have been added that implicitly panic, rather than needing to be unwrapped.

Replace `world.get_resource::<Foo>().unwrap()` with `world.resource::<Foo>()`.

## Impact

- `.unwrap` search results before: 1084
- `.unwrap` search results after: 942
- internal `unwrap_or_else` calls added: 4
- trivial unwrap calls removed from tests and code: 146
- uses of the new `try_get_resource` API: 11
- percentage of the time the unwrapping API was used internally: 93%
  • Loading branch information
alice-i-cecile committed Feb 27, 2022
1 parent 44bf66e commit 557ab98
Show file tree
Hide file tree
Showing 52 changed files with 277 additions and 276 deletions.
5 changes: 1 addition & 4 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -839,10 +839,7 @@ impl App {
#[cfg(feature = "bevy_reflect")]
pub fn register_type<T: bevy_reflect::GetTypeRegistration>(&mut self) -> &mut Self {
{
let registry = self
.world
.get_resource_mut::<bevy_reflect::TypeRegistryArc>()
.unwrap();
let registry = self.world.resource_mut::<bevy_reflect::TypeRegistryArc>();
registry.write().register::<T>();
}
self
Expand Down
12 changes: 3 additions & 9 deletions crates/bevy_asset/src/asset_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -794,24 +794,18 @@ mod test {
app.add_system(update_asset_storage_system::<PngAsset>.after(FreeUnusedAssets));

fn load_asset(path: AssetPath, world: &World) -> HandleUntyped {
let asset_server = world.get_resource::<AssetServer>().unwrap();
let asset_server = world.resource::<AssetServer>();
let id = futures_lite::future::block_on(asset_server.load_async(path.clone(), true))
.unwrap();
asset_server.get_handle_untyped(id)
}

fn get_asset(id: impl Into<HandleId>, world: &World) -> Option<&PngAsset> {
world
.get_resource::<Assets<PngAsset>>()
.unwrap()
.get(id.into())
world.resource::<Assets<PngAsset>>().get(id.into())
}

fn get_load_state(id: impl Into<HandleId>, world: &World) -> LoadState {
world
.get_resource::<AssetServer>()
.unwrap()
.get_load_state(id.into())
world.resource::<AssetServer>().get_load_state(id.into())
}

// ---
Expand Down
30 changes: 9 additions & 21 deletions crates/bevy_asset/src/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ impl AddAsset for App {
return self;
}
let assets = {
let asset_server = self.world.get_resource::<AssetServer>().unwrap();
let asset_server = self.world.resource::<AssetServer>();
asset_server.register_asset_type::<T>()
};

Expand All @@ -307,8 +307,7 @@ impl AddAsset for App {
self.add_system(crate::debug_asset_server::sync_debug_assets::<T>);
let mut app = self
.world
.get_non_send_resource_mut::<crate::debug_asset_server::DebugAssetApp>()
.unwrap();
.non_send_resource_mut::<crate::debug_asset_server::DebugAssetApp>();
app.add_asset::<T>()
.init_resource::<crate::debug_asset_server::HandleMap<T>>();
}
Expand All @@ -331,8 +330,7 @@ impl AddAsset for App {
{
let mut app = self
.world
.get_non_send_resource_mut::<crate::debug_asset_server::DebugAssetApp>()
.unwrap();
.non_send_resource_mut::<crate::debug_asset_server::DebugAssetApp>();
app.init_asset_loader::<T>();
}
self
Expand All @@ -342,10 +340,7 @@ impl AddAsset for App {
where
T: AssetLoader,
{
self.world
.get_resource_mut::<AssetServer>()
.expect("AssetServer does not exist. Consider adding it as a resource.")
.add_loader(loader);
self.world.resource_mut::<AssetServer>().add_loader(loader);
self
}
}
Expand All @@ -357,8 +352,7 @@ macro_rules! load_internal_asset {
{
let mut debug_app = $app
.world
.get_non_send_resource_mut::<bevy_asset::debug_asset_server::DebugAssetApp>()
.unwrap();
.non_send_resource_mut::<bevy_asset::debug_asset_server::DebugAssetApp>();
bevy_asset::debug_asset_server::register_handle_with_loader(
$loader,
&mut debug_app,
Expand All @@ -367,10 +361,7 @@ macro_rules! load_internal_asset {
$path_str,
);
}
let mut assets = $app
.world
.get_resource_mut::<bevy_asset::Assets<_>>()
.unwrap();
let mut assets = $app.world.resource_mut::<bevy_asset::Assets<_>>();
assets.set_untracked($handle, ($loader)(include_str!($path_str)));
}};
}
Expand All @@ -379,10 +370,7 @@ macro_rules! load_internal_asset {
#[macro_export]
macro_rules! load_internal_asset {
($app: ident, $handle: ident, $path_str: expr, $loader: expr) => {{
let mut assets = $app
.world
.get_resource_mut::<bevy_asset::Assets<_>>()
.unwrap();
let mut assets = $app.world.resource_mut::<bevy_asset::Assets<_>>();
assets.set_untracked($handle, ($loader)(include_str!($path_str)));
}};
}
Expand All @@ -402,10 +390,10 @@ mod tests {
app.add_plugin(bevy_core::CorePlugin)
.add_plugin(crate::AssetPlugin);
app.add_asset::<MyAsset>();
let mut assets_before = app.world.get_resource_mut::<Assets<MyAsset>>().unwrap();
let mut assets_before = app.world.resource_mut::<Assets<MyAsset>>();
let handle = assets_before.add(MyAsset);
app.add_asset::<MyAsset>(); // Ensure this doesn't overwrite the Asset
let assets_after = app.world.get_resource_mut::<Assets<MyAsset>>().unwrap();
let assets_after = app.world.resource_mut::<Assets<MyAsset>>();
assert!(assets_after.get(handle).is_some());
}
}
9 changes: 2 additions & 7 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,8 @@ pub fn create_platform_default_asset_io(app: &mut App) -> Box<dyn AssetIo> {

impl Plugin for AssetPlugin {
fn build(&self, app: &mut App) {
if app.world.get_resource::<AssetServer>().is_none() {
let task_pool = app
.world
.get_resource::<IoTaskPool>()
.expect("`IoTaskPool` resource not found.")
.0
.clone();
if !app.world.contains_resource::<AssetServer>() {
let task_pool = app.world.resource::<IoTaskPool>().0.clone();

let source = create_platform_default_asset_io(app);

Expand Down
16 changes: 7 additions & 9 deletions crates/bevy_core/src/time/fixed_timestep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ impl System for FixedTimestep {
)));
self.internal_system.initialize(world);
if let Some(ref label) = self.state.label {
let mut fixed_timesteps = world.get_resource_mut::<FixedTimesteps>().unwrap();
let mut fixed_timesteps = world.resource_mut::<FixedTimesteps>();
fixed_timesteps.fixed_timesteps.insert(
label.clone(),
FixedTimestepState {
Expand Down Expand Up @@ -257,25 +257,25 @@ mod test {
// if time does not progress, the step does not run
schedule.run(&mut world);
schedule.run(&mut world);
assert_eq!(0, *world.get_resource::<Count>().unwrap());
assert_eq!(0, *world.resource::<Count>());
assert_eq!(0., get_accumulator_deciseconds(&world));

// let's progress less than one step
advance_time(&mut world, instance, 0.4);
schedule.run(&mut world);
assert_eq!(0, *world.get_resource::<Count>().unwrap());
assert_eq!(0, *world.resource::<Count>());
assert_eq!(4., get_accumulator_deciseconds(&world));

// finish the first step with 0.1s above the step length
advance_time(&mut world, instance, 0.6);
schedule.run(&mut world);
assert_eq!(1, *world.get_resource::<Count>().unwrap());
assert_eq!(1, *world.resource::<Count>());
assert_eq!(1., get_accumulator_deciseconds(&world));

// runs multiple times if the delta is multiple step lengths
advance_time(&mut world, instance, 1.7);
schedule.run(&mut world);
assert_eq!(3, *world.get_resource::<Count>().unwrap());
assert_eq!(3, *world.resource::<Count>());
assert_eq!(2., get_accumulator_deciseconds(&world));
}

Expand All @@ -285,15 +285,13 @@ mod test {

fn advance_time(world: &mut World, instance: Instant, seconds: f32) {
world
.get_resource_mut::<Time>()
.unwrap()
.resource_mut::<Time>()
.update_with_instant(instance.add(Duration::from_secs_f32(seconds)));
}

fn get_accumulator_deciseconds(world: &World) -> f64 {
world
.get_resource::<FixedTimesteps>()
.unwrap()
.resource::<FixedTimesteps>()
.get(LABEL)
.unwrap()
.accumulator
Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_core_pipeline/src/clear_pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ impl Node for ClearPassNode {
world: &World,
) -> Result<(), NodeRunError> {
let mut cleared_targets = HashSet::new();
let clear_color = world.get_resource::<ClearColor>().unwrap();
let render_target_clear_colors = world.get_resource::<RenderTargetClearColors>().unwrap();
let clear_color = world.resource::<ClearColor>();
let render_target_clear_colors = world.resource::<RenderTargetClearColors>();

// This gets all ViewTargets and ViewDepthTextures and clears its attachments
// TODO: This has the potential to clear the same target multiple times, if there
Expand Down Expand Up @@ -89,8 +89,8 @@ impl Node for ClearPassNode {
// TODO: This is a hack to ensure we don't call present() on frames without any work,
// which will cause panics. The real fix here is to clear "render targets" directly
// instead of "views". This should be removed once full RenderTargets are implemented.
let windows = world.get_resource::<ExtractedWindows>().unwrap();
let images = world.get_resource::<RenderAssets<Image>>().unwrap();
let windows = world.resource::<ExtractedWindows>();
let images = world.resource::<RenderAssets<Image>>();
for target in render_target_clear_colors.colors.keys().cloned().chain(
windows
.values()
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl Plugin for CorePipelinePlugin {
let clear_pass_node = ClearPassNode::new(&mut render_app.world);
let pass_node_2d = MainPass2dNode::new(&mut render_app.world);
let pass_node_3d = MainPass3dNode::new(&mut render_app.world);
let mut graph = render_app.world.get_resource_mut::<RenderGraph>().unwrap();
let mut graph = render_app.world.resource_mut::<RenderGraph>();

let mut draw_2d_graph = RenderGraph::default();
draw_2d_graph.add_node(draw_2d_graph::node::MAIN_PASS, pass_node_2d);
Expand Down
4 changes: 1 addition & 3 deletions crates/bevy_core_pipeline/src/main_pass_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ impl Node for MainPass2dNode {
depth_stencil_attachment: None,
};

let draw_functions = world
.get_resource::<DrawFunctions<Transparent2d>>()
.unwrap();
let draw_functions = world.resource::<DrawFunctions<Transparent2d>>();

let render_pass = render_context
.command_encoder
Expand Down
8 changes: 3 additions & 5 deletions crates/bevy_core_pipeline/src/main_pass_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl Node for MainPass3dNode {
}),
};

let draw_functions = world.get_resource::<DrawFunctions<Opaque3d>>().unwrap();
let draw_functions = world.resource::<DrawFunctions<Opaque3d>>();

let render_pass = render_context
.command_encoder
Expand Down Expand Up @@ -109,7 +109,7 @@ impl Node for MainPass3dNode {
}),
};

let draw_functions = world.get_resource::<DrawFunctions<AlphaMask3d>>().unwrap();
let draw_functions = world.resource::<DrawFunctions<AlphaMask3d>>();

let render_pass = render_context
.command_encoder
Expand Down Expand Up @@ -145,9 +145,7 @@ impl Node for MainPass3dNode {
}),
};

let draw_functions = world
.get_resource::<DrawFunctions<Transparent3d>>()
.unwrap();
let draw_functions = world.resource::<DrawFunctions<Transparent3d>>();

let render_pass = render_context
.command_encoder
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/main_pass_driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ impl Node for MainPassDriverNode {
_render_context: &mut RenderContext,
world: &World,
) -> Result<(), NodeRunError> {
let extracted_cameras = world.get_resource::<ExtractedCameraNames>().unwrap();
let extracted_cameras = world.resource::<ExtractedCameraNames>();
if let Some(camera_2d) = extracted_cameras.entities.get(CameraPlugin::CAMERA_2D) {
graph.run_sub_graph(
crate::draw_2d_graph::NAME,
Expand Down
25 changes: 11 additions & 14 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1010,29 +1010,26 @@ mod tests {
.get_archetype_component_id(resource_id)
.unwrap();

assert_eq!(*world.get_resource::<i32>().expect("resource exists"), 123);
assert_eq!(*world.resource::<i32>(), 123);
assert!(world.contains_resource::<i32>());
assert!(world.is_resource_added::<i32>());
assert!(world.is_resource_changed::<i32>());

world.insert_resource(456u64);
assert_eq!(
*world.get_resource::<u64>().expect("resource exists"),
456u64
);
assert_eq!(*world.resource::<u64>(), 456u64);

world.insert_resource(789u64);
assert_eq!(*world.get_resource::<u64>().expect("resource exists"), 789);
assert_eq!(*world.resource::<u64>(), 789);

{
let mut value = world.get_resource_mut::<u64>().expect("resource exists");
let mut value = world.resource_mut::<u64>();
assert_eq!(*value, 789);
*value = 10;
}

assert_eq!(
world.get_resource::<u64>(),
Some(&10),
world.resource::<u64>(),
&10,
"resource changes are preserved"
);

Expand Down Expand Up @@ -1183,8 +1180,8 @@ mod tests {
let mut world = World::default();
world.insert_non_send_resource(123i32);
world.insert_non_send_resource(456i64);
assert_eq!(*world.get_non_send_resource::<i32>().unwrap(), 123);
assert_eq!(*world.get_non_send_resource_mut::<i64>().unwrap(), 456);
assert_eq!(*world.non_send_resource::<i32>(), 123);
assert_eq!(*world.non_send_resource_mut::<i64>(), 456);
}

#[test]
Expand All @@ -1193,7 +1190,7 @@ mod tests {
let mut world = World::default();
world.insert_non_send_resource(0i32);
std::thread::spawn(move || {
let _ = world.get_non_send_resource_mut::<i32>();
let _ = world.non_send_resource_mut::<i32>();
})
.join()
.unwrap();
Expand Down Expand Up @@ -1323,7 +1320,7 @@ mod tests {
*value += 1;
assert!(!world.contains_resource::<i32>());
});
assert_eq!(*world.get_resource::<i32>().unwrap(), 1);
assert_eq!(*world.resource::<i32>(), 1);
}

#[test]
Expand Down Expand Up @@ -1389,7 +1386,7 @@ mod tests {
"world should not have any entities"
);
assert_eq!(
*world.get_resource::<i32>().unwrap(),
*world.resource::<i32>(),
0,
"world should still contain resources"
);
Expand Down
6 changes: 1 addition & 5 deletions crates/bevy_ecs/src/schedule/executor_parallel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,7 @@ mod tests {

fn receive_events(world: &World) -> Vec<SchedulingEvent> {
let mut events = Vec::new();
while let Ok(event) = world
.get_resource::<Receiver<SchedulingEvent>>()
.unwrap()
.try_recv()
{
while let Ok(event) = world.resource::<Receiver<SchedulingEvent>>().try_recv() {
events.push(event);
}
events
Expand Down
Loading

0 comments on commit 557ab98

Please sign in to comment.