More parsing code cleanup and some better error messages.

Still more to do, but this is some decent progress.

src/parse/parse_utils.rs

@@ -76,6 +76,203 @@ pub fn ensure_close(events: &mut DataTreeReader<impl BufRead>) -> PsyResult<()>
     }
 }
 
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+pub enum Range {
+    Full,
+    From(usize),
+    To(usize),
+    Range(usize, usize),
+}
+
+impl Range {
+    pub fn contains(self, n: usize) -> bool {
+        match self {
+            Range::Full => true,
+            Range::From(start) => n >= start,
+            Range::To(end) => n < end,
+            Range::Range(start, end) => n >= start && n < end,
+        }
+    }
+
+    /// Checks if the value is within the upper bound of the range.
+    /// Ignores any lower bound.
+    pub fn contains_upper(self, n: usize) -> bool {
+        match self {
+            Range::Full | Range::From(_) => true,
+            Range::To(end) => n < end,
+            Range::Range(_, end) => n < end,
+        }
+    }
+
+    pub fn lower(self) -> usize {
+        match self {
+            Range::Full => 0,
+            Range::From(start) => start,
+            Range::To(_) => 0,
+            Range::Range(start, _) => start,
+        }
+    }
+
+    pub fn upper(self) -> usize {
+        match self {
+            Range::Full => std::usize::MAX,
+            Range::From(_) => std::usize::MAX,
+            Range::To(end) => end,
+            Range::Range(_, end) => end,
+        }
+    }
+}
+
+impl std::convert::From<std::ops::RangeFull> for Range {
+    fn from(_r: std::ops::RangeFull) -> Self {
+        Range::Full
+    }
+}
+
+impl std::convert::From<std::ops::RangeFrom<usize>> for Range {
+    fn from(r: std::ops::RangeFrom<usize>) -> Self {
+        Range::From(r.start)
+    }
+}
+
+impl std::convert::From<std::ops::RangeTo<usize>> for Range {
+    fn from(r: std::ops::RangeTo<usize>) -> Self {
+        Range::To(r.end)
+    }
+}
+
+impl std::convert::From<std::ops::Range<usize>> for Range {
+    fn from(r: std::ops::Range<usize>) -> Self {
+        Range::Range(r.start, r.end)
+    }
+}
+
+impl std::convert::From<usize> for Range {
+    fn from(r: usize) -> Self {
+        Range::Range(r, r + 1)
+    }
+}
+
+/// Acts as an intermediary for parsing, ensuring that the right number of the
+/// right subsections are encountered.  It loops over subsections, passing
+/// through the `events` object untouched, so the passed closure needs to call
+/// `next_event`.
+///
+/// Tracks a maximum of 64 different subsections.
+pub fn ensure_subsections<F, DTR: BufRead>(
+    events: &mut DataTreeReader<DTR>,
+    subsections: &[(&str, bool, Range)], // (type name, is leaf, valid count range)
+    mut f: F,
+) -> PsyResult<()>
+where
+    F: FnMut(&mut DataTreeReader<DTR>) -> PsyResult<()>,
+{
+    let mut counts = [0usize; 64];
+
+    // Loop through our events!
+    loop {
+        match events.peek_event()? {
+            Event::Leaf {
+                type_name,
+                byte_offset,
+                ..
+            } => {
+                if let Some(idx) = subsections
+                    .iter()
+                    .position(|(n, l, _)| *l == true && n == &type_name)
+                {
+                    // Increment count and make sure we're within the valid count
+                    // range for this sub-sections.
+                    counts[idx] += 1;
+                    if !subsections[idx].2.contains_upper(counts[idx]) {
+                        return Err(PsyError::WrongNodeCount(
+                            byte_offset,
+                            format!(
+                                "Expected at most {} '{}' leaf nodes but found \
+                                 at least {}.",
+                                subsections[idx].2.upper() - 1,
+                                subsections[idx].0,
+                                counts[idx],
+                            ),
+                        ));
+                    }
+
+                    // Call handler.
+                    f(events)?
+                } else {
+                    break;
+                }
+            }
+            Event::InnerOpen {
+                type_name,
+                byte_offset,
+                ..
+            } => {
+                if let Some(idx) = subsections
+                    .iter()
+                    .position(|(n, l, _)| *l == false && n == &type_name)
+                {
+                    // Increment count and make sure we're within the valid count
+                    // range for this sub-sections.
+                    counts[idx] += 1;
+                    if !subsections[idx].2.contains_upper(counts[idx]) {
+                        return Err(PsyError::WrongNodeCount(
+                            byte_offset,
+                            format!(
+                                "Expected at most {} internal '{}' node(s) but \
+                                 found at least {}.",
+                                subsections[idx].2.upper() - 1,
+                                subsections[idx].0,
+                                counts[idx],
+                            ),
+                        ));
+                    }
+
+                    // Call handler.
+                    f(events)?
+                } else {
+                    break;
+                }
+            }
+            Event::InnerClose { .. } => {
+                break;
+            }
+            Event::EOF => {
+                break;
+            }
+        }
+    }
+
+    // Validation.
+    for i in 0..subsections.len() {
+        if !subsections[i].2.contains(counts[i]) {
+            if subsections[i].1 {
+                return Err(PsyError::WrongNodeCount(
+                    events.byte_offset(),
+                    format!(
+                        "Expected at least {} '{}' leaf node(s) but only found {}.",
+                        subsections[i].2.lower(),
+                        subsections[i].0,
+                        counts[i],
+                    ),
+                ));
+            } else {
+                return Err(PsyError::WrongNodeCount(
+                    events.byte_offset(),
+                    format!(
+                        "Expected at least {} internal '{}' node(s) but only found {}.",
+                        subsections[i].2.lower(),
+                        subsections[i].0,
+                        counts[i],
+                    ),
+                ));
+            }
+        }
+    }
+
+    Ok(())
+}
+
 // ========================================================
 
 #[cfg(test)]

src/parse/psy.rs

@@ -19,7 +19,7 @@ use crate::{
 };
 
 use super::{
-    parse_utils::{ensure_close, ws_f32, ws_u32},
+    parse_utils::{ensure_close, ensure_subsections, ws_f32, ws_u32},
     psy_assembly::parse_assembly,
     psy_light::parse_distant_disk_light,
     psy_surface_shader::parse_surface_shader,
@@ -127,76 +127,76 @@ pub fn parse_scene<'a>(
     events: &mut DataTreeReader<impl BufRead>,
     _ident: Option<&str>,
 ) -> PsyResult<Scene<'a>> {
-    // Get output info.
+    // Get everything except the root assembly (which comes last).
-    let _output_info = if let Event::InnerOpen {
+    let mut output_info = None;
-        type_name: "Output",
+    let mut render_settings = None;
-        ..
+    let mut shaders = None;
-    } = events.next_event()?
+    let mut world = None;
-    {
+    let mut camera = None;
-        parse_output_info(events)?
+    let valid_subsections = &[
-    } else {
+        ("Output", false, (1).into()),
-        todo!(); // Return error.
+        ("RenderSettings", false, (1).into()),
-    };
+        ("Shaders", false, (1).into()),
+        ("World", false, (1).into()),
+        ("Camera", false, (1).into()),
+    ];
+    ensure_subsections(events, valid_subsections, |events| {
+        match events.next_event()? {
+            Event::InnerOpen {
+                type_name: "Output",
+                ..
+            } => {
+                output_info = Some(parse_output_info(events)?);
+            }
 
-    // Get render settings.
+            Event::InnerOpen {
-    let _render_settings = if let Event::InnerOpen {
+                type_name: "RenderSettings",
-        type_name: "RenderSettings",
+                ..
-        ..
+            } => {
-    } = events.next_event()?
+                render_settings = Some(parse_render_settings(events)?);
-    {
+            }
-        parse_render_settings(events)?
-    } else {
-        todo!(); // Return error.
-    };
 
-    // Get shaders.
+            Event::InnerOpen {
-    let shaders = if let Event::InnerOpen {
+                type_name: "Shaders",
-        type_name: "Shaders",
+                ..
-        ..
+            } => {
-    } = events.next_event()?
+                shaders = Some(parse_shaders(arena, events)?);
-    {
+            }
-        parse_shaders(arena, events)?
-    } else {
-        todo!(); // Return error.
-    };
 
-    // Get world.
+            Event::InnerOpen {
-    let world = if let Event::InnerOpen {
+                type_name: "World", ..
-        type_name: "World", ..
+            } => {
-    } = events.next_event()?
+                world = Some(parse_world(arena, events)?);
-    {
+            }
-        parse_world(arena, events)?
-    } else {
-        todo!(); // Return error.
-    };
 
-    // Get camera.
+            Event::InnerOpen {
-    let camera = if let Event::InnerOpen {
+                type_name: "Camera",
-        type_name: "Camera",
+                ..
-        ..
+            } => {
-    } = events.next_event()?
+                camera = Some(parse_camera(arena, events)?);
-    {
+            }
-        parse_camera(arena, events)?
+
-    } else {
+            _ => unreachable!(),
-        todo!(); // Return error.
+        }
-    };
+        Ok(())
+    })?;
 
     // Get the root assembly.
-    let root_assembly = if let Event::InnerOpen {
+    let mut root_assembly = None;
-        type_name: "Assembly",
+    let valid_subsections = &[("Assembly", false, (1).into())];
-        ..
+    ensure_subsections(events, valid_subsections, |events| {
-    } = events.next_event()?
+        match events.next_event()? {
-    {
+            Event::InnerOpen {
-        parse_assembly(arena, events)?
+                type_name: "Assembly",
-    } else {
+                ..
-        todo!(); // Return error.
+            } => root_assembly = Some(parse_assembly(arena, events)?),
-    };
+            _ => unreachable!(),
+        }
+        Ok(())
+    })?;
 
     // Make sure we're closed out properly.
-    if let Event::InnerClose { .. } = events.next_event()? {
+    ensure_close(events)?;
-    } else {
-        todo!(); // Return error.
-    }
 
     // Put scene together
     // let scene_name = if let Some(name) = ident {
@@ -205,10 +205,10 @@ pub fn parse_scene<'a>(
     //     None
     // };
     let scene = Scene {
-        camera: camera,
+        camera: camera.unwrap(),
-        world: world,
+        world: world.unwrap(),
-        shaders: shaders,
+        shaders: shaders.unwrap(),
-        root_assembly: root_assembly,
+        root_assembly: root_assembly.unwrap(),
     };
 
     // // Put renderer together
@@ -227,9 +227,10 @@ pub fn parse_scene<'a>(
 }
 
 fn parse_output_info(events: &mut DataTreeReader<impl BufRead>) -> PsyResult<String> {
-    let mut found_path = false;
     let mut path = String::new();
-    loop {
+
+    let valid_subsections = &[("Path", true, (1).into())];
+    ensure_subsections(events, valid_subsections, |events| {
         match events.next_event()? {
             Event::Leaf {
                 type_name: "Path",
@@ -253,42 +254,36 @@ fn parse_output_info(events: &mut DataTreeReader<impl BufRead>) -> PsyResult<Str
                 let len = tc.len();
                 let tc = &tc[1..len - 1];
 
-                // Parse
                 // TODO: proper string escaping
-                found_path = true;
                 path = tc.to_string();
             }
 
-            Event::InnerClose { .. } => {
-                break;
-            }
-
             _ => {
-                todo!(); // Return error.
+                unreachable!();
             }
         }
-    }
+        Ok(())
+    })?;
 
-    if found_path {
+    ensure_close(events)?;
-        return Ok(path);
+
-    } else {
+    Ok(path)
-        // return Err(PsyError::MissingNode(
-        //     tree.byte_offset(),
-        //     "Output section must contain a Path.",
-        // ));
-        todo!(); // Return error.
-    }
 }
 
 fn parse_render_settings(
     events: &mut DataTreeReader<impl BufRead>,
 ) -> PsyResult<((u32, u32), u32, u32)> {
-    let mut found_res = false;
-    let mut found_spp = false;
     let mut res = (0, 0);
     let mut spp = 0;
     let mut seed = 0;
-    loop {
+
+    // Parse
+    let valid_subsections = &[
+        ("Resolution", true, (1).into()),
+        ("SamplesPerPixel", true, (1).into()),
+        ("Seed", true, (..).into()),
+    ];
+    ensure_subsections(events, valid_subsections, |events| {
         match events.next_event()? {
             // Resolution
             Event::Leaf {
@@ -298,7 +293,6 @@ fn parse_render_settings(
             } => {
                 if let IResult::Ok((_, (w, h))) = all_consuming(tuple((ws_u32, ws_u32)))(&contents)
                 {
-                    found_res = true;
                     res = (w, h);
                 } else {
                     // Found Resolution, but its contents is not in the right format
@@ -318,7 +312,6 @@ fn parse_render_settings(
                 byte_offset,
             } => {
                 if let IResult::Ok((_, n)) = all_consuming(ws_u32)(&contents) {
-                    found_spp = true;
                     spp = n;
                 } else {
                     // Found SamplesPerPixel, but its contents is not in the right format
@@ -350,26 +343,17 @@ fn parse_render_settings(
                 }
             }
 
-            Event::InnerClose { .. } => {
-                break;
-            }
-
             _ => {
-                todo!(); // Return error.
+                unreachable!();
             }
         }
-    }
 
-    if found_res && found_spp {
+        Ok(())
-        return Ok((res, spp, seed));
+    })?;
-    } else {
+
-        // return Err(PsyError::MissingNode(
+    ensure_close(events)?;
-        //     tree.byte_offset(),
+
-        //     "RenderSettings must have both Resolution and \
+    Ok((res, spp, seed))
-        //      SamplesPerPixel specified.",
-        // ));
-        todo!(); // Return error.
-    }
 }
 
 fn parse_camera<'a>(
@@ -382,7 +366,13 @@ fn parse_camera<'a>(
     let mut aperture_radii = Vec::new();
 
     // Parse
-    loop {
+    let valid_subsections = &[
+        ("Fov", true, (1..).into()),
+        ("FocalDistance", true, (1..).into()),
+        ("ApertureRadius", true, (1..).into()),
+        ("Transform", true, (1..).into()),
+    ];
+    ensure_subsections(events, valid_subsections, |events| {
         match events.next_event()? {
             // Fov
             Event::Leaf {
@@ -455,15 +445,15 @@ fn parse_camera<'a>(
                 }
             }
 
-            Event::InnerClose { .. } => {
-                break;
-            }
-
             _ => {
-                todo!(); // Return error.
+                unreachable!();
             }
         }
-    }
+
+        Ok(())
+    })?;
+
+    ensure_close(events)?;
 
     return Ok(Camera::new(
         arena,
@@ -481,7 +471,11 @@ fn parse_world<'a>(
     let mut background_color = None;
     let mut lights: Vec<&dyn WorldLightSource> = Vec::new();
 
-    loop {
+    let valid_subsections = &[
+        ("BackgroundShader", false, (1).into()),
+        ("DistantDiskLight", false, (..).into()),
+    ];
+    ensure_subsections(events, valid_subsections, |events| {
         match events.next_event()? {
             // Parse background shader
             Event::InnerOpen {
@@ -496,7 +490,7 @@ fn parse_world<'a>(
                 {
                     contents.to_string()
                 } else {
-                    todo!(); // Return error.
+                    panic!("No type specified for the BackgroundShader"); // Return error.
                 };
 
                 match bgs_type.as_ref() {
@@ -504,12 +498,12 @@ fn parse_world<'a>(
                         if let Event::Leaf {
                             type_name: "Color",
                             contents,
-                            ..
+                            byte_offset,
                         } = events.next_event()?
                         {
-                            background_color = Some(parse_color(contents)?);
+                            background_color = Some(parse_color(byte_offset, contents)?);
                         } else {
-                            todo!(
+                            panic!(
                                 "BackgroundShader's Type is Color, \
                                  but no Color is specified."
                             ); // Return error.
@@ -517,7 +511,7 @@ fn parse_world<'a>(
                     }
 
                     _ => {
-                        todo!(
+                        panic!(
                             "The specified BackgroundShader Type \
                              isn't a recognized type.",
                         ); // Return an error.
@@ -541,16 +535,12 @@ fn parse_world<'a>(
                     ident.as_deref(),
                 )?));
             }
-
+            _ => unreachable!(),
-            Event::InnerClose { .. } => {
-                break;
-            }
-
-            _ => {
-                todo!(); // Return error.
-            }
         }
-    }
+        Ok(())
+    })?;
+
+    ensure_close(events)?;
 
     if background_color == None {
         todo!(); // Return error.
@@ -568,12 +558,13 @@ fn parse_shaders<'a>(
     events: &mut DataTreeReader<impl BufRead>,
 ) -> PsyResult<HashMap<String, Box<dyn SurfaceShader>>> {
     let mut shaders = HashMap::new();
-    loop {
+    let valid_subsections = &[("SurfaceShader", false, (..).into())];
+    ensure_subsections(events, valid_subsections, |events| {
         match events.next_event()? {
             Event::InnerOpen {
                 type_name: "SurfaceShader",
                 ident,
-                ..
+                byte_offset,
             } => {
                 if let Some(name) = ident {
                     let name = name.to_string();
@@ -582,19 +573,19 @@ fn parse_shaders<'a>(
                         parse_surface_shader(arena, events, Some(&name))?,
                     );
                 } else {
-                    todo!("Shader has no name."); // Return error.
+                    return Err(PsyError::ExpectedIdent(
+                        byte_offset,
+                        "SurfaceShader has no name.".into(),
+                    ));
                 }
             }
 
-            Event::InnerClose { .. } => {
+            _ => unreachable!(),
-                break;
-            }
-
-            _ => {
-                todo!(); // Return error.
-            }
         }
-    }
+        Ok(())
+    })?;
+
+    ensure_close(events)?;
 
     // Return the list of shaders.
     return Ok(shaders);
@@ -626,39 +617,44 @@ pub fn make_transform_format_error(byte_offset: usize) -> PsyError {
     )
 }
 
-pub fn parse_color(contents: &str) -> PsyResult<Color> {
+pub fn parse_color(byte_offset: usize, contents: &str) -> PsyResult<Color> {
     let items: Vec<_> = contents.split(',').map(|s| s.trim()).collect();
     if items.len() != 2 {
-        todo!(); // Return an error.
+        return Err(PsyError::IncorrectLeafData(
+            byte_offset,
+            "Color has invalid format.".into(),
+        ));
     }
 
     match items[0] {
         "rec709" => {
             if let IResult::Ok((_, color)) = tuple((ws_f32, ws_f32, ws_f32))(items[1]) {
                 return Ok(Color::new_xyz(rec709_e_to_xyz(color)));
-            } else {
-                todo!(); // Return an error.
             }
         }
 
         "blackbody" => {
             if let IResult::Ok((_, (temperature, factor))) = tuple((ws_f32, ws_f32))(items[1]) {
                 return Ok(Color::new_blackbody(temperature, factor));
-            } else {
-                todo!(); // Return an error.
             }
         }
 
         "color_temperature" => {
             if let IResult::Ok((_, (temperature, factor))) = tuple((ws_f32, ws_f32))(items[1]) {
                 return Ok(Color::new_temperature(temperature, factor));
-            } else {
-                todo!(); // Return an error.
             }
         }
 
-        _ => {
+        s => {
-            todo!(); // Return an error.
+            return Err(PsyError::IncorrectLeafData(
+                byte_offset,
+                format!("Unrecognized color type '{}.", s),
+            ));
         }
     }
+
+    Err(PsyError::IncorrectLeafData(
+        byte_offset,
+        "Color has invalid format.".into(),
+    ))
 }

src/parse/psy_assembly.rs

@@ -9,7 +9,7 @@ use data_tree::{DataTreeReader, Event};
 use crate::scene::{Assembly, Object, ObjectData};
 
 use super::{
-    parse_utils::ensure_close,
+    parse_utils::{ensure_close, ensure_subsections},
     psy::{parse_matrix, PsyError, PsyResult},
     psy_light::{parse_rectangle_light, parse_sphere_light},
     psy_mesh_surface::parse_mesh_surface,
@@ -20,115 +20,112 @@ pub fn parse_assembly<'a>(
     events: &mut DataTreeReader<impl BufRead>,
 ) -> PsyResult<Assembly<'a>> {
     let mut assembly = Assembly::new();
-    loop {
+    ensure_subsections(events, &[("Object", false, (1..).into())], |events| {
-        match events.next_event()? {
+        if let Event::InnerOpen {
-            Event::InnerOpen {
+            type_name: "Object",
-                type_name: "Object",
+            ident,
-                ident,
+            byte_offset,
-                byte_offset,
+        } = events.next_event()?
-            } => {
+        {
-                // Get object identifier.
+            // Get object identifier.
-                let object_ident = if let Some(id) = ident {
+            let object_ident = if let Some(id) = ident {
-                    id.to_string()
+                id.to_string()
-                } else {
+            } else {
-                    return Err(PsyError::ExpectedIdent(
+                return Err(PsyError::ExpectedIdent(
-                        byte_offset,
+                    byte_offset,
-                        "\'Object\' types must have an identifier, but the identifier is missing."
+                    "\'Object\' types must have an identifier, but the identifier is missing."
-                            .into(),
+                        .into(),
-                    ));
+                ));
-                };
+            };
 
-                // Collect instances.
+            // Collect instances.
-                let mut instance_xform_idxs = Vec::new();
+            let mut instance_xform_idxs = Vec::new();
-                while let Event::InnerOpen {
+            while let Event::InnerOpen {
-                    type_name: "Instance",
+                type_name: "Instance",
-                    ..
+                ..
-                } = events.peek_event()?
+            } = events.peek_event()?
-                {
+            {
-                    events.next_event();
+                events.next_event()?;
-                    let xforms_start_idx = assembly.xforms.len();
+                let xforms_start_idx = assembly.xforms.len();
-                    loop {
+                loop {
-                        match events.next_event()? {
+                    match events.next_event()? {
-                            Event::Leaf {
+                        Event::Leaf {
-                                type_name: "Transform",
+                            type_name: "Transform",
-                                contents,
+                            contents,
-                                ..
+                            ..
-                            } => {
+                        } => {
-                                assembly.xforms.push(parse_matrix(contents)?);
+                            assembly.xforms.push(parse_matrix(contents)?);
-                            }
+                        }
 
-                            Event::InnerClose { .. } => {
+                        Event::InnerClose { .. } => {
-                                break;
+                            break;
-                            }
+                        }
 
-                            _ => {
+                        _ => {
-                                todo!("Instances can only contain Transforms.");
+                            todo!("Instances can only contain Transforms.");
-                                // Return an error.
+                            // Return an error.
-                            }
                         }
                     }
-
-                    instance_xform_idxs.push(xforms_start_idx..assembly.xforms.len());
                 }
 
-                // Get object data.
+                instance_xform_idxs.push(xforms_start_idx..assembly.xforms.len());
-                let object_data = match events.next_event()? {
-                    Event::InnerOpen {
-                        type_name: "Assembly",
-                        ..
-                    } => ObjectData::Assembly(Box::new(parse_assembly(arena, events)?)),
-
-                    Event::InnerOpen {
-                        type_name: "MeshSurface",
-                        ..
-                    } => ObjectData::Surface(Box::new(parse_mesh_surface(arena, events)?)),
-
-                    Event::InnerOpen {
-                        type_name: "SphereLight",
-                        ..
-                    } => ObjectData::Light(Box::new(parse_sphere_light(arena, events)?)),
-
-                    Event::InnerOpen {
-                        type_name: "RectangleLight",
-                        ..
-                    } => ObjectData::Light(Box::new(parse_rectangle_light(arena, events)?)),
-
-                    Event::InnerClose { byte_offset } => {
-                        return Err(PsyError::MissingNode(
-                            byte_offset,
-                            "Object contains no object data.".into(),
-                        ));
-                    }
-
-                    _ => {
-                        return Err(PsyError::UnknownVariant(
-                            byte_offset,
-                            "Unknown data type for Object.".into(),
-                        ));
-                    }
-                };
-
-                // Close object node.
-                ensure_close(events)?;
-
-                assembly.objects.insert(
-                    object_ident,
-                    Object {
-                        data: object_data,
-                        instance_xform_idxs: instance_xform_idxs,
-                    },
-                );
             }
 
-            Event::InnerClose { .. } => {
+            // Get object data.
-                break;
+            let object_data = match events.next_event()? {
-            }
+                Event::InnerOpen {
+                    type_name: "Assembly",
+                    ..
+                } => ObjectData::Assembly(Box::new(parse_assembly(arena, events)?)),
 
-            _ => {
+                Event::InnerOpen {
-                todo!(); // Return an error.
+                    type_name: "MeshSurface",
-            }
+                    ..
+                } => ObjectData::Surface(Box::new(parse_mesh_surface(arena, events)?)),
+
+                Event::InnerOpen {
+                    type_name: "SphereLight",
+                    ..
+                } => ObjectData::Light(Box::new(parse_sphere_light(arena, events)?)),
+
+                Event::InnerOpen {
+                    type_name: "RectangleLight",
+                    ..
+                } => ObjectData::Light(Box::new(parse_rectangle_light(arena, events)?)),
+
+                Event::InnerClose { byte_offset } => {
+                    return Err(PsyError::MissingNode(
+                        byte_offset,
+                        "Object contains no object data.".into(),
+                    ));
+                }
+
+                _ => {
+                    return Err(PsyError::UnknownVariant(
+                        byte_offset,
+                        "Unknown data type for Object.".into(),
+                    ));
+                }
+            };
+
+            // Close object node.
+            ensure_close(events)?;
+
+            assembly.objects.insert(
+                object_ident,
+                Object {
+                    data: object_data,
+                    instance_xform_idxs: instance_xform_idxs,
+                },
+            );
+        } else {
+            unreachable!()
         }
-    }
+
+        Ok(())
+    })?;
+
+    ensure_close(events)?;
 
     // if !tree.is_internal() {
     //     return Err(PsyError::UnknownError(tree.byte_offset()));

src/parse/psy_light.rs

@@ -65,12 +65,7 @@ pub fn parse_distant_disk_light<'a>(
                 contents,
                 byte_offset,
             } => {
-                if let Ok(color) = parse_color(&contents) {
+                colors.push(parse_color(byte_offset, &contents)?);
-                    colors.push(color);
-                } else {
-                    // Found color, but its contents is not in the right format
-                    return Err(PsyError::UnknownError(byte_offset));
-                }
             }
 
             Event::InnerClose { .. } => {
@@ -116,12 +111,7 @@ pub fn parse_sphere_light<'a>(
                 contents,
                 byte_offset,
             } => {
-                if let Ok(color) = parse_color(&contents) {
+                colors.push(parse_color(byte_offset, &contents)?);
-                    colors.push(color);
-                } else {
-                    // Found color, but its contents is not in the right format
-                    return Err(PsyError::UnknownError(byte_offset));
-                }
             }
 
             Event::InnerClose { .. } => {
@@ -168,12 +158,7 @@ pub fn parse_rectangle_light<'a>(
                 contents,
                 byte_offset,
             } => {
-                if let Ok(color) = parse_color(&contents) {
+                colors.push(parse_color(byte_offset, &contents)?);
-                    colors.push(color);
-                } else {
-                    // Found color, but its contents is not in the right format
-                    return Err(PsyError::UnknownError(byte_offset));
-                }
             }
 
             Event::InnerClose { .. } => {

src/parse/psy_surface_shader.rs

@@ -40,12 +40,7 @@ pub fn parse_surface_shader(
                 byte_offset,
             } = events.next_event()?
             {
-                if let Ok(color) = parse_color(contents) {
+                parse_color(byte_offset, contents)?
-                    color
-                } else {
-                    // Found color, but its contents is not in the right format
-                    return Err(PsyError::UnknownError(byte_offset));
-                }
             } else {
                 return Err(PsyError::MissingNode(
                     events.byte_offset(),
@@ -76,13 +71,7 @@ pub fn parse_surface_shader(
                         contents,
                         byte_offset,
                     } => {
-                        if let Ok(col) = parse_color(contents) {
+                        color = Some(parse_color(byte_offset, contents)?);
-                            color = Some(col);
-                        } else {
-                            // Found color, but its contents is not in the right
-                            // format.
-                            return Err(PsyError::UnknownError(byte_offset));
-                        }
                     }
 
                     // Roughness
@@ -147,12 +136,7 @@ pub fn parse_surface_shader(
                 byte_offset,
             } = events.next_event()?
             {
-                if let Ok(color) = parse_color(contents) {
+                parse_color(byte_offset, contents)?
-                    color
-                } else {
-                    // Found color, but its contents is not in the right format
-                    return Err(PsyError::UnknownError(byte_offset));
-                }
             } else {
                 return Err(PsyError::MissingNode(
                     events.byte_offset(),