Merge pull request 'Refactor MapChecks Summary' (#160) from summary into staging

Reviewed-on: #160

validation/src/check.rs

@@ -12,6 +12,7 @@ pub enum Error{
 	Download(crate::download::Error),
 	ModelFileDecode(ReadDomError),
 	GetRootInstance(GetRootInstanceError),
+	ToJsonValue(serde_json::Error),
 }
 impl std::fmt::Display for Error{
 	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
@@ -47,93 +48,157 @@ impl ModeID{
 	const BONUS:Self=Self(1);
 }
 enum Zone{
-	Start(ModeID),
-	Finish(ModeID),
-	Anticheat(ModeID),
+	Start,
+	Finish,
+	Anticheat,
+}
+struct ModeElement{
+	zone:Zone,
+	mode_id:ModeID,
 }
 #[allow(dead_code)]
 pub enum IDParseError{
 	NoCaptures,
-	ParseInt(core::num::ParseIntError)
+	ParseInt(core::num::ParseIntError),
 }
 // Parse a Zone from a part name
-impl std::str::FromStr for Zone{
+impl std::str::FromStr for ModeElement{
 	type Err=IDParseError;
 	fn from_str(s:&str)->Result<Self,Self::Err>{
 		match s{
-			"MapStart"=>Ok(Self::Start(ModeID::MAIN)),
-			"MapFinish"=>Ok(Self::Finish(ModeID::MAIN)),
-			"MapAnticheat"=>Ok(Self::Anticheat(ModeID::MAIN)),
-			"BonusStart"=>Ok(Self::Start(ModeID::BONUS)),
-			"BonusFinish"=>Ok(Self::Finish(ModeID::BONUS)),
-			"BonusAnticheat"=>Ok(Self::Anticheat(ModeID::BONUS)),
+			"MapStart"=>Ok(Self{zone:Zone::Start,mode_id:ModeID::MAIN}),
+			"MapFinish"=>Ok(Self{zone:Zone::Finish,mode_id:ModeID::MAIN}),
+			"MapAnticheat"=>Ok(Self{zone:Zone::Anticheat,mode_id:ModeID::MAIN}),
+			"BonusStart"=>Ok(Self{zone:Zone::Start,mode_id:ModeID::BONUS}),
+			"BonusFinish"=>Ok(Self{zone:Zone::Finish,mode_id:ModeID::BONUS}),
+			"BonusAnticheat"=>Ok(Self{zone:Zone::Anticheat,mode_id:ModeID::BONUS}),
 			other=>{
 				let bonus_start_pattern=lazy_regex::lazy_regex!(r"^Bonus(\d+)Start$|^BonusStart(\d+)$");
 				if let Some(captures)=bonus_start_pattern.captures(other){
-					return Ok(Self::Start(ModeID(captures[1].parse().map_err(IDParseError::ParseInt)?)));
+					return Ok(Self{
+						zone:Zone::Start,
+						mode_id:ModeID(captures[1].parse().map_err(IDParseError::ParseInt)?),
+					});
 				}
 				let bonus_finish_pattern=lazy_regex::lazy_regex!(r"^Bonus(\d+)Finish$|^BonusFinish(\d+)$");
 				if let Some(captures)=bonus_finish_pattern.captures(other){
-					return Ok(Self::Finish(ModeID(captures[1].parse().map_err(IDParseError::ParseInt)?)));
+					return Ok(Self{
+						zone:Zone::Finish,
+						mode_id:ModeID(captures[1].parse().map_err(IDParseError::ParseInt)?),
+					});
 				}
 				let bonus_finish_pattern=lazy_regex::lazy_regex!(r"^Bonus(\d+)Anticheat$|^BonusAnticheat(\d+)$");
 				if let Some(captures)=bonus_finish_pattern.captures(other){
-					return Ok(Self::Anticheat(ModeID(captures[1].parse().map_err(IDParseError::ParseInt)?)));
+					return Ok(Self{
+						zone:Zone::Anticheat,
+						mode_id:ModeID(captures[1].parse().map_err(IDParseError::ParseInt)?),
+					});
 				}
 				Err(IDParseError::NoCaptures)
 			}
 		}
 	}
 }
+impl std::fmt::Display for ModeElement{
+	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
+		match self{
+			ModeElement{zone:Zone::Start,mode_id:ModeID::MAIN}=>write!(f,"MapStart"),
+			ModeElement{zone:Zone::Start,mode_id:ModeID::BONUS}=>write!(f,"BonusStart"),
+			ModeElement{zone:Zone::Start,mode_id:ModeID(mode_id)}=>write!(f,"Bonus{mode_id}Start"),
+			ModeElement{zone:Zone::Finish,mode_id:ModeID::MAIN}=>write!(f,"MapFinish"),
+			ModeElement{zone:Zone::Finish,mode_id:ModeID::BONUS}=>write!(f,"BonusFinish"),
+			ModeElement{zone:Zone::Finish,mode_id:ModeID(mode_id)}=>write!(f,"Bonus{mode_id}Finish"),
+			ModeElement{zone:Zone::Anticheat,mode_id:ModeID::MAIN}=>write!(f,"MapAnticheat"),
+			ModeElement{zone:Zone::Anticheat,mode_id:ModeID::BONUS}=>write!(f,"BonusAnticheat"),
+			ModeElement{zone:Zone::Anticheat,mode_id:ModeID(mode_id)}=>write!(f,"Bonus{mode_id}Anticheat"),
+		}
+	}
+}
 
 #[derive(Clone,Copy,Debug,Hash,Eq,PartialEq)]
-struct SpawnID(u64);
-impl SpawnID{
+struct StageID(u64);
+impl StageID{
 	const FIRST:Self=Self(1);
 }
-enum SpawnTeleport{
-	Teleport(SpawnID),
-	Spawn(SpawnID),
+enum StageElementBehaviour{
+	Teleport,
+	Spawn,
+}
+struct StageElement{
+	stage_id:StageID,
+	behaviour:StageElementBehaviour,
 }
 // Parse a SpawnTeleport from a part name
-impl std::str::FromStr for SpawnTeleport{
+impl std::str::FromStr for StageElement{
 	type Err=IDParseError;
 	fn from_str(s:&str)->Result<Self,Self::Err>{
 		// Trigger ForceTrigger Teleport ForceTeleport SpawnAt ForceSpawnAt
 		let bonus_start_pattern=lazy_regex::lazy_regex!(r"^(?:Force)?(Teleport|SpawnAt|Trigger)(\d+)$");
 		if let Some(captures)=bonus_start_pattern.captures(s){
-			return Ok(Self::Teleport(SpawnID(captures[1].parse().map_err(IDParseError::ParseInt)?)));
+			return Ok(StageElement{
+				behaviour:StageElementBehaviour::Teleport,
+				stage_id:StageID(captures[1].parse().map_err(IDParseError::ParseInt)?),
+			});
 		}
 		// Spawn
 		let bonus_finish_pattern=lazy_regex::lazy_regex!(r"^Spawn(\d+)$");
 		if let Some(captures)=bonus_finish_pattern.captures(s){
-			return Ok(Self::Spawn(SpawnID(captures[1].parse().map_err(IDParseError::ParseInt)?)));
+			return Ok(StageElement{
+				behaviour:StageElementBehaviour::Spawn,
+				stage_id:StageID(captures[1].parse().map_err(IDParseError::ParseInt)?),
+			});
 		}
 		Err(IDParseError::NoCaptures)
 	}
 }
+impl std::fmt::Display for StageElement{
+	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
+		match self{
+			StageElement{behaviour:StageElementBehaviour::Spawn,stage_id:StageID(stage_id)}=>write!(f,"Spawn{stage_id}"),
+			StageElement{behaviour:StageElementBehaviour::Teleport,stage_id:StageID(stage_id)}=>write!(f,"Teleport{stage_id}"),
+		}
+	}
+}
 
 #[derive(Clone,Copy,Debug,Hash,Eq,PartialEq)]
 struct WormholeID(u64);
-enum Wormhole{
-	In(WormholeID),
-	Out(WormholeID),
+enum WormholeBehaviour{
+	In,
+	Out,
+}
+struct WormholeElement{
+	behaviour:WormholeBehaviour,
+	wormhole_id:WormholeID,
 }
 // Parse a Wormhole from a part name
-impl std::str::FromStr for Wormhole{
+impl std::str::FromStr for WormholeElement{
 	type Err=IDParseError;
 	fn from_str(s:&str)->Result<Self,Self::Err>{
 		let bonus_start_pattern=lazy_regex::lazy_regex!(r"^WormholeIn(\d+)$");
 		if let Some(captures)=bonus_start_pattern.captures(s){
-			return Ok(Self::In(WormholeID(captures[1].parse().map_err(IDParseError::ParseInt)?)));
+			return Ok(Self{
+				behaviour:WormholeBehaviour::In,
+				wormhole_id:WormholeID(captures[1].parse().map_err(IDParseError::ParseInt)?),
+			});
 		}
 		let bonus_finish_pattern=lazy_regex::lazy_regex!(r"^WormholeOut(\d+)$");
 		if let Some(captures)=bonus_finish_pattern.captures(s){
-			return Ok(Self::Out(WormholeID(captures[1].parse().map_err(IDParseError::ParseInt)?)));
+			return Ok(Self{
+				behaviour:WormholeBehaviour::Out,
+				wormhole_id:WormholeID(captures[1].parse().map_err(IDParseError::ParseInt)?),
+			});
 		}
 		Err(IDParseError::NoCaptures)
 	}
 }
+impl std::fmt::Display for WormholeElement{
+	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
+		match self{
+			WormholeElement{behaviour:WormholeBehaviour::In,wormhole_id:WormholeID(wormhole_id)}=>write!(f,"WormholeIn{wormhole_id}"),
+			WormholeElement{behaviour:WormholeBehaviour::Out,wormhole_id:WormholeID(wormhole_id)}=>write!(f,"WormholeOut{wormhole_id}"),
+		}
+	}
+}
 
 /// Count various map elements
 #[derive(Default)]
@@ -141,8 +206,8 @@ struct Counts<'a>{
 	mode_start_counts:HashMap<ModeID,Vec<&'a str>>,
 	mode_finish_counts:HashMap<ModeID,Vec<&'a str>>,
 	mode_anticheat_counts:HashMap<ModeID,Vec<&'a str>>,
-	teleport_counts:HashMap<SpawnID,Vec<&'a str>>,
-	spawn_counts:HashMap<SpawnID,u64>,
+	teleport_counts:HashMap<StageID,Vec<&'a str>>,
+	spawn_counts:HashMap<StageID,u64>,
 	wormhole_in_counts:HashMap<WormholeID,u64>,
 	wormhole_out_counts:HashMap<WormholeID,u64>,
 }
@@ -164,21 +229,21 @@ pub fn get_model_info<'a>(dom:&'a rbx_dom_weak::WeakDom,model_instance:&'a rbx_d
 		if class_is_a(instance.class.as_str(),"BasePart"){
 			// Zones
 			match instance.name.parse(){
-				Ok(Zone::Start(mode_id))=>counts.mode_start_counts.entry(mode_id).or_default().push(instance.name.as_str()),
-				Ok(Zone::Finish(mode_id))=>counts.mode_finish_counts.entry(mode_id).or_default().push(instance.name.as_str()),
-				Ok(Zone::Anticheat(mode_id))=>counts.mode_anticheat_counts.entry(mode_id).or_default().push(instance.name.as_str()),
+				Ok(ModeElement{zone:Zone::Start,mode_id})=>counts.mode_start_counts.entry(mode_id).or_default().push(instance.name.as_str()),
+				Ok(ModeElement{zone:Zone::Finish,mode_id})=>counts.mode_finish_counts.entry(mode_id).or_default().push(instance.name.as_str()),
+				Ok(ModeElement{zone:Zone::Anticheat,mode_id})=>counts.mode_anticheat_counts.entry(mode_id).or_default().push(instance.name.as_str()),
 				Err(_)=>(),
 			}
 			// Spawns & Teleports
 			match instance.name.parse(){
-				Ok(SpawnTeleport::Teleport(spawn_id))=>counts.teleport_counts.entry(spawn_id).or_default().push(instance.name.as_str()),
-				Ok(SpawnTeleport::Spawn(spawn_id))=>*counts.spawn_counts.entry(spawn_id).or_insert(0)+=1,
+				Ok(StageElement{behaviour:StageElementBehaviour::Teleport,stage_id})=>counts.teleport_counts.entry(stage_id).or_default().push(instance.name.as_str()),
+				Ok(StageElement{behaviour:StageElementBehaviour::Spawn,stage_id})=>*counts.spawn_counts.entry(stage_id).or_insert(0)+=1,
 				Err(_)=>(),
 			}
 			// Wormholes
 			match instance.name.parse(){
-				Ok(Wormhole::In(wormhole_id))=>*counts.wormhole_in_counts.entry(wormhole_id).or_insert(0)+=1,
-				Ok(Wormhole::Out(wormhole_id))=>*counts.wormhole_out_counts.entry(wormhole_id).or_insert(0)+=1,
+				Ok(WormholeElement{behaviour:WormholeBehaviour::In,wormhole_id})=>*counts.wormhole_in_counts.entry(wormhole_id).or_insert(0)+=1,
+				Ok(WormholeElement{behaviour:WormholeBehaviour::Out,wormhole_id})=>*counts.wormhole_out_counts.entry(wormhole_id).or_insert(0)+=1,
 				Err(_)=>(),
 			}
 		}
@@ -345,9 +410,9 @@ struct MapCheck<'a>{
 	// Spawn1 must exist
 	spawn1:Result<Exists,Absent>,
 	// Check for dangling Teleport# (no associated Spawn#)
-	teleport_counts:SetDifferenceCheck<SetDifferenceCheckContextAllowNone<SpawnID,Vec<&'a str>>>,
+	teleport_counts:SetDifferenceCheck<SetDifferenceCheckContextAllowNone<StageID,Vec<&'a str>>>,
 	// No duplicate Spawn#
-	spawn_counts:DuplicateCheck<SpawnID,u64>,
+	spawn_counts:DuplicateCheck<StageID,u64>,
 	// Check for dangling WormholeIn# (no associated WormholeOut#)
 	wormhole_in_counts:SetDifferenceCheck<SetDifferenceCheckContextAtLeastOne<WormholeID,u64>>,
 	// No duplicate WormholeOut# (duplicate WormholeIn# ok)
@@ -391,7 +456,7 @@ impl<'a> ModelInfo<'a>{
 		};
 
 		// Spawn1 must exist
-		let spawn1=if self.counts.spawn_counts.contains_key(&SpawnID::FIRST){
+		let spawn1=if self.counts.spawn_counts.contains_key(&StageID::FIRST){
 			Ok(Exists)
 		}else{
 			Err(Absent)
@@ -446,7 +511,7 @@ impl<'a> ModelInfo<'a>{
 }
 
 impl MapCheck<'_>{
-	fn result(self)->Result<MapInfoOwned,String>{
+	fn result(self)->Result<MapInfoOwned,Result<MapCheckList,serde_json::Error>>{
 		match self{
 			MapCheck{
 				model_class:StringCheck(Ok(())),
@@ -470,156 +535,269 @@ impl MapCheck<'_>{
 					game_id,
 				})
 			},
-			other=>Err(other.to_string()),
+			other=>Err(other.itemize()),
 		}
 	}
 }
 
-fn write_comma_separated<T>(
-	f:&mut std::fmt::Formatter<'_>,
-	mut it:impl Iterator<Item=T>,
-	custom_write:impl Fn(&mut std::fmt::Formatter<'_>,T)->std::fmt::Result
-)->std::fmt::Result{
-	if let Some(t)=it.next(){
-		custom_write(f,t)?;
-		for t in it{
-			write!(f,", ")?;
-			custom_write(f,t)?;
-		}
+struct Separated<F>{
+	f:F,
+	separator:&'static str,
+}
+impl<F> Separated<F>{
+	fn new(separator:&'static str,f:F)->Self{
+		Self{separator,f}
+	}
+}
+impl<F,I,D> std::fmt::Display for Separated<F>
+	where
+		D:std::fmt::Display,
+		I:IntoIterator<Item=D>,
+		F:Fn()->I,
+{
+	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
+		let mut it=(self.f)().into_iter();
+		if let Some(first)=it.next(){
+			write!(f,"{first}")?;
+			for item in it{
+				write!(f,"{}{item}",self.separator)?;
+			}
+		}
+		Ok(())
 	}
-	Ok(())
 }
 
-/// Write a zone string such as BonusStart
-macro_rules! write_zone{
-	($f:expr,$mode:expr,$zone:expr)=>{
-		match $mode{
-			ModeID(0)=>write!($f,concat!("Map",$zone)),
-			ModeID(1)=>write!($f,concat!("Bonus",$zone)),
-			ModeID(other)=>write!($f,concat!("Bonus{}",$zone),other),
+struct Duplicates<D>{
+	display:D,
+	duplicates:usize,
+}
+impl<D> Duplicates<D>{
+	fn new(display:D,duplicates:usize)->Self{
+		Self{
+			display,
+			duplicates,
+		}
+	}
+}
+impl<D:std::fmt::Display> std::fmt::Display for Duplicates<D>{
+	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
+		write!(f,"{} ({} duplicates)",self.display,self.duplicates)
+	}
+}
+
+#[derive(serde::Serialize)]
+struct CheckSummary{
+	name:&'static str,
+	summary:String,
+	passed:bool,
+	details:serde_json::Value,
+}
+impl CheckSummary{
+	const fn passed(name:&'static str)->Self{
+		Self{
+			name,
+			summary:String::new(),
+			passed:true,
+			details:serde_json::Value::Null,
+		}
+	}
+}
+macro_rules! summary{
+	($name:literal,$summary:expr,$details:expr)=>{
+		CheckSummary{
+			name:$name,
+			summary:$summary,
+			passed:false,
+			details:serde_json::to_value($details)?,
+		}
+	};
+}
+macro_rules! summary_format{
+	($name:literal,$fmt:literal,$details:expr)=>{
+		CheckSummary{
+			name:$name,
+			summary:format!($fmt),
+			passed:false,
+			details:serde_json::to_value($details)?,
 		}
 	};
 }
 
 // Generate an error message for each observed issue separated by newlines.
 // This defines MapCheck.to_string() which is used in MapCheck.result()
-impl std::fmt::Display for MapCheck<'_>{
-	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
-		if let StringCheck(Err(context))=&self.model_class{
-			writeln!(f,"Invalid model class: {context}")?;
-		}
-		if let StringCheck(Err(context))=&self.model_name{
-			writeln!(f,"Model name must have snake_case: {context}")?;
-		}
-		match &self.display_name{
-			Ok(Ok(StringCheck(Ok(_))))=>(),
-			Ok(Ok(StringCheck(Err(context))))=>writeln!(f,"DisplayName must have Title Case: {context}")?,
-			Ok(Err(context))=>writeln!(f,"Invalid DisplayName: {context}")?,
-			Err(StringValueError::ObjectNotFound)=>writeln!(f,"Missing DisplayName StringValue")?,
-			Err(StringValueError::ValueNotSet)=>writeln!(f,"DisplayName Value not set")?,
-			Err(StringValueError::NonStringValue)=>writeln!(f,"DisplayName Value is not a String")?,
-		}
-		match &self.creator{
-			Ok(Ok(_))=>(),
-			Ok(Err(context))=>writeln!(f,"Invalid Creator: {context}")?,
-			Err(StringValueError::ObjectNotFound)=>writeln!(f,"Missing Creator StringValue")?,
-			Err(StringValueError::ValueNotSet)=>writeln!(f,"Creator Value not set")?,
-			Err(StringValueError::NonStringValue)=>writeln!(f,"Creator Value is not a String")?,
-		}
-		if let Err(_parse_game_id_error)=&self.game_id{
-			writeln!(f,"Model name must be prefixed with bhop_ surf_ or flytrials_")?;
-		}
-		if let Err(Absent)=&self.mapstart{
-			writeln!(f,"Model has no MapStart")?;
-		}
-		if let DuplicateCheck(Err(DuplicateCheckContext(context)))=&self.mode_start_counts{
-			write!(f,"Duplicate start zones: ")?;
-			write_comma_separated(f,context.iter(),|f,(mode_id,names)|{
-				write_zone!(f,mode_id,"Start")?;
-				write!(f," ({} duplicates)",names.len())?;
-				Ok(())
-			})?;
-			writeln!(f)?;
-		}
-		if let SetDifferenceCheck(Err(context))=&self.mode_finish_counts{
-			if !context.extra.is_empty(){
-				let plural=if context.extra.len()==1{"zone"}else{"zones"};
-				write!(f,"No matching start zone for finish {plural}: ")?;
-				write_comma_separated(f,context.extra.iter(),|f,(mode_id,_names)|
-					write_zone!(f,mode_id,"Finish")
-				)?;
-				writeln!(f)?;
+impl MapCheck<'_>{
+	fn itemize(&self)->Result<MapCheckList,serde_json::Error>{
+		let model_class=match &self.model_class{
+			StringCheck(Ok(()))=>CheckSummary::passed("ModelClass"),
+			StringCheck(Err(context))=>summary_format!("ModelClass","Invalid model class: {context}",()),
+		};
+		let model_name=match &self.model_name{
+			StringCheck(Ok(()))=>CheckSummary::passed("ModelName"),
+			StringCheck(Err(context))=>summary_format!("ModelName","Model name must have snake_case: {context}",()),
+		};
+		let display_name=match &self.display_name{
+			Ok(Ok(StringCheck(Ok(_))))=>CheckSummary::passed("DisplayName"),
+			Ok(Ok(StringCheck(Err(context))))=>summary_format!("DisplayName","DisplayName must have Title Case: {context}",()),
+			Ok(Err(context))=>summary_format!("DisplayName","Invalid DisplayName: {context}",()),
+			Err(StringValueError::ObjectNotFound)=>summary!("DisplayName","Missing DisplayName StringValue".to_owned(),()),
+			Err(StringValueError::ValueNotSet)=>summary!("DisplayName","DisplayName Value not set".to_owned(),()),
+			Err(StringValueError::NonStringValue)=>summary!("DisplayName","DisplayName Value is not a String".to_owned(),()),
+		};
+		let creator=match &self.creator{
+			Ok(Ok(_))=>CheckSummary::passed("Creator"),
+			Ok(Err(context))=>summary_format!("Creator","Invalid Creator: {context}",()),
+			Err(StringValueError::ObjectNotFound)=>summary!("Creator","Missing Creator StringValue".to_owned(),()),
+			Err(StringValueError::ValueNotSet)=>summary!("Creator","Creator Value not set".to_owned(),()),
+			Err(StringValueError::NonStringValue)=>summary!("Creator","Creator Value is not a String".to_owned(),()),
+		};
+		let game_id=match &self.game_id{
+			Ok(_)=>CheckSummary::passed("GameID"),
+			Err(ParseGameIDError)=>summary!("GameID","Model name must be prefixed with bhop_ surf_ or flytrials_".to_owned(),()),
+		};
+		let mapstart=match &self.mapstart{
+			Ok(Exists)=>CheckSummary::passed("MapStart"),
+			Err(Absent)=>summary_format!("MapStart","Model has no MapStart",()),
+		};
+		let duplicate_start=match &self.mode_start_counts{
+			DuplicateCheck(Ok(()))=>CheckSummary::passed("DuplicateStart"),
+			DuplicateCheck(Err(DuplicateCheckContext(context)))=>{
+				let context=Separated::new(", ",||context.iter().map(|(&mode_id,names)|
+					Duplicates::new(ModeElement{zone:Zone::Start,mode_id},names.len())
+				));
+				summary_format!("DuplicateStart","Duplicate start zones: {context}",())
 			}
-			if !context.missing.is_empty(){
-				let plural=if context.missing.len()==1{"zone"}else{"zones"};
-				write!(f,"Missing finish {plural}: ")?;
-				write_comma_separated(f,context.missing.iter(),|f,mode_id|
-					write_zone!(f,mode_id,"Finish")
-				)?;
-				writeln!(f)?;
+		};
+		let (extra_finish,missing_finish)=match &self.mode_finish_counts{
+			SetDifferenceCheck(Ok(()))=>(CheckSummary::passed("ExtraFinish"),CheckSummary::passed("MissingFinish")),
+			SetDifferenceCheck(Err(context))=>(
+				if context.extra.is_empty(){
+					CheckSummary::passed("ExtraFinish")
+				}else{
+					let plural=if context.extra.len()==1{"zone"}else{"zones"};
+					let context=Separated::new(", ",||context.extra.iter().map(|(&mode_id,_names)|
+						ModeElement{zone:Zone::Finish,mode_id}
+					));
+					summary_format!("ExtraFinish","No matching start zone for finish {plural}: {context}",())
+				},
+				if context.missing.is_empty(){
+					CheckSummary::passed("MissingFinish")
+				}else{
+					let plural=if context.missing.len()==1{"zone"}else{"zones"};
+					let context=Separated::new(", ",||context.missing.iter().map(|&mode_id|
+						ModeElement{zone:Zone::Finish,mode_id}
+					));
+					summary_format!("MissingFinish","Missing finish {plural}: {context}",())
+				}
+			),
+		};
+		let dangling_anticheat=match &self.mode_anticheat_counts{
+			SetDifferenceCheck(Ok(()))=>CheckSummary::passed("DanglingAnticheat"),
+			SetDifferenceCheck(Err(context))=>{
+				if context.extra.is_empty(){
+					CheckSummary::passed("DanglingAnticheat")
+				}else{
+					let plural=if context.extra.len()==1{"zone"}else{"zones"};
+					let context=Separated::new(", ",||context.extra.iter().map(|(&mode_id,_names)|
+						ModeElement{zone:Zone::Anticheat,mode_id}
+					));
+					summary_format!("DanglingAnticheat","No matching start zone for anticheat {plural}: {context}",())
+				}
 			}
-		}
-		if let SetDifferenceCheck(Err(context))=&self.mode_anticheat_counts{
-			if !context.extra.is_empty(){
-				let plural=if context.extra.len()==1{"zone"}else{"zones"};
-				write!(f,"No matching start zone for anticheat {plural}: ")?;
-				write_comma_separated(f,context.extra.iter(),|f,(mode_id,_names)|
-					write_zone!(f,mode_id,"Anticheat")
-				)?;
-				writeln!(f)?;
+		};
+		let spawn1=match &self.spawn1{
+			Ok(Exists)=>CheckSummary::passed("Spawn1"),
+			Err(Absent)=>summary_format!("Spawn1","Model has no Spawn1",()),
+		};
+		let dangling_teleport=match &self.teleport_counts{
+			SetDifferenceCheck(Ok(()))=>CheckSummary::passed("DanglingTeleport"),
+			SetDifferenceCheck(Err(context))=>{
+				let unique_names:HashSet<_>=context.extra.values().flat_map(|names|names.iter().copied()).collect();
+				let plural=if unique_names.len()==1{"object"}else{"objects"};
+				let context=Separated::new(", ",||&unique_names);
+				summary_format!("DanglingTeleport","No matching Spawn for {plural}: {context}",())
 			}
-		}
-		if let Err(Absent)=&self.spawn1{
-			writeln!(f,"Model has no Spawn1")?;
-		}
-		if let SetDifferenceCheck(Err(context))=&self.teleport_counts{
-			for names in context.extra.values(){
-				let plural=if names.len()==1{"object"}else{"objects"};
-				write!(f,"No matching Spawn for {plural}: ")?;
-				write_comma_separated(f,names.iter(),|f,&name|{
-					write!(f,"{name}")
-				})?;
-				writeln!(f)?;
+		};
+		let duplicate_spawns=match &self.spawn_counts{
+			DuplicateCheck(Ok(()))=>CheckSummary::passed("DuplicateSpawn"),
+			DuplicateCheck(Err(DuplicateCheckContext(context)))=>{
+				let context=Separated::new(", ",||context.iter().map(|(&stage_id,&names)|
+					Duplicates::new(StageElement{behaviour:StageElementBehaviour::Spawn,stage_id},names as usize)
+				));
+				summary_format!("DuplicateSpawn","Duplicate Spawn: {context}",())
 			}
-		}
-		if let DuplicateCheck(Err(DuplicateCheckContext(context)))=&self.spawn_counts{
-			write!(f,"Duplicate Spawn: ")?;
-			write_comma_separated(f,context.iter(),|f,(SpawnID(spawn_id),count)|
-				write!(f,"Spawn{spawn_id} ({count} duplicates)")
-			)?;
-			writeln!(f)?;
-		}
-		if let SetDifferenceCheck(Err(context))=&self.wormhole_in_counts{
-			if !context.extra.is_empty(){
-				write!(f,"WormholeIn with no matching WormholeOut: ")?;
-				write_comma_separated(f,context.extra.iter(),|f,(WormholeID(wormhole_id),_count)|
-					write!(f,"WormholeIn{wormhole_id}")
-				)?;
-				writeln!(f)?;
+		};
+		let (extra_wormhole_in,missing_wormhole_in)=match &self.wormhole_in_counts{
+			SetDifferenceCheck(Ok(()))=>(CheckSummary::passed("ExtraWormholeIn"),CheckSummary::passed("MissingWormholeIn")),
+			SetDifferenceCheck(Err(context))=>(
+				if context.extra.is_empty(){
+					CheckSummary::passed("ExtraWormholeIn")
+				}else{
+					let context=Separated::new(", ",||context.extra.iter().map(|(&wormhole_id,_names)|
+						WormholeElement{behaviour:WormholeBehaviour::In,wormhole_id}
+					));
+					summary_format!("ExtraWormholeIn","WormholeIn with no matching WormholeOut: {context}",())
+				},
+				if context.missing.is_empty(){
+					CheckSummary::passed("MissingWormholeIn")
+				}else{
+					// This counts WormholeIn objects, but
+					// flipped logic is easier to understand
+					let context=Separated::new(", ",||context.missing.iter().map(|&wormhole_id|
+						WormholeElement{behaviour:WormholeBehaviour::Out,wormhole_id}
+					));
+					summary_format!("MissingWormholeIn","WormholeOut with no matching WormholeIn: {context}",())
+				}
+			)
+		};
+		let duplicate_wormhole_out=match &self.wormhole_out_counts{
+			DuplicateCheck(Ok(()))=>CheckSummary::passed("DuplicateWormholeOut"),
+			DuplicateCheck(Err(DuplicateCheckContext(context)))=>{
+				let context=Separated::new(", ",||context.iter().map(|(&wormhole_id,&names)|
+					Duplicates::new(WormholeElement{behaviour:WormholeBehaviour::Out,wormhole_id},names as usize)
+				));
+				summary_format!("DuplicateWormholeOut","Duplicate WormholeOut: {context}",())
 			}
-			if !context.missing.is_empty(){
-				// This counts WormholeIn objects, but
-				// flipped logic is easier to understand
-				write!(f,"WormholeOut with no matching WormholeIn: ")?;
-				write_comma_separated(f,context.missing.iter(),|f,WormholeID(wormhole_id)|
-					write!(f,"WormholeOut{wormhole_id}")
-				)?;
-				writeln!(f)?;
-			}
-		}
-		if let DuplicateCheck(Err(DuplicateCheckContext(context)))=&self.wormhole_out_counts{
-			write!(f,"Duplicate WormholeOut: ")?;
-			write_comma_separated(f,context.iter(),|f,(WormholeID(wormhole_id),count)|
-				write!(f,"WormholeOut{wormhole_id} ({count} duplicates)")
-			)?;
-			writeln!(f)?;
-		}
-		Ok(())
+		};
+		Ok(MapCheckList{checks:Box::new([
+			model_class,
+			model_name,
+			display_name,
+			creator,
+			game_id,
+			mapstart,
+			duplicate_start,
+			extra_finish,
+			missing_finish,
+			dangling_anticheat,
+			spawn1,
+			dangling_teleport,
+			duplicate_spawns,
+			extra_wormhole_in,
+			missing_wormhole_in,
+			duplicate_wormhole_out,
+		])})
 	}
 }
 
+#[derive(serde::Serialize)]
+struct MapCheckList{
+	checks:Box<[CheckSummary;16]>,
+}
+impl MapCheckList{
+	fn summary(&self)->String{
+		Separated::new("; ",||self.checks.iter().filter_map(|check|
+			(!check.passed).then_some(check.summary.as_str())
+		)).to_string()
+	}
+}
+
+pub struct Summary{
+	pub summary:String,
+	pub json:serde_json::Value,
+}
+
 pub struct CheckReportAndVersion{
-	pub status:Result<MapInfoOwned,String>,
+	pub status:Result<MapInfoOwned,Summary>,
 	pub version:u64,
 }
 
@@ -656,7 +834,14 @@ impl crate::message_handler::MessageHandler{
 		let map_check=model_info.check();
 
 		// check the report, generate an error message if it fails the check
-		let status=map_check.result();
+		let status=match map_check.result(){
+			Ok(map_info)=>Ok(map_info),
+			Err(Ok(summary))=>Err(Summary{
+				summary:summary.summary(),
+				json:serde_json::to_value(&summary).map_err(Error::ToJsonValue)?,
+			}),
+			Err(Err(e))=>return Err(Error::ToJsonValue(e)),
+		};
 
 		Ok(CheckReportAndVersion{status,version})
 	}

validation/src/check_mapfix.rs

@@ -34,7 +34,7 @@ impl crate::message_handler::MessageHandler{
 			Ok(CheckReportAndVersion{status:Err(report),..})=>self.api.action_mapfix_request_changes(
 				submissions_api::types::ActionMapfixRequestChangesRequest{
 					MapfixID:mapfix_id,
-					ErrorMessage:report,
+					ErrorMessage:report.summary,
 				}
 			).await.map_err(Error::ApiActionMapfixCheck)?,
 			// update the mapfix model status to request changes

validation/src/check_submission.rs

@@ -35,7 +35,7 @@ impl crate::message_handler::MessageHandler{
 			Ok(CheckReportAndVersion{status:Err(report),..})=>self.api.action_submission_request_changes(
 				submissions_api::types::ActionSubmissionRequestChangesRequest{
 					SubmissionID:submission_id,
-					ErrorMessage:report,
+					ErrorMessage:report.summary,
 				}
 			).await.map_err(Error::ApiActionSubmissionCheck)?,
 			// update the submission model status to request changes