From ae48542626bb4e4292ca2b452e929ff8c514af32 Mon Sep 17 00:00:00 2001
From: Quaternions <krakow20@gmail.com>
Date: Mon, 3 Feb 2025 10:02:15 -0800
Subject: [PATCH] refactor Loader trait to use generics

---
 lib/bsp_loader/src/loader.rs               |  20 +--
 lib/bsp_loader/src/mesh.rs                 | 143 +++++++++------------
 lib/deferred_loader/src/deferred_loader.rs |   4 +-
 lib/deferred_loader/src/loader.rs          |   5 +-
 lib/rbx_loader/src/loader.rs               |  10 +-
 5 files changed, 83 insertions(+), 99 deletions(-)

diff --git a/lib/bsp_loader/src/loader.rs b/lib/bsp_loader/src/loader.rs
index 6f69fe4..a9b5c23 100644
--- a/lib/bsp_loader/src/loader.rs
+++ b/lib/bsp_loader/src/loader.rs
@@ -3,7 +3,7 @@ use std::{borrow::Cow, io::Read};
 use strafesnet_common::model::Mesh;
 use strafesnet_deferred_loader::{loader::Loader,texture::Texture};
 
-use crate::{mesh::ModelData, Bsp};
+use crate::Bsp;
 
 #[allow(dead_code)]
 #[derive(Debug)]
@@ -28,11 +28,10 @@ impl TextureLoader<'_>{
 		Self(std::marker::PhantomData)
 	}
 }
-impl<'a> Loader for TextureLoader<'a>{
+impl<'a> Loader<Texture> for TextureLoader<'a>{
 	type Error=TextureError;
 	type Index=Cow<'a,str>;
-	type Resource=Texture;
-	fn load(&mut self,index:Self::Index)->Result<Self::Resource,Self::Error>{
+	fn load(&mut self,index:Self::Index)->Result<Texture,Self::Error>{
 		let file_name=format!("textures/{}.dds",index);
 		let mut file=std::fs::File::open(file_name)?;
 		let mut data=Vec::new();
@@ -88,11 +87,10 @@ impl MeshLoader<'_,'_>{
 		}
 	}
 }
-impl<'a> Loader for MeshLoader<'a,'_>{
+impl<'a> Loader<Mesh> for MeshLoader<'a,'_>{
 	type Error=MeshError;
 	type Index=&'a str;
-	type Resource=Mesh;
-	fn load(&mut self,index:Self::Index)->Result<Self::Resource,Self::Error>{
+	fn load(&mut self,index:Self::Index)->Result<Mesh,Self::Error>{
 		let mdl_path_lower=index.to_lowercase();
 		//.mdl, .vvd, .dx90.vtx
 		let path=std::path::PathBuf::from(mdl_path_lower.as_str());
@@ -105,8 +103,12 @@ impl<'a> Loader for MeshLoader<'a,'_>{
 		let mdl=bsp.pack.get(mdl_path_lower.as_str())?.ok_or(MeshError::MissingMdl)?;
 		let vtx=bsp.pack.get(vtx_path.as_os_str().to_str().unwrap())?.ok_or(MeshError::MissingVtx)?;
 		let vvd=bsp.pack.get(vvd_path.as_os_str().to_str().unwrap())?.ok_or(MeshError::MissingVvd)?;
-		let model=ModelData{mdl,vtx,vvd};
-		let mesh=model.convert_mesh(&mut self.deferred_loader)?;
+		let model=vmdl::Model::from_parts(
+			vmdl::mdl::Mdl::read(mdl.as_ref())?,
+			vmdl::vtx::Vtx::read(vtx.as_ref())?,
+			vmdl::vvd::Vvd::read(vvd.as_ref())?,
+		);
+		let mesh=crate::mesh::convert_mesh(model,&mut self.deferred_loader);
 		Ok(mesh)
 	}
 }
diff --git a/lib/bsp_loader/src/mesh.rs b/lib/bsp_loader/src/mesh.rs
index d6a3339..2c09008 100644
--- a/lib/bsp_loader/src/mesh.rs
+++ b/lib/bsp_loader/src/mesh.rs
@@ -5,85 +5,70 @@ use strafesnet_deferred_loader::deferred_loader::RenderConfigDeferredLoader;
 
 use crate::valve_transform;
 
-pub struct ModelData{
-	pub mdl:Vec<u8>,
-	pub vtx:Vec<u8>,
-	pub vvd:Vec<u8>,
-}
-impl ModelData{
-	fn read_model(&self)->Result<vmdl::Model,vmdl::ModelError>{
-		Ok(vmdl::Model::from_parts(
-			vmdl::mdl::Mdl::read(self.mdl.as_ref())?,
-			vmdl::vtx::Vtx::read(self.vtx.as_ref())?,
-			vmdl::vvd::Vvd::read(self.vvd.as_ref())?,
+pub fn convert_mesh<'a>(model:vmdl::Model,deferred_loader:&mut RenderConfigDeferredLoader<Cow<'a,str>>)->model::Mesh{
+	let texture_paths=model.texture_directories();
+	if texture_paths.len()!=1{
+		println!("WARNING: multiple texture paths");
+	}
+	let skin=model.skin_tables().nth(0).unwrap();
+
+	let mut spam_pos=Vec::with_capacity(model.vertices().len());
+	let mut spam_normal=Vec::with_capacity(model.vertices().len());
+	let mut spam_tex=Vec::with_capacity(model.vertices().len());
+	let mut spam_vertices=Vec::with_capacity(model.vertices().len());
+	for (i,vertex) in model.vertices().iter().enumerate(){
+		spam_pos.push(valve_transform(vertex.position.into()));
+		spam_normal.push(valve_transform(vertex.normal.into()));
+		spam_tex.push(glam::Vec2::from_array(vertex.texture_coordinates));
+		spam_vertices.push(model::IndexedVertex{
+			pos:model::PositionId::new(i as u32),
+			tex:model::TextureCoordinateId::new(i as u32),
+			normal:model::NormalId::new(i as u32),
+			color:model::ColorId::new(0),
+		});
+	}
+	let mut graphics_groups=Vec::new();
+	let mut physics_groups=Vec::new();
+	let polygon_groups=model.meshes().enumerate().map(|(polygon_group_id,mesh)|{
+		let polygon_group_id=model::PolygonGroupId::new(polygon_group_id as u32);
+
+		let render_id=if let (Some(texture_path),Some(texture_name))=(texture_paths.get(0),skin.texture(mesh.material_index())){
+			let mut path=std::path::PathBuf::from(texture_path.as_str());
+			path.push(texture_name);
+			let index=path.as_os_str().to_str().map(|s|Cow::Owned(s.to_owned()));
+			deferred_loader.acquire_render_config_id(index)
+		}else{
+			deferred_loader.acquire_render_config_id(None)
+		};
+
+		graphics_groups.push(model::IndexedGraphicsGroup{
+			render:render_id,
+			groups:vec![polygon_group_id],
+		});
+		physics_groups.push(model::IndexedPhysicsGroup{
+			groups:vec![polygon_group_id],
+		});
+		model::PolygonGroup::PolygonList(model::PolygonList::new(
+			//looking at the code, it would seem that the strips are pre-deindexed into triangle lists when calling this function
+			mesh.vertex_strip_indices().flat_map(|mut strip|
+				std::iter::from_fn(move||{
+					match (strip.next(),strip.next(),strip.next()){
+						(Some(v1),Some(v2),Some(v3))=>Some([v1,v2,v3].map(|vertex_id|model::VertexId::new(vertex_id as u32)).to_vec()),
+						//ignore extra vertices, not sure what to do in this case, failing the whole conversion could be appropriate
+						_=>None,
+					}
+				})
+			).collect()
 		))
-	}
-	pub fn convert_mesh<'a>(self,deferred_loader:&mut RenderConfigDeferredLoader<Cow<'a,str>>)->Result<model::Mesh,vmdl::ModelError>{
-		let model=self.read_model()?;
-		let texture_paths=model.texture_directories();
-		if texture_paths.len()!=1{
-			println!("WARNING: multiple texture paths");
-		}
-		let skin=model.skin_tables().nth(0).unwrap();
-
-		let mut spam_pos=Vec::with_capacity(model.vertices().len());
-		let mut spam_normal=Vec::with_capacity(model.vertices().len());
-		let mut spam_tex=Vec::with_capacity(model.vertices().len());
-		let mut spam_vertices=Vec::with_capacity(model.vertices().len());
-		for (i,vertex) in model.vertices().iter().enumerate(){
-			spam_pos.push(valve_transform(vertex.position.into()));
-			spam_normal.push(valve_transform(vertex.normal.into()));
-			spam_tex.push(glam::Vec2::from_array(vertex.texture_coordinates));
-			spam_vertices.push(model::IndexedVertex{
-				pos:model::PositionId::new(i as u32),
-				tex:model::TextureCoordinateId::new(i as u32),
-				normal:model::NormalId::new(i as u32),
-				color:model::ColorId::new(0),
-			});
-		}
-		let mut graphics_groups=Vec::new();
-		let mut physics_groups=Vec::new();
-		let polygon_groups=model.meshes().enumerate().map(|(polygon_group_id,mesh)|{
-			let polygon_group_id=model::PolygonGroupId::new(polygon_group_id as u32);
-
-			let render_id=if let (Some(texture_path),Some(texture_name))=(texture_paths.get(0),skin.texture(mesh.material_index())){
-				let mut path=std::path::PathBuf::from(texture_path.as_str());
-				path.push(texture_name);
-				let index=path.as_os_str().to_str().map(|s|Cow::Owned(s.to_owned()));
-				deferred_loader.acquire_render_config_id(index)
-			}else{
-				deferred_loader.acquire_render_config_id(None)
-			};
-
-			graphics_groups.push(model::IndexedGraphicsGroup{
-				render:render_id,
-				groups:vec![polygon_group_id],
-			});
-			physics_groups.push(model::IndexedPhysicsGroup{
-				groups:vec![polygon_group_id],
-			});
-			model::PolygonGroup::PolygonList(model::PolygonList::new(
-				//looking at the code, it would seem that the strips are pre-deindexed into triangle lists when calling this function
-				mesh.vertex_strip_indices().flat_map(|mut strip|
-					std::iter::from_fn(move||{
-						match (strip.next(),strip.next(),strip.next()){
-							(Some(v1),Some(v2),Some(v3))=>Some([v1,v2,v3].map(|vertex_id|model::VertexId::new(vertex_id as u32)).to_vec()),
-							//ignore extra vertices, not sure what to do in this case, failing the whole conversion could be appropriate
-							_=>None,
-						}
-					})
-				).collect()
-			))
-		}).collect();
-		Ok(model::Mesh{
-			unique_pos:spam_pos,
-			unique_normal:spam_normal,
-			unique_tex:spam_tex,
-			unique_color:vec![glam::Vec4::ONE],
-			unique_vertices:spam_vertices,
-			polygon_groups,
-			graphics_groups,
-			physics_groups,
-		})
+	}).collect();
+	model::Mesh{
+		unique_pos:spam_pos,
+		unique_normal:spam_normal,
+		unique_tex:spam_tex,
+		unique_color:vec![glam::Vec4::ONE],
+		unique_vertices:spam_vertices,
+		polygon_groups,
+		graphics_groups,
+		physics_groups,
 	}
 }
diff --git a/lib/deferred_loader/src/deferred_loader.rs b/lib/deferred_loader/src/deferred_loader.rs
index 83a5c76..3ec9e60 100644
--- a/lib/deferred_loader/src/deferred_loader.rs
+++ b/lib/deferred_loader/src/deferred_loader.rs
@@ -45,7 +45,7 @@ impl<H:core::hash::Hash+Eq> RenderConfigDeferredLoader<H>{
 	pub fn indices(&self)->impl Iterator<Item=&H>{
 		self.render_config_id_from_asset_id.keys().flatten()
 	}
-	pub fn into_render_configs<L:Loader<Index=H,Resource=Texture>>(mut self,loader:&mut L,failure_mode:LoadFailureMode)->Result<RenderConfigs,L::Error>{
+	pub fn into_render_configs<L:Loader<Texture,Index=H>>(mut self,loader:&mut L,failure_mode:LoadFailureMode)->Result<RenderConfigs,L::Error>{
 		let mut sorted_textures=vec![None;self.texture_count as usize];
 		for (index_option,render_config_id) in self.render_config_id_from_asset_id{
 			let render_config=&mut self.render_configs[render_config_id.get() as usize];
@@ -93,7 +93,7 @@ impl<H:core::hash::Hash+Eq> MeshDeferredLoader<H>{
 	pub fn indices(&self)->impl Iterator<Item=&H>{
 		self.mesh_id_from_asset_id.keys()
 	}
-	pub fn into_meshes<L:Loader<Index=H,Resource=Mesh>>(self,loader:&mut L,failure_mode:LoadFailureMode)->Result<Meshes,L::Error>{
+	pub fn into_meshes<L:Loader<Mesh,Index=H>>(self,loader:&mut L,failure_mode:LoadFailureMode)->Result<Meshes,L::Error>{
 		let mut mesh_list=vec![None;self.mesh_id_from_asset_id.len()];
 		for (index,mesh_id) in self.mesh_id_from_asset_id{
 			let resource_result=loader.load(index);
diff --git a/lib/deferred_loader/src/loader.rs b/lib/deferred_loader/src/loader.rs
index adc9710..b69ef46 100644
--- a/lib/deferred_loader/src/loader.rs
+++ b/lib/deferred_loader/src/loader.rs
@@ -1,8 +1,7 @@
 use std::error::Error;
 
-pub trait Loader{
+pub trait Loader<Resource>{
 	type Error:Error;
 	type Index;
-	type Resource;
-	fn load(&mut self,index:Self::Index)->Result<Self::Resource,Self::Error>;
+	fn load(&mut self,index:Self::Index)->Result<Resource,Self::Error>;
 }
diff --git a/lib/rbx_loader/src/loader.rs b/lib/rbx_loader/src/loader.rs
index 9cfa8ab..2cfe4a5 100644
--- a/lib/rbx_loader/src/loader.rs
+++ b/lib/rbx_loader/src/loader.rs
@@ -42,11 +42,10 @@ impl TextureLoader<'_>{
 		Self(std::marker::PhantomData)
 	}
 }
-impl<'a> Loader for TextureLoader<'a>{
+impl<'a> Loader<Texture> for TextureLoader<'a>{
 	type Error=TextureError;
 	type Index=&'a str;
-	type Resource=Texture;
-	fn load(&mut self,index:Self::Index)->Result<Self::Resource,Self::Error>{
+	fn load(&mut self,index:Self::Index)->Result<Texture,Self::Error>{
 		let RobloxAssetId(asset_id)=index.parse()?;
 		let file_name=format!("textures/{}.dds",asset_id);
 		let data=read_entire_file(file_name)?;
@@ -144,11 +143,10 @@ impl MeshLoader<'_>{
 		Self(std::marker::PhantomData)
 	}
 }
-impl<'a> Loader for MeshLoader<'a>{
+impl<'a> Loader<Mesh> for MeshLoader<'a>{
 	type Error=MeshError;
 	type Index=MeshIndex<'a>;
-	type Resource=Mesh;
-	fn load(&mut self,index:Self::Index)->Result<Self::Resource,Self::Error>{
+	fn load(&mut self,index:Self::Index)->Result<Mesh,Self::Error>{
 		let mesh=match index.mesh_type{
 			MeshType::FileMesh=>{
 				let RobloxAssetId(asset_id)=index.content.parse()?;