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 |