diff --git a/validation/src/check.rs b/validation/src/check.rs index 5acf80f..ff195d2 100644 --- a/validation/src/check.rs +++ b/validation/src/check.rs @@ -1,5 +1,6 @@ +use std::collections::{HashSet,HashMap}; use crate::download::download_asset_version; -use crate::rbx_util::{class_is_a,get_mapinfo,get_root_instance,read_dom,ReadDomError,GameID}; +use crate::rbx_util::{class_is_a,get_mapinfo,get_root_instance,read_dom,ReadDomError,GameID,ParseGameIDError,MapInfo,GetRootInstanceError,StringValueError}; use heck::{ToSnakeCase,ToTitleCase}; @@ -10,6 +11,7 @@ pub enum Error{ CreatorTypeMustBeUser, Download(crate::download::Error), ModelFileDecode(ReadDomError), + GetRootInstance(GetRootInstanceError), } impl std::fmt::Display for Error{ fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{ @@ -38,124 +40,12 @@ impl From<crate::nats_types::CheckSubmissionRequest> for CheckRequest{ } } -#[derive(Default)] -pub enum Check{ - Pass, - #[default] - Fail, -} -impl Check{ - fn pass(&self)->bool{ - match self{ - Check::Pass=>true, - Check::Fail=>false, - } - } -} -impl std::fmt::Display for Check{ - fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{ - match self{ - Check::Pass=>write!(f,"passed"), - Check::Fail=>write!(f,"failed"), - } - } -} - -pub enum CheckStatus{ - Passed{ - display_name:String, - creator:String, - game_id:GameID, - }, - Failed{ - report:CheckReport, - } -} - -#[derive(Default)] -pub struct CheckReport{ - // === METADATA CHECKS === - // the model must have exactly 1 root part (models uploaded to roblox can have multiple roots) - exactly_one_root:Check, - // the root must be of class Model - root_is_model:Check, - // the prefix of the model's name must match the game it was submitted for. bhop_ for bhop, and surf_ for surf - model_name_prefix_is_valid:Check, - // your model's name must match this regex: ^[a-z0-9_] - model_name_is_snake_case:Check, - // map must have a StringValue named Creator and DisplayName. additionally, they must both have a value - has_display_name:Check, - has_creator:Check, - // the display name must be capitalized - display_name_is_title_case:Check, - // you cannot have any Scripts or ModuleScripts that have the keyword 'getfenv" or 'require' - // you cannot have more than 50 duplicate scripts - - // === MODE CHECKS === - // Exactly one MapStart - exactly_one_mapstart:Check, - // At least one MapFinish - at_least_one_mapfinish:Check, - // Spawn0 or Spawn1 must exist - spawn1_exists:Check, - // No duplicate Spawn# - no_duplicate_spawns:Check, - // No duplicate WormholeOut# (duplicate WormholeIn# ok) - no_duplicate_wormhole_out:Check, -} -impl CheckReport{ - pub fn pass(&self)->bool{ - return self.exactly_one_root.pass() - &&self.root_is_model.pass() - &&self.model_name_prefix_is_valid.pass() - &&self.model_name_is_snake_case.pass() - &&self.has_display_name.pass() - &&self.has_creator.pass() - &&self.display_name_is_title_case.pass() - &&self.exactly_one_mapstart.pass() - &&self.at_least_one_mapfinish.pass() - &&self.spawn1_exists.pass() - &&self.no_duplicate_spawns.pass() - &&self.no_duplicate_wormhole_out.pass() - } -} -impl std::fmt::Display for CheckReport{ - fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{ - write!(f, - "exactly_one_root={}\n -root_is_model={}\n -model_name_prefix_is_valid={}\n -model_name_is_snake_case={}\n -has_display_name={}\n -has_creator={}\n -display_name_is_title_case={}\n -exactly_one_mapstart={}\n -at_least_one_mapfinish={}\n -spawn1_exists={}\n -no_duplicate_spawns={}\n -no_duplicate_wormhole_out={}", - self.exactly_one_root, - self.root_is_model, - self.model_name_prefix_is_valid, - self.model_name_is_snake_case, - self.has_display_name, - self.has_creator, - self.display_name_is_title_case, - self.exactly_one_mapstart, - self.at_least_one_mapfinish, - self.spawn1_exists, - self.no_duplicate_spawns, - self.no_duplicate_wormhole_out, - ) - } -} - enum Zone{ Start(ModeID), Finish(ModeID), } -#[derive(Debug,Hash,Eq,PartialEq)] +#[derive(Clone,Copy,Debug,Hash,Eq,PartialEq)] struct ModeID(u64); impl ModeID{ const MAIN:Self=Self(0); @@ -199,66 +89,37 @@ impl SpawnID{ struct WormholeOutID(u64); struct Counts{ - mode_start_counts:std::collections::HashMap<ModeID,u64>, - mode_finish_counts:std::collections::HashMap<ModeID,u64>, - spawn_counts:std::collections::HashMap<SpawnID,u64>, - wormhole_out_counts:std::collections::HashMap<WormholeOutID,u64>, + mode_start_counts:HashMap<ModeID,u64>, + mode_finish_counts:HashMap<ModeID,u64>, + spawn_counts:HashMap<SpawnID,u64>, + wormhole_out_counts:HashMap<WormholeOutID,u64>, } impl Counts{ fn new()->Self{ Self{ - mode_start_counts:std::collections::HashMap::new(), - mode_finish_counts:std::collections::HashMap::new(), - spawn_counts:std::collections::HashMap::new(), - wormhole_out_counts:std::collections::HashMap::new(), + mode_start_counts:HashMap::new(), + mode_finish_counts:HashMap::new(), + spawn_counts:HashMap::new(), + wormhole_out_counts:HashMap::new(), } } } -pub fn check(dom:&rbx_dom_weak::WeakDom)->CheckStatus{ - // empty report with all checks failed - let mut report=CheckReport::default(); +pub struct ModelInfo<'a>{ + model_class:&'a str, + model_name:&'a str, + map_info:MapInfo<'a>, + counts:Counts, +} + +pub fn get_model_info(dom:&rbx_dom_weak::WeakDom)->Result<ModelInfo,GetRootInstanceError>{ // extract the root instance, otherwise immediately return - let Ok(model_instance)=get_root_instance(&dom)else{ - return CheckStatus::Failed{report}; - }; - - report.exactly_one_root=Check::Pass; - - if model_instance.class=="Model"{ - report.root_is_model=Check::Pass; - } - if model_instance.name==model_instance.name.to_snake_case(){ - report.model_name_is_snake_case=Check::Pass; - } + let model_instance=get_root_instance(&dom)?; // extract model info let map_info=get_mapinfo(&dom,model_instance); - // check DisplayName - if let Ok(display_name)=map_info.display_name{ - if !display_name.is_empty(){ - report.has_display_name=Check::Pass; - if display_name==display_name.to_title_case(){ - report.display_name_is_title_case=Check::Pass; - } - } - } - - // check Creator - if let Ok(creator)=map_info.creator{ - if !creator.is_empty(){ - report.has_creator=Check::Pass; - } - } - - // check GameID - if map_info.game_id.is_ok(){ - report.model_name_prefix_is_valid=Check::Pass; - } - - // === MODE CHECKS === // count objects let mut counts=Counts::new(); for instance in dom.descendants_of(model_instance.referent()){ @@ -286,43 +147,216 @@ pub fn check(dom:&rbx_dom_weak::WeakDom)->CheckStatus{ } } - // MapStart must exist && there must be exactly one of any bonus start zones. - if counts.mode_start_counts.get(&ModeID::MAIN)==Some(&1) - &&counts.mode_start_counts.iter().all(|(_,&c)|c==1){ - report.exactly_one_mapstart=Check::Pass; - } - // iterate over start zones - if counts.mode_start_counts.iter().all(|(mode_id,_)| - // ensure that at least one end zone exists with the same mode id - counts.mode_finish_counts.get(mode_id).is_some_and(|&num|0<num) - ){ - report.at_least_one_mapfinish=Check::Pass; - } - // Spawn1 must exist - if counts.spawn_counts.get(&SpawnID::FIRST).is_some(){ - report.spawn1_exists=Check::Pass; - } - if counts.spawn_counts.iter().all(|(_,&c)|c==1){ - report.no_duplicate_spawns=Check::Pass; - } - if counts.wormhole_out_counts.iter().all(|(_,&c)|c==1){ - report.no_duplicate_wormhole_out=Check::Pass; - } + Ok(ModelInfo{ + model_class:model_instance.class.as_str(), + model_name:model_instance.name.as_str(), + map_info, + counts, + }) +} - if report.pass(){ - CheckStatus::Passed{ - // TODO: refactor data structure to avoid pain - display_name:map_info.display_name.unwrap().to_owned(), - creator:map_info.creator.unwrap().to_owned(), - game_id:map_info.game_id.unwrap(), +// check if an observed string matches and expected string +pub struct StringCheck<'a,T,Str>(Result<T,StringCheckContext<'a,Str>>); +pub struct StringCheckContext<'a,Str>{ + observed:&'a str, + expected:Str, +} +impl<'a,Str> StringCheckContext<'a,Str> + where + &'a str:PartialEq<Str>, +{ + fn check<T>(self,value:T)->StringCheck<'a,T,Str>{ + if self.observed==self.expected{ + StringCheck(Ok(value)) + }else{ + StringCheck(Err(self)) + } + } +} + +// check if a string is empty +pub struct StringEmpty; +pub struct StringEmptyCheck<Context>(Result<Context,StringEmpty>); + +// check for duplicate objects +pub struct DuplicateCheckContext<ID>(HashMap<ID,u64>); +pub struct DuplicateCheck<ID>(Result<(),DuplicateCheckContext<ID>>); +impl<ID> DuplicateCheckContext<ID>{ + fn check(self)->DuplicateCheck<ID>{ + let Self(mut set)=self; + // remove correct entries + set.retain(|_,&mut c|c!=1); + // if any entries remain, they are incorrect + if set.is_empty(){ + DuplicateCheck(Ok(())) + }else{ + DuplicateCheck(Err(Self(set))) + } + } +} + +// check that there is at least one +pub struct AtLeastOneMatchingAndNoExtraCheckContext<ID>{ + extra:HashMap<ID,u64>, + missing:HashSet<ID>, +} +pub struct AtLeastOneMatchingAndNoExtraCheck<ID>(Result<(),AtLeastOneMatchingAndNoExtraCheckContext<ID>>); +impl<ID> AtLeastOneMatchingAndNoExtraCheckContext<ID>{ + fn new(initial_set:HashMap<ID,u64>)->Self{ + Self{ + extra:initial_set, + missing:HashSet::new(), + } + } +} +impl<ID:Copy+Eq+std::hash::Hash> AtLeastOneMatchingAndNoExtraCheckContext<ID>{ + fn check<T>(self,reference_set:&HashMap<ID,T>)->AtLeastOneMatchingAndNoExtraCheck<ID>{ + let Self{mut extra,mut missing}=self; + // remove correct entries + for (id,_) in reference_set{ + if extra.remove(id).is_none(){ + // the set did not contain a required item. This is a fail + missing.insert(*id); + } + } + // if any entries remain, they are incorrect + if extra.is_empty()&&missing.is_empty(){ + AtLeastOneMatchingAndNoExtraCheck(Ok(())) + }else{ + AtLeastOneMatchingAndNoExtraCheck(Err(Self{extra,missing})) + } + } +} + +pub struct MapInfoOwned{ + pub display_name:String, + pub creator:String, + pub game_id:GameID, +} + +// crazy! +pub struct MapCheck<'a>{ + model_class:StringCheck<'a,(),&'a str>, + model_name:StringCheck<'a,(),String>, + display_name:Result<StringEmptyCheck<StringCheck<'a,&'a str,String>>,StringValueError>, + creator:Result<StringEmptyCheck<&'a str>,StringValueError>, + game_id:Result<GameID,ParseGameIDError>, + mapstart:Result<(),()>, + mode_start_counts:DuplicateCheck<ModeID>, + mode_finish_counts:AtLeastOneMatchingAndNoExtraCheck<ModeID>, + spawn1:Result<(),()>, + spawn_counts:DuplicateCheck<SpawnID>, + wormhole_out_counts:DuplicateCheck<WormholeOutID>, +} + +impl<'a> ModelInfo<'a>{ + fn check(self)->MapCheck<'a>{ + let model_class=StringCheckContext{ + observed:self.model_class, + expected:"Model", + }.check(()); + + let model_name=StringCheckContext{ + observed:self.model_name, + expected:self.model_name.to_snake_case(), + }.check(()); + + // check display name + let display_name=self.map_info.display_name.map(|display_name|{ + if display_name.is_empty(){ + StringEmptyCheck(Err(StringEmpty)) + }else{ + StringEmptyCheck(Ok(StringCheckContext{ + observed:display_name, + expected:display_name.to_title_case(), + }.check(display_name))) + } + }); + + // check Creator + let creator=self.map_info.creator.map(|creator|{ + if creator.is_empty(){ + StringEmptyCheck(Err(StringEmpty)) + }else{ + StringEmptyCheck(Ok(creator)) + } + }); + + // check GameID + let game_id=self.map_info.game_id; + + // MapStart must exist + let mapstart=if self.counts.mode_start_counts.get(&ModeID::MAIN).is_some(){ + Ok(()) + }else{ + Err(()) + }; + + // Spawn1 must exist + let spawn1=if self.counts.spawn_counts.get(&SpawnID::FIRST).is_some(){ + Ok(()) + }else{ + Err(()) + }; + + // check that at least one end zone exists for each start zone. + let mode_finish_counts=AtLeastOneMatchingAndNoExtraCheckContext::new(self.counts.mode_finish_counts) + .check(&self.counts.mode_start_counts); + + // there must be exactly one start zone for every mode in the map. + let mode_start_counts=DuplicateCheckContext(self.counts.mode_start_counts).check(); + + // there must be exactly one of any perticular spawn id in the map. + let spawn_counts=DuplicateCheckContext(self.counts.spawn_counts).check(); + + // 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(); + + MapCheck{ + model_class, + model_name, + display_name, + creator, + game_id, + mapstart, + mode_start_counts, + mode_finish_counts, + spawn1, + spawn_counts, + wormhole_out_counts, + } + } +} + +impl<'a> MapCheck<'a>{ + fn pass(self)->Result<MapInfoOwned,Self>{ + match self{ + MapCheck{ + model_class:StringCheck(Ok(())), + model_name:StringCheck(Ok(())), + display_name:Ok(StringEmptyCheck(Ok(StringCheck(Ok(display_name))))), + creator:Ok(StringEmptyCheck(Ok(creator))), + game_id:Ok(game_id), + mapstart:Ok(()), + mode_start_counts:DuplicateCheck(Ok(())), + mode_finish_counts:AtLeastOneMatchingAndNoExtraCheck(Ok(())), + spawn1:Ok(()), + spawn_counts:DuplicateCheck(Ok(())), + wormhole_out_counts:DuplicateCheck(Ok(())), + }=>{ + Ok(MapInfoOwned{ + display_name:display_name.to_owned(), + creator:creator.to_owned(), + game_id, + }) + }, + other=>Err(other), } - }else{ - CheckStatus::Failed{report} } } pub struct CheckReportAndVersion{ - pub status:CheckStatus, + pub status:Result<MapInfoOwned,String>, pub version:u64, } @@ -349,7 +383,14 @@ impl crate::message_handler::MessageHandler{ // decode dom (slow!) let dom=maybe_gzip.read_with(read_dom,read_dom).map_err(Error::ModelFileDecode)?; - let status=check(&dom); + // extract information from the model + let model_info=get_model_info(&dom).map_err(Error::GetRootInstance)?; + + // convert the model information into a structured report + let map_check=model_info.check(); + + // check the report, generate an error message if it fails the check + let status=map_check.pass().map_err(|e|e.to_string()); Ok(CheckReportAndVersion{status,version}) } diff --git a/validation/src/check_mapfix.rs b/validation/src/check_mapfix.rs index 09585dd..2187fe3 100644 --- a/validation/src/check_mapfix.rs +++ b/validation/src/check_mapfix.rs @@ -1,4 +1,4 @@ -use crate::check::{CheckStatus,CheckReportAndVersion}; +use crate::check::CheckReportAndVersion; use crate::nats_types::CheckMapfixRequest; #[allow(dead_code)] @@ -24,22 +24,22 @@ impl crate::message_handler::MessageHandler{ Ok(CheckReportAndVersion{status,version})=>{ match status{ // update the mapfix model status to submitted - CheckStatus::Passed{display_name,creator,game_id}=> + Ok(map_info)=> self.api.action_mapfix_submitted( submissions_api::types::ActionMapfixSubmittedRequest{ MapfixID:mapfix_id, ModelVersion:version, - DisplayName:display_name, - Creator:creator, - GameID:game_id as u32, + DisplayName:map_info.display_name, + Creator:map_info.creator, + GameID:map_info.game_id as u32, } ).await.map_err(Error::ApiActionMapfixCheck)?, // update the mapfix model status to request changes - CheckStatus::Failed{report}=> + Err(report)=> self.api.action_mapfix_request_changes( submissions_api::types::ActionMapfixRequestChangesRequest{ MapfixID:mapfix_id, - ErrorMessage:report.to_string(), + ErrorMessage:report, } ).await.map_err(Error::ApiActionMapfixCheck)?, } diff --git a/validation/src/check_submission.rs b/validation/src/check_submission.rs index b20f976..d0bd80c 100644 --- a/validation/src/check_submission.rs +++ b/validation/src/check_submission.rs @@ -1,4 +1,4 @@ -use crate::check::{CheckStatus,CheckReportAndVersion}; +use crate::check::CheckReportAndVersion; use crate::nats_types::CheckSubmissionRequest; #[allow(dead_code)] @@ -24,22 +24,22 @@ impl crate::message_handler::MessageHandler{ Ok(CheckReportAndVersion{status,version})=>{ match status{ // update the submission model status to submitted - CheckStatus::Passed{display_name,creator,game_id}=> + Ok(map_info)=> self.api.action_submission_submitted( submissions_api::types::ActionSubmissionSubmittedRequest{ SubmissionID:submission_id, ModelVersion:version, - DisplayName:display_name, - Creator:creator, - GameID:game_id as u32, + DisplayName:map_info.display_name, + Creator:map_info.creator, + GameID:map_info.game_id as u32, } ).await.map_err(Error::ApiActionSubmissionCheck)?, // update the submission model status to request changes - CheckStatus::Failed{report}=> + Err(report)=> self.api.action_submission_request_changes( submissions_api::types::ActionSubmissionRequestChangesRequest{ SubmissionID:submission_id, - ErrorMessage:report.to_string(), + ErrorMessage:report, } ).await.map_err(Error::ApiActionSubmissionCheck)?, }