From a3e42829a00dadb7e00e9c12304c71d3ea39b000 Mon Sep 17 00:00:00 2001 From: jam1garner Date: Mon, 21 Oct 2019 15:47:01 -0400 Subject: [PATCH] Fix clippy suggestions and general touch ups --- src/motion_lib/asm.rs | 12 ++++++---- src/motion_lib/disasm.rs | 51 ++++++++++++++++++++-------------------- src/motion_lib/hash40.rs | 22 +++++++---------- src/motion_lib/lib.rs | 9 +++---- src/motion_lib/mlist.rs | 4 +--- src/yamlist/main.rs | 32 ++++++++++++------------- 6 files changed, 63 insertions(+), 67 deletions(-) diff --git a/src/motion_lib/asm.rs b/src/motion_lib/asm.rs index ecbafe2..497148d 100644 --- a/src/motion_lib/asm.rs +++ b/src/motion_lib/asm.rs @@ -27,10 +27,10 @@ pub fn assemble(cursor: &mut Cursor>, mlist: &MList) -> Result<(), Error fn write_motion(cursor: &mut Cursor>, motion: &Motion) -> Result<(), Error> { let anm_cnt = motion.animations.len(); if anm_cnt > 3 { - return Err(Error::new( + Err(Error::new( ErrorKind::InvalidData, "Animation count cannot exceed 3", - )); + ))?; } cursor.write_hash40::(&motion.game_script)?; @@ -39,7 +39,7 @@ fn write_motion(cursor: &mut Cursor>, motion: &Motion) -> Result<(), Err cursor.write_u8(anm_cnt as u8)?; let temp = (motion.scripts.len() * 8) as u32; - let size = temp + (if let Some(_) = &motion.extra { 4 } else { 0 }); + let size = temp + (motion.extra.is_some() as u32) * 4; cursor.write_u32::(size)?; for i in motion.animations.iter() { @@ -49,7 +49,9 @@ fn write_motion(cursor: &mut Cursor>, motion: &Motion) -> Result<(), Err cursor.write_u8(i.unk)?; } //align - cursor.set_position((cursor.position() + 3 >> 2) << 2); + const ALIGN: u64 = 4; + const ALIGN_MASK: u64 = ALIGN - 1; + cursor.set_position((cursor.position() + ALIGN_MASK) & !ALIGN_MASK); for script in motion.scripts.iter() { cursor.write_hash40::(&script)?; @@ -59,7 +61,7 @@ fn write_motion(cursor: &mut Cursor>, motion: &Motion) -> Result<(), Err cursor.write_u8(x.xlu_start)?; cursor.write_u8(x.xlu_end)?; cursor.write_u8(x.cancel_frame)?; - cursor.write_u8(if x.no_stop_intp { 1 } else { 0 })?; + cursor.write_u8(x.no_stop_intp as u8)?; } Ok(()) diff --git a/src/motion_lib/disasm.rs b/src/motion_lib/disasm.rs index 9202920..c8dacee 100644 --- a/src/motion_lib/disasm.rs +++ b/src/motion_lib/disasm.rs @@ -29,37 +29,36 @@ pub fn disassemble(cursor: &mut Cursor>) -> Result { } fn read_motion(cursor: &mut Cursor>) -> Result { - let game = cursor.read_hash40::()?; + let game_script = cursor.read_hash40::()?; let flags = cursor.read_u16::()?; - let frames = cursor.read_u8()?; + let transition = cursor.read_u8()?; let anm_cnt = cursor.read_u8()?; if anm_cnt > 3 { - return Err(Error::new( + Err(Error::new( ErrorKind::InvalidData, "Animation count cannot exceed 3", - )); + ))?; } let size = cursor.read_u32::()?; - let mut anims = Vec::::with_capacity(anm_cnt as usize); - for _ in 0..anm_cnt { - anims.push(Animation { - name: cursor.read_hash40::()?, - unk: 0, - }); - } - for i in 0..anm_cnt { - anims[i as usize].unk = cursor.read_u8()? - } + let animations = (0..anm_cnt) + .map(|_| cursor.read_hash40::()) + .collect::, Error>>()? + .iter() + .map(|name| Ok(Animation { + name: *name, + unk: cursor.read_u8()? + })) + .collect::, Error>>()?; //align by 4 - cursor.set_position((cursor.position() + 3 >> 2) << 2); + const ALIGN: u64 = 4; + const ALIGN_MASK: u64 = ALIGN - 1; + cursor.set_position((cursor.position() + ALIGN_MASK) & !ALIGN_MASK); - let count = size / 8; - let mut scripts = Vec::::with_capacity(count as usize); - for _ in 0..count { - scripts.push(cursor.read_hash40::()?); - } + let scripts = (0..size / 8) + .map(|_| cursor.read_hash40::()) + .collect::, Error>>()?; let extra: Option = if size % 8 == 4 { Some(Extra { @@ -73,11 +72,11 @@ fn read_motion(cursor: &mut Cursor>) -> Result { }; Ok(Motion { - game_script: game, - flags: flags, - transition: frames, - animations: anims, - scripts: scripts, - extra: extra, + game_script, + flags, + transition, + animations, + scripts, + extra, }) } diff --git a/src/motion_lib/hash40.rs b/src/motion_lib/hash40.rs index cfbabef..4081948 100644 --- a/src/motion_lib/hash40.rs +++ b/src/motion_lib/hash40.rs @@ -35,20 +35,18 @@ pub fn load_labels(file: &str) -> Result<(), Error> { } } -#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] -pub struct Hash40 { - pub value: u64, -} +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub struct Hash40(pub u64); impl Hash40 { #[inline] pub fn crc(&self) -> u32 { - self.value as u32 + self.0 as u32 } #[inline] pub fn len(&self) -> u8 { - (self.value >> 32) as u8 + (self.0 >> 32) as u8 } pub fn to_label(&self) -> String { @@ -69,7 +67,7 @@ pub trait ReadHash40: ReadBytesExt { impl ReadHash40 for R { fn read_hash40(&mut self) -> Result { match self.read_u64::() { - Ok(x) => Ok(Hash40 { value: x }), + Ok(x) => Ok(Hash40(x)), Err(y) => Err(y), } } @@ -81,14 +79,14 @@ pub trait WriteHash40: WriteBytesExt { } impl WriteHash40 for W { fn write_hash40(&mut self, hash: &Hash40) -> Result<(), Error> { - self.write_u64::(hash.value) + self.write_u64::(hash.0) } } // Hash40 -> string impl ToString for Hash40 { fn to_string(&self) -> String { - format!("0x{:010x}", self.value) + format!("0x{:010x}", self.0) } } @@ -109,7 +107,7 @@ impl<'de> de::Visitor<'de> for Hash40Visitor { fn visit_str(self, value: &str) -> Result { if value.starts_with("0x") { match u64::from_str_radix(&value[2..], 16) { - Ok(x) => Ok(Hash40 { value: x }), + Ok(x) => Ok(Hash40(x)), Err(y) => Err(E::custom(y)), } } else { @@ -132,9 +130,7 @@ impl<'de> Deserialize<'de> for Hash40 { //exposed function to compute a hash pub fn to_hash40(word: &str) -> Hash40 { - Hash40 { - value: crc32_with_len(word), - } + Hash40(crc32_with_len(word)) } fn crc32_with_len(word: &str) -> u64 { diff --git a/src/motion_lib/lib.rs b/src/motion_lib/lib.rs index 919379f..4cfbcf9 100644 --- a/src/motion_lib/lib.rs +++ b/src/motion_lib/lib.rs @@ -1,5 +1,6 @@ mod asm; mod disasm; +#[allow(clippy::all)] pub mod hash40; pub mod mlist; @@ -18,12 +19,12 @@ pub fn open>(file: P) -> Result { } } -pub fn save>(file: P, mlist: &MList) -> Result<(), Error> { - match File::create(file) { - Ok(mut x) => { +pub fn save>(path: P, mlist: &MList) -> Result<(), Error> { + match File::create(path) { + Ok(mut file) => { let mut cursor = Cursor::new(Vec::::new()); asm::assemble(&mut cursor, mlist)?; - x.write_all(&cursor.into_inner())?; + file.write_all(&cursor.into_inner())?; Ok(()) } Err(y) => Err(y), diff --git a/src/motion_lib/mlist.rs b/src/motion_lib/mlist.rs index 69ae454..2c8fa11 100644 --- a/src/motion_lib/mlist.rs +++ b/src/motion_lib/mlist.rs @@ -3,9 +3,7 @@ use indexmap::IndexMap; use serde::{Deserialize, Serialize, Serializer}; // "motion" -pub const MAGIC: Hash40 = Hash40 { - value: 0x06f5fea1e8, -}; +pub const MAGIC: Hash40 = Hash40(0x06f5fea1e8); //TODO: overuse of public attributes? Create .new method instead? #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/yamlist/main.rs b/src/yamlist/main.rs index 161eb34..07489c3 100644 --- a/src/yamlist/main.rs +++ b/src/yamlist/main.rs @@ -118,7 +118,7 @@ fn main() { arg_index += 1; } - if labelname.len() > 0 { + if !labelname.is_empty() { if let Err(e) = motion_lib::hash40::load_labels(&labelname) { println!("Error loading labels: {}", e); } @@ -126,7 +126,7 @@ fn main() { match mode { Mode::Disasm {file} => { - let o = if outname.len() > 0 { + let o = if !outname.is_empty() { &outname } else { "out.yml" @@ -135,13 +135,13 @@ fn main() { match convert_to_yaml(&file, o) { Ok(_) => {} Err(y) => { - let e: &Error = y.borrow(); + let e: &dyn Error = y.borrow(); println!("ERROR: {}", e); } } } Mode::Asm {file} => { - let o = if outname.len() > 0 { + let o = if !outname.is_empty() { &outname } else { "out.bin" @@ -150,16 +150,16 @@ fn main() { match convert_to_bin(&file, o) { Ok(_) => {} Err(y) => { - let e: &Error = y.borrow(); + let e: &dyn Error = y.borrow(); println!("ERROR: {}", e); } } } - Mode::Patch {file, patch} => { - + Mode::Patch {..} => { + unimplemented!() } - Mode::Compare {a, b} => { - + Mode::Compare {..} => { + unimplemented!() } } } @@ -177,7 +177,7 @@ fn print_help_text() { println!(" -o (out) "); } -fn convert_to_yaml(i: &str, o: &str) -> Result<(), Box> { +fn convert_to_yaml(i: &str, o: &str) -> Result<(), Box> { match motion_lib::open(i) { Ok(x) => { let mut f = File::create(o)?; @@ -189,12 +189,12 @@ fn convert_to_yaml(i: &str, o: &str) -> Result<(), Box> { } } -fn convert_to_bin(i: &str, o: &str) -> Result<(), Box> { - let mut f = File::open(i)?; - let mut s: String = String::default(); - f.read_to_string(&mut s)?; - match from_str::(&s) { - Ok(x) => match motion_lib::save(o, &x) { +fn convert_to_bin(in_path: &str, out_path: &str) -> Result<(), Box> { + let mut file = File::open(in_path)?; + let mut contents: String = String::default(); + file.read_to_string(&mut contents)?; + match from_str(&contents) { + Ok(mlist) => match motion_lib::save(out_path, &mlist) { Ok(_) => Ok(()), Err(y) => Err(Box::new(y)), },