From a6bcc763e9353e187fc40fad6063028ff5c2e36f Mon Sep 17 00:00:00 2001 From: Max Bradbury Date: Fri, 16 Oct 2020 11:55:15 +0100 Subject: [PATCH] more clippy fixes --- src/exit.rs | 4 +- src/game.rs | 137 ++++++++++++++++++++++--------------------------- src/image.rs | 6 +-- src/lib.rs | 2 +- src/palette.rs | 2 +- src/room.rs | 20 ++++---- 6 files changed, 79 insertions(+), 92 deletions(-) diff --git a/src/exit.rs b/src/exit.rs index 9f1d14d..a0d6c74 100644 --- a/src/exit.rs +++ b/src/exit.rs @@ -74,8 +74,8 @@ impl FromStr for Exit { let position = position.unwrap(); - let effect = if parts.next().is_some() { - Transition::from(parts.next().unwrap()) + let effect = if let Some(transition_line) = parts.next() { + Transition::from(transition_line) } else { Transition::None }; diff --git a/src/game.rs b/src/game.rs index b23d92f..3633af9 100644 --- a/src/game.rs +++ b/src/game.rs @@ -6,7 +6,7 @@ use std::str::FromStr; use std::collections::HashMap; use std::borrow::BorrowMut; use std::fmt; -use std::fmt::{Display, Formatter}; +use std::fmt::Display; /// in very early versions of Bitsy, room tiles were defined as single alphanumeric characters - /// so there was a maximum of 36 unique tiles. later versions are comma-separated. @@ -60,7 +60,8 @@ pub struct InvalidVersion; impl Version { fn from(str: &str) -> Result { - let parts: Vec<&str> = str.split(".").collect(); + let parts: Vec<&str> = str.split('.').collect(); + if parts.len() == 2 { Ok(Version { major: parts[0].parse().unwrap(), @@ -85,11 +86,11 @@ pub enum NotFound { impl Display for NotFound { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f,"Not found: {} data", match self { - &NotFound::Anything => "game", - &NotFound::Avatar => "avatar", - &NotFound::Room => "room", - &NotFound::Sprite => "sprite", - &NotFound::Tile => "tile", + NotFound::Anything => "game", + NotFound::Avatar => "avatar", + NotFound::Room => "room", + NotFound::Sprite => "sprite", + NotFound::Tile => "tile", }) } } @@ -124,7 +125,7 @@ impl Game { // would be nice to *try* to parse a game, and catalogue any and all errors without crashing, // for display purposes etc. pub fn from(string: String) -> Result { - if string.clone().trim() == "".to_string() { + if string.trim() == "" { return Err(NotFound::Anything); } @@ -134,7 +135,7 @@ impl Game { string = transform_line_endings(string, TransformMode::LF) } - let string = string.trim_start_matches("\n").to_string(); + let string = string.trim_start_matches('\n').to_string(); let mut segments = segments_from_string(string); let mut name = "".to_string(); @@ -187,8 +188,8 @@ impl Game { if segment.starts_with("# BITSY VERSION") { let segment = segment.replace("# BITSY VERSION ", ""); let segment = Version::from(&segment); - if segment.is_ok() { - version = Some(segment.unwrap()); + if let Ok(segment) = segment { + version = Some(segment); } } else if segment.starts_with("! ROOM_FORMAT") { let segment = segment.replace("! ROOM_FORMAT ", ""); @@ -203,7 +204,7 @@ impl Game { if font == Font::Custom { custom_font = Some(segment.to_string()); } - } else if segment.trim() == "TEXT_DIRECTION RTL".to_string() { + } else if segment.trim() == "TEXT_DIRECTION RTL" { text_direction = TextDirection::RightToLeft; } else if segment.starts_with("PAL ") { palettes.push(Palette::from(segment)); @@ -216,11 +217,10 @@ impl Game { tiles.push(Tile::from(segment)); } else if segment.starts_with("SPR ") { let sprite = Sprite::from(segment); - if sprite.is_ok() { - let sprite = sprite.unwrap(); - if ! avatar_exists && sprite.id == "A".to_string() { - avatar_exists = true; - } + + if let Ok(sprite) = sprite { + avatar_exists |= sprite.id == "A"; + sprites.push(sprite); } } else if segment.starts_with("ITM ") { @@ -228,9 +228,8 @@ impl Game { } else if segment.starts_with("DLG ") { dialogues.push(Dialogue::from(segment)); } else if segment.starts_with("END ") { - let ending = Ending::from_str(&segment); - if ending.is_ok() { - endings.push(ending.unwrap()); + if let Ok(ending) = Ending::from_str(&segment) { + endings.push(ending); } } else if segment.starts_with("VAR ") { variables.push(Variable::from(segment)); @@ -272,10 +271,9 @@ impl Game { |sprite| sprite.id == id ); - if index.is_some() { - Ok(&self.sprites[index.unwrap()]) - } else { - Err(NotFound::Sprite) + match index { + Some(index) => Ok(&self.sprites[index]), + None => Err(NotFound::Sprite), } } @@ -284,10 +282,9 @@ impl Game { |tile| tile.id == id ); - if index.is_some() { - Ok(&self.tiles[index.unwrap()]) - } else { - Err(NotFound::Tile) + match index { + Some(index) => Ok(&self.tiles[index]), + None => Err(NotFound::Tile), } } @@ -296,10 +293,9 @@ impl Game { |room| room.id == id ); - if index.is_some() { - Ok(&self.rooms[index.unwrap()]) - } else { - Err(NotFound::Room) + match index { + Some(index) => Ok(&self.rooms[index]), + None => Err(NotFound::Room), } } @@ -312,9 +308,8 @@ impl Game { let mut tiles: Vec<&Tile> = Vec::new(); for id in ids { - let tile = self.get_tile_by_id(id); - if tile.is_ok() { - tiles.push(tile.unwrap()); + if let Ok(tile) = self.get_tile_by_id(id) { + tiles.push(tile); } } @@ -331,9 +326,9 @@ impl Game { tile_ids.dedup(); // remove 0 as this isn't a real tile let zero_index = tile_ids.iter() - .position(|i| i == &"0".to_string()); - if zero_index.is_some() { - tile_ids.remove(zero_index.unwrap()); + .position(|i| i == "0"); + if let Some(zero_index) = zero_index { + tile_ids.remove(zero_index); } // remove Ok once this function returns a result Ok(self.get_tiles_by_ids(tile_ids)) @@ -428,11 +423,9 @@ impl Game { item.id = item_id_changes[&item.id].clone(); } - if item.dialogue_id.is_some() { - let key = item.dialogue_id.clone().unwrap(); - let change = dialogue_id_changes.get(&key); - if change.is_some() { - item.dialogue_id = Some(change.unwrap().clone()); + if let Some(key) = item.dialogue_id.clone() { + if let Some(change) = dialogue_id_changes.get(&key) { + item.dialogue_id = Some(change.clone()); } } @@ -443,7 +436,7 @@ impl Game { // to insert any new room, we need to know the new IDs of every room // to maintain the integrity of exits and endings - let mut all_room_ids = self.room_ids().clone(); + let mut all_room_ids = self.room_ids(); for room in &game.rooms { let old = room.id.clone(); @@ -457,16 +450,13 @@ impl Game { for room in &game.rooms { let mut room = room.clone(); - let room_id_change = room_id_changes.get(&room.id); - if room_id_change.is_some() { - room.id = room_id_change.unwrap().clone(); + if let Some(room_id_change) = room_id_changes.get(&room.id) { + room.id = room_id_change.clone(); } - if room.palette_id.is_some() { - let key = room.palette_id.clone().unwrap(); - let change = palette_id_changes.get(&key); - if change.is_some() { - room.palette_id = Some(change.unwrap().clone()); + if let Some(key) = room.palette_id.clone() { + if let Some(change) = palette_id_changes.get(&key) { + room.palette_id = Some(change.clone()); } } @@ -487,16 +477,14 @@ impl Game { let mut exit = exit.clone(); let key = exit.exit.room_id.clone(); - let change = room_id_changes.get(&key); - if change.is_some() { - exit.exit.room_id = change.unwrap().clone(); + + if let Some(change) = room_id_changes.get(&key) { + exit.exit.room_id = change.clone(); } - if exit.dialogue_id.is_some() { - let key = exit.dialogue_id.clone().unwrap(); - let dialogue_change = dialogue_id_changes.get(&key); - if dialogue_change.is_some() { - exit.dialogue_id = Some(dialogue_change.unwrap().clone()); + if let Some(key) = exit.dialogue_id.clone() { + if let Some(dialogue_change) = dialogue_id_changes.get(&key) { + exit.dialogue_id = Some(dialogue_change.clone()); } } @@ -506,10 +494,11 @@ impl Game { room.endings = room.endings.iter().map(|ending| { let mut ending = ending.clone(); let key = ending.id.clone(); - let change = ending_id_changes.get(&key); - if change.is_some() { - ending.id = change.unwrap().clone(); + + if let Some(change) = ending_id_changes.get(&key) { + ending.id = change.clone(); } + ending }).collect(); @@ -521,22 +510,19 @@ impl Game { for sprite in &game.sprites { let mut sprite = sprite.clone(); // avoid having two avatars - if sprite.id == "A".to_string() { + if sprite.id == "A" { sprite.id = "0".to_string(); // just a default value for replacement } - if sprite.dialogue_id.is_some() { - let key = sprite.dialogue_id.clone().unwrap(); + if let Some(key) = sprite.dialogue_id.clone() { if dialogue_id_changes.contains_key(&key) { sprite.dialogue_id = Some(dialogue_id_changes[&key].clone()); } } - if sprite.room_id.is_some() { - let key = sprite.room_id.clone().unwrap(); - let change = room_id_changes.get(&key); - if change.is_some() { - sprite.room_id = Some(change.unwrap().clone()); + if let Some(key) = sprite.room_id.clone() { + if let Some(change) = room_id_changes.get(&key) { + sprite.room_id = Some(change.clone()); } } @@ -689,8 +675,8 @@ impl Game { None } - pub fn find_tile_with_animation(&self, animation: &Vec) -> Option<&Tile> { - self.tiles.iter().find(|&tile| &tile.animation_frames == animation) + pub fn find_tile_with_animation(&self, animation: &[Image]) -> Option<&Tile> { + self.tiles.iter().find(|&tile| tile.animation_frames.as_slice() == animation) } /// adds a palette safely and returns the ID @@ -705,12 +691,13 @@ impl Game { /// adds a tile safely and returns the ID pub fn add_tile(&mut self, mut tile: Tile) -> String { - if tile.id == "0".to_string() || self.tile_ids().contains(&tile.id) { + if tile.id == "0" || self.tile_ids().contains(&tile.id) { let new_id = self.new_tile_id(); if new_id != tile.id { tile.id = new_id; } } + let id = tile.id.clone(); self.tiles.push(tile); id @@ -784,7 +771,7 @@ impl Game { let mut unique_tiles: Vec = Vec::new(); let mut tile_id_changes: HashMap = HashMap::new(); - while tiles_temp.len() > 0 { + while !tiles_temp.is_empty() { let tile = tiles_temp.pop().unwrap(); if tile == crate::mock::tile_background() { diff --git a/src/image.rs b/src/image.rs index c38bb7e..7312c0c 100644 --- a/src/image.rs +++ b/src/image.rs @@ -89,7 +89,7 @@ impl ToString for Image { } pub(crate) fn animation_frames_from_string(string: String) -> Vec { - let frames: Vec<&str> = string.split(">").collect(); + let frames: Vec<&str> = string.split('>').collect(); frames.iter().map(|&frame| Image::from(frame.to_string())).collect() } @@ -146,7 +146,7 @@ mod test { let output = Image::from( include_str!("test-resources/image-oob").to_string() ); - + let expected = Image { pixels: vec![ 1,1,1,1,1,1,1,1, @@ -159,7 +159,7 @@ mod test { 0,0,0,0,0,0,0,0, ] }; - + assert_eq!(output, expected); } diff --git a/src/lib.rs b/src/lib.rs index b82079c..f127f6e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,7 +62,7 @@ impl AnimationFrames for Vec { let mut string = String::new(); let last_frame = self.len() - 1; - for (i, frame) in self.into_iter().enumerate() { + for (i, frame) in self.iter().enumerate() { string.push_str(&frame.to_string()); if i < last_frame { diff --git a/src/palette.rs b/src/palette.rs index 024f3bd..99eaf43 100644 --- a/src/palette.rs +++ b/src/palette.rs @@ -14,7 +14,7 @@ impl From for Palette { let id = lines[0].replace("PAL ", ""); let name = match lines[1].starts_with("NAME") { - true => Some(lines[1].replace("NAME ", "").to_string()), + true => Some(lines[1].replace("NAME ", "")), false => None, }; diff --git a/src/room.rs b/src/room.rs index 980ebef..03d38d5 100644 --- a/src/room.rs +++ b/src/room.rs @@ -33,18 +33,17 @@ impl Room { } fn wall_line(&self) -> String { - if self.walls.len() > 0 { - optional_data_line("WAL", Some(self.walls.join(","))) - } else { + if self.walls.is_empty() { "".to_string() + } else { + optional_data_line("WAL", Some(self.walls.join(","))) } } fn palette_line(&self) -> String { - if self.palette_id.is_some() { - optional_data_line("PAL", Some(self.palette_id.as_ref().unwrap())) - } else { - "".to_string() + match &self.palette_id { + Some(id) => optional_data_line("PAL", Some(id.clone())), + None => "".to_string(), } } } @@ -195,9 +194,10 @@ impl Room { "\nEXT {} {}{}{}{}", instance.position.to_string(), instance.exit.to_string(), - if instance.transition.is_some() { - instance.transition.as_ref().unwrap().to_string() - } else {"".to_string()}, + match &instance.transition { + Some(transition) => transition, + None => &Transition::None, + }.to_string(), if instance.dialogue_id.is_some() {" DLG "} else {""}, instance.dialogue_id.as_ref().unwrap_or(&"".to_string()), ));