rbx_loader: refactor to make RecoverableError report

Cargo.lock

@@ -3803,6 +3803,7 @@ dependencies = [
  "rbx_binary",
  "rbx_dom_weak",
  "rbx_mesh",
+ "rbx_reflection",
  "rbx_reflection_database",
  "rbx_xml",
  "rbxassetid 0.1.0",

lib/rbx_loader/Cargo.toml

@@ -16,6 +16,7 @@ lazy-regex = "3.1.0"
 rbx_binary  = { version = "1.1.0-sn4", registry = "strafesnet" }
 rbx_dom_weak =  { version = "3.1.0-sn4", registry = "strafesnet", features = ["instance-userdata"] }
 rbx_mesh = "0.3.1"
+rbx_reflection = "5.0.0"
 rbx_reflection_database = "1.0.0"
 rbx_xml  = { version = "1.1.0-sn4", registry = "strafesnet" }
 rbxassetid = { version = "0.1.0", path = "../rbxassetid", registry = "strafesnet" }

lib/rbx_loader/src/error.rs

@@ -0,0 +1,117 @@
+use std::collections::HashSet;
+use std::num::ParseIntError;
+
+use strafesnet_common::gameplay_modes::{StageId,ModeId};
+use strafesnet_common::integer::{FixedFromFloatError,Planar64TryFromFloatError};
+
+/// A collection of errors which can be ignored at your peril
+#[derive(Debug,Default)]
+pub struct RecoverableErrors{
+	/// A basepart has an invalid / missing property.
+	pub basepart_property:Vec<InstancePath>,
+	/// A part has an unconvertable CFrame.
+	pub basepart_cframe:Vec<CFrameError>,
+	/// A part has an unconvertable Velocity.
+	pub basepart_velocity:Vec<Planar64ConvertError>,
+	/// A part has an invalid / missing property.
+	pub part_property:Vec<InstancePath>,
+	/// A part has an invalid shape.
+	pub part_shape:Vec<ShapeError>,
+	/// A meshpart has an invalid / missing property.
+	pub meshpart_property:Vec<InstancePath>,
+	/// A meshpart has no mesh.
+	pub meshpart_content:Vec<InstancePath>,
+	/// A basepart has an unsupported subclass.
+	pub unsupported_class:HashSet<String>,
+	/// A decal has an invalid / missing property.
+	pub decal_property:Vec<InstancePath>,
+	/// A decal has an invalid normal_id.
+	pub normal_id:Vec<NormalIdError>,
+	/// A texture has an invalid / missing property.
+	pub texture_property:Vec<InstancePath>,
+	/// A mode_id failed to parse.
+	pub mode_id_parse_int:Vec<ParseIntContext>,
+	/// There is a duplicate mode.
+	pub duplicate_mode:HashSet<ModeId>,
+	/// A mode_id failed to parse.
+	pub stage_id_parse_int:Vec<ParseIntContext>,
+	/// A Stage was duplicated leading to undefined behaviour.
+	pub duplicate_stage:HashSet<DuplicateStageError>,
+	/// A WormholeOut id failed to parse.
+	pub wormhole_out_id_parse_int:Vec<ParseIntContext>,
+	/// A WormholeOut was duplicated leading to undefined behaviour.
+	pub duplicate_wormhole_out:HashSet<u32>,
+	/// A WormholeIn id failed to parse.
+	pub wormhole_in_id_parse_int:Vec<ParseIntContext>,
+	/// A jump limit failed to parse.
+	pub jump_limit_parse_int:Vec<ParseIntContext>,
+}
+
+/// A Decal was missing required properties
+#[derive(Debug)]
+pub struct InstancePath(pub String);
+
+impl InstancePath{
+	pub fn new(dom:&rbx_dom_weak::WeakDom,instance:&rbx_dom_weak::Instance)->InstancePath{
+		let mut names:Vec<_>=core::iter::successors(
+			Some(instance),
+			|i|dom.get_by_ref(i.parent())
+		).map(
+			|i|i.name.as_str()
+		).collect();
+		// discard the name of the root object
+		names.pop();
+		names.reverse();
+		InstancePath(names.join("."))
+	}
+}
+
+#[derive(Debug)]
+pub struct ParseIntContext{
+	pub context:String,
+	pub error:ParseIntError,
+}
+impl ParseIntContext{
+	pub fn parse<T:core::str::FromStr<Err=ParseIntError>>(input:&str)->Result<T,Self>{
+		input.parse().map_err(|error|ParseIntContext{
+			context:input.to_owned(),
+			error,
+		})
+	}
+}
+
+#[derive(Debug)]
+pub struct NormalIdError{
+	pub path:InstancePath,
+	pub normal_id:u32,
+}
+
+#[derive(Debug)]
+pub struct ShapeError{
+	pub path:InstancePath,
+	pub shape:u32,
+}
+
+#[derive(Debug)]
+pub enum CFrameErrorType{
+	ZeroDeterminant,
+	Convert(FixedFromFloatError),
+}
+
+#[derive(Debug)]
+pub struct CFrameError{
+	pub path:InstancePath,
+	pub error:CFrameErrorType,
+}
+
+#[derive(Debug)]
+pub struct Planar64ConvertError{
+	pub path:InstancePath,
+	pub error:Planar64TryFromFloatError,
+}
+
+#[derive(Debug,Hash,Eq,PartialEq)]
+pub struct DuplicateStageError{
+	pub mode_id:ModeId,
+	pub stage_id:StageId,
+}

lib/rbx_loader/src/lib.rs

@@ -1,10 +1,14 @@
 use std::io::Read;
 use rbx_dom_weak::WeakDom;
 use roblox_emulator::context::Context;
+use strafesnet_common::map::CompleteMap;
 use strafesnet_deferred_loader::deferred_loader::{LoadFailureMode,MeshDeferredLoader,RenderConfigDeferredLoader};
 
+pub use error::RecoverableErrors;
+
 mod rbx;
 mod mesh;
+mod error;
 mod union;
 pub mod loader;
 mod primitives;
@@ -28,7 +32,7 @@ impl Model{
 	fn new(dom:WeakDom)->Self{
 		Self{dom}
 	}
-	pub fn to_snf(&self,failure_mode:LoadFailureMode)->Result<strafesnet_common::map::CompleteMap,LoadError>{
+	pub fn to_snf(&self,failure_mode:LoadFailureMode)->Result<(CompleteMap,RecoverableErrors),LoadError>{
 		to_snf(self,failure_mode)
 	}
 }
@@ -59,7 +63,7 @@ impl Place{
 			}
 		}
 	}
-	pub fn to_snf(&self,failure_mode:LoadFailureMode)->Result<strafesnet_common::map::CompleteMap,LoadError>{
+	pub fn to_snf(&self,failure_mode:LoadFailureMode)->Result<(CompleteMap,RecoverableErrors),LoadError>{
 		to_snf(self,failure_mode)
 	}
 }
@@ -123,7 +127,7 @@ impl From<loader::MeshError> for LoadError{
 	}
 }
 
-fn to_snf(dom:impl AsRef<WeakDom>,failure_mode:LoadFailureMode)->Result<strafesnet_common::map::CompleteMap,LoadError>{
+fn to_snf(dom:impl AsRef<WeakDom>,failure_mode:LoadFailureMode)->Result<(CompleteMap,RecoverableErrors),LoadError>{
 	let dom=dom.as_ref();
 
 	let mut texture_deferred_loader=RenderConfigDeferredLoader::new();
@@ -143,7 +147,5 @@ fn to_snf(dom:impl AsRef<WeakDom>,failure_mode:LoadFailureMode)->Result<strafesn
 	let mut texture_loader=loader::TextureLoader::new();
 	let render_configs=texture_deferred_loader.into_render_configs(&mut texture_loader,failure_mode).map_err(LoadError::Texture)?;
 
-	let map=map_step2.add_render_configs_and_textures(render_configs);
+	Ok(map_step2.add_render_configs_and_textures(render_configs))
-
-	Ok(map)
 }

lib/rbx_loader/src/rbx.rs

@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use crate::error::{RecoverableErrors,CFrameError,CFrameErrorType,DuplicateStageError,InstancePath,NormalIdError,Planar64ConvertError,ParseIntContext,ShapeError};
 use crate::loader::{MeshWithSize,MeshIndex};
 use crate::primitives::{self,CubeFace,CubeFaceDescription,WedgeFaceDescription,CornerWedgeFaceDescription,FaceDescription,Primitives};
 use strafesnet_common::map;
@@ -6,7 +7,7 @@ use strafesnet_common::model;
 use strafesnet_common::gameplay_modes::{NormalizedModes,Mode,ModeId,ModeUpdate,ModesBuilder,Stage,StageElement,StageElementBehaviour,StageId,Zone};
 use strafesnet_common::gameplay_style;
 use strafesnet_common::gameplay_attributes as attr;
-use strafesnet_common::integer::{self,vec3,Planar64,Planar64Vec3,Planar64Mat3,Planar64Affine3};
+use strafesnet_common::integer::{self,vec3,Planar64TryFromFloatError,Planar64,Planar64Vec3,Planar64Mat3,Planar64Affine3};
 use strafesnet_common::model::RenderConfigId;
 use strafesnet_deferred_loader::deferred_loader::{RenderConfigDeferredLoader,MeshDeferredLoader};
 use strafesnet_deferred_loader::mesh::Meshes;
@@ -17,40 +18,32 @@ fn static_ustr(s:&'static str)->rbx_dom_weak::Ustr{
 	rbx_dom_weak::ustr(s)
 }
 
-fn recursive_collect_superclass(
+fn planar64_affine3_from_roblox(cf:&rbx_dom_weak::types::CFrame,size:&rbx_dom_weak::types::Vector3)->Result<Planar64Affine3,Planar64TryFromFloatError>{
-	objects:&mut std::vec::Vec<rbx_dom_weak::types::Ref>,
+	Ok(Planar64Affine3::new(
-	dom:&rbx_dom_weak::WeakDom,
-	instance:&rbx_dom_weak::Instance,
-	superclass:&str
-){
-	let instance=instance;
-	let db=rbx_reflection_database::get();
-	let Some(superclass)=db.classes.get(superclass)else{
-		return;
-	};
-	objects.extend(
-		dom.descendants_of(instance.referent()).filter_map(|instance|{
-			let class=db.classes.get(instance.class.as_str())?;
-			db.has_superclass(class,superclass).then(||instance.referent())
-		})
-	);
-}
-
-fn planar64_affine3_from_roblox(cf:&rbx_dom_weak::types::CFrame,size:&rbx_dom_weak::types::Vector3)->Planar64Affine3{
-	Planar64Affine3::new(
 		Planar64Mat3::from_cols([
-			vec3::try_from_f32_array([cf.orientation.x.x,cf.orientation.y.x,cf.orientation.z.x]).unwrap()
+			(vec3::try_from_f32_array([cf.orientation.x.x,cf.orientation.y.x,cf.orientation.z.x])?
-			*integer::try_from_f32(size.x/2.0).unwrap(),
+			*integer::try_from_f32(size.x/2.0)?).narrow_1().unwrap(),//.map_err(Planar64ConvertError::Narrow)?
-			vec3::try_from_f32_array([cf.orientation.x.y,cf.orientation.y.y,cf.orientation.z.y]).unwrap()
+			(vec3::try_from_f32_array([cf.orientation.x.y,cf.orientation.y.y,cf.orientation.z.y])?
-			*integer::try_from_f32(size.y/2.0).unwrap(),
+			*integer::try_from_f32(size.y/2.0)?).narrow_1().unwrap(),//.map_err(Planar64ConvertError::Narrow)?
-			vec3::try_from_f32_array([cf.orientation.x.z,cf.orientation.y.z,cf.orientation.z.z]).unwrap()
+			(vec3::try_from_f32_array([cf.orientation.x.z,cf.orientation.y.z,cf.orientation.z.z])?
-			*integer::try_from_f32(size.z/2.0).unwrap(),
+			*integer::try_from_f32(size.z/2.0)?).narrow_1().unwrap(),//.map_err(Planar64ConvertError::Narrow)?
-			].map(|t|t.narrow_1().unwrap())),
+		]),
-		vec3::try_from_f32_array([cf.position.x,cf.position.y,cf.position.z]).unwrap()
+		vec3::try_from_f32_array([cf.position.x,cf.position.y,cf.position.z])?
-	)
+	))
 }
 
-fn get_attributes(name:&str,can_collide:bool,velocity:Planar64Vec3,model_id:model::ModelId,modes_builder:&mut ModesBuilder,wormhole_in_model_to_id:&mut HashMap<model::ModelId,u32>,wormhole_id_to_out_model:&mut HashMap<u32,model::ModelId>)->attr::CollisionAttributes{
+enum GetAttributesError{
+	ModeIdParseInt(ParseIntContext),
+	DuplicateMode(ModeId),
+	StageIdParseInt(ParseIntContext),
+	DuplicateStage(DuplicateStageError),
+	WormholeOutIdParseInt(ParseIntContext),
+	DuplicateWormholeOut(u32),
+	WormholeInIdParseInt(ParseIntContext),
+	JumpLimitParseInt(ParseIntContext),
+}
+
+fn get_attributes(name:&str,can_collide:bool,velocity:Planar64Vec3,model_id:model::ModelId,modes_builder:&mut ModesBuilder,wormhole_in_model_to_id:&mut HashMap<model::ModelId,u32>,wormhole_id_to_out_model:&mut HashMap<u32,model::ModelId>)->Result<attr::CollisionAttributes,GetAttributesError>{
 	let mut general=attr::GeneralAttributes::default();
 	let mut intersecting=attr::IntersectingAttributes::default();
 	let mut contacting=attr::ContactingAttributes::default();
@@ -84,13 +77,14 @@ fn get_attributes(name:&str,can_collide:bool,velocity:Planar64Vec3,model_id:mode
 		"MapStart"=>{
 			force_can_collide=false;
 			force_intersecting=true;
+			let mode_id=ModeId::MAIN;
 			modes_builder.insert_mode(
-				ModeId::MAIN,
+				mode_id,
 				Mode::empty(
 					gameplay_style::StyleModifiers::roblox_bhop(),
 					model_id
 				)
-			).unwrap();
+			).map_err(|_|GetAttributesError::DuplicateMode(mode_id))?;
 		},
 		"MapFinish"=>{
 			force_can_collide=false;
@@ -130,26 +124,30 @@ fn get_attributes(name:&str,can_collide:bool,velocity:Planar64Vec3,model_id:mode
 					"BonusStart"=>{
 						force_can_collide=false;
 						force_intersecting=true;
+						let mode_id=ModeId::new(ParseIntContext::parse(&captures[2]).map_err(GetAttributesError::ModeIdParseInt)?);
 						modes_builder.insert_mode(
-							ModeId::new(captures[2].parse::<u32>().unwrap()),
+							mode_id,
 							Mode::empty(
 								gameplay_style::StyleModifiers::roblox_bhop(),
 								model_id
 							)
-						).unwrap();
+						).map_err(|_|GetAttributesError::DuplicateMode(mode_id))?;
 					},
 					"WormholeOut"=>{
 						//the PhysicsModelId has to exist for it to be teleported to!
 						force_intersecting=true;
 						//this object is not special in strafe client, but the roblox mapping needs to be converted to model id
-						assert!(wormhole_id_to_out_model.insert(captures[2].parse::<u32>().unwrap(),model_id).is_none(),"Cannot have multiple WormholeOut with same id");
+						let wormhole_id=ParseIntContext::parse(&captures[2]).map_err(GetAttributesError::WormholeOutIdParseInt)?;
+						if wormhole_id_to_out_model.insert(wormhole_id,model_id).is_some(){
+							return Err(GetAttributesError::DuplicateWormholeOut(wormhole_id));
+						}
 					},
 					_=>(),
 				}
 			}else if let Some(captures)=lazy_regex::regex!(r"^(Force)?(Spawn|SpawnAt|Trigger|Teleport|Platform)(\d+)$")
 			.captures(other){
 				force_intersecting=true;
-				let stage_id=StageId::new(captures[3].parse::<u32>().unwrap());
+				let stage_id=StageId::new(ParseIntContext::parse(&captures[3]).map_err(GetAttributesError::StageIdParseInt)?);
 				let stage_element=StageElement::new(
 					//stage_id:
 					stage_id,
@@ -161,11 +159,12 @@ fn get_attributes(name:&str,can_collide:bool,velocity:Planar64Vec3,model_id:mode
 					//behaviour:
 					match &captures[2]{
 						"Spawn"=>{
+							let mode_id=ModeId::MAIN;
 							modes_builder.insert_stage(
-								ModeId::MAIN,
+								mode_id,
 								stage_id,
 								Stage::empty(model_id),
-							).unwrap();
+							).map_err(|_|GetAttributesError::DuplicateStage(DuplicateStageError{mode_id,stage_id}))?;
 							//TODO: let denormalize handle this
 							StageElementBehaviour::SpawnAt
 						},
@@ -175,7 +174,7 @@ fn get_attributes(name:&str,can_collide:bool,velocity:Planar64Vec3,model_id:mode
 						"Trigger"=>{force_can_collide=false;StageElementBehaviour::Trigger},
 						"Teleport"=>{force_can_collide=false;StageElementBehaviour::Teleport},
 						"Platform"=>StageElementBehaviour::Platform,
-						_=>panic!("regex1[2] messed up bad"),
+						_=>unreachable!("regex1[2] messed up bad"),
 					},
 					None
 				);
@@ -198,30 +197,33 @@ fn get_attributes(name:&str,can_collide:bool,velocity:Planar64Vec3,model_id:mode
 								StageId::FIRST,
 								false,
 								StageElementBehaviour::Check,
-								Some(captures[2].parse::<u8>().unwrap())
+								Some(ParseIntContext::parse(&captures[2]).map_err(GetAttributesError::JumpLimitParseInt)?)
 							)
 						),
 					),
 					"WormholeIn"=>{
 						force_can_collide=false;
 						force_intersecting=true;
-						assert!(wormhole_in_model_to_id.insert(model_id,captures[2].parse::<u32>().unwrap()).is_none(),"Impossible");
+						let wormhole_id=ParseIntContext::parse(&captures[2]).map_err(GetAttributesError::WormholeInIdParseInt)?;
+						// It is impossible for two different objects to have the same model id
+						assert!(wormhole_in_model_to_id.insert(model_id,wormhole_id).is_none(),"Impossible");
 					},
-					_=>panic!("regex2[1] messed up bad"),
+					_=>unreachable!("regex2[1] messed up bad"),
 				}
 			}else if let Some(captures)=lazy_regex::regex!(r"^Bonus(Finish|Anticheat)(\d+)$")
 			.captures(other){
 				force_can_collide=false;
 				force_intersecting=true;
+				let mode_id=ModeId::new(ParseIntContext::parse(&captures[2]).map_err(GetAttributesError::ModeIdParseInt)?);
 				modes_builder.push_mode_update(
-					ModeId::new(captures[2].parse::<u32>().unwrap()),
+					mode_id,
 					ModeUpdate::zone(
 						model_id,
 						//zone:
 						match &captures[1]{
 							"Finish"=>Zone::Finish,
 							"Anticheat"=>Zone::Anticheat,
-							_=>panic!("regex3[1] messed up bad"),
+							_=>unreachable!("regex3[1] messed up bad"),
 						},
 					),
 				);
@@ -243,7 +245,7 @@ fn get_attributes(name:&str,can_collide:bool,velocity:Planar64Vec3,model_id:mode
 	if allow_booster&&velocity!=vec3::ZERO{
 		general.booster=Some(attr::Booster::Velocity(velocity));
 	}
-	match force_can_collide{
+	Ok(match force_can_collide{
 		true=>{
 			match name{
 				"Bounce"=>contacting.contact_behaviour=Some(attr::ContactingBehaviour::Elastic(u32::MAX)),
@@ -261,7 +263,7 @@ fn get_attributes(name:&str,can_collide:bool,velocity:Planar64Vec3,model_id:mode
 		}else{
 			attr::CollisionAttributes::Decoration
 		},
-	}
+	})
 }
 
 #[derive(Clone,Copy)]
@@ -407,21 +409,25 @@ fn get_content_url(content:&rbx_dom_weak::types::Content)->Option<&str>{
 }
 
 fn get_texture_description<'a>(
-	temp_objects:&mut Vec<rbx_dom_weak::types::Ref>,
 	render_config_deferred_loader:&mut RenderConfigDeferredLoader<&'a str>,
+	recoverable_errors:&mut RecoverableErrors,
+	db:&rbx_reflection::ReflectionDatabase,
 	dom:&'a rbx_dom_weak::WeakDom,
 	object:&rbx_dom_weak::Instance,
 	size:&rbx_dom_weak::types::Vector3,
 )->RobloxPartDescription{
 	//use the biggest one and cut it down later...
 	let mut part_texture_description=RobloxPartDescription::default();
-	temp_objects.clear();
+	let decal=&db.classes["Decal"];
-	recursive_collect_superclass(temp_objects,&dom,object,"Decal");
+	let decals=object.children().iter().filter_map(|&referent|{
-	for &mut decal_ref in temp_objects{
+		let instance=dom.get_by_ref(referent)?;
-		let Some(decal)=dom.get_by_ref(decal_ref) else{
+		db.classes.get(instance.class.as_str()).is_some_and(|class|
-			println!("Decal get_by_ref failed");
+			db.has_superclass(class,decal)
-			continue;
+		).then_some(instance)
-		};
+	});
+	for decal in decals{
+		// decals should always have these properties,
+		// but it is not guaranteed by the rbx_dom_weak data structure.
 		let (
 			Some(rbx_dom_weak::types::Variant::Content(content)),
 			Some(rbx_dom_weak::types::Variant::Enum(normalid)),
@@ -433,16 +439,16 @@ fn get_texture_description<'a>(
 			decal.properties.get(&static_ustr("Color3")),
 			decal.properties.get(&static_ustr("Transparency")),
 		)else{
-			println!("Decal is missing a required property");
+			recoverable_errors.decal_property.push(InstancePath::new(dom,decal));
 			continue;
 		};
-		let texture_id=match content.value(){
+		let texture_id=get_content_url(content);
-			rbx_dom_weak::types::ContentType::Uri(uri)=>Some(uri.as_str()),
-			_=>None,
-		};
 		let render_id=render_config_deferred_loader.acquire_render_config_id(texture_id);
 		let Ok(cube_face)=normalid.to_u32().try_into()else{
-			println!("NormalId is invalid");
+			recoverable_errors.normal_id.push(NormalIdError{
+				path:InstancePath::new(dom,decal),
+				normal_id:normalid.to_u32(),
+			});
 			continue;
 		};
 		let (roblox_texture_color,roblox_texture_transform)=if decal.class=="Texture"{
@@ -479,6 +485,7 @@ fn get_texture_description<'a>(
 					}
 				)
 			}else{
+				recoverable_errors.texture_property.push(InstancePath::new(dom,decal));
 				(glam::Vec4::ONE,RobloxTextureTransform::identity())
 			}
 		}else{
@@ -532,6 +539,8 @@ pub fn convert<'a>(
 	render_config_deferred_loader:&mut RenderConfigDeferredLoader<&'a str>,
 	mesh_deferred_loader:&mut MeshDeferredLoader<MeshIndex<'a>>,
 )->PartialMap1<'a>{
+	let mut recoverable_errors=RecoverableErrors::default();
+
 	let mut deferred_models_deferred_attributes=Vec::new();
 	let mut deferred_unions_deferred_attributes=Vec::new();
 	let mut primitive_models_deferred_attributes=Vec::new();
@@ -541,63 +550,82 @@ pub fn convert<'a>(
 	//just going to leave it like this for now instead of reworking the data structures for this whole thing
 	let textureless_render_group=render_config_deferred_loader.acquire_render_config_id(None);
 
-	let mut object_refs=Vec::new();
+	let db=rbx_reflection_database::get();
-	let mut temp_objects=Vec::new();
+	let basepart=&db.classes["BasePart"];
-	recursive_collect_superclass(&mut object_refs, &dom, dom.root(),"BasePart");
+	let baseparts=dom.descendants().filter(|&instance|
-	for object_ref in object_refs {
+		db.classes.get(instance.class.as_str()).is_some_and(|class|
-		if let Some(object)=dom.get_by_ref(object_ref){
+			db.has_superclass(class,basepart)
-			if let (
+		)
-					Some(rbx_dom_weak::types::Variant::CFrame(cf)),
+	);
-					Some(rbx_dom_weak::types::Variant::Vector3(size)),
+	for object in baseparts{
-					Some(rbx_dom_weak::types::Variant::Vector3(velocity)),
+		let (
-					Some(rbx_dom_weak::types::Variant::Float32(transparency)),
+			Some(rbx_dom_weak::types::Variant::CFrame(cf)),
-					Some(rbx_dom_weak::types::Variant::Color3uint8(color3)),
+			Some(rbx_dom_weak::types::Variant::Vector3(size)),
-					Some(rbx_dom_weak::types::Variant::Bool(can_collide)),
+			Some(rbx_dom_weak::types::Variant::Vector3(velocity)),
-				) = (
+			Some(rbx_dom_weak::types::Variant::Float32(transparency)),
-					object.properties.get(&static_ustr("CFrame")),
+			Some(rbx_dom_weak::types::Variant::Color3uint8(color3)),
-					object.properties.get(&static_ustr("Size")),
+			Some(&rbx_dom_weak::types::Variant::Bool(can_collide)),
-					object.properties.get(&static_ustr("Velocity")),
+		) = (
-					object.properties.get(&static_ustr("Transparency")),
+			object.properties.get(&static_ustr("CFrame")),
-					object.properties.get(&static_ustr("Color")),
+			object.properties.get(&static_ustr("Size")),
-					object.properties.get(&static_ustr("CanCollide")),
+			object.properties.get(&static_ustr("Velocity")),
-				)
+			object.properties.get(&static_ustr("Transparency")),
-			{
+			object.properties.get(&static_ustr("Color")),
-				let model_transform=planar64_affine3_from_roblox(cf,size);
+			object.properties.get(&static_ustr("CanCollide")),
-
+		)else{
+			recoverable_errors.basepart_property.push(InstancePath::new(dom,object));
+			continue;
+		};
+		let model_transform=match planar64_affine3_from_roblox(cf,size){
+			Ok(model_transform)=>{
 				if model_transform.matrix3.det().is_zero(){
-					let mut parent_ref=object.parent();
+					recoverable_errors.basepart_cframe.push(CFrameError{
-					let mut full_path=object.name.clone();
+						path:InstancePath::new(dom,object),
-					while let Some(parent)=dom.get_by_ref(parent_ref){
+						error:CFrameErrorType::ZeroDeterminant,
-						full_path=format!("{}.{}",parent.name,full_path);
+					});
-						parent_ref=parent.parent();
-					}
-					println!("Zero determinant CFrame at location {}",full_path);
-					println!("matrix3:{}",model_transform.matrix3);
 					continue;
 				}
+				model_transform
+			},
+			Err(e)=>{
+				recoverable_errors.basepart_cframe.push(CFrameError{
+					path:InstancePath::new(dom,object),
+					error:CFrameErrorType::Convert(e),
+				});
+				continue;
+			}
+		};
 
-				//TODO: also detect "CylinderMesh" etc here
+		//TODO: also detect "CylinderMesh" etc here
-				let shape=match object.class.as_str(){
+		let shape=match object.class.as_str(){
-					"Part"=>if let Some(rbx_dom_weak::types::Variant::Enum(shape))=object.properties.get(&static_ustr("Shape")){
+			"Part"=>{
-						Shape::Primitive(shape.to_u32().try_into().expect("Funky roblox PartType"))
+				let Some(rbx_dom_weak::types::Variant::Enum(shape))=object.properties.get(&static_ustr("Shape"))else{
-					}else{
+					recoverable_errors.part_property.push(InstancePath::new(dom,object));
-						panic!("Part has no Shape!");
+					continue;
-					},
-					"TrussPart"=>Shape::Primitive(Primitives::Cube),
-					"WedgePart"=>Shape::Primitive(Primitives::Wedge),
-					"CornerWedgePart"=>Shape::Primitive(Primitives::CornerWedge),
-					"MeshPart"=>Shape::MeshPart,
-					"UnionOperation"=>Shape::PhysicsData,
-					_=>{
-						println!("Unsupported BasePart ClassName={}; defaulting to cube",object.class);
-						Shape::Primitive(Primitives::Cube)
-					}
 				};
+				let Ok(shape)=shape.to_u32().try_into()else{
+					recoverable_errors.part_shape.push(ShapeError{
+						path:InstancePath::new(dom,object),
+						shape:shape.to_u32(),
+					});
+					continue;
+				};
+				Shape::Primitive(shape)
+			},
+			"TrussPart"=>Shape::Primitive(Primitives::Cube),
+			"WedgePart"=>Shape::Primitive(Primitives::Wedge),
+			"CornerWedgePart"=>Shape::Primitive(Primitives::CornerWedge),
+			"MeshPart"=>Shape::MeshPart,
+			"UnionOperation"=>Shape::PhysicsData,
+			_=>{
+				recoverable_errors.unsupported_class.insert(object.class.as_str().to_owned());
+				Shape::Primitive(Primitives::Cube)
+			}
+		};
 
-				let (availability,mesh_id)=match shape{
+		let (availability,mesh_id)=match shape{
-					Shape::Primitive(primitive_shape)=>{
+			Shape::Primitive(primitive_shape)=>{
-				//TODO: TAB TAB
+				let part_texture_description=get_texture_description(render_config_deferred_loader,&mut recoverable_errors,db,dom,object,size);
-				let part_texture_description=get_texture_description(&mut temp_objects,render_config_deferred_loader,dom,object,size);
 				//obscure rust syntax "slice pattern"
 				let RobloxPartDescription([
 					f0,//Cube::Right
@@ -646,66 +674,83 @@ pub fn convert<'a>(
 					mesh_id
 				};
 				(MeshAvailability::Immediate,mesh_id)
-					},
+			},
-					Shape::MeshPart=>if let (
+			Shape::MeshPart=>{
-						Some(rbx_dom_weak::types::Variant::Content(mesh_content)),
+				let (
-						Some(rbx_dom_weak::types::Variant::Content(texture_content)),
+					Some(rbx_dom_weak::types::Variant::Content(mesh_content)),
-					)=(
+					Some(rbx_dom_weak::types::Variant::Content(texture_content)),
-						// mesh must exist
+				)=(
-						object.properties.get(&static_ustr("MeshContent")),
+					// mesh must exist
-						// texture is allowed to be none
+					object.properties.get(&static_ustr("MeshContent")),
-						object.properties.get(&static_ustr("TextureContent")),
+					// texture is allowed to be none
-					){
+					object.properties.get(&static_ustr("TextureContent")),
-						let mesh_asset_id=get_content_url(mesh_content).unwrap_or_default();
+				)else{
-						let texture_asset_id=get_content_url(texture_content);
+					recoverable_errors.meshpart_property.push(InstancePath::new(dom,object));
-						(
+					continue;
-							MeshAvailability::DeferredMesh(render_config_deferred_loader.acquire_render_config_id(texture_asset_id)),
-							mesh_deferred_loader.acquire_mesh_id(MeshIndex::file_mesh(mesh_asset_id)),
-						)
-					}else{
-						panic!("Mesh has no Mesh or Texture");
-					},
-					Shape::PhysicsData=>{
-						let mut content="";
-						let mut mesh_data:&[u8]=&[];
-						let mut physics_data:&[u8]=&[];
-						if let Some(rbx_dom_weak::types::Variant::ContentId(asset_id))=object.properties.get(&static_ustr("AssetId")){
-							content=asset_id.as_ref();
-						}
-						if let Some(rbx_dom_weak::types::Variant::BinaryString(data))=object.properties.get(&static_ustr("MeshData")){
-							mesh_data=data.as_ref();
-						}
-						if let Some(rbx_dom_weak::types::Variant::BinaryString(data))=object.properties.get(&static_ustr("PhysicsData")){
-							physics_data=data.as_ref();
-						}
-						let part_texture_description=get_texture_description(&mut temp_objects,render_config_deferred_loader,dom,object,size);
-						let mesh_index=MeshIndex::union(content,mesh_data,physics_data,size,part_texture_description.clone());
-						let mesh_id=mesh_deferred_loader.acquire_mesh_id(mesh_index);
-						(MeshAvailability::DeferredUnion(part_texture_description),mesh_id)
-					},
 				};
-				let model_deferred_attributes=ModelDeferredAttributes{
+				let mesh_asset_id=match get_content_url(mesh_content){
-					mesh:mesh_id,
+					Some(mesh_asset_id)=>mesh_asset_id,
-					transform:model_transform,
+					None=>{
-					color:glam::vec4(color3.r as f32/255f32, color3.g as f32/255f32, color3.b as f32/255f32, 1.0-*transparency),
+						recoverable_errors.meshpart_content.push(InstancePath::new(dom,object));
-					deferred_attributes:GetAttributesArgs{
+						// Return an empty string which will fail to parse as an asset id
-						name:object.name.as_str(),
+						""
-						can_collide:*can_collide,
+					}
-						velocity:vec3::try_from_f32_array([velocity.x,velocity.y,velocity.z]).unwrap(),
-					},
 				};
-				match availability{
+				let texture_asset_id=get_content_url(texture_content);
-					MeshAvailability::Immediate=>primitive_models_deferred_attributes.push(model_deferred_attributes),
+				(
-					MeshAvailability::DeferredMesh(render)=>deferred_models_deferred_attributes.push(DeferredModelDeferredAttributes{
+					MeshAvailability::DeferredMesh(render_config_deferred_loader.acquire_render_config_id(texture_asset_id)),
-						render,
+					mesh_deferred_loader.acquire_mesh_id(MeshIndex::file_mesh(mesh_asset_id)),
-						model:model_deferred_attributes
+				)
-					}),
+			},
-					MeshAvailability::DeferredUnion(part_texture_description)=>deferred_unions_deferred_attributes.push(DeferredUnionDeferredAttributes{
+			Shape::PhysicsData=>{
-						render:part_texture_description,
+				let mut content="";
-						model:model_deferred_attributes,
+				let mut mesh_data:&[u8]=&[];
-					}),
+				let mut physics_data:&[u8]=&[];
+				if let Some(rbx_dom_weak::types::Variant::ContentId(asset_id))=object.properties.get(&static_ustr("AssetId")){
+					content=asset_id.as_ref();
 				}
+				if let Some(rbx_dom_weak::types::Variant::BinaryString(data))=object.properties.get(&static_ustr("MeshData")){
+					mesh_data=data.as_ref();
+				}
+				if let Some(rbx_dom_weak::types::Variant::BinaryString(data))=object.properties.get(&static_ustr("PhysicsData")){
+					physics_data=data.as_ref();
+				}
+				let part_texture_description=get_texture_description(render_config_deferred_loader,&mut recoverable_errors,db,dom,object,size);
+				let mesh_index=MeshIndex::union(content,mesh_data,physics_data,size,part_texture_description.clone());
+				let mesh_id=mesh_deferred_loader.acquire_mesh_id(mesh_index);
+				(MeshAvailability::DeferredUnion(part_texture_description),mesh_id)
+			},
+		};
+		let velocity=match vec3::try_from_f32_array([velocity.x,velocity.y,velocity.z]){
+			Ok(velocity)=>velocity,
+			Err(e)=>{
+				recoverable_errors.basepart_velocity.push(Planar64ConvertError{
+					path:InstancePath::new(dom,object),
+					error:e,
+				});
+				continue;
 			}
+		};
+		let model_deferred_attributes=ModelDeferredAttributes{
+			mesh:mesh_id,
+			transform:model_transform,
+			color:glam::vec4(color3.r as f32/255f32,color3.g as f32/255f32,color3.b as f32/255f32,1.0-*transparency),
+			deferred_attributes:GetAttributesArgs{
+				name:object.name.as_str(),
+				can_collide,
+				velocity,
+			},
+		};
+		match availability{
+			MeshAvailability::Immediate=>primitive_models_deferred_attributes.push(model_deferred_attributes),
+			MeshAvailability::DeferredMesh(render)=>deferred_models_deferred_attributes.push(DeferredModelDeferredAttributes{
+				render,
+				model:model_deferred_attributes
+			}),
+			MeshAvailability::DeferredUnion(part_texture_description)=>deferred_unions_deferred_attributes.push(DeferredUnionDeferredAttributes{
+				render:part_texture_description,
+				model:model_deferred_attributes,
+			}),
 		}
 	}
 	PartialMap1{
@@ -713,6 +758,7 @@ pub fn convert<'a>(
 		primitive_models_deferred_attributes,
 		deferred_models_deferred_attributes,
 		deferred_unions_deferred_attributes,
+		recoverable_errors,
 	}
 }
 struct MeshIdWithSize{
@@ -772,6 +818,7 @@ pub struct PartialMap1<'a>{
 	primitive_models_deferred_attributes:Vec<ModelDeferredAttributes<'a>>,
 	deferred_models_deferred_attributes:Vec<DeferredModelDeferredAttributes<'a>>,
 	deferred_unions_deferred_attributes:Vec<DeferredUnionDeferredAttributes<'a>>,
+	recoverable_errors:RecoverableErrors,
 }
 impl PartialMap1<'_>{
 	pub fn add_meshpart_meshes_and_calculate_attributes(
@@ -795,6 +842,7 @@ impl PartialMap1<'_>{
 		// I just want to chain iterators together man
 		let aint_no_way=core::cell::UnsafeCell::new(&mut self.primitive_meshes);
 
+		let mut model_counter=0;
 		let mut mesh_id_from_render_config_id=HashMap::new();
 		let mut union_id_from_render_config_id=HashMap::new();
 		//now that the meshes are loaded, these models can be generated
@@ -847,61 +895,78 @@ impl PartialMap1<'_>{
 			})
 		}))
 		.chain(self.primitive_models_deferred_attributes.into_iter())
-		.enumerate().map(|(model_id,model_deferred_attributes)|{
+		.filter_map(|model_deferred_attributes|{
-			let model_id=model::ModelId::new(model_id as u32);
+			let model_id=model::ModelId::new(model_counter);
-			ModelOwnedAttributes{
+			let attributes=match get_attributes(
+				&model_deferred_attributes.deferred_attributes.name,
+				model_deferred_attributes.deferred_attributes.can_collide,
+				model_deferred_attributes.deferred_attributes.velocity,
+				model_id,
+				&mut modes_builder,
+				&mut wormhole_in_model_to_id,
+				&mut wormhole_id_to_out_model,
+			){
+				Ok(attributes)=>attributes,
+				Err(e)=>{
+					match e{
+						GetAttributesError::ModeIdParseInt(e)=>self.recoverable_errors.mode_id_parse_int.push(e),
+						GetAttributesError::DuplicateMode(mode_id)=>{self.recoverable_errors.duplicate_mode.insert(mode_id);},
+						GetAttributesError::StageIdParseInt(e)=>self.recoverable_errors.stage_id_parse_int.push(e),
+						GetAttributesError::DuplicateStage(duplicate_stage)=>{self.recoverable_errors.duplicate_stage.insert(duplicate_stage);},
+						GetAttributesError::WormholeOutIdParseInt(e)=>self.recoverable_errors.wormhole_out_id_parse_int.push(e),
+						GetAttributesError::DuplicateWormholeOut(wormhole_id)=>{self.recoverable_errors.duplicate_wormhole_out.insert(wormhole_id);},
+						GetAttributesError::WormholeInIdParseInt(e)=>self.recoverable_errors.wormhole_in_id_parse_int.push(e),
+						GetAttributesError::JumpLimitParseInt(e)=>self.recoverable_errors.jump_limit_parse_int.push(e),
+					}
+					return None;
+				}
+			};
+			model_counter+=1;
+			Some(ModelOwnedAttributes{
 				mesh:model_deferred_attributes.mesh,
-				attributes:get_attributes(
+				attributes,
-					&model_deferred_attributes.deferred_attributes.name,
-					model_deferred_attributes.deferred_attributes.can_collide,
-					model_deferred_attributes.deferred_attributes.velocity,
-					model_id,
-					&mut modes_builder,
-					&mut wormhole_in_model_to_id,
-					&mut wormhole_id_to_out_model,
-				),
 				color:model_deferred_attributes.color,
 				transform:model_deferred_attributes.transform,
-			}
+			})
 		}).collect();
 		let models=models_owned_attributes.into_iter().enumerate().map(|(model_id,mut model_owned_attributes)|{
-		//TODO: TAB
+			let model_id=model::ModelId::new(model_id as u32);
-		let model_id=model::ModelId::new(model_id as u32);
+			//update attributes with wormhole id
-		//update attributes with wormhole id
+			//TODO: errors/prints
-		//TODO: errors/prints
+			if let Some(wormhole_id)=wormhole_in_model_to_id.get(&model_id){
-		if let Some(wormhole_id)=wormhole_in_model_to_id.get(&model_id){
+				if let Some(&wormhole_out_model_id)=wormhole_id_to_out_model.get(wormhole_id){
-			if let Some(&wormhole_out_model_id)=wormhole_id_to_out_model.get(wormhole_id){
+					match &mut model_owned_attributes.attributes{
-				match &mut model_owned_attributes.attributes{
+						attr::CollisionAttributes::Contact(attr::ContactAttributes{contacting:_,general})
-					attr::CollisionAttributes::Contact(attr::ContactAttributes{contacting:_,general})
+						|attr::CollisionAttributes::Intersect(attr::IntersectAttributes{intersecting:_,general})
-					|attr::CollisionAttributes::Intersect(attr::IntersectAttributes{intersecting:_,general})
+						=>general.wormhole=Some(attr::Wormhole{destination_model:wormhole_out_model_id}),
-					=>general.wormhole=Some(attr::Wormhole{destination_model:wormhole_out_model_id}),
+						attr::CollisionAttributes::Decoration=>println!("Not a wormhole"),
-					attr::CollisionAttributes::Decoration=>println!("Not a wormhole"),
+					}
 				}
 			}
-		}
 
-		//index the attributes
+			//index the attributes
-		let attributes_id=if let Some(&attributes_id)=attributes_id_from_attributes.get(&model_owned_attributes.attributes){
+			let attributes_id=if let Some(&attributes_id)=attributes_id_from_attributes.get(&model_owned_attributes.attributes){
-			attributes_id
+				attributes_id
-		}else{
+			}else{
-			let attributes_id=attr::CollisionAttributesId::new(unique_attributes.len() as u32);
+				let attributes_id=attr::CollisionAttributesId::new(unique_attributes.len() as u32);
-			attributes_id_from_attributes.insert(model_owned_attributes.attributes.clone(),attributes_id);
+				attributes_id_from_attributes.insert(model_owned_attributes.attributes.clone(),attributes_id);
-			unique_attributes.push(model_owned_attributes.attributes);
+				unique_attributes.push(model_owned_attributes.attributes);
-			attributes_id
+				attributes_id
-		};
+			};
-		model::Model{
+			model::Model{
-			mesh:model_owned_attributes.mesh,
+				mesh:model_owned_attributes.mesh,
-			transform:model_owned_attributes.transform,
+				transform:model_owned_attributes.transform,
-			color:model_owned_attributes.color,
+				color:model_owned_attributes.color,
-			attributes:attributes_id,
+				attributes:attributes_id,
+			}
+		}).collect();
+		PartialMap2{
+			meshes:self.primitive_meshes,
+			models,
+			modes:modes_builder.build_normalized(),
+			attributes:unique_attributes,
+			recoverable_errors:self.recoverable_errors,
 		}
-	}).collect();
-	PartialMap2{
-		meshes:self.primitive_meshes,
-		models,
-		modes:modes_builder.build_normalized(),
-		attributes:unique_attributes,
-	}
 	}
 }
 
@@ -910,12 +975,13 @@ pub struct PartialMap2{
 	models:Vec<model::Model>,
 	modes:NormalizedModes,
 	attributes:Vec<strafesnet_common::gameplay_attributes::CollisionAttributes>,
+	recoverable_errors:RecoverableErrors,
 }
 impl PartialMap2{
 	pub fn add_render_configs_and_textures(
 		self,
 		render_configs:RenderConfigs,
-	)->map::CompleteMap{
+	)->(map::CompleteMap,RecoverableErrors){
 		let (textures,render_configs)=render_configs.consume();
 		let (textures,texture_id_map):(Vec<Vec<u8>>,HashMap<model::TextureId,model::TextureId>)
 		=textures.into_iter().enumerate().map(|(new_texture_id,(old_texture_id,Texture::ImageDDS(texture)))|{
@@ -934,14 +1000,17 @@ impl PartialMap2{
 			);
 			render_config
 		}).collect();
-		map::CompleteMap{
+		(
-			modes:self.modes,
+			map::CompleteMap{
-			attributes:self.attributes,
+				modes:self.modes,
-			meshes:self.meshes,
+				attributes:self.attributes,
-			models:self.models,
+				meshes:self.meshes,
-			//the roblox legacy texture thing always works
+				models:self.models,
-			textures,
+				//the roblox legacy texture thing always works
-			render_configs,
+				textures,
-		}
+				render_configs,
+			},
+			self.recoverable_errors,
+		)
 	}
 }