error handling for position

src/exit.rs

@@ -1,4 +1,5 @@
 use crate::{from_base36, Position, ToBase36};
+use std::str::FromStr;
 
 #[derive(Debug, Eq, PartialEq)]
 pub enum Transition {
@@ -63,7 +64,7 @@ impl From<String> for Exit {
         // e.g. "EXT 6,4 0 10,12 FX fade_w"
         let room_position_effect: Vec<&str> = string.split_whitespace().collect();
         let room_id = from_base36(room_position_effect[0]);
-        let position = Position::from(room_position_effect[1].to_string()).unwrap();
+        let position = Position::from_str(room_position_effect[1]).unwrap();
 
         let effect = if room_position_effect.len() == 4 {
             Transition::from(room_position_effect[3])

src/position.rs

@@ -1,4 +1,7 @@
 use std::error::Error;
+use std::fmt;
+use std::fmt::Formatter;
+use std::str::FromStr;
 
 #[derive(Debug, Eq, PartialEq)]
 pub struct Position {
@@ -6,9 +9,14 @@ pub struct Position {
     pub y: u8,
 }
 
-impl Position {
+impl Error for Position {}
-    #[inline]
+
-    pub(crate) fn from(string: String) -> Result<Position, &'static dyn Error> {
+impl FromStr for Position {
+    type Err = ();
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let string = s.to_string();
+
         // e.g. "2,5"
         let xy: Vec<&str> = string.split(',').collect();
         let x = xy[0].parse().expect("Bad x coordinate supplied for Position");
@@ -23,21 +31,22 @@ impl Position {
     }
 }
 
-impl ToString for Position {
+impl fmt::Display for Position {
     #[inline]
-    fn to_string(&self) -> String {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        format!("{},{}", self.x, self.y)
+        write!(f, "{},{}", self.x, self.y)
     }
 }
 
 #[cfg(test)]
 mod test {
     use crate::position::Position;
+    use std::str::FromStr;
 
     #[test]
-    fn test_position_from_string() {
+    fn test_position_from_str() {
         assert_eq!(
-            Position::from("4,12".to_string()).unwrap(),
+            Position::from_str(&"4,12").unwrap(),
             Position { x: 4, y: 12 }
         );
     }

src/room.rs

@@ -1,6 +1,7 @@
 use crate::{from_base36, optional_data_line, Exit, ExitInstance, Instance, Position, ToBase36};
 use crate::game::{RoomType, RoomFormat};
 use crate::exit::Transition;
+use std::str::FromStr;
 
 #[derive(Debug, Eq, PartialEq)]
 pub struct Room {
@@ -70,39 +71,43 @@ impl From<String> for Room {
                 let item_position: Vec<&str> = last_line.split(' ').collect();
                 let item_id = item_position[0];
                 let position = item_position[1];
-                let position = Position::from(position.to_string()).unwrap();
+                let position = Position::from_str(position);
 
-                items.push(Instance {
+                if position.is_ok() {
-                    position,
+                    let position = position.unwrap();
-                    id: item_id.to_string(),
+                    items.push(Instance { position, id: item_id.to_string() });
-                });
+                }
             } else if last_line.starts_with("EXT") {
                 let last_line = last_line.replace("EXT ", "");
                 let parts: Vec<&str> = last_line.split(' ').collect();
-                let position = Position::from(parts[0].to_string()).unwrap();
+                let position = Position::from_str(parts[0]);
-                let exit = Exit::from(format!("{} {}", parts[1], parts[2]));
+
-                let mut transition = None;
+                if position.is_ok() {
-                let mut dialogue_id = None;
+                    let position = position.unwrap();
-                let chunks = parts[3..].chunks(2);
+                    let exit = Exit::from(format!("{} {}", parts[1], parts[2]));
-                for chunk in chunks {
+                    let mut transition = None;
-                    if chunk[0] == "FX" {
+                    let mut dialogue_id = None;
-                        transition = Some(Transition::from(chunk[1]));
+                    let chunks = parts[3..].chunks(2);
-                    } else if chunk[0] == "DLG" {
+                    for chunk in chunks {
-                        dialogue_id = Some(chunk[1].to_string());
+                        if chunk[0] == "FX" {
+                            transition = Some(Transition::from(chunk[1]));
+                        } else if chunk[0] == "DLG" {
+                            dialogue_id = Some(chunk[1].to_string());
+                        }
                     }
+                    exits.push(ExitInstance { position, exit, transition, dialogue_id });
                 }
-                exits.push(ExitInstance { position, exit, transition, dialogue_id });
             } else if last_line.starts_with("END") {
                 let last_line = last_line.replace("END ", "");
                 let ending_position: Vec<&str> = last_line.split(' ').collect();
                 let ending = ending_position[0].to_string();
-                let position = ending_position[1].to_string();
+                let position = ending_position[1];
-                let position = Position::from(position).unwrap();
+                let position = Position::from_str(position);
 
-                endings.push(Instance {
+                if position.is_ok() {
-                    position,
+                    let position = position.unwrap();
-                    id: ending,
+                    endings.push(Instance { position, id: ending });
-                });
+                }
             } else {
                 lines.push(last_line);
                 break;

src/sprite.rs

@@ -1,5 +1,6 @@
 use crate::{from_base36, optional_data_line, AnimationFrames, Image, Position, ToBase36};
 use crate::image::animation_frames_from_string;
+use std::str::FromStr;
 
 #[derive(Debug, Eq, PartialEq)]
 pub struct Sprite {
@@ -82,7 +83,7 @@ impl From<String> for Sprite {
                     panic!("Bad room/position for sprite: {}", string);
                 }
 
-                position = Some(Position::from(room_position[1].to_string()).unwrap());
+                position = Some(Position::from_str(room_position[1]).unwrap());
             } else if last_line.starts_with("COL") {
                 colour_id = Some(last_line.replace("COL ", "").parse().unwrap());
             } else if last_line.starts_with("ITM") {