tirbofish/dropbear · commit
824aa2553eba98a05b3757180380b9779e7ca089
adding a custom clippy lint for hecs::World::query_one_mut so i don't deal with that mistake.
Signature present but could not be verified.
Unverified
@@ -6,7 +6,7 @@ package.repository = "https://github.com/4tkbytes/dropbear-engine" package.readme = "README.md" resolver = "3" -members = ["dropbear-engine", "eucalyptus-core", "eucalyptus-editor"] +members = ["deny_query_one_mut_lint", "dropbear-engine", "eucalyptus-core", "eucalyptus-editor"] # members = ["dropbear-engine", "eucalyptus-core", "eucalyptus-editor", "redback-runtime"] @@ -42,8 +42,6 @@ open = "5" parking_lot = {version = "0.12", features = ["deadlock_detection"] } rfd = "0.15.4" ron = "0.11" -# russimp-ng = { version = "3.2", features = ["static-link"] } -# rustyscript = { version = "0.12" } serde = { version = "1.0.219", features = ["derive"] } spin_sleep = "1.3" transform-gizmo-egui = { git = "https://github.com/4tkbytes/transform-gizmo"} @@ -52,7 +50,6 @@ wgpu = "25" winit = { version = "0.30", features = [] } zip = "5.1" walkdir = "2.3" -# wasmer = { version = "6.1.0-rc.3" } rayon = "1.11" flate2 = "1.1" reqwest = { version = "0.11", features = ["stream"] } @@ -61,10 +58,8 @@ async-trait = "0.1" futures-util = "0.3" boa_engine = "0.20" backtrace = "0.3" - - gltf = "1" -thiserror = "2.0" +os_info = "3.12" [workspace.dependencies.image] version = "0.25" @@ -0,0 +1,16 @@ +[package] +name = "deny_query_one_mut_lint" +version.workspace = true +edition.workspace = true +license = "MIT" +repository.workspace = true +readme = "README.md" + +[lib] +crate-type = ["dylib"] + +[dependencies] + + +[features] +default = [] @@ -0,0 +1,28 @@ +# deny_query_one_mut_lint + +A custom lint for all codebases in the dropbear-engine that check if the usage of [`hecs::World::query_one_mut`] +is used. + +### Cause + +The editor is required to have the world be threadsafe. This is achieved with the help of `Arc<RwLock<T>>`. +This is a great way to achieve threadsafety, however writing to the world often creates a +[deadlock](https://en.wikipedia.org/wiki/Deadlock_(computer_science)). A deadlock in Rust is undetectable during +compile-time and when ran during runtime, it can cause the program to freeze and not respond. + +### Fix + +To solve this conundrum, I have implemented a couple of fixes: +1. Using a [`std::sync::Mutex`](https://doc.rust-lang.org/std/sync/struct.Mutex.html) allows for locking, which allows +for mutability but blocks that thread. Furthermore, it cannot/shouldn't be used in threads where a mutex uses `Send`. +To fix this, I switched to [`parking_lot::RwLock`](https://docs.rs/parking_lot/latest/parking_lot/type.RwLock.html), +which doesn't use `Send` (making it even more threadsafe). +Conveniently, it also includes deadlock detection (as an enabled feature), which can point out if a deadlock is in progress or if an expensive +operation is being run. +2. I have switched from `hecs::World::query_one_mut` to `hecs::World::query_one` to use only the `RwLock::read(&self)`, +which doesn't require a mutable reference from `self.world`. + +Even with such fixes, deadlocks are inevitable in the codebase. This crate aims to fix this by creating a custom rustc +lint. + +Note: https://blog.guillaume-gomez.fr/articles/2024-01-18+Writing+your+own+Rust+linter @@ -0,0 +1,14 @@ +pub fn add(left: u64, right: u64) -> u64 { + left + right +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_works() { + let result = add(2, 2); + assert_eq!(result, 4); + } +} @@ -23,13 +23,11 @@ gilrs.workspace = true glam.workspace = true log.workspace = true log-once.workspace = true -# russimp-ng.workspace = true serde.workspace = true spin_sleep.workspace = true wgpu.workspace = true winit.workspace = true hecs.workspace = true -# once_cell.workspace = true parking_lot.workspace = true lazy_static.workspace = true gltf.workspace = true @@ -37,7 +35,7 @@ rayon.workspace = true tokio.workspace = true async-trait.workspace = true backtrace.workspace = true -os_info = "*" +os_info.workspace = true [target.'cfg(not(target_os = "android"))'.dependencies] rfd.workspace = true @@ -28,9 +28,6 @@ tokio.workspace = true boa_engine.workspace = true rayon.workspace = true -[dev-dependencies] -pollster = "*" - [features] editor = [] @@ -386,47 +386,49 @@ impl<'a> TabViewer for EditorTabViewer<'a> { if !matches!(self.viewport_mode, ViewportMode::None) { if let Some(entity_id) = self.selected_entity { { - if let Ok(transform) = self.world.write() - .query_one_mut::<&mut Transform>(*entity_id) + if let Ok(mut q) = self.world.read() + .query_one::<&mut Transform>(*entity_id) { - let was_focused = cfg.is_focused; - cfg.is_focused = self.gizmo.is_focused(); + if let Some(transform) = q.get() { + let was_focused = cfg.is_focused; + cfg.is_focused = self.gizmo.is_focused(); - if cfg.is_focused && !was_focused { - cfg.old_pos = *transform; - } + if cfg.is_focused && !was_focused { + cfg.old_pos = *transform; + } - let gizmo_transform = - transform_gizmo_egui::math::Transform::from_scale_rotation_translation( - transform.scale, - transform.rotation, - transform.position, - ); + let gizmo_transform = + transform_gizmo_egui::math::Transform::from_scale_rotation_translation( + transform.scale, + transform.rotation, + transform.position, + ); - if let Some((_result, new_transforms)) = - self.gizmo.interact(ui, &[gizmo_transform]) - { - if let Some(new_transform) = new_transforms.first() { - transform.position = new_transform.translation.into(); - transform.rotation = new_transform.rotation.into(); - transform.scale = new_transform.scale.into(); + if let Some((_result, new_transforms)) = + self.gizmo.interact(ui, &[gizmo_transform]) + { + if let Some(new_transform) = new_transforms.first() { + transform.position = new_transform.translation.into(); + transform.rotation = new_transform.rotation.into(); + transform.scale = new_transform.scale.into(); + } } - } - if was_focused && !cfg.is_focused { - let transform_changed = cfg.old_pos.position != transform.position - || cfg.old_pos.rotation != transform.rotation - || cfg.old_pos.scale != transform.scale; - - if transform_changed { - UndoableAction::push_to_undo( - &mut self.undo_stack, - UndoableAction::Transform( - entity_id.clone(), - cfg.old_pos.clone(), - ), - ); - log::debug!("Pushed transform action to stack"); + if was_focused && !cfg.is_focused { + let transform_changed = cfg.old_pos.position != transform.position + || cfg.old_pos.rotation != transform.rotation + || cfg.old_pos.scale != transform.scale; + + if transform_changed { + UndoableAction::push_to_undo( + &mut self.undo_stack, + UndoableAction::Transform( + entity_id.clone(), + cfg.old_pos.clone(), + ), + ); + log::debug!("Pushed transform action to stack"); + } } } } @@ -1018,21 +1020,25 @@ impl<'a> TabViewer for EditorTabViewer<'a> { log::debug!("Add Component clicked"); if let Some(entity) = self.selected_entity { { - if let Ok(..) = self.world.write() - .query_one_mut::<&AdoptedEntity>(*entity) + if let Ok(mut q) = self.world.read() + .query_one::<&AdoptedEntity>(*entity) { - log::debug!("Queried selected entity, it is an entity"); - *self.signal = - Signal::AddComponent(*entity, EntityType::Entity); + if let Some(..) = q.get() { + log::debug!("Queried selected entity, it is an entity"); + *self.signal = + Signal::AddComponent(*entity, EntityType::Entity); + } } } { - if let Ok(..) = self.world.write() - .query_one_mut::<&Light>(*entity) + if let Ok(mut q) = self.world.read() + .query_one::<&Light>(*entity) { - log::debug!("Queried selected entity, it is a light"); - *self.signal = Signal::AddComponent(*entity, EntityType::Light); + if let Some(..) = q.get() { + log::debug!("Queried selected entity, it is a light"); + *self.signal = Signal::AddComponent(*entity, EntityType::Light); + } } } } else { @@ -1054,16 +1060,18 @@ impl<'a> TabViewer for EditorTabViewer<'a> { EditorTabMenuAction::RemoveComponent => { log::debug!("Remove Component clicked"); if let Some(entity) = self.selected_entity { - if let Ok(script) = self.world.write() - .query_one_mut::<&ScriptComponent>(*entity) + if let Ok(mut q) = self.world.write() + .query_one::<&ScriptComponent>(*entity) { - log::debug!( - "Queried selected entity, it has a script component" - ); - *self.signal = Signal::RemoveComponent( - *entity, - ComponentType::Script(script.clone()), - ); + if let Some(script) = q.get() { + log::debug!( + "Queried selected entity, it has a script component" + ); + *self.signal = Signal::RemoveComponent( + *entity, + ComponentType::Script(script.clone()), + ); + } } else { warn!( "Selected entity does not have a script component to remove" @@ -1092,41 +1092,49 @@ impl UndoableAction { match action { UndoableCameraAction::Speed(entity, speed) => { { - if let Ok((cam, comp)) = - world.write().query_one_mut::<(&mut Camera, &mut CameraComponent)>(*entity) + if let Ok(mut q) = + world.read().query_one::<(&mut Camera, &mut CameraComponent)>(*entity) { - comp.speed = *speed; - comp.update(cam); + if let Some((cam, comp)) = q.get() { + comp.speed = *speed; + comp.update(cam); + } } } } UndoableCameraAction::Sensitivity(entity, sensitivity) => { { - if let Ok((cam, comp)) = - world.write().query_one_mut::<(&mut Camera, &mut CameraComponent)>(*entity) + if let Ok(mut q) = + world.read().query_one::<(&mut Camera, &mut CameraComponent)>(*entity) { - comp.sensitivity = *sensitivity; - comp.update(cam); + if let Some((cam, comp)) = q.get() { + comp.sensitivity = *sensitivity; + comp.update(cam); + } } } } UndoableCameraAction::FOV(entity, fov) => { { - if let Ok((cam, comp)) = - world.write().query_one_mut::<(&mut Camera, &mut CameraComponent)>(*entity) + if let Ok(mut q) = + world.read().query_one::<(&mut Camera, &mut CameraComponent)>(*entity) { - comp.fov_y = *fov; - comp.update(cam); + if let Some((cam, comp)) = q.get() { + comp.fov_y = *fov; + comp.update(cam); + } } } } UndoableCameraAction::Type(entity, camera_type) => { { - if let Ok((cam, comp)) = - world.write().query_one_mut::<(&mut Camera, &mut CameraComponent)>(*entity) + if let Ok(mut q) = + world.read().query_one::<(&mut Camera, &mut CameraComponent)>(*entity) { - comp.camera_type = *camera_type; - comp.update(cam); + if let Some((cam, comp)) = q.get() { + comp.camera_type = *camera_type; + comp.update(cam); + } } } } @@ -767,93 +767,95 @@ impl Scene for Editor { Signal::AddComponent(entity, e_type) => { match e_type { EntityType::Entity => { - if let Ok(e) = self.world.write() - .query_one_mut::<&AdoptedEntity>(*entity) + if let Ok(mut q) = self.world.read() + .query_one::<&AdoptedEntity>(*entity) { - let mut local_signal: Option<Signal> = None; - let label = e.label().clone(); - let mut show = true; - egui::Window::new(format!("Add component for {}", label)) - .title_bar(true) - .open(&mut show) - .scroll([false, true]) - .anchor(Align2::CENTER_CENTER, [0.0, 0.0]) - .enabled(true) - .show(&graphics.shared.get_egui_context(), |ui| { - if ui - .add_sized( - [ui.available_width(), 30.0], - egui::Button::new("Scripting"), - ) - .clicked() - { - log::debug!( + if let Some(e) = q.get() { + let mut local_signal: Option<Signal> = None; + let label = e.label().clone(); + let mut show = true; + egui::Window::new(format!("Add component for {}", label)) + .title_bar(true) + .open(&mut show) + .scroll([false, true]) + .anchor(Align2::CENTER_CENTER, [0.0, 0.0]) + .enabled(true) + .show(&graphics.shared.get_egui_context(), |ui| { + if ui + .add_sized( + [ui.available_width(), 30.0], + egui::Button::new("Scripting"), + ) + .clicked() + { + log::debug!( "Adding scripting component to entity [{}]", label ); - { - if let Err(e) = self.world.write() - .insert_one(*entity, ScriptComponent::default()) { - warn!( + if let Err(e) = self.world.write() + .insert_one(*entity, ScriptComponent::default()) + { + warn!( "Failed to add scripting component to entity: {}", e ); - } else { - success!("Added the scripting component"); + } else { + success!("Added the scripting component"); + } } + local_signal = Some(Signal::None); } - local_signal = Some(Signal::None); - } - if ui - .add_sized( - [ui.available_width(), 30.0], - egui::Button::new("Camera"), - ) - .clicked() - { - log::debug!( + if ui + .add_sized( + [ui.available_width(), 30.0], + egui::Button::new("Camera"), + ) + .clicked() + { + log::debug!( "Adding camera component to entity [{}]", label ); - let has_camera = self.world.read() - .query_one::<(&Camera, &CameraComponent)>(*entity) - .is_ok(); + let has_camera = self.world.read() + .query_one::<(&Camera, &CameraComponent)>(*entity) + .is_ok(); - if has_camera { - warn!( + if has_camera { + warn!( "Entity [{}] already has a camera component", label ); - } else { - let camera = Camera::predetermined( - graphics.shared.clone(), - Some(&format!("{} Camera", label)), - ); - let component = CameraComponent::new(); + } else { + let camera = Camera::predetermined( + graphics.shared.clone(), + Some(&format!("{} Camera", label)), + ); + let component = CameraComponent::new(); - { - if let Err(e) = self.world.write() - .insert(*entity, (camera, component)) { - warn!( + if let Err(e) = self.world.write() + .insert(*entity, (camera, component)) + { + warn!( "Failed to add camera component to entity: {}", e ); - } else { - success!("Added the camera component"); + } else { + success!("Added the camera component"); + } } } + local_signal = Some(Signal::None); } - local_signal = Some(Signal::None); - } - }); - if !show { - self.signal = Signal::None; - } - if let Some(signal) = local_signal { - self.signal = signal + }); + if !show { + self.signal = Signal::None; + } + if let Some(signal) = local_signal { + self.signal = signal + } } } else { log_once::warn_once!( @@ -863,38 +865,40 @@ impl Scene for Editor { } EntityType::Light => { { - if let Ok(light) = self.world.write() - .query_one_mut::<&Light>(*entity) + if let Ok(mut q) = self.world.read() + .query_one::<&Light>(*entity) { - let mut show = true; - egui::Window::new(format!("Add component for {}", light.label)) - .scroll([false, true]) - .anchor(Align2::CENTER_CENTER, [0.0, 0.0]) - .enabled(true) - .open(&mut show) - .title_bar(true) - .show(&graphics.shared.get_egui_context(), |ui| { - if ui - .add_sized( - [ui.available_width(), 30.0], - egui::Button::new("Scripting"), - ) - .clicked() - { - log::debug!( + if let Some(light) = q.get() { + let mut show = true; + egui::Window::new(format!("Add component for {}", light.label)) + .scroll([false, true]) + .anchor(Align2::CENTER_CENTER, [0.0, 0.0]) + .enabled(true) + .open(&mut show) + .title_bar(true) + .show(&graphics.shared.get_egui_context(), |ui| { + if ui + .add_sized( + [ui.available_width(), 30.0], + egui::Button::new("Scripting"), + ) + .clicked() + { + log::debug!( "Adding scripting component to light [{}]", light.label ); - success!( + success!( "Added the scripting component to light [{}]", light.label ); - self.signal = Signal::None; - } - }); - if !show { - self.signal = Signal::None; + self.signal = Signal::None; + } + }); + if !show { + self.signal = Signal::None; + } } } else { log_once::warn_once!( @@ -905,33 +909,35 @@ impl Scene for Editor { } EntityType::Camera => { { - if let Ok((cam, _comp)) = self.world.write() - .query_one_mut::<(&Camera, &CameraComponent)>(*entity) + if let Ok(mut q) = self.world.write() + .query_one::<(&Camera, &CameraComponent)>(*entity) { - let mut show = true; - egui::Window::new(format!("Add component for {}", cam.label)) - .scroll([false, true]) - .anchor(Align2::CENTER_CENTER, [0.0, 0.0]) - .enabled(true) - .open(&mut show) - .title_bar(true) - .show(&graphics.shared.get_egui_context(), |ui| { - egui_extras::install_image_loaders(ui.ctx()); - ui.add(Image::from_bytes( - "bytes://theres_nothing.jpg", - include_bytes!("../../../resources/theres_nothing.jpg"), - )); - ui.label("Theres nothing..."); - // // scripting - // if ui.add_sized([ui.available_width(), 30.0], egui::Button::new("Scripting")).clicked() { - // log::debug!("Adding scripting component to camera [{}]", cam.label); - - // success!("Added the scripting component to camera [{}]", cam.label); - // self.signal = Signal::None; - // } - }); - if !show { - self.signal = Signal::None; + if let Some((cam, _comp)) = q.get() { + let mut show = true; + egui::Window::new(format!("Add component for {}", cam.label)) + .scroll([false, true]) + .anchor(Align2::CENTER_CENTER, [0.0, 0.0]) + .enabled(true) + .open(&mut show) + .title_bar(true) + .show(&graphics.shared.get_egui_context(), |ui| { + egui_extras::install_image_loaders(ui.ctx()); + ui.add(Image::from_bytes( + "bytes://theres_nothing.jpg", + include_bytes!("../../../resources/theres_nothing.jpg"), + )); + ui.label("Theres nothing..."); + // // scripting + // if ui.add_sized([ui.available_width(), 30.0], egui::Button::new("Scripting")).clicked() { + // log::debug!("Adding scripting component to camera [{}]", cam.label); + + // success!("Added the scripting component to camera [{}]", cam.label); + // self.signal = Signal::None; + // } + }); + if !show { + self.signal = Signal::None; + } } } else { log_once::warn_once!( @@ -1,3 +0,0 @@ - -npm install --save-dev typedoc -npx typedoc resources/dropbear.ts @@ -1,2 +0,0 @@ -npm install --save-dev typedoc -npx typedoc resources/dropbear.ts @@ -1,8 +0,0 @@ -{ - "compilerOptions": { - "lib": ["es6", "DOM"], - "target": "es5", - "forceConsistentCasingInFileNames": true, - "strict": true - }, -} @@ -1,4 +0,0 @@ -{ - "name": "dropbear", - "projectDocuments": ["ts/articles/*.md"] -}