More parsing code cleanup and better error messages.

src/parse/psy_light.rs

@@ -14,7 +14,7 @@ use crate::{
 };
 
 use super::{
-    parse_utils::ws_f32,
+    parse_utils::{ensure_close, ensure_subsections, ws_f32},
     psy::{parse_color, PsyError, PsyResult},
 };
 
@@ -28,7 +28,12 @@ pub fn parse_distant_disk_light<'a>(
     let mut colors = Vec::new();
 
     // Parse
-    loop {
+    let valid_subsections = &[
+        ("Radius", true, (1..).into()),
+        ("Direction", true, (1..).into()),
+        ("Color", true, (1..).into()),
+    ];
+    ensure_subsections(events, valid_subsections, |events| {
         match events.next_event()? {
             Event::Leaf {
                 type_name: "Radius",
@@ -39,7 +44,12 @@ pub fn parse_distant_disk_light<'a>(
                     radii.push(radius);
                 } else {
                     // Found radius, but its contents is not in the right format
-                    return Err(PsyError::UnknownError(byte_offset));
+                    return Err(PsyError::IncorrectLeafData(
+                        byte_offset,
+                        "Radius data isn't in the right format.  It should \
+                         contain a single floating point value."
+                            .into(),
+                    ));
                 }
             }
 
@@ -55,7 +65,12 @@ pub fn parse_distant_disk_light<'a>(
                     directions.push(Vector::new(direction.0, direction.1, direction.2));
                 } else {
                     // Found direction, but its contents is not in the right format
-                    return Err(PsyError::UnknownError(byte_offset));
+                    return Err(PsyError::IncorrectLeafData(
+                        byte_offset,
+                        "Direction data isn't in the right format.  It should \
+                         contain a single floating point value."
+                            .into(),
+                    ));
                 }
             }
 
@@ -68,15 +83,12 @@ pub fn parse_distant_disk_light<'a>(
                 colors.push(parse_color(byte_offset, &contents)?);
             }
 
-            Event::InnerClose { .. } => {
+            _ => unreachable!(),
-                break;
         }
+        Ok(())
+    })?;
 
-            _ => {
+    ensure_close(events)?;
-                todo!(); // Return error.
-            }
-        }
-    }
 
     return Ok(DistantDiskLight::new(arena, &radii, &directions, &colors));
 }
@@ -89,9 +101,12 @@ pub fn parse_sphere_light<'a>(
     let mut colors = Vec::new();
 
     // Parse
-    loop {
+    let valid_subsections = &[
+        ("Radius", true, (1..).into()),
+        ("Color", true, (1..).into()),
+    ];
+    ensure_subsections(events, valid_subsections, |events| {
         match events.next_event()? {
-            // Radius
             Event::Leaf {
                 type_name: "Radius",
                 contents,
@@ -101,7 +116,12 @@ pub fn parse_sphere_light<'a>(
                     radii.push(radius);
                 } else {
                     // Found radius, but its contents is not in the right format
-                    return Err(PsyError::UnknownError(byte_offset));
+                    return Err(PsyError::IncorrectLeafData(
+                        byte_offset,
+                        "Radius data isn't in the right format.  It should \
+                         contain a single floating point value."
+                            .into(),
+                    ));
                 }
             }
 
@@ -114,15 +134,12 @@ pub fn parse_sphere_light<'a>(
                 colors.push(parse_color(byte_offset, &contents)?);
             }
 
-            Event::InnerClose { .. } => {
+            _ => unreachable!(),
-                break;
         }
+        Ok(())
+    })?;
 
-            _ => {
+    ensure_close(events)?;
-                todo!(); // Return error.
-            }
-        }
-    }
 
     return Ok(SphereLight::new(arena, &radii, &colors));
 }
@@ -135,7 +152,11 @@ pub fn parse_rectangle_light<'a>(
     let mut colors = Vec::new();
 
     // Parse
-    loop {
+    let valid_subsections = &[
+        ("Dimensions", true, (1..).into()),
+        ("Color", true, (1..).into()),
+    ];
+    ensure_subsections(events, valid_subsections, |events| {
         match events.next_event()? {
             // Dimensions
             Event::Leaf {
@@ -148,7 +169,12 @@ pub fn parse_rectangle_light<'a>(
                     dimensions.push(radius);
                 } else {
                     // Found dimensions, but its contents is not in the right format
-                    return Err(PsyError::UnknownError(byte_offset));
+                    return Err(PsyError::IncorrectLeafData(
+                        byte_offset,
+                        "Dimensions data isn't in the right format.  It should \
+                         contain two space-separated floating point values."
+                            .into(),
+                    ));
                 }
             }
 
@@ -161,15 +187,12 @@ pub fn parse_rectangle_light<'a>(
                 colors.push(parse_color(byte_offset, &contents)?);
             }
 
-            Event::InnerClose { .. } => {
+            _ => unreachable!(),
-                break;
         }
+        Ok(())
+    })?;
 
-            _ => {
+    ensure_close(events)?;
-                todo!(); // Return error.
-            }
-        }
-    }
 
     return Ok(RectangleLight::new(arena, &dimensions, &colors));
 }

src/parse/psy_mesh_surface.rs

@@ -14,7 +14,7 @@ use crate::{
 };
 
 use super::{
-    parse_utils::{ws_f32, ws_usize},
+    parse_utils::{ensure_close, ensure_subsections, ws_f32, ws_usize},
     psy::{PsyError, PsyResult},
 };
 
@@ -34,7 +34,14 @@ pub fn parse_mesh_surface<'a>(
     let mut face_vert_counts = Vec::new();
     let mut face_vert_indices = Vec::new();
 
-    loop {
+    let valid_subsections = &[
+        ("SurfaceShaderBind", true, (1).into()),
+        ("Vertices", true, (1..).into()),
+        ("Normals", true, (..).into()),
+        ("FaceVertCounts", true, (1).into()),
+        ("FaceVertIndices", true, (1).into()),
+    ];
+    ensure_subsections(events, valid_subsections, |events| {
         match events.next_event()? {
             Event::Leaf {
                 type_name: "SurfaceShaderBind",
@@ -46,7 +53,7 @@ pub fn parse_mesh_surface<'a>(
             Event::Leaf {
                 type_name: "Vertices",
                 contents,
-                ..
+                byte_offset,
             } => {
                 // Collect verts for this time sample
                 let mut text = contents;
@@ -56,13 +63,21 @@ pub fn parse_mesh_surface<'a>(
 
                     tverts.push(Point::new(vert.0, vert.1, vert.2));
                 }
+                if !text.is_empty() {
+                    return Err(PsyError::IncorrectLeafData(
+                        byte_offset,
+                        "Vertices are not in the right format.  Each vertex \
+                         must be specified by three decimal values."
+                            .into(),
+                    ));
+                }
                 verts.push(tverts);
             }
 
             Event::Leaf {
                 type_name: "Normals",
                 contents,
-                ..
+                byte_offset,
             } => {
                 // Collect normals for this time sample
                 let mut text = contents;
@@ -72,6 +87,14 @@ pub fn parse_mesh_surface<'a>(
 
                     tnormals.push(Normal::new(nor.0, nor.1, nor.2).normalized());
                 }
+                if !text.is_empty() {
+                    return Err(PsyError::IncorrectLeafData(
+                        byte_offset,
+                        "Normals are not in the right format.  Each normal \
+                         must be specified by three decimal values."
+                            .into(),
+                    ));
+                }
                 normals.push(tnormals);
             }
 
@@ -80,17 +103,19 @@ pub fn parse_mesh_surface<'a>(
                 contents,
                 byte_offset,
             } => {
-                if !face_vert_counts.is_empty() {
-                    return Err(PsyError::WrongNodeCount(
-                        byte_offset,
-                        "Meshes can only have one FaceVertCounts section.".into(),
-                    ));
-                }
                 let mut text = contents;
                 while let IResult::Ok((remaining, count)) = ws_usize(text) {
                     text = remaining;
                     face_vert_counts.push(count);
                 }
+                if !text.is_empty() {
+                    return Err(PsyError::IncorrectLeafData(
+                        byte_offset,
+                        "FaceVertCounts are not in the right format.  Should be \
+                         a simple list of space-separated integers."
+                            .into(),
+                    ));
+                }
             }
 
             Event::Leaf {
@@ -98,28 +123,27 @@ pub fn parse_mesh_surface<'a>(
                 contents,
                 byte_offset,
             } => {
-                if !face_vert_indices.is_empty() {
-                    return Err(PsyError::WrongNodeCount(
-                        byte_offset,
-                        "Meshes can only have one FaceVertIndices section.".into(),
-                    ));
-                }
                 let mut text = contents;
                 while let IResult::Ok((remaining, index)) = ws_usize(text) {
                     text = remaining;
                     face_vert_indices.push(index);
                 }
+                if !text.is_empty() {
+                    return Err(PsyError::IncorrectLeafData(
+                        byte_offset,
+                        "FaceVertCounts are not in the right format.  Should be \
+                         a simple list of space-separated integers."
+                            .into(),
+                    ));
+                }
             }
 
-            Event::InnerClose { .. } => {
+            _ => unreachable!(),
-                break;
         }
+        Ok(())
+    })?;
 
-            _ => {
+    ensure_close(events)?;
-                todo!(); // Return error.
-            }
-        }
-    }
 
     // Validation: make sure all time samples have same vert count.
     let vert_count = verts[0].len();
@@ -136,12 +160,6 @@ pub fn parse_mesh_surface<'a>(
         }
     }
 
-    // Validation: make sure we have any mesh data.
-    if verts.is_empty() || face_vert_counts.is_empty() || face_vert_indices.is_empty() {
-        todo!("Meshes must have at least one non-empty of each of the following sections: Vertices, FaceVertCounts, FaceVertIndices.");
-        // Return an error.
-    }
-
     // Build triangle mesh
     let mut tri_vert_indices = Vec::new();
     let mut ii = 0;

src/parse/psy_surface_shader.rs

@@ -11,7 +11,7 @@ use data_tree::{DataTreeReader, Event};
 use crate::shading::{SimpleSurfaceShader, SurfaceShader};
 
 use super::{
-    parse_utils::{ensure_close, ws_f32},
+    parse_utils::{ensure_close, ensure_subsections, ws_f32},
     psy::{parse_color, PsyError, PsyResult},
 };
 
@@ -63,7 +63,12 @@ pub fn parse_surface_shader(
             let mut roughness = None;
             let mut fresnel = None;
 
-            loop {
+            let valid_subsections = &[
+                ("Color", true, (1).into()),
+                ("Roughness", true, (1).into()),
+                ("Fresnel", true, (1).into()),
+            ];
+            ensure_subsections(events, valid_subsections, |events| {
                 match events.next_event()? {
                     // Color
                     Event::Leaf {
@@ -83,7 +88,12 @@ pub fn parse_surface_shader(
                         if let IResult::Ok((_, rgh)) = all_consuming(ws_f32)(contents) {
                             roughness = Some(rgh);
                         } else {
-                            return Err(PsyError::UnknownError(byte_offset));
+                            return Err(PsyError::IncorrectLeafData(
+                                byte_offset,
+                                "Roughness data isn't in the right format.  It \
+                                 should contain a single floating point value."
+                                    .into(),
+                            ));
                         }
                     }
 
@@ -96,27 +106,21 @@ pub fn parse_surface_shader(
                         if let IResult::Ok((_, frs)) = all_consuming(ws_f32)(contents) {
                             fresnel = Some(frs);
                         } else {
-                            return Err(PsyError::UnknownError(byte_offset));
+                            return Err(PsyError::IncorrectLeafData(
-                        }
+                                byte_offset,
-                    }
+                                "Fresnel data isn't in the right format.  It \
-
+                                 should contain a single floating point value."
-                    Event::InnerClose { .. } => {
+                                    .into(),
-                        break;
-                    }
-
-                    _ => {
-                        todo!(); // Return an error.
-                    }
-                }
-            }
-
-            // Validation: make sure all fields are present.
-            if color == None || roughness == None || fresnel == None {
-                return Err(PsyError::MissingNode(
-                    events.byte_offset(),
-                    "GGX shader requires one of each field: Color, Roughness, Fresnel.".into(),
                             ));
                         }
+                    }
+
+                    _ => unreachable!(),
+                }
+                Ok(())
+            })?;
+
+            ensure_close(events)?;
 
             Box::new(SimpleSurfaceShader::GGX {
                 color: color.unwrap(),