From de37de7f454ae53f6d52174a7109ea181d4b67e2 Mon Sep 17 00:00:00 2001 From: Ben Wallis Date: Mon, 8 Jun 2020 19:37:41 +0100 Subject: [PATCH] Initial clippy fixes as discussed in #587 --- common/src/assets/mod.rs | 2 +- common/src/cmd.rs | 14 ++++++-------- common/src/comp/ability.rs | 7 ++++--- common/src/comp/agent.rs | 2 +- common/src/comp/inventory/mod.rs | 4 ++-- common/src/comp/inventory/test.rs | 13 ++++++------- common/src/figure/mod.rs | 4 ++-- common/src/net/post2.rs | 2 +- common/src/path.rs | 7 +------ common/src/state.rs | 6 ++---- common/src/states/climb.rs | 4 +++- common/src/states/glide.rs | 2 +- common/src/states/triple_strike.rs | 13 ++++++------- common/src/sys/agent.rs | 8 +------- common/src/sys/combat.rs | 4 ++-- common/src/sys/projectile.rs | 14 ++++++-------- common/src/sys/stats.rs | 6 ++++-- common/src/terrain/mod.rs | 7 +------ common/src/terrain/structure.rs | 10 +++------- common/src/util/color.rs | 4 ++++ common/src/util/dir.rs | 15 ++++++++++----- common/src/util/mod.rs | 4 ++-- common/src/vol.rs | 2 +- 23 files changed, 70 insertions(+), 84 deletions(-) diff --git a/common/src/assets/mod.rs b/common/src/assets/mod.rs index 1aff030e79..82263a2e2c 100644 --- a/common/src/assets/mod.rs +++ b/common/src/assets/mod.rs @@ -281,7 +281,7 @@ lazy_static! { // VELOREN_ASSETS environment variable if let Ok(var) = std::env::var("VELOREN_ASSETS") { - paths.push(var.to_owned().into()); + paths.push(var.into()); } // Executable path diff --git a/common/src/cmd.rs b/common/src/cmd.rs index 66531fbd92..f54a27997c 100644 --- a/common/src/cmd.rs +++ b/common/src/cmd.rs @@ -56,7 +56,7 @@ pub enum ChatCommand { } // Thank you for keeping this sorted alphabetically :-) -pub static CHAT_COMMANDS: &'static [ChatCommand] = &[ +pub static CHAT_COMMANDS: &[ChatCommand] = &[ ChatCommand::Adminify, ChatCommand::Alias, ChatCommand::Build, @@ -118,11 +118,9 @@ lazy_static! { let path = entry?.path(); if path.is_dir(){ list_items(&path, &base, &mut items)?; - } else { - if let Ok(path) = path.strip_prefix(base) { - let path = path.to_string_lossy().trim_end_matches(".ron").replace('/', "."); - items.push(path); - } + } else if let Ok(path) = path.strip_prefix(base) { + let path = path.to_string_lossy().trim_end_matches(".ron").replace('/', "."); + items.push(path); } } Ok(()) @@ -348,7 +346,7 @@ impl FromStr for ChatCommand { type Err = (); fn from_str(keyword: &str) -> Result { - let kwd = if keyword.chars().next() == Some('/') { + let kwd = if keyword.starts_with('/') { &keyword[1..] } else { &keyword[..] @@ -358,7 +356,7 @@ impl FromStr for ChatCommand { return Ok(*c); } } - return Err(()); + Err(()) } } diff --git a/common/src/comp/ability.rs b/common/src/comp/ability.rs index 7673123e86..eaa2b7ae6b 100644 --- a/common/src/comp/ability.rs +++ b/common/src/comp/ability.rs @@ -203,9 +203,10 @@ impl From<&CharacterAbility> for CharacterState { stage_exhausted: false, stage_time_active: Duration::default(), initialized: false, - transition_style: match *needs_timing { - true => TransitionStyle::Timed(TimingState::NotPressed), - false => TransitionStyle::Hold(HoldingState::Holding), + transition_style: if *needs_timing { + TransitionStyle::Timed(TimingState::NotPressed) + } else { + TransitionStyle::Hold(HoldingState::Holding) }, }), } diff --git a/common/src/comp/agent.rs b/common/src/comp/agent.rs index dffde7484e..d2bbb837b3 100644 --- a/common/src/comp/agent.rs +++ b/common/src/comp/agent.rs @@ -150,7 +150,7 @@ impl SpeechBubble { { match &self.message { SpeechBubbleMessage::Plain(m) => m.to_string(), - SpeechBubbleMessage::Localized(k, i) => i18n_variation(k.to_string(), *i).to_string(), + SpeechBubbleMessage::Localized(k, i) => i18n_variation(k.to_string(), *i), } } } diff --git a/common/src/comp/inventory/mod.rs b/common/src/comp/inventory/mod.rs index ef186d103f..b8767db06e 100644 --- a/common/src/comp/inventory/mod.rs +++ b/common/src/comp/inventory/mod.rs @@ -164,7 +164,7 @@ impl Inventory { } } self.recount_items(); - if leftovers.len() > 0 { + if !leftovers.is_empty() { Err(Error::Full(leftovers)) } else { Ok(()) @@ -187,7 +187,7 @@ impl Inventory { self.push(item).map(|overflow| leftovers.push(overflow)); } // else drop item if it was already in } - if leftovers.len() > 0 { + if !leftovers.is_empty() { Err(Error::Full(leftovers)) } else { Ok(()) diff --git a/common/src/comp/inventory/test.rs b/common/src/comp/inventory/test.rs index 30f9d0b63b..4b3db9f94d 100644 --- a/common/src/comp/inventory/test.rs +++ b/common/src/comp/inventory/test.rs @@ -31,7 +31,7 @@ fn push_all_full() { amount: 0, }; let Error::Full(leftovers) = inv - .push_all(TEST_ITEMS.iter().map(|a| a.clone())) + .push_all(TEST_ITEMS.iter().cloned()) .expect_err("Pushing into a full inventory somehow worked!"); assert_eq!(leftovers, TEST_ITEMS.clone()) } @@ -44,7 +44,7 @@ fn push_unique_all_full() { slots: TEST_ITEMS.iter().map(|a| Some(a.clone())).collect(), amount: 0, }; - inv.push_all_unique(TEST_ITEMS.iter().map(|a| a.clone())) + inv.push_all_unique(TEST_ITEMS.iter().cloned()) .expect("Pushing unique items into an inventory that already contains them didn't work!"); } @@ -56,7 +56,7 @@ fn push_all_empty() { slots: vec![None, None], amount: 0, }; - inv.push_all(TEST_ITEMS.iter().map(|a| a.clone())) + inv.push_all(TEST_ITEMS.iter().cloned()) .expect("Pushing items into an empty inventory didn't work!"); } @@ -68,8 +68,7 @@ fn push_all_unique_empty() { slots: vec![None, None], amount: 0, }; - inv.push_all_unique(TEST_ITEMS.iter().map(|a| a.clone())) - .expect( - "Pushing unique items into an empty inventory that didn't contain them didn't work!", - ); + inv.push_all_unique(TEST_ITEMS.iter().cloned()).expect( + "Pushing unique items into an empty inventory that didn't contain them didn't work!", + ); } diff --git a/common/src/figure/mod.rs b/common/src/figure/mod.rs index 4fb39425cd..876a2c7453 100644 --- a/common/src/figure/mod.rs +++ b/common/src/figure/mod.rs @@ -36,7 +36,7 @@ impl From<&DotVoxData> for Segment { if let Some(&color) = palette.get(voxel.i as usize) { segment .set( - Vec3::new(voxel.x, voxel.y, voxel.z).map(|e| i32::from(e)), + Vec3::new(voxel.x, voxel.y, voxel.z).map(i32::from), Cell::new(color), ) .unwrap(); @@ -195,7 +195,7 @@ impl MatSegment { voxel.y, voxel.z, ) - .map(|e| i32::from(e)), + .map(i32::from), block, ) .unwrap(); diff --git a/common/src/net/post2.rs b/common/src/net/post2.rs index 95307ac4bd..6429eedb24 100644 --- a/common/src/net/post2.rs +++ b/common/src/net/post2.rs @@ -374,7 +374,7 @@ mod tests { let mut server = postoffice.new_postboxes().next().unwrap(); for msg in &test_msgs { - client.send_message(msg.clone()); + client.send_message(*msg); } let mut recv_msgs = Vec::new(); diff --git a/common/src/path.rs b/common/src/path.rs index b64b74e669..156565cb64 100644 --- a/common/src/path.rs +++ b/common/src/path.rs @@ -268,12 +268,7 @@ where let satisfied = |pos: &Vec3| pos == &end; let mut new_astar = match astar.take() { - None => Astar::new( - 20_000, - start, - heuristic.clone(), - DefaultHashBuilder::default(), - ), + None => Astar::new(20_000, start, heuristic, DefaultHashBuilder::default()), Some(astar) => astar, }; diff --git a/common/src/state.rs b/common/src/state.rs index b48d4ad35a..fa7daf985b 100644 --- a/common/src/state.rs +++ b/common/src/state.rs @@ -306,10 +306,8 @@ impl State { // Apply terrain changes pub fn apply_terrain_changes(&self) { let mut terrain = self.ecs.write_resource::(); - let mut modified_blocks = std::mem::replace( - &mut self.ecs.write_resource::().blocks, - Default::default(), - ); + let mut modified_blocks = + std::mem::take(&mut self.ecs.write_resource::().blocks); // Apply block modifications // Only include in `TerrainChanges` if successful modified_blocks.retain(|pos, block| terrain.set(*pos, *block).is_ok()); diff --git a/common/src/states/climb.rs b/common/src/states/climb.rs index 7ddf2a7d57..21be05230b 100644 --- a/common/src/states/climb.rs +++ b/common/src/states/climb.rs @@ -54,9 +54,11 @@ impl CharacterBehavior for Data { Climb::Up | Climb::Down => 8, Climb::Hold => 1, }; - if let Err(_) = update + + if update .energy .try_change_by(-energy_use, EnergySource::Climb) + .is_err() { update.character = CharacterState::Idle {}; } diff --git a/common/src/states/glide.rs b/common/src/states/glide.rs index 3e6759780d..dc052e48c2 100644 --- a/common/src/states/glide.rs +++ b/common/src/states/glide.rs @@ -24,7 +24,7 @@ impl CharacterBehavior for Data { } // If there is a wall in front of character go to climb - if let Some(_) = data.physics.on_wall { + if data.physics.on_wall.is_some() { update.character = CharacterState::Climb; return update; } diff --git a/common/src/states/triple_strike.rs b/common/src/states/triple_strike.rs index 9421dbcbd7..924d8b0b1a 100644 --- a/common/src/states/triple_strike.rs +++ b/common/src/states/triple_strike.rs @@ -128,13 +128,12 @@ impl CharacterBehavior for Data { // Move player forward while in first third of each stage if update.vel.0.magnitude_squared() < BASE_SPEED.powf(2.0) { - update.vel.0 = update.vel.0 - + data.dt.0 - * (if data.physics.on_ground { - Vec3::new(0.0, 0.0, 500.0) // Jump upwards if on ground - } else { - Vec3::one() - } + adjusted_accel * Vec3::from(data.ori.0.xy())); + update.vel.0 += data.dt.0 + * (if data.physics.on_ground { + Vec3::new(0.0, 0.0, 500.0) // Jump upwards if on ground + } else { + Vec3::one() + } + adjusted_accel * Vec3::from(data.ori.0.xy())); let mag2 = update.vel.0.magnitude_squared(); if mag2 > BASE_SPEED.powf(2.0) { update.vel.0 = update.vel.0.normalized() * BASE_SPEED; diff --git a/common/src/sys/agent.rs b/common/src/sys/agent.rs index 7dbff74b1c..f2aa4891f7 100644 --- a/common/src/sys/agent.rs +++ b/common/src/sys/agent.rs @@ -88,13 +88,7 @@ impl<'a> System<'a> for Sys { { // Skip mounted entities if mount_state - .map(|ms| { - if let MountState::Unmounted = ms { - false - } else { - true - } - }) + .map(|ms| *ms != MountState::Unmounted) .unwrap_or(false) { continue; diff --git a/common/src/sys/combat.rs b/common/src/sys/combat.rs index 1d2675e679..b650692f31 100644 --- a/common/src/sys/combat.rs +++ b/common/src/sys/combat.rs @@ -124,14 +124,14 @@ impl<'a> System<'a> for Sys { } if rand::random() { - healthchange = healthchange * 1.2; + healthchange *= 1.2; } // Block if character_b.is_block() && ori_b.0.angle_between(pos.0 - pos_b.0) < BLOCK_ANGLE.to_radians() / 2.0 { - healthchange = healthchange * (1.0 - BLOCK_EFFICIENCY) + healthchange *= 1.0 - BLOCK_EFFICIENCY } server_emitter.emit(ServerEvent::Damage { diff --git a/common/src/sys/projectile.rs b/common/src/sys/projectile.rs index dc7c8212a6..c9471eeebf 100644 --- a/common/src/sys/projectile.rs +++ b/common/src/sys/projectile.rs @@ -119,20 +119,18 @@ impl<'a> System<'a> for Sys { projectile::Effect::Possess => { if other != projectile.owner.unwrap() { if let Some(owner) = projectile.owner { - server_emitter.emit(ServerEvent::Possess(owner.into(), other)); + server_emitter.emit(ServerEvent::Possess(owner, other)); } } }, _ => {}, } } - } else { - if let Some(dir) = velocities - .get(entity) - .and_then(|vel| vel.0.try_normalized()) - { - ori.0 = dir.into(); - } + } else if let Some(dir) = velocities + .get(entity) + .and_then(|vel| vel.0.try_normalized()) + { + ori.0 = dir.into(); } if projectile.time_left == Duration::default() { diff --git a/common/src/sys/stats.rs b/common/src/sys/stats.rs index 5321b54c7a..aacf15e00a 100644 --- a/common/src/sys/stats.rs +++ b/common/src/sys/stats.rs @@ -77,10 +77,12 @@ impl<'a> System<'a> for Sys { // Accelerate recharging energy if not wielding. match character_state { CharacterState::Idle { .. } | CharacterState::Sit { .. } => { - if { + let res = { let energy = energy.get_unchecked(); energy.current() < energy.maximum() - } { + }; + + if res { let mut energy = energy.get_mut_unchecked(); // Have to account for Calc I differential equations due to acceleration energy.change_by( diff --git a/common/src/terrain/mod.rs b/common/src/terrain/mod.rs index c2135f5c74..0b9a348325 100644 --- a/common/src/terrain/mod.rs +++ b/common/src/terrain/mod.rs @@ -41,12 +41,7 @@ impl TerrainChunkMeta { } } - pub fn name(&self) -> &str { - self.name - .as_ref() - .map(|s| s.as_str()) - .unwrap_or("Wilderness") - } + pub fn name(&self) -> &str { self.name.as_deref().unwrap_or("Wilderness") } pub fn biome(&self) -> BiomeKind { self.biome } } diff --git a/common/src/terrain/structure.rs b/common/src/terrain/structure.rs index 14ea9806be..3242c4deac 100644 --- a/common/src/terrain/structure.rs +++ b/common/src/terrain/structure.rs @@ -53,8 +53,7 @@ pub struct Structure { impl Structure { pub fn load_group(specifier: &str) -> Vec> { let spec = assets::load::(&["world.manifests.", specifier].concat()); - return spec - .unwrap() + spec.unwrap() .0 .iter() .map(|sp| { @@ -63,7 +62,7 @@ impl Structure { }) .unwrap() }) - .collect(); + .collect() } pub fn with_center(mut self, center: Vec3) -> Self { @@ -145,10 +144,7 @@ impl Asset for Structure { }, }; - let _ = vol.set( - Vec3::new(voxel.x, voxel.y, voxel.z).map(|e| i32::from(e)), - block, - ); + let _ = vol.set(Vec3::new(voxel.x, voxel.y, voxel.z).map(i32::from), block); } Ok(Structure { diff --git a/common/src/util/color.rs b/common/src/util/color.rs index a1b91394da..907144828f 100644 --- a/common/src/util/color.rs +++ b/common/src/util/color.rs @@ -1,6 +1,7 @@ use vek::{Mat3, Rgb, Rgba, Vec3}; #[inline(always)] +#[allow(clippy::excessive_precision)] pub fn srgb_to_linear(col: Rgb) -> Rgb { col.map(|c| { if c <= 0.104 { @@ -12,6 +13,7 @@ pub fn srgb_to_linear(col: Rgb) -> Rgb { } #[inline(always)] +#[allow(clippy::excessive_precision)] pub fn linear_to_srgb(col: Rgb) -> Rgb { col.map(|c| { if c <= 0.0060 { @@ -37,6 +39,7 @@ pub fn linear_to_srgba(col: Rgba) -> Rgba { /// Convert rgb to hsv. Expects rgb to be [0, 1]. #[inline(always)] +#[allow(clippy::many_single_char_names)] pub fn rgb_to_hsv(rgb: Rgb) -> Vec3 { let (r, g, b) = rgb.into_tuple(); let (max, min, diff, add) = { @@ -69,6 +72,7 @@ pub fn rgb_to_hsv(rgb: Rgb) -> Vec3 { /// Convert hsv to rgb. Expects h [0, 360], s [0, 1], v [0, 1] #[inline(always)] +#[allow(clippy::many_single_char_names)] pub fn hsv_to_rgb(hsv: Vec3) -> Rgb { let (h, s, v) = hsv.into_tuple(); let c = s * v; diff --git a/common/src/util/dir.rs b/common/src/util/dir.rs index 4ea54d2bc7..763c527fdd 100644 --- a/common/src/util/dir.rs +++ b/common/src/util/dir.rs @@ -88,7 +88,7 @@ impl std::ops::Deref for Dir { } impl From> for Dir { - fn from(dir: Vec3) -> Self { Dir::new(dir.into()) } + fn from(dir: Vec3) -> Self { Dir::new(dir) } } /// Begone ye NaN's /// Slerp two `Vec3`s skipping the slerp if their directions are very close @@ -102,20 +102,25 @@ fn slerp_normalized(from: vek::Vec3, to: vek::Vec3, factor: f32) -> ve // Ensure from is normalized #[cfg(debug_assertions)] { - if { + let unnormalized = { let len_sq = from.magnitude_squared(); len_sq < 0.999 || len_sq > 1.001 - } { + }; + + if unnormalized { panic!("Called slerp_normalized with unnormalized from: {:?}", from); } } + // Ensure to is normalized #[cfg(debug_assertions)] { - if { + let unnormalized = { let len_sq = from.magnitude_squared(); len_sq < 0.999 || len_sq > 1.001 - } { + }; + + if unnormalized { panic!("Called slerp_normalized with unnormalized to: {:?}", to); } } diff --git a/common/src/util/mod.rs b/common/src/util/mod.rs index ff0d99e80b..4dbe8eb127 100644 --- a/common/src/util/mod.rs +++ b/common/src/util/mod.rs @@ -4,8 +4,8 @@ mod dir; pub const GIT_VERSION: &str = include_str!(concat!(env!("OUT_DIR"), "/githash")); lazy_static::lazy_static! { - pub static ref GIT_HASH: &'static str = GIT_VERSION.split("/").nth(0).expect("failed to retrieve git_hash!"); - pub static ref GIT_DATE: &'static str = GIT_VERSION.split("/").nth(1).expect("failed to retrieve git_date!"); + pub static ref GIT_HASH: &'static str = GIT_VERSION.split('/').nth(0).expect("failed to retrieve git_hash!"); + pub static ref GIT_DATE: &'static str = GIT_VERSION.split('/').nth(1).expect("failed to retrieve git_date!"); } pub use color::*; diff --git a/common/src/vol.rs b/common/src/vol.rs index 85626c37d7..45edcfb905 100644 --- a/common/src/vol.rs +++ b/common/src/vol.rs @@ -269,6 +269,6 @@ impl<'a, T: ReadVol> Iterator for DefaultVolIterator<'a, T> { return Some((pos, vox)); } } - return None; + None } }