From a7a4a34ab85a20abea88098c09da9f55a4ec1da0 Mon Sep 17 00:00:00 2001
From: Max Bradbury <max@tinybird.info>
Date: Sun, 18 Oct 2020 14:07:28 +0100
Subject: [PATCH] error handling for exit, position, transition

---
 src/error.rs    |  1 +
 src/exit.rs     | 46 ++++++++++++++++++++++------------------------
 src/position.rs | 13 +++----------
 src/room.rs     |  6 ++++--
 src/sprite.rs   |  1 -
 5 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/src/error.rs b/src/error.rs
index 96c005a..dfe8a28 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -38,6 +38,7 @@ pub enum Error {
     Sprite,
     Text,
     Tile,
+    Transition,
     Variable,
     Version,
 }
diff --git a/src/exit.rs b/src/exit.rs
index 4c77214..d61da5d 100644
--- a/src/exit.rs
+++ b/src/exit.rs
@@ -1,5 +1,5 @@
 use crate::Position;
-use std::str::FromStr;
+
 use std::fmt;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -15,18 +15,18 @@ pub enum Transition {
     SlideRight,
 }
 
-impl From<&str> for Transition {
-    fn from(str: &str) -> Transition {
+impl Transition {
+    pub(crate) fn from_str(str: &str) -> Result<Transition, crate::Error> {
         match str {
-            "fade_w"  => Transition::FadeToWhite,
-            "fade_b"  => Transition::FadeToBlack,
-            "wave"    => Transition::Wave,
-            "tunnel"  => Transition::Tunnel,
-            "slide_u" => Transition::SlideUp,
-            "slide_d" => Transition::SlideDown,
-            "slide_l" => Transition::SlideLeft,
-            "slide_r" => Transition::SlideRight,
-            _         => Transition::None,
+            "fade_w"  => Ok(Transition::FadeToWhite),
+            "fade_b"  => Ok(Transition::FadeToBlack),
+            "wave"    => Ok(Transition::Wave),
+            "tunnel"  => Ok(Transition::Tunnel),
+            "slide_u" => Ok(Transition::SlideUp),
+            "slide_d" => Ok(Transition::SlideDown),
+            "slide_l" => Ok(Transition::SlideLeft),
+            "slide_r" => Ok(Transition::SlideRight),
+            _         => Err(crate::Error::Transition),
         }
     }
 }
@@ -56,22 +56,21 @@ pub struct Exit {
     pub effect: Transition,
 }
 
-impl FromStr for Exit {
-    type Err = String;
+impl Exit {
+    pub(crate) fn from_str(s: &str) -> Result<Self, crate::Error> {
+        let parts: Vec<&str> = s.split_whitespace().collect();
 
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let mut parts = s.split_whitespace();
-        let room_id = parts.next().unwrap().to_string();
-        let position = Position::from_str(parts.next().unwrap());
-
-        if position.is_err() {
-            return Err("Invalid position for exit".to_string());
+        if parts.len() < 2 {
+            return Err(crate::Error::Exit);
         }
 
-        let position = position.unwrap();
+        let mut parts = parts.iter();
+
+        let room_id = parts.next().unwrap().to_string();
+        let position = Position::from_str(parts.next().unwrap())?;
 
         let effect = if parts.next().is_some() {
-            Transition::from(parts.next().unwrap())
+            Transition::from_str(parts.next().unwrap())?
         } else {
             Transition::None
         };
@@ -95,7 +94,6 @@ impl fmt::Display for Exit {
 #[cfg(test)]
 mod test {
     use crate::{Transition, Exit, Position};
-    use std::str::FromStr;
 
     #[test]
     fn exit_from_string() {
diff --git a/src/position.rs b/src/position.rs
index 9db6dac..4f88592 100644
--- a/src/position.rs
+++ b/src/position.rs
@@ -1,6 +1,4 @@
-use std::error::Error;
 use std::fmt;
-use std::str::FromStr;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct Position {
@@ -8,12 +6,8 @@ pub struct Position {
     pub y: u8,
 }
 
-impl Error for Position {}
-
-impl FromStr for Position {
-    type Err = String;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+impl Position {
+    pub fn from_str(s: &str) -> Result<Self, crate::Error> {
         let mut parts = s.split(',');
 
         let x = parts.next().unwrap();
@@ -22,7 +16,7 @@ impl FromStr for Position {
         if let (Ok(x), Ok(y)) = (x.parse(), y.parse()) {
             Ok(Position { x, y })
         } else {
-            Err(format!("Malformed position string: {}", s))
+            Err(crate::Error::Position)
         }
     }
 }
@@ -36,7 +30,6 @@ impl fmt::Display for Position {
 #[cfg(test)]
 mod test {
     use crate::Position;
-    use std::str::FromStr;
 
     #[test]
     fn position_from_str() {
diff --git a/src/room.rs b/src/room.rs
index b8af00e..6afee5b 100644
--- a/src/room.rs
+++ b/src/room.rs
@@ -9,7 +9,6 @@ use crate::{
     Transition
 };
 
-use std::str::FromStr;
 use std::collections::HashMap;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -94,14 +93,17 @@ impl From<String> for Room {
                     if let Ok(exit) = exit {
                         let mut transition = None;
                         let mut dialogue_id = None;
+
                         let chunks = parts[3..].chunks(2);
+
                         for chunk in chunks {
                             if chunk[0] == "FX" {
-                                transition = Some(Transition::from(chunk[1]));
+                                transition = Some(Transition::from_str(chunk[1]).unwrap());
                             } else if chunk[0] == "DLG" {
                                 dialogue_id = Some(chunk[1].to_string());
                             }
                         }
+
                         exits.push(ExitInstance { position, exit, transition, dialogue_id });
                     }
                 }
diff --git a/src/sprite.rs b/src/sprite.rs
index 2e51445..05c647b 100644
--- a/src/sprite.rs
+++ b/src/sprite.rs
@@ -1,6 +1,5 @@
 use crate::{optional_data_line, AnimationFrames, Image, Position};
 use crate::image::animation_frames_from_string;
-use std::str::FromStr;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct Sprite {