replace own from_raw with TryFrom implementation

Hooking into the standard Rust system is probably better in the long-run
than having those separate from_raw methods on all of our objects.

Most end users probably won't even need them, as they will use the
higher level functionality provided by evtclib::process.

2 files changed, 71 insertions, 52 deletions

diff --git a/src/event.rs b/src/event.rs
index 03ef4c0..40dcc02 100644
--- a/src/event.rs
+++ b/src/event.rs
@@ -1,9 +1,21 @@
 use super::raw;
 
+use std::convert::TryFrom;
 use std::io;
 
 use byteorder::{BigEndian, WriteBytesExt};
 use num_traits::FromPrimitive;
+use thiserror::Error;
+
+/// Any error that can occur when trying to convert a raw [`CbtEvent`][raw::CbtEvent] to a
+/// [`Event`][Event].
+#[derive(Clone, Debug, Error)]
+pub enum FromRawEventError {
+    #[error("event contains an unknown state change: {0:?}")]
+    UnknownStateChange(raw::CbtStateChange),
+    #[error("event contains an unknown damage event")]
+    UnknownDamageEvent,
+}
 
 /// A rusty enum for all possible combat events.
 ///
@@ -123,17 +135,10 @@ pub enum EventKind {
     },
 
     /// The agent is facing in the given direction.
-    Facing {
-        agent_addr: u64,
-        x: f32,
-        y: f32,
-    },
+    Facing { agent_addr: u64, x: f32, y: f32 },
 
     /// The given agent changed their team.
-    TeamChange {
-        agent_addr: u64,
-        team_id: u64,
-    },
+    TeamChange { agent_addr: u64, team_id: u64 },
 
     /// Establishes an "attack target" relationship between two agents.
     ///
@@ -157,10 +162,7 @@ pub enum EventKind {
     },
 
     /// Updates the targetable state for the given agent.
-    Targetable {
-        agent_addr: u64,
-        targetable: bool,
-    },
+    Targetable { agent_addr: u64, targetable: bool },
 
     /// Information about the map id.
     MapId { map_id: u64 },
@@ -199,14 +201,16 @@ pub struct Event {
     pub is_shields: bool,
 }
 
-impl Event {
+impl TryFrom<&raw::CbtEvent> for Event {
+    type Error = FromRawEventError;
+
     /// Transform a raw event to a "high-level" event.
     ///
     /// If the event is not known, or some other error occured, `None` is
     /// returned.
     ///
     /// * `raw_event` - the raw event to transform.
-    pub fn from_raw(raw_event: &raw::CbtEvent) -> Option<Event> {
+    fn try_from(raw_event: &raw::CbtEvent) -> Result<Self, Self::Error> {
         use raw::CbtStateChange;
         let kind = match raw_event.is_statechange {
             // Check for state change events first.
@@ -314,11 +318,15 @@ impl Event {
             | CbtStateChange::BuffInfo
             | CbtStateChange::BuffFormula
             | CbtStateChange::SkillInfo
-            | CbtStateChange::SkillTiming => return None,
+            | CbtStateChange::SkillTiming => {
+                return Err(FromRawEventError::UnknownStateChange(
+                    raw_event.is_statechange,
+                ))
+            }
 
             CbtStateChange::None => check_activation(raw_event)?,
         };
-        Some(Event {
+        Ok(Event {
             time: raw_event.time,
             kind,
             is_ninety: raw_event.is_ninety,
@@ -330,12 +338,12 @@ impl Event {
     }
 }
 
-fn check_activation(raw_event: &raw::CbtEvent) -> Option<EventKind> {
+fn check_activation(raw_event: &raw::CbtEvent) -> Result<EventKind, FromRawEventError> {
     use raw::CbtActivation;
     match raw_event.is_activation {
         CbtActivation::None => check_buffremove(raw_event),
 
-        activation => Some(EventKind::SkillUse {
+        activation => Ok(EventKind::SkillUse {
             source_agent_addr: raw_event.src_agent,
             skill_id: raw_event.skillid,
             activation: match activation {
@@ -351,12 +359,12 @@ fn check_activation(raw_event: &raw::CbtEvent) -> Option<EventKind> {
     }
 }
 
-fn check_buffremove(raw_event: &raw::CbtEvent) -> Option<EventKind> {
+fn check_buffremove(raw_event: &raw::CbtEvent) -> Result<EventKind, FromRawEventError> {
     use raw::CbtBuffRemove;
     match raw_event.is_buffremove {
         CbtBuffRemove::None => check_damage(raw_event),
 
-        removal => Some(EventKind::BuffRemove {
+        removal => Ok(EventKind::BuffRemove {
             source_agent_addr: raw_event.src_agent,
             destination_agent_addr: raw_event.dst_agent,
             buff_id: raw_event.skillid,
@@ -367,9 +375,9 @@ fn check_buffremove(raw_event: &raw::CbtEvent) -> Option<EventKind> {
     }
 }
 
-fn check_damage(raw_event: &raw::CbtEvent) -> Option<EventKind> {
+fn check_damage(raw_event: &raw::CbtEvent) -> Result<EventKind, FromRawEventError> {
     if raw_event.buff == 0 && raw_event.iff == raw::IFF::Foe && raw_event.dst_agent != 0 {
-        Some(EventKind::Physical {
+        Ok(EventKind::Physical {
             source_agent_addr: raw_event.src_agent,
             destination_agent_addr: raw_event.dst_agent,
             skill_id: raw_event.skillid,
@@ -381,14 +389,14 @@ fn check_damage(raw_event: &raw::CbtEvent) -> Option<EventKind> {
         && raw_event.dst_agent != 0
         && raw_event.value == 0
     {
-        Some(EventKind::ConditionTick {
+        Ok(EventKind::ConditionTick {
             source_agent_addr: raw_event.src_agent,
             destination_agent_addr: raw_event.dst_agent,
             condition_id: raw_event.skillid,
             damage: raw_event.buff_dmg,
         })
     } else if raw_event.buff == 1 && raw_event.buff_dmg == 0 && raw_event.value != 0 {
-        Some(EventKind::BuffApplication {
+        Ok(EventKind::BuffApplication {
             source_agent_addr: raw_event.src_agent,
             destination_agent_addr: raw_event.dst_agent,
             buff_id: raw_event.skillid,
@@ -396,13 +404,13 @@ fn check_damage(raw_event: &raw::CbtEvent) -> Option<EventKind> {
             overstack: raw_event.overstack_value,
         })
     } else if raw_event.buff == 1 && raw_event.buff_dmg == 0 && raw_event.value == 0 {
-        Some(EventKind::InvulnTick {
+        Ok(EventKind::InvulnTick {
             source_agent_addr: raw_event.src_agent,
             destination_agent_addr: raw_event.dst_agent,
             condition_id: raw_event.skillid,
         })
     } else {
-        None
+        Err(FromRawEventError::UnknownDamageEvent)
     }
 }
 
diff --git a/src/lib.rs b/src/lib.rs
index b99d6a7..3f098e1 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -17,6 +17,7 @@
 //! (Look at the note on "Buffering" in the [parser
 //! module](raw/parser/index.html#buffering))
 
+use std::convert::TryFrom;
 use std::marker::PhantomData;
 
 use getset::{CopyGetters, Getters};
@@ -176,26 +177,6 @@ impl AgentKind {
         })
     }
 
-    /// Extract the correct `AgentKind` from the given [raw agent][raw::Agent].
-    ///
-    /// This automatically discerns between player, gadget and characters.
-    ///
-    /// Note that in most cases, you probably want to use [`Agent::from_raw`][Agent::from_raw] or
-    /// even [`process`][process] instead of this function.
-    pub fn from_raw(raw_agent: &raw::Agent) -> Result<AgentKind, EvtcError> {
-        if raw_agent.is_character() {
-            Ok(AgentKind::Character(AgentKind::from_raw_character(
-                raw_agent,
-            )?))
-        } else if raw_agent.is_gadget() {
-            Ok(AgentKind::Gadget(AgentKind::from_raw_gadget(raw_agent)?))
-        } else if raw_agent.is_player() {
-            Ok(AgentKind::Player(AgentKind::from_raw_player(raw_agent)?))
-        } else {
-            Err(EvtcError::InvalidData)
-        }
-    }
-
     /// Accesses the inner [`Player`][Player] struct, if available.
     pub fn as_player(&self) -> Option<&Player> {
         if let AgentKind::Player(ref player) = *self {
@@ -239,6 +220,30 @@ impl AgentKind {
     }
 }
 
+impl TryFrom<&raw::Agent> for AgentKind {
+    type Error = EvtcError;
+
+    /// Extract the correct `AgentKind` from the given [raw agent][raw::Agent].
+    ///
+    /// This automatically discerns between player, gadget and characters.
+    ///
+    /// Note that in most cases, you probably want to use `Agent::try_from` or even
+    /// [`process`][process] instead of this function.
+    fn try_from(raw_agent: &raw::Agent) -> Result<Self, Self::Error> {
+        if raw_agent.is_character() {
+            Ok(AgentKind::Character(AgentKind::from_raw_character(
+                raw_agent,
+            )?))
+        } else if raw_agent.is_gadget() {
+            Ok(AgentKind::Gadget(AgentKind::from_raw_gadget(raw_agent)?))
+        } else if raw_agent.is_player() {
+            Ok(AgentKind::Player(AgentKind::from_raw_player(raw_agent)?))
+        } else {
+            Err(EvtcError::InvalidData)
+        }
+    }
+}
+
 /// An agent.
 ///
 /// Agents in arcdps are very versatile, as a lot of things end up being an "agent". This includes:
@@ -348,10 +353,12 @@ pub struct Agent<Kind = ()> {
     phantom_data: PhantomData<Kind>,
 }
 
-impl Agent {
+impl TryFrom<&raw::Agent> for Agent {
+    type Error = EvtcError;
+
     /// Parse a raw agent.
-    pub fn from_raw(raw_agent: &raw::Agent) -> Result<Agent, EvtcError> {
-        let kind = AgentKind::from_raw(raw_agent)?;
+    fn try_from(raw_agent: &raw::Agent) -> Result<Self, Self::Error> {
+        let kind = AgentKind::try_from(raw_agent)?;
         Ok(Agent {
             addr: raw_agent.addr,
             kind,
@@ -630,7 +637,11 @@ pub fn process(data: &raw::Evtc) -> Result<Log, EvtcError> {
     // Set the master addr field
     set_agent_masters(data, &mut agents)?;
 
-    let events = data.events.iter().filter_map(Event::from_raw).collect();
+    let events = data
+        .events
+        .iter()
+        .filter_map(|e| Event::try_from(e).ok())
+        .collect();
 
     Ok(Log {
         agents,
@@ -643,7 +654,7 @@ fn setup_agents(data: &raw::Evtc) -> Result<Vec<Agent>, EvtcError> {
     let mut agents = Vec::with_capacity(data.agents.len());
 
     for raw_agent in &data.agents {
-        agents.push(Agent::from_raw(raw_agent)?);
+        agents.push(Agent::try_from(raw_agent)?);
     }
     Ok(agents)
 }