View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003085 | FSSCP | tables | public | 2014-08-06 00:16 | 2014-08-08 14:48 |
| Reporter | MageKing17 | Assigned To | MageKing17 | ||
| Priority | low | Severity | feature | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Summary | 0003085: Unhelpful error reporting in hud_gauges.tbl if an invalid gauge type is specified. | ||||
| Description | Per 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. | ||||
| Tags | No tags attached. | ||||
|
|
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;
}
}
|
|
|
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;
}
}
|
|
|
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. |
|
|
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. |
|
|
Fix committed to trunk@10978. |
| 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 |