View Issue Details

IDProjectCategoryView StatusLast Update
0003085FSSCPtablespublic2014-08-08 14:48
ReporterMageKing17 Assigned ToMageKing17  
PrioritylowSeverityfeatureReproducibilityalways
Status resolvedResolutionfixed 
Summary0003085: Unhelpful error reporting in hud_gauges.tbl if an invalid gauge type is specified.
DescriptionPer http://www.hard-light.net/forums/index.php?topic=88106.0 (posted by X3N0-Life-Form), specifying an invalid gauge type in hud_gauges.tbl leads to the unhelpful error message "Invalid gauge found in hud_gauges.tbl" instead of, say, a line number or the actual line in question. Attached are two patches that try to fix this in two different ways.

The first just adds an error to parse_gauge_type() (and changes the error in load_gauge() into an Assertion() instead). This is a pretty low-impact change and definitely provides a more helpful error.

The second includes the load_gauge() change from the first patch, but makes parse_gauge_type() use required_string_one_of() and switches based on the result, so that the error is thrown as a matter of course. On the one hand, this is a more involved change. On the other hand, this gets rid of the ugly series of "if(optional_string(...))" statements. On the other other hand, the error will enumerate every single possible gauge type, which may be a bit of an information overload; changing that would require making required_string_one_of() more terse if more than a certain number of arguments are supplied.
TagsNo tags attached.

Activities

MageKing17

2014-08-06 00:16

developer  

hudparse.cpp.patch (773 bytes)   
Index: code/hud/hudparse.cpp
===================================================================
--- code/hud/hudparse.cpp	(revision 10971)
+++ code/hud/hudparse.cpp	(working copy)
@@ -861,6 +861,8 @@
 	if ( optional_string("+Secondary Weapons:") )
 		return HUD_OBJECT_SECONDARY_WEAPONS;
 
+	error_display(1, "Invalid gauge type [%.32s]", next_tokens());
+	
 	return -1;
 }
 
@@ -1041,7 +1043,8 @@
 		load_gauge_secondary_weapons(base_w, base_h, hud_font, scale_gauge, ship_idx, use_clr);
 		break;
 	default:
-		Warning(LOCATION, "Invalid gauge found in hud_gauges.tbl");
+		// It's either -1, indicating we're ignoring a parse error, or it's a coding error.
+		Assertion(gauge == -1, "Invalid value '%d' passed to load_gauge(); get a coder!\n", gauge);
 		break;
 	}
 }
hudparse.cpp.patch (773 bytes)   

MageKing17

2014-08-06 00:26

developer  

hudparse2.cpp.patch (7,116 bytes)   
Index: code/hud/hudparse.cpp
===================================================================
--- code/hud/hudparse.cpp	(revision 10971)
+++ code/hud/hudparse.cpp	(working copy)
@@ -688,180 +688,186 @@
 {
 	// probably a more elegant way to do this. 
 	// Likely involving a for loop, an array of strings and only one if statement with a strcmp.
-	if(optional_string("+Custom:")) 
+	switch (required_string_one_of(56, "+Custom:", "+Messages:", "+Training Messages:", "+Support:", "+Damage:", "+Wingman Status:", "+Auto Speed:", "+Auto Target:", "+Countermeasures:", "+Talking Head:", "+Directives:", "+Weapons:", "+Objective Notify:", "+Squad Message:", "+Lag:", "+Mini Target Shields:", "+Player Shields:", "+Target Shields:", "+Escort View:", "+Mission Time:", "+ETS Weapons:", "+ETS Shields:", "+ETS Engines:", "+ETS Retail:", "+Target Monitor:", "+Extra Target Data:", "+Radar:", "+Radar Orb:", "+Radar BSG:", "+Afterburner Energy:", "+Weapon Energy:", "+Text Warnings:", "+Center Reticle:", "+Throttle:", "+Threat Indicator:", "+Lead Indicator:", "+Lead Sight:", "+Lock Indicator:", "+Weapon Linking:", "+Multiplayer Messages:", "+Voice Status:", "+Ping:", "+Supernova:", "+Orientation Tee:", "+Offscreen Indicator:", "+Target Brackets:", "+Hostile Triangle:", "+Target Triangle:", "+Missile Triangles:", "+Kills:", "+Fixed Messages:", "+Flight Path Marker:", "+Warhead Count:", "+Hardpoints:", "+Primary Weapons:", "+Secondary Weapons:")) {
+	case 0:
 		return HUD_OBJECT_CUSTOM;
 
-	if(optional_string("+Messages:")) 
+	case 1:
 		return HUD_OBJECT_MESSAGES;
 
-	if(optional_string("+Training Messages:")) 
+	case 2:
 		return HUD_OBJECT_TRAINING_MESSAGES;
 
-	if(optional_string("+Support:")) 
+	case 3:
 		return HUD_OBJECT_SUPPORT;
 
-	if(optional_string("+Damage:")) 
+	case 4:
 		return HUD_OBJECT_DAMAGE;
 
-	if(optional_string("+Wingman Status:")) 
+	case 5:
 		return HUD_OBJECT_WINGMAN_STATUS;
 
-	if(optional_string("+Auto Speed:")) 
+	case 6:
 		return HUD_OBJECT_AUTO_SPEED;
 
-	if(optional_string("+Auto Target:")) 
+	case 7:
 		return HUD_OBJECT_AUTO_TARGET;
 
-	if(optional_string("+Countermeasures:")) 
+	case 8:
 		return HUD_OBJECT_CMEASURES;
 
-	if(optional_string("+Talking Head:")) 
+	case 9:
 		return HUD_OBJECT_TALKING_HEAD;
 
-	if(optional_string("+Directives:")) 
+	case 10:
 		return HUD_OBJECT_DIRECTIVES;
 
-	if(optional_string("+Weapons:")) 
+	case 11:
 		return HUD_OBJECT_WEAPONS;
 
-	if(optional_string("+Objective Notify:")) 
+	case 12:
 		return HUD_OBJECT_OBJ_NOTIFY;
 
-	if(optional_string("+Squad Message:")) 
+	case 13:
 		return HUD_OBJECT_SQUAD_MSG;
 
-	if(optional_string("+Lag:")) 
+	case 14:
 		return HUD_OBJECT_LAG;
 
-	if(optional_string("+Mini Target Shields:")) 
+	case 15:
 		return HUD_OBJECT_MINI_SHIELD;
 
-	if(optional_string("+Player Shields:")) 
+	case 16:
 		return HUD_OBJECT_PLAYER_SHIELD;
 
-	if(optional_string("+Target Shields:")) 
+	case 17:
 		return HUD_OBJECT_TARGET_SHIELD;
 
-	if(optional_string("+Escort View:")) 
+	case 18:
 		return HUD_OBJECT_ESCORT;
 
-	if(optional_string("+Mission Time:")) 
+	case 19:
 		return HUD_OBJECT_MISSION_TIME;
 
-	if(optional_string("+ETS Weapons:")) 
+	case 20:
 		return HUD_OBJECT_ETS_WEAPONS;
 
-	if(optional_string("+ETS Shields:")) 
+	case 21:
 		return HUD_OBJECT_ETS_SHIELDS;
 
-	if(optional_string("+ETS Engines:")) 
+	case 22:
 		return HUD_OBJECT_ETS_ENGINES;
 
-	if(optional_string("+ETS Retail:"))
+	case 23:
 		return HUD_OBJECT_ETS_RETAIL;
 
-	if(optional_string("+Target Monitor:")) 
+	case 24:
 		return HUD_OBJECT_TARGET_MONITOR;
 
-	if(optional_string("+Extra Target Data:")) 
+	case 25:
 		return HUD_OBJECT_EXTRA_TARGET_DATA;
 
-	if(optional_string("+Radar:")) {
+	case 26:
 		if(Cmdline_orb_radar) {
 			return HUD_OBJECT_RADAR_ORB;
 		} else {
 			return HUD_OBJECT_RADAR_STD;
 		}
-	}
 
-	if(optional_string("+Radar Orb:")) 
+	case 27:
 		return HUD_OBJECT_RADAR_ORB;
 
-	if(optional_string("+Radar BSG:")) 
+	case 28:
 		return HUD_OBJECT_RADAR_BSG;
 
-	if(optional_string("+Afterburner Energy:")) 
+	case 29:
 		return HUD_OBJECT_AFTERBURNER;
 
-	if(optional_string("+Weapon Energy:")) 
+	case 30:
 		return HUD_OBJECT_WEAPON_ENERGY;
 
-	if(optional_string("+Text Warnings:")) 
+	case 31:
 		return HUD_OBJECT_TEXT_WARNINGS;
 
-	if(optional_string("+Center Reticle:")) 
+	case 32:
 		return HUD_OBJECT_CENTER_RETICLE;
 
-	if(optional_string("+Throttle:")) 
+	case 33:
 		return HUD_OBJECT_THROTTLE;
 
-	if(optional_string("+Threat Indicator:")) 
+	case 34:
 		return HUD_OBJECT_THREAT;
 	
-	if(optional_string("+Lead Indicator:"))
+	case 35:
 		return HUD_OBJECT_LEAD;
 
-	if(optional_string("+Lead Sight:"))
+	case 36:
 		return HUD_OBJECT_LEAD_SIGHT;
 
-	if(optional_string("+Lock Indicator:"))
+	case 37:
 		return HUD_OBJECT_LOCK;
 
-	if(optional_string("+Weapon Linking:"))
+	case 38:
 		return HUD_OBJECT_WEAPON_LINKING;
 
-	if(optional_string("+Multiplayer Messages:"))
+	case 39:
 		return HUD_OBJECT_MULTI_MSG;
 
-	if(optional_string("+Voice Status:"))
+	case 40:
 		return HUD_OBJECT_VOICE_STATUS;
 
-	if(optional_string("+Ping:"))
+	case 41:
 		return HUD_OBJECT_PING;
 	
-	if(optional_string("+Supernova:"))
+	case 42:
 		return HUD_OBJECT_SUPERNOVA;
 
-	if(optional_string("+Orientation Tee:"))
+	case 43:
 		return HUD_OBJECT_ORIENTATION_TEE;
 
-	if(optional_string("+Offscreen Indicator:"))
+	case 44:
 		return HUD_OBJECT_OFFSCREEN;
 
-	if(optional_string("+Target Brackets:"))
+	case 45:
 		return HUD_OBJECT_BRACKETS;
 
-	if(optional_string("+Hostile Triangle:"))
+	case 46:
 		return HUD_OBJECT_HOSTILE_TRI;
 
-	if(optional_string("+Target Triangle:"))
+	case 47:
 		return HUD_OBJECT_TARGET_TRI;
 
-	if(optional_string("+Missile Triangles:"))
+	case 48:
 		return HUD_OBJECT_MISSILE_TRI;
 
-	if(optional_string("+Kills:"))
+	case 49:
 		return HUD_OBJECT_KILLS;
 
-	if(optional_string("+Fixed Messages:"))
+	case 50:
 		return HUD_OBJECT_FIXED_MESSAGES;
 
-	if(optional_string("+Flight Path Marker:"))
+	case 51:
 		return HUD_OBJECT_FLIGHT_PATH;
 
-	if ( optional_string("+Warhead Count:") )
+	case 52:
 		return HUD_OBJECT_WARHEAD_COUNT;
 
-	if ( optional_string("+Hardpoints:") )
+	case 53:
 		return HUD_OBJECT_HARDPOINTS;
 
-	if ( optional_string("+Primary Weapons:") )
+	case 54:
 		return HUD_OBJECT_PRIMARY_WEAPONS;
 
-	if ( optional_string("+Secondary Weapons:") )
+	case 55:
 		return HUD_OBJECT_SECONDARY_WEAPONS;
 
-	return -1;
+	case -1:	// -noparseerrors is set
+		return -1;
+
+	default:
+		Assertion(false, "This should never happen.\n");	// Impossible return value from required_string_one_of()
+		return -1;
+	}
 }
 
 void load_gauge(int gauge, int base_w, int base_h, int hud_font, bool scale_gauge, SCP_vector<int>* ship_idx, color *use_clr)
@@ -1041,7 +1047,8 @@
 		load_gauge_secondary_weapons(base_w, base_h, hud_font, scale_gauge, ship_idx, use_clr);
 		break;
 	default:
-		Warning(LOCATION, "Invalid gauge found in hud_gauges.tbl");
+		// It's either -1, indicating we're ignoring a parse error, or it's a coding error.
+		Assertion(gauge == -1, "Invalid value '%d' passed to load_gauge(); get a coder!\n", gauge);
 		break;
 	}
 }
hudparse2.cpp.patch (7,116 bytes)   

m_m

2014-08-08 08:46

developer   ~0016186

I prefer the first patch as the optional_string() calls make clear what type of gauge is being parsed and adding a new gauge type only involves adding two lines at one place as opposed to adding string to the required_string_of and adding a case.
You changed the Warning about the unrecognized gauge to an Assertion, can that assertion be hit if there is a user data error?
Your comment above the Assertion suggests that that could be the case.

MageKing17

2014-08-08 14:46

developer   ~0016189

The Assertion() will specifically never trigger if gauge is -1, which is our indicator of ignoring bad user data with -noparseerrors, so it should only be triggering on an actual coding mistake.

m_m

2014-08-08 14:48

developer   ~0016190

Fix committed to trunk@10978.

Related Changesets

fs2open: trunk r10978

2014-08-08 11:16

m_m


Ported: N/A

Details Diff
From MageKing17: Fix for Mantis 3085: Unhelpful error reporting in hud_gauges.tbl if an invalid gauge type is specified. Affected Issues
0003085
mod - /trunk/fs2_open/code/hud/hudparse.cpp Diff File

Issue History

Date Modified Username Field Change
2014-08-06 00:16 MageKing17 New Issue
2014-08-06 00:16 MageKing17 Status new => assigned
2014-08-06 00:16 MageKing17 Assigned To => MageKing17
2014-08-06 00:16 MageKing17 File Added: hudparse.cpp.patch
2014-08-06 00:26 MageKing17 File Added: hudparse2.cpp.patch
2014-08-06 00:28 MageKing17 Status assigned => code review
2014-08-08 08:46 m_m Note Added: 0016186
2014-08-08 14:46 MageKing17 Note Added: 0016189
2014-08-08 14:48 m_m Changeset attached => fs2open trunk r10978
2014-08-08 14:48 m_m Note Added: 0016190
2014-08-08 14:48 m_m Status code review => resolved
2014-08-08 14:48 m_m Resolution open => fixed