validation: check for unanchored parts (#230)

Unanchored parts are always invalid.  Make a check to detect them.

Reviewed-on: #230
Co-authored-by: Quaternions <krakow20@gmail.com>
Co-committed-by: Quaternions <krakow20@gmail.com>

Cargo.lock

@@ -1898,7 +1898,7 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3"
 
 [[package]]
 name = "submissions-api"
-version = "0.8.1"
+version = "0.8.2"
 dependencies = [
  "chrono",
  "reqwest",

pkg/datastore/gormstore/audit_events.go

@@ -56,7 +56,7 @@ func (env *AuditEvents) Delete(ctx context.Context, id int64) error {
 
 func (env *AuditEvents) List(ctx context.Context, filters datastore.OptionalMap, page model.Page) ([]model.AuditEvent, error) {
 	var events []model.AuditEvent
-	if err := env.db.Where(filters.Map()).Offset(int((page.Number - 1) * page.Size)).Limit(int(page.Size)).Find(&events).Error; err != nil {
+	if err := env.db.Where(filters.Map()).Order("id ASC").Offset(int((page.Number - 1) * page.Size)).Limit(int(page.Size)).Find(&events).Error; err != nil {
 		return nil, err
 	}
 

validation/api/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "submissions-api"
-version = "0.8.1"
+version = "0.8.2"
 edition = "2021"
 publish = ["strafesnet"]
 repository = "https://git.itzana.me/StrafesNET/maps-service"

validation/api/src/types.rs

@@ -114,9 +114,9 @@ pub struct SubmissionIDResponse{
 	pub SubmissionID:SubmissionID,
 }
 
-#[derive(Clone,Copy,Debug,PartialEq,Eq,serde::Serialize,serde::Deserialize)]
+#[derive(Clone,Copy,Debug,Hash,Eq,PartialEq,serde::Serialize,serde::Deserialize)]
 pub struct ScriptID(pub(crate)i64);
-#[derive(Clone,Copy,Debug,serde::Serialize,serde::Deserialize)]
+#[derive(Clone,Copy,Debug,Hash,Eq,PartialEq,serde::Serialize,serde::Deserialize)]
 pub struct ScriptPolicyID(pub(crate)i64);
 
 #[derive(Clone,Copy,Debug,PartialEq,Eq,serde_repr::Serialize_repr,serde_repr::Deserialize_repr)]
@@ -385,7 +385,7 @@ pub struct CreateSubmissionAuditCheckListRequest<'a>{
 	pub CheckList:&'a [Check],
 }
 
-#[derive(Clone,Copy,Debug,serde::Serialize,serde::Deserialize)]
+#[derive(Clone,Copy,Debug,Hash,Eq,PartialEq,serde::Serialize,serde::Deserialize)]
 pub struct SubmissionID(pub(crate)i64);
 
 #[allow(nonstandard_style)]
@@ -438,7 +438,7 @@ pub struct CreateMapfixAuditCheckListRequest<'a>{
 	pub CheckList:&'a [Check],
 }
 
-#[derive(Clone,Copy,Debug,serde::Serialize,serde::Deserialize)]
+#[derive(Clone,Copy,Debug,Hash,Eq,PartialEq,serde::Serialize,serde::Deserialize)]
 pub struct MapfixID(pub(crate)i64);
 
 #[allow(nonstandard_style)]
@@ -448,10 +448,10 @@ pub struct ActionOperationFailedRequest{
 	pub StatusMessage:String,
 }
 
-#[derive(Clone,Copy,Debug,serde::Serialize,serde::Deserialize)]
+#[derive(Clone,Copy,Debug,Hash,Eq,PartialEq,serde::Serialize,serde::Deserialize)]
 pub struct OperationID(pub(crate)i64);
 
-#[derive(Clone,Copy,Debug,serde::Serialize,serde::Deserialize)]
+#[derive(Clone,Copy,Debug,Hash,Eq,PartialEq,serde::Serialize,serde::Deserialize)]
 pub struct ResourceID(pub(crate)i64);
 
 #[derive(Clone,Copy,Debug)]

validation/src/check.rs

@@ -3,6 +3,7 @@ use crate::download::download_asset_version;
 use crate::rbx_util::{get_mapinfo,get_root_instance,read_dom,ReadDomError,GameID,ParseGameIDError,MapInfo,GetRootInstanceError,StringValueError};
 
 use heck::{ToSnakeCase,ToTitleCase};
+use rbx_dom_weak::Instance;
 use submissions_api::types::Check;
 
 #[allow(dead_code)]
@@ -208,10 +209,10 @@ impl std::fmt::Display for WormholeElement{
 /// Count various map elements
 #[derive(Default)]
 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<StageID,Vec<&'a str>>,
+	mode_start_counts:HashMap<ModeID,Vec<&'a Instance>>,
+	mode_finish_counts:HashMap<ModeID,Vec<&'a Instance>>,
+	mode_anticheat_counts:HashMap<ModeID,Vec<&'a Instance>>,
+	teleport_counts:HashMap<StageID,Vec<&'a Instance>>,
 	spawn_counts:HashMap<StageID,u64>,
 	wormhole_in_counts:HashMap<WormholeID,u64>,
 	wormhole_out_counts:HashMap<WormholeID,u64>,
@@ -222,6 +223,7 @@ pub struct ModelInfo<'a>{
 	model_name:&'a str,
 	map_info:MapInfo<'a>,
 	counts:Counts<'a>,
+	unanchored_parts:Vec<&'a Instance>,
 }
 
 pub fn get_model_info<'a>(dom:&'a rbx_dom_weak::WeakDom,model_instance:&'a rbx_dom_weak::Instance)->ModelInfo<'a>{
@@ -231,6 +233,10 @@ pub fn get_model_info<'a>(dom:&'a rbx_dom_weak::WeakDom,model_instance:&'a rbx_d
 	// count objects (default count is 0)
 	let mut counts=Counts::default();
 
+	// locate unanchored parts
+	let mut unanchored_parts=Vec::new();
+	let anchored_ustr=rbx_dom_weak::ustr("Anchored");
+
 	let db=rbx_reflection_database::get();
 	let base_part=&db.classes["BasePart"];
 	let base_parts=dom.descendants_of(model_instance.referent()).filter(|&instance|
@@ -241,14 +247,14 @@ pub fn get_model_info<'a>(dom:&'a rbx_dom_weak::WeakDom,model_instance:&'a rbx_d
 	for instance in base_parts{
 		// Zones
 		match instance.name.parse(){
-			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()),
+			Ok(ModeElement{zone:Zone::Start,mode_id})=>counts.mode_start_counts.entry(mode_id).or_default().push(instance),
+			Ok(ModeElement{zone:Zone::Finish,mode_id})=>counts.mode_finish_counts.entry(mode_id).or_default().push(instance),
+			Ok(ModeElement{zone:Zone::Anticheat,mode_id})=>counts.mode_anticheat_counts.entry(mode_id).or_default().push(instance),
 			Err(_)=>(),
 		}
 		// Spawns & Teleports
 		match instance.name.parse(){
-			Ok(StageElement{behaviour:StageElementBehaviour::Teleport,stage_id})=>counts.teleport_counts.entry(stage_id).or_default().push(instance.name.as_str()),
+			Ok(StageElement{behaviour:StageElementBehaviour::Teleport,stage_id})=>counts.teleport_counts.entry(stage_id).or_default().push(instance),
 			Ok(StageElement{behaviour:StageElementBehaviour::Spawn,stage_id})=>*counts.spawn_counts.entry(stage_id).or_insert(0)+=1,
 			Err(_)=>(),
 		}
@@ -258,6 +264,10 @@ pub fn get_model_info<'a>(dom:&'a rbx_dom_weak::WeakDom,model_instance:&'a rbx_d
 			Ok(WormholeElement{behaviour:WormholeBehaviour::Out,wormhole_id})=>*counts.wormhole_out_counts.entry(wormhole_id).or_insert(0)+=1,
 			Err(_)=>(),
 		}
+		// Unanchored parts
+		if let Some(rbx_dom_weak::types::Variant::Bool(false))=instance.properties.get(&anchored_ustr){
+			unanchored_parts.push(instance);
+		}
 	}
 
 	ModelInfo{
@@ -265,6 +275,7 @@ pub fn get_model_info<'a>(dom:&'a rbx_dom_weak::WeakDom,model_instance:&'a rbx_d
 		model_name:model_instance.name.as_str(),
 		map_info,
 		counts,
+		unanchored_parts,
 	}
 }
 
@@ -430,15 +441,15 @@ struct MapCheck<'a>{
 	// MapStart must exist
 	mapstart:Result<Exists,Absent>,
 	// No duplicate map starts (including bonuses)
-	mode_start_counts:DuplicateCheck<ModeID,Vec<&'a str>>,
+	mode_start_counts:DuplicateCheck<ModeID,Vec<&'a Instance>>,
 	// At least one finish zone for each start zone, and no finishes with no start
-	mode_finish_counts:SetDifferenceCheck<SetDifferenceCheckContextAtLeastOne<ModeID,Vec<&'a str>>>,
+	mode_finish_counts:SetDifferenceCheck<SetDifferenceCheckContextAtLeastOne<ModeID,Vec<&'a Instance>>>,
 	// Check for dangling MapAnticheat zones (no associated MapStart)
-	mode_anticheat_counts:SetDifferenceCheck<SetDifferenceCheckContextAllowNone<ModeID,Vec<&'a str>>>,
+	mode_anticheat_counts:SetDifferenceCheck<SetDifferenceCheckContextAllowNone<ModeID,Vec<&'a Instance>>>,
 	// Spawn1 must exist
 	spawn1:Result<Exists,Absent>,
 	// Check for dangling Teleport# (no associated Spawn#)
-	teleport_counts:SetDifferenceCheck<SetDifferenceCheckContextAllowNone<StageID,Vec<&'a str>>>,
+	teleport_counts:SetDifferenceCheck<SetDifferenceCheckContextAllowNone<StageID,Vec<&'a Instance>>>,
 	// No duplicate Spawn#
 	spawn_counts:DuplicateCheck<StageID,u64>,
 	// Check for dangling WormholeIn# (no associated WormholeOut#)
@@ -446,6 +457,9 @@ struct MapCheck<'a>{
 	// No duplicate WormholeOut# (duplicate WormholeIn# ok)
 	// No dangling WormholeOut#
 	wormhole_out_counts:DuplicateCheck<WormholeID,u64>,
+
+	// === GENERAL CHECKS ===
+	unanchored_parts:Result<(),Vec<&'a Instance>>,
 }
 
 impl<'a> ModelInfo<'a>{
@@ -519,6 +533,13 @@ impl<'a> ModelInfo<'a>{
 		// There must be exactly one of any perticular wormhole out id in the map.
 		let wormhole_out_counts=DuplicateCheckContext(self.counts.wormhole_out_counts).check(|&c|1<c);
 
+		// There must not be any unanchored parts
+		let unanchored_parts=if self.unanchored_parts.is_empty(){
+			Ok(())
+		}else{
+			Err(self.unanchored_parts)
+		};
+
 		MapCheck{
 			model_class,
 			model_name,
@@ -534,6 +555,7 @@ impl<'a> ModelInfo<'a>{
 			spawn_counts,
 			wormhole_in_counts,
 			wormhole_out_counts,
+			unanchored_parts,
 		}
 	}
 }
@@ -556,6 +578,7 @@ impl MapCheck<'_>{
 				spawn_counts:DuplicateCheck(Ok(())),
 				wormhole_in_counts:SetDifferenceCheck(Ok(())),
 				wormhole_out_counts:DuplicateCheck(Ok(())),
+				unanchored_parts:Ok(()),
 			}=>{
 				Ok(MapInfoOwned{
 					display_name:display_name.to_owned(),
@@ -680,8 +703,8 @@ impl MapCheck<'_>{
 		let duplicate_start=match &self.mode_start_counts{
 			DuplicateCheck(Ok(()))=>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())
+				let context=Separated::new(", ",||context.iter().map(|(&mode_id,instances)|
+					Duplicates::new(ModeElement{zone:Zone::Start,mode_id},instances.len())
 				));
 				summary_format!("DuplicateStart","Duplicate start zones: {context}")
 			}
@@ -693,7 +716,7 @@ impl MapCheck<'_>{
 					passed!("DanglingFinish")
 				}else{
 					let plural=if context.extra.len()==1{"zone"}else{"zones"};
-					let context=Separated::new(", ",||context.extra.iter().map(|(&mode_id,_names)|
+					let context=Separated::new(", ",||context.extra.iter().map(|(&mode_id,_instances)|
 						ModeElement{zone:Zone::Finish,mode_id}
 					));
 					summary_format!("DanglingFinish","No matching start zone for finish {plural}: {context}")
@@ -716,7 +739,7 @@ impl MapCheck<'_>{
 					passed!("DanglingAnticheat")
 				}else{
 					let plural=if context.extra.len()==1{"zone"}else{"zones"};
-					let context=Separated::new(", ",||context.extra.iter().map(|(&mode_id,_names)|
+					let context=Separated::new(", ",||context.extra.iter().map(|(&mode_id,_instances)|
 						ModeElement{zone:Zone::Anticheat,mode_id}
 					));
 					summary_format!("DanglingAnticheat","No matching start zone for anticheat {plural}: {context}")
@@ -730,7 +753,9 @@ impl MapCheck<'_>{
 		let dangling_teleport=match &self.teleport_counts{
 			SetDifferenceCheck(Ok(()))=>passed!("DanglingTeleport"),
 			SetDifferenceCheck(Err(context))=>{
-				let unique_names:HashSet<_>=context.extra.values().flat_map(|names|names.iter().copied()).collect();
+				let unique_names:HashSet<_>=context.extra.values().flat_map(|instances|
+					instances.iter().map(|instance|instance.name.as_str())
+				).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}")
@@ -739,8 +764,8 @@ impl MapCheck<'_>{
 		let duplicate_spawns=match &self.spawn_counts{
 			DuplicateCheck(Ok(()))=>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)
+				let context=Separated::new(", ",||context.iter().map(|(&stage_id,&instances)|
+					Duplicates::new(StageElement{behaviour:StageElementBehaviour::Spawn,stage_id},instances as usize)
 				));
 				summary_format!("DuplicateSpawn","Duplicate Spawn: {context}")
 			}
@@ -751,7 +776,7 @@ impl MapCheck<'_>{
 				if context.extra.is_empty(){
 					passed!("ExtraWormholeIn")
 				}else{
-					let context=Separated::new(", ",||context.extra.iter().map(|(&wormhole_id,_names)|
+					let context=Separated::new(", ",||context.extra.iter().map(|(&wormhole_id,_instances)|
 						WormholeElement{behaviour:WormholeBehaviour::In,wormhole_id}
 					));
 					summary_format!("ExtraWormholeIn","WormholeIn with no matching WormholeOut: {context}")
@@ -771,12 +796,23 @@ impl MapCheck<'_>{
 		let duplicate_wormhole_out=match &self.wormhole_out_counts{
 			DuplicateCheck(Ok(()))=>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)
+				let context=Separated::new(", ",||context.iter().map(|(&wormhole_id,&instances)|
+					Duplicates::new(WormholeElement{behaviour:WormholeBehaviour::Out,wormhole_id},instances as usize)
 				));
 				summary_format!("DuplicateWormholeOut","Duplicate WormholeOut: {context}")
 			}
 		};
+		let unanchored_parts=match &self.unanchored_parts{
+			Ok(())=>passed!("UnanchoredParts"),
+			Err(unanchored_parts)=>{
+				let count=unanchored_parts.len();
+				let plural=if count==1{"part"}else{"parts"};
+				let context=Separated::new(", ",||unanchored_parts.iter().map(|&instance|
+					instance.name.as_str()
+				).take(20));
+				summary_format!("UnanchoredParts","{count} unanchored {plural}: {context}")
+			}
+		};
 		Ok(MapCheckList{checks:Box::new([
 			model_class,
 			model_name,
@@ -794,13 +830,14 @@ impl MapCheck<'_>{
 			extra_wormhole_in,
 			missing_wormhole_in,
 			duplicate_wormhole_out,
+			unanchored_parts,
 		])})
 	}
 }
 
 #[derive(serde::Serialize)]
 pub struct MapCheckList{
-	pub checks:Box<[Check;16]>,
+	pub checks:Box<[Check;17]>,
 }
 
 pub struct CheckListAndVersion{