💥 Improve the docs and error handling

README.md

@@ -20,7 +20,7 @@ use wotbreplay_parser::replay::Replay;
 fn main() -> Result<()> {
     let battle_results = Replay::open(File::open("replays/20221203_player_results.wotbreplay")?)?.read_battle_results()?;
 
-    assert_eq!(battle_results.timestamp, 1670083956);
+    assert_eq!(battle_results.timestamp_secs, 1670083956);
     assert_eq!(battle_results.players.len(), 14);
 
     assert_eq!(battle_results.players[0].account_id, 595693744);

src/error.rs

@@ -21,9 +21,13 @@ pub enum Error {
     #[error("JSON error")]
     JsonError(#[from] serde_json::Error),
 
-    #[error("invalid magic: {0:#X}, expected: {1:#X}")]
-    InvalidMagic(u32, u32),
+    #[error("invalid magic: {actual:#X}, expected: {expected:#X}")]
+    InvalidMagic { expected: u32, actual: u32 },
 
-    #[error("failed to read a packet length, got {0} bytes")]
-    PacketLengthError(usize),
+    #[error("failed to parse type {type_} packet's payload at replay time {clock_secs}s")]
+    PacketPayloadParsingError {
+        source: Box<Error>,
+        type_: u32,
+        clock_secs: f32,
+    },
 }

src/models/battle_results.rs

@@ -9,7 +9,7 @@ impl BattleResults {
     ///
     /// # Note
     ///
-    /// This does **not** parse `battle_results.dat` itself, but the un-pickled tuple element.
+    /// This can **not** parse `battle_results.dat` itself, but the un-pickled tuple element.
     /// To parse `battle_results.dat`, use [`crate::models::battle_results_dat::BattleResultsDat`].
     pub fn from_buffer(buffer: impl Buf) -> Result<Self> {
         Ok(Self::decode(buffer)?)
@@ -23,7 +23,7 @@ impl BattleResults {
 pub struct BattleResults {
     /// Battle timestamp.
     #[prost(int64, tag = "2")]
-    pub timestamp: i64,
+    pub timestamp_secs: i64,
 
     #[prost(enumeration = "TeamNumber", optional, tag = "3")]
     pub winner_team_number: Option<i32>,
@@ -40,7 +40,7 @@ pub struct BattleResults {
     #[prost(message, repeated, tag = "201")]
     pub players: Vec<Player>,
 
-    /// Player's results.
+    /// Individual player's results.
     #[prost(message, repeated, tag = "301")]
     pub player_results: Vec<PlayerResults>,
 }
@@ -65,8 +65,9 @@ pub struct PlayerInfo {
     pub nickname: String,
 
     /// Some sort of platoon ID:
-    /// - contains same ID for a platoon members, or
-    /// - `None` for non-platoon players
+    ///
+    /// - contains same ID for the platoon members, or
+    /// - [`None`] for non-platoon players
     #[prost(uint32, optional, tag = "2")]
     pub platoon_id: Option<u32>,
 
@@ -165,6 +166,8 @@ pub struct PlayerResultsInfo {
     ///
     /// Note, that this is **not** the game client's displayed rating.
     /// This field matches the `mm_rating` as returned by the Wargaming.net API.
+    ///
+    /// The display rating is calculated as: `3000.0 + mm_rating * 10.0`.
     #[prost(float, optional, tag = "107")]
     pub mm_rating: Option<f32>,
 
@@ -173,7 +176,7 @@ pub struct PlayerResultsInfo {
 }
 
 impl PlayerResultsInfo {
-    /// Returns displayed rating as per the game client.
+    /// Calculate displayed rating according to the game client.
     pub fn display_rating(&self) -> Option<u32> {
         self.mm_rating
             .map(|mm_rating| (mm_rating * 10.0 + 3000.0) as u32)

src/models/data.rs

@@ -53,7 +53,10 @@ fn assert_magic<T: Into<u32> + PartialEq>(actual: T, expected: T) -> Result {
     if actual == expected {
         Ok(())
     } else {
-        Err(Error::InvalidMagic(actual.into(), expected.into()))
+        Err(Error::InvalidMagic {
+            actual: actual.into(),
+            expected: expected.into(),
+        })
     }
 }
 
@@ -82,7 +85,11 @@ fn read_pickled<T: DeserializeOwned>(reader: &mut impl Read, length: usize) -> R
 /// Read 2-byte length surrounded by the magical `0xFF` and `0x00`,
 /// unless it's a simple 1-byte length.
 ///
-/// P.S. Okay, Wargaming, I've no idea what you smoke, but I want some!
+/// # Context
+///
+/// It seems that for payloads larger than 254 bytes, Wargaming writes `00 <XX XX> FF`,
+/// where the word in the middle is the actual length of a payload, and the `00` and `FF`
+/// are magic values.
 #[inline]
 fn read_quirky_length(reader: &mut impl Read) -> Result<usize> {
     match reader.read_u8()? {

src/models/data/base_player_create.rs

@@ -22,8 +22,16 @@ pub struct BasePlayerCreate {
     #[serde(rename(deserialize = "camouflageSlot"))]
     pub camouflage_slot: u8,
 
+    /// Average team ratings for rating battles.
+    ///
+    /// # Contains
+    ///
+    /// - [`Some`] 2-vector, if the attribute is present in the replay. Each element represents one team:
+    ///     - [`Some`] rating, if known
+    ///     - [`None`], if the average rating is unknown (all the players are calibrating)
+    /// - [`None`], if the attribute is missing in the replay
     #[serde(default, rename(deserialize = "avgMmr"))]
-    pub average_mmr: Option<Vec<Option<f64>>>,
+    pub average_mm_ratings: Option<Vec<Option<f64>>>,
 
     #[serde(default, rename(deserialize = "playerWaitTimes"))]
     pub player_wait_times: Option<HashMap<u32, f64>>,
@@ -35,6 +43,10 @@ pub struct BasePlayerCreate {
     pub turbo_battles_statistics: Option<TurboBattlesStatistics>,
 }
 
+/// I don't know what that is:
+///
+/// - On EU, Asia, and NA the values are just constants
+/// - On Russia, the `alpha` and `beta` are different constants, and the chances actually differ for different replays
 #[derive(Debug, Serialize, Deserialize)]
 pub struct TurboBattlesStatistics {
     #[serde(rename(deserialize = "turboBattleChance"))]

src/models/data/entity_method.rs

@@ -26,7 +26,7 @@ pub enum EntityMethod {
 }
 
 impl EntityMethod {
-    /// Parse the entity method payload.
+    /// Parse the entity method raw payload.
     pub fn new(raw_payload: &[u8]) -> Result<Self> {
         let mut raw_payload = raw_payload;
 

src/models/data/packet.rs

@@ -6,6 +6,7 @@ use serde_with::serde_as;
 
 use crate::models::data::payload::Payload;
 use crate::result::Result;
+use crate::Error;
 
 /// «Packet» is a single traffic message in a replay,
 /// packets tell us what is going on during the battle.
@@ -28,14 +29,26 @@ pub struct Packet {
 }
 
 impl Packet {
+    /// Parse next packet from the reader.
+    ///
+    /// # Returns
+    ///
+    /// Parsed packet, or [`None`], when no packets are left.
     pub fn from_reader(reader: &mut impl Read) -> Result<Option<Self>> {
         let Ok(length) = reader.read_u32::<LittleEndian>() else { return Ok(None) };
         let type_ = reader.read_u32::<LittleEndian>()?;
         let clock_secs = reader.read_f32::<LittleEndian>()?;
         let raw_payload = Self::read_raw_payload(reader, length as usize)?;
+        let payload = Payload::new(type_, &raw_payload).map_err(|source| {
+            Error::PacketPayloadParsingError {
+                source: Box::new(source),
+                type_,
+                clock_secs,
+            }
+        })?;
         let this = Self {
             clock_secs,
-            payload: Payload::new(type_, &raw_payload)?,
+            payload,
 
             #[cfg(feature = "raw-payload")]
             raw_payload,

src/models/data/payload.rs

@@ -29,7 +29,7 @@ pub enum Payload {
 }
 
 impl Payload {
-    /// Parse the packet payload.
+    /// Parse the raw packet payload.
     pub fn new(packet_type: u32, raw_payload: &[u8]) -> Result<Self> {
         let mut raw_payload = raw_payload;
 

src/replay.rs

@@ -21,7 +21,7 @@ use crate::result::Result;
 pub struct Replay<R>(pub(crate) ZipArchive<R>);
 
 impl<R: Read + Seek> Replay<R> {
-    /// Opens a replay from the reader which contains a `*.wotbreplay`.
+    /// Open a replay from the reader which contains a `*.wotbreplay`.
     pub fn open(reader: R) -> Result<Self> {
         Ok(Self(ZipArchive::new(reader)?))
     }

tests/data.rs

@@ -50,7 +50,7 @@ fn base_player_create_ok() -> Result<()> {
     assert_eq!(arguments.matchmaker_type, 2);
     assert_eq!(arguments.camouflage_slot, 1);
     assert_eq!(
-        arguments.average_mmr,
+        arguments.average_mm_ratings,
         Some(vec![Some(59.29457753400008), Some(99.34182121604681)])
     );
     assert_eq!(