Fix clippy suggestions and general touch ups

src/motion_lib/asm.rs

@@ -27,10 +27,10 @@ pub fn assemble(cursor: &mut Cursor<Vec<u8>>, mlist: &MList) -> Result<(), Error
 fn write_motion(cursor: &mut Cursor<Vec<u8>>, 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::<LittleEndian>(&motion.game_script)?;
@@ -39,7 +39,7 @@ fn write_motion(cursor: &mut Cursor<Vec<u8>>, 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::<LittleEndian>(size)?;
 
     for i in motion.animations.iter() {
@@ -49,7 +49,9 @@ fn write_motion(cursor: &mut Cursor<Vec<u8>>, 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::<LittleEndian>(&script)?;
@@ -59,7 +61,7 @@ fn write_motion(cursor: &mut Cursor<Vec<u8>>, 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(())

src/motion_lib/disasm.rs

@@ -29,37 +29,36 @@ pub fn disassemble(cursor: &mut Cursor<Vec<u8>>) -> Result<MList, Error> {
 }
 
 fn read_motion(cursor: &mut Cursor<Vec<u8>>) -> Result<Motion, Error> {
-    let game = cursor.read_hash40::<LittleEndian>()?;
+    let game_script = cursor.read_hash40::<LittleEndian>()?;
     let flags = cursor.read_u16::<LittleEndian>()?;
-    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::<LittleEndian>()?;
 
-    let mut anims = Vec::<Animation>::with_capacity(anm_cnt as usize);
-    for _ in 0..anm_cnt {
-        anims.push(Animation {
-            name: cursor.read_hash40::<LittleEndian>()?,
-            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::<LittleEndian>())
+        .collect::<Result<Vec<_>, Error>>()?
+        .iter()
+        .map(|name| Ok(Animation {
+            name: *name,
+            unk: cursor.read_u8()?
+        }))
+        .collect::<Result<Vec<_>, 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::<Hash40>::with_capacity(count as usize);
-    for _ in 0..count {
-        scripts.push(cursor.read_hash40::<LittleEndian>()?);
-    }
+    let scripts = (0..size / 8)
+        .map(|_| cursor.read_hash40::<LittleEndian>())
+        .collect::<Result<Vec<_>, Error>>()?;
 
     let extra: Option<Extra> = if size % 8 == 4 {
         Some(Extra {
@@ -73,11 +72,11 @@ fn read_motion(cursor: &mut Cursor<Vec<u8>>) -> Result<Motion, Error> {
     };
 
     Ok(Motion {
-        game_script: game,
-        flags: flags,
-        transition: frames,
-        animations: anims,
-        scripts: scripts,
-        extra: extra,
+        game_script,
+        flags,
+        transition,
+        animations,
+        scripts,
+        extra,
     })
 }

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<R: Read> ReadHash40 for R {
     fn read_hash40<T: ByteOrder>(&mut self) -> Result<Hash40, Error> {
         match self.read_u64::<T>() {
-            Ok(x) => Ok(Hash40 { value: x }),
+            Ok(x) => Ok(Hash40(x)),
             Err(y) => Err(y),
         }
     }
@@ -81,14 +79,14 @@ pub trait WriteHash40: WriteBytesExt {
 }
 impl<W: Write> WriteHash40 for W {
     fn write_hash40<T: ByteOrder>(&mut self, hash: &Hash40) -> Result<(), Error> {
-        self.write_u64::<T>(hash.value)
+        self.write_u64::<T>(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<E: de::Error>(self, value: &str) -> Result<Self::Value, E> {
         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 {

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<P: AsRef<Path>>(file: P) -> Result<MList, Error> {
     }
 }
 
-pub fn save<P: AsRef<Path>>(file: P, mlist: &MList) -> Result<(), Error> {
-    match File::create(file) {
-        Ok(mut x) => {
+pub fn save<P: AsRef<Path>>(path: P, mlist: &MList) -> Result<(), Error> {
+    match File::create(path) {
+        Ok(mut file) => {
             let mut cursor = Cursor::new(Vec::<u8>::new());
             asm::assemble(&mut cursor, mlist)?;
-            x.write_all(&cursor.into_inner())?;
+            file.write_all(&cursor.into_inner())?;
             Ok(())
         }
         Err(y) => Err(y),

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)]

src/yamlist/main.rs

@@ -118,15 +118,15 @@ 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);
         }
     }
 
     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)         <OUTNAME>");
 }
 
-fn convert_to_yaml(i: &str, o: &str) -> Result<(), Box<Error>> {
+fn convert_to_yaml(i: &str, o: &str) -> Result<(), Box<dyn Error>> {
     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<Error>> {
     }
 }
 
-fn convert_to_bin(i: &str, o: &str) -> Result<(), Box<Error>> {
-    let mut f = File::open(i)?;
-    let mut s: String = String::default();
-    f.read_to_string(&mut s)?;
-    match from_str::<motion_lib::mlist::MList>(&s) {
-        Ok(x) => match motion_lib::save(o, &x) {
+fn convert_to_bin(in_path: &str, out_path: &str) -> Result<(), Box<dyn Error>> {
+    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)),
         },