Skip to content

Commit

Permalink
Issue 279 remove unnecessary unwraps (#292)
Browse files Browse the repository at this point in the history
* addresses: resource => get_resource and resource_mut => get_resource_mut
* addresses critical unwraps
  • Loading branch information
naomijub authored Apr 7, 2024
1 parent b31471b commit 7d78b9b
Show file tree
Hide file tree
Showing 19 changed files with 201 additions and 59 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ jobs:
- name: Install stable@stable toolchain
uses: dtolnay/rust-toolchain@stable
- name: Run cargo test
run: cargo test --workspace --all-features --profile=ci
run: cargo test --workspace --all-features --release

# Run cargo clippy -- -D warnings
clippy_check:
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ backtrace = ["backtrace-on-stack-overflow"]
bevy_xpbd_3d = ["dep:space_bevy_xpbd_plugin"]
persistence_editor = []
no_event_registration = ["space_prefab/no_event_registration"]
default = ["bevy_xpbd_3d", "persistence_editor", "bevy/dynamic_linking"]
default = ["bevy_xpbd_3d", "persistence_editor", "bevy/dynamic_linking", "space_prefab/editor"]

[[example]]
name = "platformer"
Expand Down
45 changes: 39 additions & 6 deletions crates/editor_core/src/gltf_unpack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ struct UnpackContext<'a> {

fn unpack_gltf(world: &mut World) {
let loaded_scenes = {
let mut events = world.resource_mut::<Events<GltfLoaded>>();
let Some(mut events) = world.get_resource_mut::<Events<GltfLoaded>>() else {
return;

Check warning on line 82 in crates/editor_core/src/gltf_unpack.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/gltf_unpack.rs#L81-L82

Added lines #L81 - L82 were not covered by tests
};
let mut reader = events.get_reader();
let loaded = reader.read(&events).cloned().collect::<Vec<GltfLoaded>>();
events.clear();
Expand All @@ -95,15 +97,44 @@ fn unpack_gltf(world: &mut World) {
};
info!("Path: {:?}", &gltf_path);

let Some(gltf) = world.resource::<Assets<Gltf>>().get(&gltf.0) else {
let Some(gltf) = world
.get_resource::<Assets<Gltf>>()
.and_then(|gltfs| gltfs.get(&gltf.0))

Check warning on line 102 in crates/editor_core/src/gltf_unpack.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/gltf_unpack.rs#L100-L102

Added lines #L100 - L102 were not covered by tests
else {
#[cfg(feature = "editor")]
world.send_event(space_shared::toast::ToastMessage::new(
"Gltf asset not found or empty",
space_shared::toast::ToastKind::Error,
));
continue;
};

let mut commands = Commands::new(&mut command_queue, world);

let gltf_nodes = world.resource::<Assets<GltfNode>>();
let gltf_meshs = world.resource::<Assets<GltfMesh>>();
let scenes = world.resource::<Assets<Scene>>();
let Some(gltf_nodes) = world.get_resource::<Assets<GltfNode>>() else {

Check warning on line 114 in crates/editor_core/src/gltf_unpack.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/gltf_unpack.rs#L114

Added line #L114 was not covered by tests
#[cfg(feature = "editor")]
world.send_event(space_shared::toast::ToastMessage::new(
"Gltf Node asset not found",
space_shared::toast::ToastKind::Error,
));
continue;

Check warning on line 120 in crates/editor_core/src/gltf_unpack.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/gltf_unpack.rs#L120

Added line #L120 was not covered by tests
};
let Some(gltf_meshs) = world.get_resource::<Assets<GltfMesh>>() else {

Check warning on line 122 in crates/editor_core/src/gltf_unpack.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/gltf_unpack.rs#L122

Added line #L122 was not covered by tests
#[cfg(feature = "editor")]
world.send_event(space_shared::toast::ToastMessage::new(
"Gltf Mesh asset not found",
space_shared::toast::ToastKind::Error,
));
continue;

Check warning on line 128 in crates/editor_core/src/gltf_unpack.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/gltf_unpack.rs#L128

Added line #L128 was not covered by tests
};
let Some(scenes) = world.get_resource::<Assets<Scene>>() else {

Check warning on line 130 in crates/editor_core/src/gltf_unpack.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/gltf_unpack.rs#L130

Added line #L130 was not covered by tests
#[cfg(feature = "editor")]
world.send_event(space_shared::toast::ToastMessage::new(
"Scene asset not found",
space_shared::toast::ToastKind::Error,
));
continue;

Check warning on line 136 in crates/editor_core/src/gltf_unpack.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/gltf_unpack.rs#L136

Added line #L136 was not covered by tests
};

let mut mesh_map = HashMap::new();
for idx in 0..gltf.meshes.len() {
Expand All @@ -125,7 +156,9 @@ fn unpack_gltf(world: &mut World) {
let mut roots = vec![];
for e in scene.world.iter_entities() {
if !e.contains::<Parent>() && e.contains::<Children>() {
let children = e.get::<Children>().unwrap();
let Some(children) = e.get::<Children>() else {
continue;

Check warning on line 160 in crates/editor_core/src/gltf_unpack.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/gltf_unpack.rs#L159-L160

Added lines #L159 - L160 were not covered by tests
};
for child in children.iter() {
if let Some(name) = scene.world.entity(*child).get::<Name>() {
info!("Name: {:?}", &name);
Expand Down
6 changes: 5 additions & 1 deletion crates/editor_core/src/hotkeys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ impl HotkeyAppExt for App {
self.register_type::<HashMap<T, Vec<KeyCode>>>();
self.register_type::<T>();
self.world
// Safe, was injected in this function
.resource_mut::<AllHotkeys>()
.mappers
.push(Box::new(|w, map_fun| {
Expand All @@ -141,6 +142,7 @@ impl HotkeyAppExt for App {
}));

self.world
// Safe, was injected in this function
.resource_mut::<AllHotkeys>()
.global_mapper
.push(Box::new(|w, map_fun| {
Expand All @@ -150,7 +152,9 @@ impl HotkeyAppExt for App {
}))
}

let mut set = self.world.get_resource_mut::<HotkeySet<T>>().unwrap();
let Some(mut set) = self.world.get_resource_mut::<HotkeySet<T>>() else {
return self;

Check warning on line 156 in crates/editor_core/src/hotkeys.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/hotkeys.rs#L156

Added line #L156 was not covered by tests
};
set.bindings.insert(key, binding);
self
}
Expand Down
19 changes: 16 additions & 3 deletions crates/editor_core/src/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ pub fn load_listener(world: &mut World) {
let assets = world.resource::<Assets<DynamicScene>>();
if let Some(scene) = &load_server.scene {
if let Some(scene) = assets.get(scene) {
let mut scene = Scene::from_dynamic_scene(scene, &app_registry).unwrap();
let Ok(mut scene) = Scene::from_dynamic_scene(scene, &app_registry) else {
return;

Check warning on line 15 in crates/editor_core/src/load.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/load.rs#L14-L15

Added lines #L14 - L15 were not covered by tests
};
scene.world.insert_resource(app_registry);
prefab = DynamicScene::from_scene(&scene); //kill me, is it clone() analog for DynamicScene
} else {
Expand All @@ -21,7 +23,14 @@ pub fn load_listener(world: &mut World) {
return;
}
}
world.resource_mut::<EditorLoader>().scene = None;
let Some(mut editor_loader) = world.get_resource_mut::<EditorLoader>() else {
world.send_event(ToastMessage::new(
"Failed to get prefab loader",
egui_toast::ToastKind::Error,
));
return;

Check warning on line 31 in crates/editor_core/src/load.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/load.rs#L26-L31

Added lines #L26 - L31 were not covered by tests
};
editor_loader.scene = None;

Check warning on line 33 in crates/editor_core/src/load.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/load.rs#L33

Added line #L33 was not covered by tests

let mut query = world.query_filtered::<(Entity, Option<&Name>), With<PrefabMarker>>();
let mark_to_delete: Vec<_> = query
Expand All @@ -38,7 +47,11 @@ pub fn load_listener(world: &mut World) {
if despawned {
world.send_event(ToastMessage::new(
&if name.is_some() {
format!("Despawning {}: {:?}", name.unwrap(), entity)
format!(
"Despawning {}: {:?}",
name.unwrap_or_else(|| Name::from(format!("{entity:?}"))),
entity
)

Check warning on line 54 in crates/editor_core/src/load.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_core/src/load.rs#L50-L54

Added lines #L50 - L54 were not covered by tests
} else {
format!("Despawning {:?}", entity)
},
Expand Down
15 changes: 12 additions & 3 deletions crates/editor_tabs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,12 @@ impl EditorUi {
.show_add_popup(true)
.show(ctx, &mut tab_viewer);

Check warning on line 138 in crates/editor_tabs/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_tabs/src/lib.rs#L114-L138

Added lines #L114 - L138 were not covered by tests

let windows_setting = unsafe { cell.world_mut().resource_mut::<NewWindowSettings>() };
let Some(windows_setting) =
(unsafe { cell.world_mut().get_resource_mut::<NewWindowSettings>() })

Check warning on line 141 in crates/editor_tabs/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_tabs/src/lib.rs#L140-L141

Added lines #L140 - L141 were not covered by tests
else {
error!("Failed to load new window settings");
return;

Check warning on line 144 in crates/editor_tabs/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_tabs/src/lib.rs#L143-L144

Added lines #L143 - L144 were not covered by tests
};
for command in tab_viewer.tab_commands {
match command {
EditorTabCommand::Add {
Expand All @@ -162,7 +167,7 @@ impl EditorUi {
if let Some(surface) = self.tree.get_surface_mut(surface) {
surface
.node_tree_mut()
.unwrap()
.unwrap() // Guaranteed to exist
.split_right(node, 0.5, vec![name]);
}

Check warning on line 172 in crates/editor_tabs/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_tabs/src/lib.rs#L167-L172

Added lines #L167 - L172 were not covered by tests
}
Expand Down Expand Up @@ -216,7 +221,10 @@ impl EditorUiAppExt for App {
});

to_label(
world.resource_mut::<T>().tab_name().title.as_str(),
&world
.get_resource::<T>()
.map(|tab| tab.tab_name().title)
.unwrap_or_else(|| "Unknown".to_string()),
text_size,
)
.into()
Expand All @@ -243,6 +251,7 @@ impl EditorUiAppExt for App {

tab.schedule.add_systems(tab_systems);

// Not much we can do here
self.world
.resource_mut::<ScheduleEditorTabStorage>()
.0
Expand Down
8 changes: 6 additions & 2 deletions crates/editor_tabs/src/start_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ impl GroupLayoutExt for App {
group: G,
tab: N,
) -> &mut Self {
self.world.resource_mut::<R>().push(group, tab);
if let Some(mut layout) = self.world.get_resource_mut::<R>() {
layout.push(group, tab)
}
self
}

Check warning on line 93 in crates/editor_tabs/src/start_layout.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_tabs/src/start_layout.rs#L84-L93

Added lines #L84 - L93 were not covered by tests

Expand All @@ -95,7 +97,9 @@ impl GroupLayoutExt for App {
group: G,
tab: N,
) -> &mut Self {
self.world.resource_mut::<R>().push_front(group, tab);
if let Some(mut layout) = self.world.get_resource_mut::<R>() {
layout.push_front(group, tab)
}
self
}

Check warning on line 104 in crates/editor_tabs/src/start_layout.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_tabs/src/start_layout.rs#L95-L104

Added lines #L95 - L104 were not covered by tests

Expand Down
24 changes: 18 additions & 6 deletions crates/editor_ui/src/camera_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,18 @@ impl EditorTab for CameraViewTab {
let mut need_recreate_texture = false;

if self.target_image.is_none() {
let handle = world
.resource_mut::<Assets<Image>>()
.add(create_camera_image(
let Some(handle) = world.get_resource_mut::<Assets<Image>>().map(|mut assets| {
assets.add(create_camera_image(

Check warning on line 195 in crates/editor_ui/src/camera_view.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/camera_view.rs#L194-L195

Added lines #L194 - L195 were not covered by tests
clipped.width() as u32,
clipped.height() as u32,
))
}) else {
world.send_event(ToastMessage::new(
"No camera image target found.",
toast::ToastKind::Error,

Check warning on line 202 in crates/editor_ui/src/camera_view.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/camera_view.rs#L198-L202

Added lines #L198 - L202 were not covered by tests
));
return;

Check warning on line 204 in crates/editor_ui/src/camera_view.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/camera_view.rs#L204

Added line #L204 was not covered by tests
};
self.target_image = Some(handle);
self.need_reinit_egui_tex = true;
let msg = format!(
Expand All @@ -220,12 +226,18 @@ impl EditorTab for CameraViewTab {
}

if need_recreate_texture {
let handle = world
.resource_mut::<Assets<Image>>()
.add(create_camera_image(
let Some(handle) = world.get_resource_mut::<Assets<Image>>().map(|mut assets| {
assets.add(create_camera_image(

Check warning on line 230 in crates/editor_ui/src/camera_view.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/camera_view.rs#L229-L230

Added lines #L229 - L230 were not covered by tests
clipped.width() as u32,
clipped.height() as u32,
))
}) else {
world.send_event(ToastMessage::new(
"No camera image target found.",
toast::ToastKind::Error,

Check warning on line 237 in crates/editor_ui/src/camera_view.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/camera_view.rs#L233-L237

Added lines #L233 - L237 were not covered by tests
));
return;

Check warning on line 239 in crates/editor_ui/src/camera_view.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/camera_view.rs#L239

Added line #L239 was not covered by tests
};

self.target_image = Some(handle);
self.need_reinit_egui_tex = true;
Expand Down
8 changes: 5 additions & 3 deletions crates/editor_ui/src/game_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ impl EditorTab for GameViewTab {

ui.spacing();
//Draw FPS
let dt = world.get_resource::<Time>().unwrap().delta_seconds();
self.smoothed_dt = self.smoothed_dt.mul_add(0.98, dt * 0.02);
ui.colored_label(TEXT_COLOR, format!("FPS: {:.0}", 1.0 / self.smoothed_dt));
if let Some(dt) = world.get_resource::<Time>() {
let dt = dt.delta_seconds();
self.smoothed_dt = self.smoothed_dt.mul_add(0.98, dt * 0.02);
ui.colored_label(TEXT_COLOR, format!("FPS: {:.0}", 1.0 / self.smoothed_dt));
}

Check warning on line 97 in crates/editor_ui/src/game_view.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/game_view.rs#L93-L97

Added lines #L93 - L97 were not covered by tests

#[cfg(debug_assertions)]
{
Expand Down
1 change: 1 addition & 0 deletions crates/editor_ui/src/hierarchy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ fn draw_entity<F: QueryFilter>(
}
})
.body(|ui| {
// already checked that children is some
for child in children.unwrap().iter() {
draw_entity(
commands,
Expand Down
16 changes: 13 additions & 3 deletions crates/editor_ui/src/inspector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ use bevy_egui::{egui::TextEdit, *};

use space_editor_core::prelude::*;
use space_prefab::{component::EntityLink, editor_registry::EditorRegistry};
use space_shared::ext::bevy_inspector_egui::{
inspector_egui_impls::InspectorEguiImpl, reflect_inspector::InspectorUi,
use space_shared::{
ext::bevy_inspector_egui::{
inspector_egui_impls::InspectorEguiImpl, reflect_inspector::InspectorUi,
},
toast::{ToastKind, ToastMessage},
};

use crate::{editor_tab_name::EditorTabName, icons::add_component_icon};
Expand Down Expand Up @@ -114,7 +117,14 @@ impl EditorTab for InspectorTab {
});

let cell = world.as_unsafe_world_cell();
let mut state = unsafe { cell.get_resource_mut::<InspectState>().unwrap() };
let Some(mut state) = (unsafe { cell.get_resource_mut::<InspectState>() }) else {
error!("Failed to load inspect state");
world.send_event(ToastMessage::new(
"Failed to load inspect state",
ToastKind::Error,
));
return;

Check warning on line 126 in crates/editor_ui/src/inspector/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/inspector/mod.rs#L120-L126

Added lines #L120 - L126 were not covered by tests
};

let mut commands: Vec<InspectCommand> = vec![];
let mut queue = CommandQueue::default();
Expand Down
1 change: 1 addition & 0 deletions crates/editor_ui/src/meshless_visualizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ pub struct EditorIconAssets {
fn register_assets(mut commands: Commands, asset_server: Res<AssetServer>) {
use space_shared::asset_fs::*;
let assets = EditorIconAssets {
// Unwraps are logged
unknown: asset_server.add(
create_unknown_image()
.inspect_err(|err| error!("failed to load image `Unknown`: {err}"))
Expand Down
9 changes: 6 additions & 3 deletions crates/editor_ui/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,12 @@ impl RegisterSettingsBlockExt for App {
block: impl FnMut(&mut egui::Ui, &mut Commands, &mut World) + Send + Sync + 'static,
) {
self.world
.resource_mut::<SettingsWindow>()
.sub_blocks
.insert(name.to_string(), Box::new(block));
.get_resource_mut::<SettingsWindow>()
.map(|mut settings| {
settings
.sub_blocks
.insert(name.to_string(), Box::new(block))
});

Check warning on line 143 in crates/editor_ui/src/settings.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/settings.rs#L138-L143

Added lines #L138 - L143 were not covered by tests
}
}

Expand Down
6 changes: 5 additions & 1 deletion crates/editor_ui/src/ui_registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,11 @@ impl EditorUiExt for App {
reg
} else {
self.init_resource::<BundleReg>();
self.world.get_resource_mut::<BundleReg>().unwrap()
if let Some(reg) = self.world.get_resource_mut::<BundleReg>() {
reg

Check warning on line 74 in crates/editor_ui/src/ui_registration.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/ui_registration.rs#L73-L74

Added lines #L73 - L74 were not covered by tests
} else {
return;

Check warning on line 76 in crates/editor_ui/src/ui_registration.rs

View check run for this annotation

Codecov / codecov/patch

crates/editor_ui/src/ui_registration.rs#L76

Added line #L76 was not covered by tests
}
};

reg.add_bundle(EditorBundle {
Expand Down
1 change: 1 addition & 0 deletions crates/prefab/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ description = "Subcrate for the space_editor crate. Contains the prefab systems

[features]
no_event_registration = []
editor = []

[dependencies]
bevy.workspace = true
Expand Down
Loading

0 comments on commit 7d78b9b

Please sign in to comment.