validation: Check that mapfixes do not change DisplayName, Creator, GameID

Cargo.lock

@@ -1044,6 +1044,7 @@ dependencies = [
  "rust-grpc",
  "serde",
  "serde_json",
+ "serde_repr",
  "siphasher",
  "tokio",
  "tonic",

pkg/model/nats.go

@@ -31,9 +31,12 @@ type CheckSubmissionRequest struct{
 }
 
 type CheckMapfixRequest struct{
-	MapfixID   int64
-	ModelID    uint64
-	SkipChecks bool
+	MapfixID    int64
+	ModelID     uint64
+	SkipChecks  bool
+	DisplayName string
+	Creator     string
+	GameID      uint32
 }
 
 type ValidateSubmissionRequest struct {

pkg/service/nats_mapfix.go

@@ -33,14 +33,20 @@ func (svc *Service) NatsCreateMapfix(
 }
 
 func (svc *Service) NatsCheckMapfix(
-	MapfixID   int64,
-	ModelID    uint64,
-	SkipChecks bool,
+	MapfixID    int64,
+	ModelID     uint64,
+	SkipChecks  bool,
+	DisplayName string,
+	Creator     string,
+	GameID      uint32,
 ) error {
 	validate_request := model.CheckMapfixRequest{
 		MapfixID:   MapfixID,
 		ModelID:    ModelID,
 		SkipChecks: SkipChecks,
+		DisplayName: DisplayName,
+		Creator: Creator,
+		GameID: GameID,
 	}
 
 	j, err := json.Marshal(validate_request)

pkg/web_api/mapfixes.go

@@ -538,13 +538,13 @@ func (svc *Service) ActionMapfixTriggerSubmit(ctx context.Context, params api.Ac
 		return ErrUserInfo
 	}
 
-	// read mapfix (this could be done with a transaction WHERE clause)
-	mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID)
+	userId, err := userInfo.GetUserID()
 	if err != nil {
 		return err
 	}
 
-	userId, err := userInfo.GetUserID()
+	// read mapfix (this could be done with a transaction WHERE clause)
+	mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID)
 	if err != nil {
 		return err
 	}
@@ -555,6 +555,12 @@ func (svc *Service) ActionMapfixTriggerSubmit(ctx context.Context, params api.Ac
 		return ErrPermissionDeniedNotSubmitter
 	}
 
+	// read map to get current DisplayName and such
+	target_map, err := svc.inner.GetMap(ctx, int64(mapfix.TargetAssetID))
+	if err != nil {
+		return err
+	}
+
 	// transaction
 	target_status := model.MapfixStatusSubmitting
 	update := service.NewMapfixUpdate()
@@ -569,6 +575,9 @@ func (svc *Service) ActionMapfixTriggerSubmit(ctx context.Context, params api.Ac
 		mapfix.ID,
 		mapfix.AssetID,
 		false,
+		target_map.DisplayName,
+		target_map.Creator,
+		target_map.GameID,
 	)
 	if err != nil {
 		return err
@@ -600,13 +609,13 @@ func (svc *Service) ActionMapfixTriggerSubmitUnchecked(ctx context.Context, para
 		return ErrUserInfo
 	}
 
-	// read mapfix (this could be done with a transaction WHERE clause)
-	mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID)
+	userId, err := userInfo.GetUserID()
 	if err != nil {
 		return err
 	}
 
-	userId, err := userInfo.GetUserID()
+	// read mapfix (this could be done with a transaction WHERE clause)
+	mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID)
 	if err != nil {
 		return err
 	}
@@ -626,6 +635,12 @@ func (svc *Service) ActionMapfixTriggerSubmitUnchecked(ctx context.Context, para
 		return ErrPermissionDeniedNeedRoleMapfixReview
 	}
 
+	// read map to get current DisplayName and such
+	target_map, err := svc.inner.GetMap(ctx, int64(mapfix.TargetAssetID))
+	if err != nil {
+		return err
+	}
+
 	// transaction
 	target_status := model.MapfixStatusSubmitting
 	update := service.NewMapfixUpdate()
@@ -640,6 +655,9 @@ func (svc *Service) ActionMapfixTriggerSubmitUnchecked(ctx context.Context, para
 		mapfix.ID,
 		mapfix.AssetID,
 		true,
+		target_map.DisplayName,
+		target_map.Creator,
+		target_map.GameID,
 	)
 	if err != nil {
 		return err

validation/Cargo.toml

@@ -14,6 +14,7 @@ rbx_xml = "2.0.0"
 regex = { version = "1.11.3", default-features = false }
 serde = { version = "1.0.215", features = ["derive"] }
 serde_json = "1.0.133"
+serde_repr = "0.1.19"
 siphasher = "1.0.1"
 tokio = { version = "1.41.1", features = ["macros", "rt-multi-thread", "signal"] }
 heck = "0.5.0"

validation/src/check.rs

@@ -33,29 +33,6 @@ macro_rules! lazy_regex{
 	}};
 }
 
-#[expect(nonstandard_style)]
-pub struct CheckRequest{
-	ModelID:u64,
-	SkipChecks:bool,
-}
-
-impl From<crate::nats_types::CheckMapfixRequest> for CheckRequest{
-	fn from(value:crate::nats_types::CheckMapfixRequest)->Self{
-		Self{
-			ModelID:value.ModelID,
-			SkipChecks:value.SkipChecks,
-		}
-	}
-}
-impl From<crate::nats_types::CheckSubmissionRequest> for CheckRequest{
-	fn from(value:crate::nats_types::CheckSubmissionRequest)->Self{
-		Self{
-			ModelID:value.ModelID,
-			SkipChecks:value.SkipChecks,
-		}
-	}
-}
-
 #[derive(Clone,Copy,Debug,Hash,Eq,PartialEq,Ord,PartialOrd)]
 struct ModeID(u64);
 impl ModeID{
@@ -323,26 +300,25 @@ pub fn get_model_info<'a>(dom:&'a rbx_dom_weak::WeakDom,model_instance:&'a rbx_d
 	}
 }
 
-// check if an observed string matches an expected string
-pub struct StringCheck<'a,T,Str>(Result<T,StringCheckContext<'a,Str>>);
-pub struct StringCheckContext<'a,Str>{
-	observed:&'a str,
-	expected:Str,
+// check if an observed value matches an expected value
+pub struct EqualityCheck<Obs,Exp>{
+	observed:Obs,
+	expected:Exp,
 }
-impl<'a,Str> StringCheckContext<'a,Str>
+impl<Obs,Exp> EqualityCheck<Obs,Exp>
 	where
-		&'a str:PartialEq<Str>,
+		Obs:PartialEq<Exp>,
 {
 	/// Compute the StringCheck, passing through the provided value on success.
-	fn check<T>(self,value:T)->StringCheck<'a,T,Str>{
+	fn check<T>(self,value:T)->Result<T,Self>{
 		if self.observed==self.expected{
-			StringCheck(Ok(value))
+			Ok(value)
 		}else{
-			StringCheck(Err(self))
+			Err(self)
 		}
 	}
 }
-impl<Str:std::fmt::Display> std::fmt::Display for StringCheckContext<'_,Str>{
+impl<Obs:std::fmt::Display,Exp:std::fmt::Display> std::fmt::Display for EqualityCheck<Obs,Exp>{
 	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
 		write!(f,"expected: {}, observed: {}",self.expected,self.observed)
 	}
@@ -464,22 +440,164 @@ impl TryFrom<MapInfo<'_>> for MapInfoOwned{
 struct Exists;
 struct Absent;
 
+enum DisplayNameError<'a>{
+	TitleCase(EqualityCheck<&'a str,String>),
+	CannotChange(EqualityCheck<&'a str,String>),
+	Empty(StringEmpty),
+	TooLong(usize),
+	StringValue(StringValueError),
+}
+
+enum CreatorError<'a>{
+	CannotChange(EqualityCheck<&'a str,String>),
+	Empty(StringEmpty),
+	TooLong(usize),
+	StringValue(StringValueError),
+}
+
+enum GameIDError{
+	CannotChange(EqualityCheck<GameID,GameID>),
+	Parse(ParseGameIDError),
+}
+
+pub struct CheckedMapInfo<'a>{
+	display_name:Result<&'a str,DisplayNameError<'a>>,
+	creator:Result<&'a str,CreatorError<'a>>,
+	game_id:Result<GameID,GameIDError>,
+}
+
+pub struct NoMapInfo;
+impl CheckModelInfo for NoMapInfo{
+	fn check<'a>(self,map_info:MapInfo<'a>)->CheckedMapInfo<'a>{
+		fn check_display_name(display_name:Result<&str,StringValueError>)->Result<&str,DisplayNameError<'_>>{
+			// DisplayName StringValue can be missing or whatever
+			let display_name=display_name.map_err(DisplayNameError::StringValue)?;
+
+			// DisplayName cannot be ""
+			let display_name=check_empty(display_name).map_err(DisplayNameError::Empty)?;
+
+			// DisplayName cannot exceed 50 characters
+			if 50<display_name.len(){
+				return Err(DisplayNameError::TooLong(display_name.len()));
+			}
+
+			// Check title case
+			let display_name=EqualityCheck{
+				observed:display_name,
+				expected:display_name.to_title_case(),
+			}.check(display_name).map_err(DisplayNameError::TitleCase)?;
+
+			Ok(display_name)
+		}
+		fn check_creator(creator:Result<&str,StringValueError>)->Result<&str,CreatorError<'_>>{
+			// Creator StringValue can be missing or whatever
+			let creator=creator.map_err(CreatorError::StringValue)?;
+
+			// Creator cannot be ""
+			let creator=check_empty(creator).map_err(CreatorError::Empty)?;
+
+			// Creator cannot exceed 50 characters
+			if 50<creator.len(){
+				return Err(CreatorError::TooLong(creator.len()));
+			}
+
+			Ok(creator)
+		}
+		fn check_game_id(game_id:Result<GameID,ParseGameIDError>)->Result<GameID,GameIDError>{
+			// Creator StringValue can be missing or whatever
+			let game_id=game_id.map_err(GameIDError::Parse)?;
+
+			Ok(game_id)
+		}
+
+		// Check display name is not empty and has title case
+		let display_name=check_display_name(map_info.display_name);
+
+		// Check Creator is not empty
+		let creator=check_creator(map_info.creator);
+
+		// Check GameID (model name was prefixed with bhop_ surf_ etc)
+		let game_id=check_game_id(map_info.game_id);
+
+		CheckedMapInfo{
+			display_name,
+			creator,
+			game_id,
+		}
+	}
+}
+impl CheckModelInfo for MapInfoOwned{
+	fn check<'a>(self,map_info:MapInfo<'a>)->CheckedMapInfo<'a>{
+		fn check_display_name(display_name:Result<&str,StringValueError>,target_display_name:String)->Result<&str,DisplayNameError<'_>>{
+			// DisplayName StringValue can be missing or whatever
+			let display_name=display_name.map_err(DisplayNameError::StringValue)?;
+
+			// Mapfix cannot change display name
+			let display_name=EqualityCheck{
+				observed:display_name,
+				expected:target_display_name,
+			}.check(display_name).map_err(DisplayNameError::CannotChange)?;
+
+			Ok(display_name)
+		}
+		fn check_creator(creator:Result<&str,StringValueError>,target_creator:String)->Result<&str,CreatorError<'_>>{
+			// Creator StringValue can be missing or whatever
+			let creator=creator.map_err(CreatorError::StringValue)?;
+
+			// Mapfix cannot change creator
+			let creator=EqualityCheck{
+				observed:creator,
+				expected:target_creator,
+			}.check(creator).map_err(CreatorError::CannotChange)?;
+
+			Ok(creator)
+		}
+		fn check_game_id(game_id:Result<GameID,ParseGameIDError>,target_game_id:GameID)->Result<GameID,GameIDError>{
+			// Creator StringValue can be missing or whatever
+			let game_id=game_id.map_err(GameIDError::Parse)?;
+
+			// Mapfix cannot change game_id
+			let game_id=EqualityCheck{
+				observed:game_id,
+				expected:target_game_id,
+			}.check(game_id).map_err(GameIDError::CannotChange)?;
+
+			Ok(game_id)
+		}
+
+		// Check display name is not empty and has title case
+		let display_name=check_display_name(map_info.display_name,self.display_name);
+
+		// Check Creator is not empty
+		let creator=check_creator(map_info.creator,self.creator);
+
+		// Check GameID (model name was prefixed with bhop_ surf_ etc)
+		let game_id=check_game_id(map_info.game_id,self.game_id);
+
+		CheckedMapInfo{
+			display_name,
+			creator,
+			game_id,
+		}
+	}
+}
+
 /// The result of every map check.
 struct MapCheck<'a>{
 	// === METADATA CHECKS ===
 	// The root must be of class Model
-	model_class:StringCheck<'a,(),&'static str>,
+	model_class:Result<(),EqualityCheck<&'a str,&'static str>>,
 	// Model's name must be in snake case
-	model_name:StringCheck<'a,(),String>,
+	model_name:Result<(),EqualityCheck<&'a str,String>>,
 	// Map must have a StringValue named DisplayName.
 	// Value must not be empty, must be in title case.
-	display_name:Result<Result<StringCheck<'a,&'a str,String>,StringEmpty>,StringValueError>,
+	display_name:Result<&'a str,DisplayNameError<'a>>,
 	// Map must have a StringValue named Creator.
 	// Value must not be empty.
-	creator:Result<Result<&'a str,StringEmpty>,StringValueError>,
+	creator:Result<&'a str,CreatorError<'a>>,
 	// The prefix of the model's name must match the game it was submitted for.
 	// bhop_ for bhop, and surf_ for surf
-	game_id:Result<GameID,ParseGameIDError>,
+	game_id:Result<GameID,GameIDError>,
 
 	// === MODE CHECKS ===
 	// MapStart must exist
@@ -509,32 +627,24 @@ struct MapCheck<'a>{
 }
 
 impl<'a> ModelInfo<'a>{
-	fn check(self)->MapCheck<'a>{
+	fn check<I:CheckModelInfo>(self,model_info:I)->MapCheck<'a>{
 		// Check class is exactly "Model"
-		let model_class=StringCheckContext{
+		let model_class=EqualityCheck{
 			observed:self.model_class,
 			expected:"Model",
 		}.check(());
 
 		// Check model name is snake case
-		let model_name=StringCheckContext{
+		let model_name=EqualityCheck{
 			observed:self.model_name,
 			expected:self.model_name.to_snake_case(),
 		}.check(());
 
-		// Check display name is not empty and has title case
-		let display_name=self.map_info.display_name.map(|display_name|{
-			check_empty(display_name).map(|display_name|StringCheckContext{
-				observed:display_name,
-				expected:display_name.to_title_case(),
-			}.check(display_name))
-		});
-
-		// Check Creator is not empty
-		let creator=self.map_info.creator.map(check_empty);
-
-		// Check GameID (model name was prefixed with bhop_ surf_ etc)
-		let game_id=self.map_info.game_id;
+		let CheckedMapInfo{
+			display_name,
+			creator,
+			game_id,
+		}=model_info.check(self.map_info);
 
 		// MapStart must exist
 		let mapstart=if self.counts.mode_start_counts.contains_key(&ModeID::MAIN){
@@ -630,10 +740,10 @@ impl MapCheck<'_>{
 	fn result(self)->Result<MapInfoOwned,Result<MapCheckList,serde_json::Error>>{
 		match self{
 			MapCheck{
-				model_class:StringCheck(Ok(())),
-				model_name:StringCheck(Ok(())),
-				display_name:Ok(Ok(StringCheck(Ok(display_name)))),
-				creator:Ok(Ok(creator)),
+				model_class:Ok(()),
+				model_name:Ok(()),
+				display_name:Ok(display_name),
+				creator:Ok(creator),
 				game_id:Ok(game_id),
 				mapstart:Ok(Exists),
 				mode_start_counts:DuplicateCheck(Ok(())),
@@ -737,31 +847,32 @@ macro_rules! summary_format{
 impl MapCheck<'_>{
 	fn itemize(&self)->Result<MapCheckList,serde_json::Error>{
 		let model_class=match &self.model_class{
-			StringCheck(Ok(()))=>passed!("ModelClass"),
-			StringCheck(Err(context))=>summary_format!("ModelClass","Invalid model class: {context}"),
+			Ok(())=>passed!("ModelClass"),
+			Err(context)=>summary_format!("ModelClass","Invalid model class: {context}"),
 		};
 		let model_name=match &self.model_name{
-			StringCheck(Ok(()))=>passed!("ModelName"),
-			StringCheck(Err(context))=>summary_format!("ModelName","Model name must have snake_case: {context}"),
+			Ok(())=>passed!("ModelName"),
+			Err(context)=>summary_format!("ModelName","Model name must have snake_case: {context}"),
 		};
 		let display_name=match &self.display_name{
-			Ok(Ok(StringCheck(Ok(_))))=>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()),
+			Ok(_)=>passed!("DisplayName"),
+			Err(DisplayNameError::TitleCase(context))=>summary_format!("DisplayName","DisplayName must have Title Case: {context}"),
+			Err(DisplayNameError::CannotChange(context))=>summary_format!("DisplayName","DisplayName cannot be changed: {context}"),
+			Err(DisplayNameError::Empty(context))=>summary_format!("DisplayName","Invalid DisplayName: {context}"),
+			Err(DisplayNameError::TooLong(context))=>summary_format!("DisplayName","DisplayName is too long: {context} characters (50 characters max)"),
+			Err(DisplayNameError::StringValue(context))=>summary_format!("DisplayName","DisplayName StringValue: {context}"),
 		};
 		let creator=match &self.creator{
-			Ok(Ok(_))=>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()),
+			Ok(_)=>passed!("Creator"),
+			Err(CreatorError::CannotChange(context))=>summary_format!("Creator","Creator cannot be changed: {context}"),
+			Err(CreatorError::Empty(context))=>summary_format!("Creator","Invalid Creator: {context}"),
+			Err(CreatorError::TooLong(context))=>summary_format!("Creator","Creator is too long: {context} characters (50 characters max)"),
+			Err(CreatorError::StringValue(context))=>summary_format!("Creator","Creator StringValue: {context}"),
 		};
 		let game_id=match &self.game_id{
 			Ok(_)=>passed!("GameID"),
-			Err(ParseGameIDError)=>summary!("GameID","Model name must be prefixed with bhop_ surf_ or flytrials_".to_owned()),
+			Err(GameIDError::CannotChange(context))=>summary_format!("GameID","GameID cannot be changed: {context}"),
+			Err(GameIDError::Parse(ParseGameIDError))=>summary!("GameID","Model name must be prefixed with bhop_ surf_ or flytrials_".to_owned()),
 		};
 		let mapstart=match &self.mapstart{
 			Ok(Exists)=>passed!("MapStart"),
@@ -922,8 +1033,55 @@ pub struct CheckListAndVersion{
 	pub version:u64,
 }
 
+pub trait CheckModelInfo{
+	fn check<'a>(self,map_info:MapInfo<'a>)->CheckedMapInfo<'a>;
+}
+
+pub trait CheckSpecialization{
+	type ModelInfo:CheckModelInfo;
+	fn info(self)->(CheckRequest,Self::ModelInfo);
+}
+
+
+#[expect(nonstandard_style)]
+pub struct CheckRequest{
+	ModelID:u64,
+	SkipChecks:bool,
+}
+
+impl CheckSpecialization for crate::nats_types::CheckMapfixRequest{
+	type ModelInfo=MapInfoOwned;
+	fn info(self)->(CheckRequest,Self::ModelInfo) {
+		(
+			CheckRequest{
+				ModelID:self.ModelID,
+				SkipChecks:self.SkipChecks,
+			},
+			MapInfoOwned{
+				display_name:self.DisplayName,
+				creator:self.Creator,
+				game_id:self.GameID,
+			}
+		)
+	}
+}
+impl CheckSpecialization for crate::nats_types::CheckSubmissionRequest{
+	type ModelInfo=NoMapInfo;
+	fn info(self)->(CheckRequest,Self::ModelInfo) {
+		(
+			CheckRequest{
+				ModelID:self.ModelID,
+				SkipChecks:self.SkipChecks,
+			},
+			NoMapInfo
+		)
+	}
+}
+
 impl crate::message_handler::MessageHandler{
-	pub async fn check_inner(&self,check_info:CheckRequest)->Result<CheckListAndVersion,Error>{
+	pub async fn check_inner<R:CheckSpecialization>(&self,check_info:R)->Result<CheckListAndVersion,Error>{
+		let (check_info,target_model_info)=check_info.info();
+
 		// discover asset creator and latest version
 		let info=self.cloud_context.get_asset_info(
 			rbx_asset::cloud::GetAssetLatestRequest{asset_id:check_info.ModelID}
@@ -963,7 +1121,7 @@ impl crate::message_handler::MessageHandler{
 		let model_info=get_model_info(&dom,model_instance);
 
 		// convert the model information into a structured report
-		let map_check=model_info.check();
+		let map_check=model_info.check(target_model_info);
 
 		// check the report, generate an error message if it fails the check
 		let status=match map_check.result(){

validation/src/check_mapfix.rs

@@ -17,7 +17,7 @@ impl std::error::Error for Error{}
 impl crate::message_handler::MessageHandler{
 	pub async fn check_mapfix(&self,check_info:CheckMapfixRequest)->Result<(),Error>{
 		let mapfix_id=check_info.MapfixID;
-		let check_result=self.check_inner(check_info.into()).await;
+		let check_result=self.check_inner(check_info).await;
 
 		// update the mapfix depending on the result
 		match check_result{

validation/src/check_submission.rs

@@ -17,7 +17,7 @@ impl std::error::Error for Error{}
 impl crate::message_handler::MessageHandler{
 	pub async fn check_submission(&self,check_info:CheckSubmissionRequest)->Result<(),Error>{
 		let submission_id=check_info.SubmissionID;
-		let check_result=self.check_inner(check_info.into()).await;
+		let check_result=self.check_inner(check_info).await;
 
 		// update the submission depending on the result
 		match check_result{

validation/src/nats_types.rs

@@ -41,6 +41,10 @@ pub struct CheckMapfixRequest{
 	pub MapfixID:u64,
 	pub ModelID:u64,
 	pub SkipChecks:bool,
+	// target map info
+	pub DisplayName:String,
+	pub Creator:String,
+	pub GameID:crate::rbx_util::GameID,
 }
 
 #[expect(nonstandard_style)]

validation/src/rbx_util.rs

@@ -31,6 +31,9 @@ fn find_first_child_name_and_class<'a>(dom:&'a rbx_dom_weak::WeakDom,instance:&r
 	instance.children().iter().filter_map(|&r|dom.get_by_ref(r)).find(|inst|inst.name==name&&inst.class==class)
 }
 
+#[derive(serde_repr::Deserialize_repr)]
+#[repr(u32)]
+#[derive(Clone,Copy,PartialEq)]
 pub enum GameID{
 	Bhop=1,
 	Surf=2,
@@ -66,6 +69,15 @@ impl TryFrom<u32> for GameID{
 		}
 	}
 }
+impl std::fmt::Display for GameID{
+	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
+		match self{
+			GameID::Bhop=>write!(f,"Bhop"),
+			GameID::Surf=>write!(f,"Surf"),
+			GameID::FlyTrials=>write!(f,"FlyTrials"),
+		}
+	}
+}
 
 pub struct MapInfo<'a>{
 	pub display_name:Result<&'a str,StringValueError>,
@@ -79,6 +91,15 @@ pub enum StringValueError{
 	ValueNotSet,
 	NonStringValue,
 }
+impl std::fmt::Display for StringValueError{
+	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
+		match self{
+			StringValueError::ObjectNotFound=>write!(f,"Missing StringValue"),
+			StringValueError::ValueNotSet=>write!(f,"Value not set"),
+			StringValueError::NonStringValue=>write!(f,"Value is not a String"),
+		}
+	}
+}
 
 fn string_value(instance:Option<&rbx_dom_weak::Instance>)->Result<&str,StringValueError>{
 	let instance=instance.ok_or(StringValueError::ObjectNotFound)?;