From c8d17d97532255880e115bb75935789b280617d5 Mon Sep 17 00:00:00 2001 From: Ben Wallis Date: Fri, 18 Jun 2021 22:04:24 +0100 Subject: [PATCH] Addressed MR comments --- Cargo.lock | 9 ------- common/src/clock.rs | 2 +- common/src/states/utils.rs | 1 - voxygen/anim/Cargo.toml | 14 +++++------ voxygen/anim/src/lib.rs | 18 +++++--------- voxygen/dynlib/Cargo.toml | 1 - voxygen/dynlib/src/lib.rs | 22 ++++++++-------- voxygen/egui/Cargo.toml | 8 ++---- voxygen/egui/dyn/Cargo.toml | 3 --- voxygen/egui/src/character_states.rs | 36 +++++++++++++++++---------- voxygen/egui/src/lib.rs | 28 ++++++++++----------- voxygen/src/render/renderer/drawer.rs | 14 ++++------- voxygen/src/session/mod.rs | 6 +++-- voxygen/src/ui/egui/mod.rs | 5 ++-- 14 files changed, 74 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 09d40facb6..4f9c525c5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6184,11 +6184,7 @@ name = "veloren-voxygen-anim" version = "0.10.0" dependencies = [ "bytemuck", - "find_folder", "lazy_static", - "libloading 0.7.0", - "notify 5.0.0-pre.9", - "tracing", "vek", "veloren-common", "veloren-voxygen-dynlib", @@ -6206,7 +6202,6 @@ name = "veloren-voxygen-dynlib" version = "0.1.0" dependencies = [ "find_folder", - "lazy_static", "libloading 0.7.0", "notify 5.0.0-pre.9", "tracing", @@ -6218,11 +6213,7 @@ version = "0.9.0" dependencies = [ "egui", "egui_winit_platform", - "find_folder", "lazy_static", - "libloading 0.7.0", - "notify 5.0.0-pre.9", - "tracing", "veloren-client", "veloren-common", "veloren-voxygen-dynlib", diff --git a/common/src/clock.rs b/common/src/clock.rs index 0eed375d95..4654fa6065 100644 --- a/common/src/clock.rs +++ b/common/src/clock.rs @@ -85,7 +85,7 @@ impl Clock { .map_or(self.last_dt.as_secs_f32(), |t| t.into_inner()), ); if self.last_dts.len() >= NUMBER_OF_DELTAS_COMPARED && self.last_dt > 2 * stable_dt { - tracing::debug!(?self.last_dt, ?self.total_tick_time, "lag spike detected, unusually slow tick"); + tracing::trace!(?self.last_dt, ?self.total_tick_time, "lag spike detected, unusually slow tick"); stable_dt } else { self.last_dt diff --git a/common/src/states/utils.rs b/common/src/states/utils.rs index 28533925ff..245987015f 100644 --- a/common/src/states/utils.rs +++ b/common/src/states/utils.rs @@ -16,7 +16,6 @@ use crate::{ }; use serde::{Deserialize, Serialize}; use std::time::Duration; - use strum_macros::Display; use vek::*; diff --git a/voxygen/anim/Cargo.toml b/voxygen/anim/Cargo.toml index e8a83a2b71..5081c532b8 100644 --- a/voxygen/anim/Cargo.toml +++ b/voxygen/anim/Cargo.toml @@ -5,7 +5,7 @@ name = "veloren-voxygen-anim" version = "0.10.0" [features] -use-dyn-lib = ["libloading", "notify", "lazy_static", "tracing", "find_folder"] +use-dyn-lib = ["lazy_static", "voxygen-dynlib"] be-dyn-lib = [] simd = ["vek/platform_intrinsics"] @@ -13,12 +13,10 @@ default = ["simd"] [dependencies] common = {package = "veloren-common", path = "../../common"} -find_folder = {version = "0.3.0", optional = true} # inline_tweak = "1.0.2" -lazy_static = {version = "1.4.0", optional = true} -libloading = {version = "0.7", optional = true} -notify = {version = "5.0.0-pre.2", optional = true} -tracing = {version = "0.1", optional = true} +bytemuck = { version = "1.4", features=["derive"] } vek = {version = "=0.14.1", features = ["serde"]} -bytemuck = { version="1.4", features=["derive"] } -voxygen-dynlib = {package = "veloren-voxygen-dynlib", path = "../dynlib"} +voxygen-dynlib = {package = "veloren-voxygen-dynlib", path = "../dynlib", optional = true} + +# Hot Reloading +lazy_static = {version = "1.4.0", optional = true} diff --git a/voxygen/anim/src/lib.rs b/voxygen/anim/src/lib.rs index a1b62ca8e5..18a68762db 100644 --- a/voxygen/anim/src/lib.rs +++ b/voxygen/anim/src/lib.rs @@ -66,19 +66,13 @@ pub mod ship; pub mod theropod; pub mod vek; -#[cfg(feature = "use-dyn-lib")] -use std::ffi::CStr; - use self::vek::*; use bytemuck::{Pod, Zeroable}; #[cfg(feature = "use-dyn-lib")] -use lazy_static::lazy_static; -#[cfg(feature = "use-dyn-lib")] -use std::sync::Arc; -#[cfg(feature = "use-dyn-lib")] -use std::sync::Mutex; -#[cfg(feature = "use-dyn-lib")] -use voxygen_dynlib::LoadedLib; +use { + lazy_static::lazy_static, std::ffi::CStr, std::sync::Arc, std::sync::Mutex, + voxygen_dynlib::LoadedLib, +}; type MatRaw = [[f32; 4]; 4]; @@ -131,7 +125,7 @@ pub fn compute_matrices( let lib = &lock.as_ref().unwrap().lib; #[allow(clippy::type_complexity)] - let compute_fn: libloading::Symbol< + let compute_fn: voxygen_dynlib::Symbol< fn(&S, Mat4, &mut [FigureBoneData; MAX_BONE_COUNT]) -> Vec3, > = unsafe { lib.get(S::COMPUTE_FN) }.unwrap_or_else(|e| { panic!( @@ -183,7 +177,7 @@ pub trait Animation { let lib = &lock.as_ref().unwrap().lib; #[allow(clippy::type_complexity)] - let update_fn: libloading::Symbol< + let update_fn: voxygen_dynlib::Symbol< fn( &Self::Skeleton, Self::Dependency<'a>, diff --git a/voxygen/dynlib/Cargo.toml b/voxygen/dynlib/Cargo.toml index 94f28d3516..7208c27386 100644 --- a/voxygen/dynlib/Cargo.toml +++ b/voxygen/dynlib/Cargo.toml @@ -6,7 +6,6 @@ edition = "2018" [dependencies] find_folder = {version = "0.3.0"} -lazy_static = {version = "1.4.0"} libloading = {version = "0.7"} notify = {version = "5.0.0-pre.2"} tracing = "0.1" diff --git a/voxygen/dynlib/src/lib.rs b/voxygen/dynlib/src/lib.rs index 3602459c76..5507755638 100644 --- a/voxygen/dynlib/src/lib.rs +++ b/voxygen/dynlib/src/lib.rs @@ -9,11 +9,15 @@ use std::{ use find_folder::Search; use std::{ env, + env::consts::DLL_SUFFIX, path::{Path, PathBuf}, sync::Arc, }; use tracing::{debug, error, info}; +// Re-exports +pub use libloading::Symbol; + /// LoadedLib holds a loaded dynamic library and the location of library file /// with the appropriate OS specific name and extension i.e. /// `libvoxygen_anim_dyn_active.dylib`, `voxygen_anim_dyn_active.dll`. @@ -111,14 +115,17 @@ impl LoadedLib { } } +/// Initialise a watcher. +/// +/// This will search for the directory named `package_source_dir` and watch the +/// files within it for any changes. pub fn init( lib_storage: Arc>>, package: &'static str, dyn_package: &'static str, package_source_dir: &'static str, ) { - let mut lock = lib_storage.lock().unwrap(); - *lock = Some(LoadedLib::compile_load(dyn_package)); + *lib_storage.lock().unwrap() = Some(LoadedLib::compile_load(dyn_package)); // TODO: use crossbeam let (reload_send, reload_recv) = mpsc::channel(); @@ -138,7 +145,6 @@ pub fn init( watcher.watch(watch_dir, RecursiveMode::Recursive).unwrap(); - let loaded_lib_clone = Arc::clone(&lib_storage); // Start reloader that watcher signals // "Debounces" events since I can't find the option to do this in the latest // `notify` @@ -158,7 +164,7 @@ pub fn init( "Hot reloading {} because files in `{}` modified.", package, package_source_dir ); - hotreload(dyn_package, loaded_lib_clone.clone()); + hotreload(dyn_package, &lib_storage); } }) .unwrap(); @@ -172,15 +178,11 @@ fn compiled_file(dyn_package: &str) -> String { dyn_lib_file(dyn_package, false) fn active_file(dyn_package: &str) -> String { dyn_lib_file(dyn_package, true) } fn dyn_lib_file(dyn_package: &str, active: bool) -> String { - #[cfg(target_os = "windows")] - const FILE_EXT: &str = ".dll"; - #[cfg(not(target_os = "windows"))] - const FILE_EXT: &str = ".so"; format!( "{}{}{}", dyn_package.replace("-", "_"), if active { "_active" } else { "" }, - FILE_EXT + DLL_SUFFIX ) } @@ -209,7 +211,7 @@ fn event_fn(res: notify::Result, sender: &mpsc::Sender) { /// /// This will reload the dynamic library by first internally calling compile /// and then reloading the library. -fn hotreload(dyn_package: &str, loaded_lib: Arc>>) { +fn hotreload(dyn_package: &str, loaded_lib: &Mutex>) { // Do nothing if recompile failed. if compile(dyn_package) { let mut lock = loaded_lib.lock().unwrap(); diff --git a/voxygen/egui/Cargo.toml b/voxygen/egui/Cargo.toml index ac20af4b53..6c59194d8c 100644 --- a/voxygen/egui/Cargo.toml +++ b/voxygen/egui/Cargo.toml @@ -5,7 +5,7 @@ edition = "2018" version = "0.9.0" [features] -use-dyn-lib = ["libloading", "notify", "lazy_static", "tracing", "find_folder"] +use-dyn-lib = ["lazy_static", "voxygen-dynlib"] be-dyn-lib = [] [dependencies] @@ -13,12 +13,8 @@ client = {package = "veloren-client", path = "../../client"} common = {package = "veloren-common", path = "../../common"} egui = "0.12" egui_winit_platform = "0.8" -voxygen-dynlib = {package = "veloren-voxygen-dynlib", path = "../dynlib"} +voxygen-dynlib = {package = "veloren-voxygen-dynlib", path = "../dynlib", optional = true} # Hot Reloading -find_folder = {version = "0.3.0", optional = true} lazy_static = {version = "1.4.0", optional = true} -libloading = {version = "0.7", optional = true} -notify = {version = "5.0.0-pre.2", optional = true} -tracing = {version = "0.1", optional = true} diff --git a/voxygen/egui/dyn/Cargo.toml b/voxygen/egui/dyn/Cargo.toml index a5db1c1161..d4865fcacb 100644 --- a/voxygen/egui/dyn/Cargo.toml +++ b/voxygen/egui/dyn/Cargo.toml @@ -5,9 +5,6 @@ name = "veloren-voxygen-egui-dyn" version = "0.9.0" [lib] -# Using dylib instead of cdylib increases the size 3 -> 13 mb -# but it is needed to expose the symbols from the anim crate :( -# effect on compile time appears to be insignificant crate-type = ["dylib"] [features] diff --git a/voxygen/egui/src/character_states.rs b/voxygen/egui/src/character_states.rs index b250a3ad99..52b1c9df13 100644 --- a/voxygen/egui/src/character_states.rs +++ b/voxygen/egui/src/character_states.rs @@ -20,21 +20,31 @@ pub fn draw_char_state_group( }, CharacterState::DashMelee(data) => dash_melee_grid(ui, data), CharacterState::ChargedMelee(data) => charged_melee_grid(ui, data), + // Character states with no associated data to display + CharacterState::Dance + | CharacterState::Idle + | CharacterState::Sit + | CharacterState::GlideWield + | CharacterState::Sneak + | CharacterState::Talk + | CharacterState::Wielding => {}, CharacterState::LeapMelee(data) => leap_melee_grid(ui, data), - _ => {}, + _ => { + ui.label(""); + }, }; } fn charged_melee_grid(ui: &mut Ui, data: &charged_melee::Data) { Grid::new("selected_entity_charged_melee_grid") - .spacing([40.0, 4.0]) - .max_col_width(100.0) - .striped(true) - .show(ui, |ui| #[rustfmt::skip] { + .spacing([40.0, 4.0]) + .max_col_width(100.0) + .striped(true) + .show(ui, |ui| #[rustfmt::skip] { two_col_row(ui, "Stage Section", data.stage_section.to_string()); two_col_row(ui, "Timer", format!("{}ms", data.timer.as_millis())); two_col_row(ui, "Charge Amount", format!("{:.1}", data.charge_amount)); - two_col_row(ui, "Exhausted", (if data.exhausted { "True" } else { "False" }).to_string()); + two_col_row(ui, "Exhausted", if data.exhausted { "True" } else { "False" }); }); } @@ -56,22 +66,22 @@ fn dash_melee_grid(ui: &mut Ui, data: &dash_melee::Data) { .max_col_width(100.0) .striped(true) .show(ui, |ui| #[rustfmt::skip] { - two_col_row(ui, "Auto Charge", (if data.auto_charge { "True" } else { "False " }).to_string()); + two_col_row(ui, "Auto Charge", if data.auto_charge { "True" } else { "False " }); two_col_row(ui, "Timer", format!("{}ms", data.timer.as_millis())); two_col_row(ui, "Stage Section", data.stage_section.to_string()); - two_col_row(ui, "Exhausted", (if data.exhausted { "True" } else { "False " }).to_string()); + two_col_row(ui, "Exhausted", if data.exhausted { "True" } else { "False " }); two_col_row(ui, "Charge End Timer", format!("{}ms", data.charge_end_timer.as_millis())); }); } fn leap_melee_grid(ui: &mut Ui, data: &leap_melee::Data) { Grid::new("selected_entity_leap_melee_grid") - .spacing([40.0, 4.0]) - .max_col_width(100.0) - .striped(true) - .show(ui, |ui| #[rustfmt::skip] { + .spacing([40.0, 4.0]) + .max_col_width(100.0) + .striped(true) + .show(ui, |ui| #[rustfmt::skip] { two_col_row(ui, "Stage Section", data.stage_section.to_string()); two_col_row(ui, "Timer", format!("{}ms", data.timer.as_millis())); - two_col_row(ui, "Exhausted", (if data.exhausted { "True" } else { "False " }).to_string()); + two_col_row(ui, "Exhausted", if data.exhausted { "True" } else { "False " }); }); } diff --git a/voxygen/egui/src/lib.rs b/voxygen/egui/src/lib.rs index 41da0b9d36..c188523e31 100644 --- a/voxygen/egui/src/lib.rs +++ b/voxygen/egui/src/lib.rs @@ -1,7 +1,10 @@ #![feature(stmt_expr_attributes)] -mod character_states; + #[cfg(all(feature = "be-dyn-lib", feature = "use-dyn-lib"))] compile_error!("Can't use both \"be-dyn-lib\" and \"use-dyn-lib\" features at once"); + +mod character_states; + use client::{Client, Join, World, WorldExt}; use common::{ comp, @@ -25,15 +28,10 @@ use crate::character_states::draw_char_state_group; use common::comp::{aura::AuraKind::Buff, Body, Fluid}; use egui_winit_platform::Platform; #[cfg(feature = "use-dyn-lib")] -use lazy_static::lazy_static; -#[cfg(feature = "use-dyn-lib")] -use std::ffi::CStr; -#[cfg(feature = "use-dyn-lib")] -use std::sync::Arc; -#[cfg(feature = "use-dyn-lib")] -use std::sync::Mutex; -#[cfg(feature = "use-dyn-lib")] -use voxygen_dynlib::LoadedLib; +use { + lazy_static::lazy_static, std::ffi::CStr, std::sync::Arc, std::sync::Mutex, + voxygen_dynlib::LoadedLib, +}; #[cfg(feature = "use-dyn-lib")] lazy_static! { @@ -69,7 +67,7 @@ pub fn maintain( let lib = &lock.as_ref().unwrap().lib; #[allow(clippy::type_complexity)] - let maintain_fn: libloading::Symbol< + let maintain_fn: voxygen_dynlib::Symbol< fn( &mut Platform, &mut EguiInnerState, @@ -148,12 +146,12 @@ pub enum DebugShapeAction { radius: f32, height: f32, }, + RemoveShape(u64), SetPosAndColor { id: u64, pos: [f32; 4], color: [f32; 4], }, - RemoveCylinder(u64), } #[derive(Default)] @@ -217,7 +215,7 @@ pub fn maintain_egui_inner( ui.label("Show EGUI Windows"); ui.horizontal(|ui| { ui.checkbox(&mut egui_windows.egui_inspection, "🔍 Inspection"); - ui.checkbox(&mut egui_windows.egui_settings, "🔍 Settings"); + ui.checkbox(&mut egui_windows.egui_settings, "🔧 Settings"); ui.checkbox(&mut egui_windows.egui_memory, "📝 Memory"); }) }) @@ -398,7 +396,7 @@ pub fn maintain_egui_inner( if let Some(debug_shape_id) = previous.debug_shape_id { egui_actions .actions - .push(DebugShapeAction::RemoveCylinder(debug_shape_id)); + .push(DebugShapeAction::RemoveShape(debug_shape_id)); } }; @@ -409,7 +407,7 @@ pub fn maintain_egui_inner( { egui_actions .actions - .push(DebugShapeAction::RemoveCylinder(debug_shape_id)); + .push(DebugShapeAction::RemoveShape(debug_shape_id)); egui_actions.actions.push(DebugShapeAction::AddCylinder { radius: 1.0, height: selected_entity_cylinder_height, diff --git a/voxygen/src/render/renderer/drawer.rs b/voxygen/src/render/renderer/drawer.rs index 44acb5269d..e103923285 100644 --- a/voxygen/src/render/renderer/drawer.rs +++ b/voxygen/src/render/renderer/drawer.rs @@ -57,7 +57,8 @@ impl<'frame> Pipelines<'frame> { struct RendererBorrow<'frame> { queue: &'frame wgpu::Queue, device: &'frame wgpu::Device, - _sc_desc: &'frame wgpu::SwapChainDescriptor, + #[cfg(feature = "egui-ui")] + sc_desc: &'frame wgpu::SwapChainDescriptor, shadow: Option<&'frame super::Shadow>, pipelines: Pipelines<'frame>, locals: &'frame super::locals::Locals, @@ -107,7 +108,7 @@ impl<'frame> Drawer<'frame> { let borrow = RendererBorrow { queue: &renderer.queue, device: &renderer.device, - _sc_desc: &renderer.sc_desc, + sc_desc: &renderer.sc_desc, shadow, pipelines, locals: &renderer.locals, @@ -278,8 +279,8 @@ impl<'frame> Drawer<'frame> { let paint_jobs = platform.context().tessellate(paint_commands); let screen_descriptor = ScreenDescriptor { - physical_width: self.borrow._sc_desc.width, - physical_height: self.borrow._sc_desc.height, + physical_width: self.borrow.sc_desc.width, + physical_height: self.borrow.sc_desc.height, scale_factor: scale_factor as f32, }; @@ -309,11 +310,6 @@ impl<'frame> Drawer<'frame> { ); } - #[cfg(feature = "egui-ui")] - pub fn egui_renderpass(&mut self) -> &mut egui_wgpu_backend::RenderPass { - self.borrow.egui_render_pass - } - /// Does nothing if the shadow pipelines are not available or shadow map /// rendering is disabled pub fn draw_point_shadows<'data: 'frame>( diff --git a/voxygen/src/session/mod.rs b/voxygen/src/session/mod.rs index 2e49a8e7fa..21317b91b1 100644 --- a/voxygen/src/session/mod.rs +++ b/voxygen/src/session/mod.rs @@ -1408,7 +1408,9 @@ impl PlayState for SessionState { /// This method should be called once per frame. fn render(&mut self, global_state: &mut GlobalState) { - let _scale_factor = global_state.window.window().scale_factor() as f32; + #[cfg(feature = "egui-ui")] + let scale_factor = global_state.window.window().scale_factor() as f32; + let renderer = global_state.window.renderer_mut(); let settings = &global_state.settings; @@ -1470,7 +1472,7 @@ impl PlayState for SessionState { #[cfg(feature = "egui-ui")] if global_state.settings.interface.toggle_debug { - drawer.draw_egui(&mut global_state.egui_state.platform, _scale_factor); + drawer.draw_egui(&mut global_state.egui_state.platform, scale_factor); } } } diff --git a/voxygen/src/ui/egui/mod.rs b/voxygen/src/ui/egui/mod.rs index fe4143dd71..d0cbfa5ed3 100644 --- a/voxygen/src/ui/egui/mod.rs +++ b/voxygen/src/ui/egui/mod.rs @@ -4,7 +4,6 @@ use crate::{ }; use client::Client; use common::debug_info::DebugInfo; -use core::mem; use egui::FontDefinitions; use egui_winit_platform::{Platform, PlatformDescriptor}; use voxygen_egui::{DebugShapeAction, EguiInnerState, EguiWindows}; @@ -41,7 +40,7 @@ impl EguiState { &mut self.egui_windows, client, debug_info, - mem::take(&mut self.new_debug_shape_id), + self.new_debug_shape_id.take(), ); egui_actions.actions.iter().for_each(|action| match action { @@ -52,7 +51,7 @@ impl EguiState { }); self.new_debug_shape_id = Some(shape_id.0); }, - DebugShapeAction::RemoveCylinder(debug_shape_id) => { + DebugShapeAction::RemoveShape(debug_shape_id) => { scene.debug.remove_shape(DebugShapeId(*debug_shape_id)); }, DebugShapeAction::SetPosAndColor { id, pos, color } => {