View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002151 | FSSCP | tables | public | 2010-03-13 08:45 | 2010-05-02 05:43 |
| Reporter | FUBAR-BDHR | Assigned To | Genghis | ||
| Priority | normal | Severity | feature | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.6.12 RC1 | ||||
| Fixed in Version | 3.6.12 | ||||
| Summary | 0002151: No error checking for engine wash info. | ||||
| Description | Set up engine wash info and edited all ships in ships.tbl to use them. Needed to test new builds and see if any ships were using invalid engine wash values so I ran the build and loaded the basic all ships mission. No errors. Figured OK no one added engine wash values to any .tbm files. Go to add it to the first .tbm and it already had engine wash set to a value not in ships.tbl. It wasn't caught and neither were any other ships with invalid wash values. Guessing (haven't looked at the code) that there is no error checking for this. | ||||
| Additional Information | 3.6.13 r6005. | ||||
| Tags | No tags attached. | ||||
|
|
Candidate fix uploaded as fix_2151.diff |
|
2010-03-18 04:47
|
fix_2151.diff (597 bytes)
Index: code/ship/ship.cpp
===================================================================
--- code/ship/ship.cpp (revision 6012)
+++ code/ship/ship.cpp (working copy)
@@ -2804,6 +2804,9 @@
stuff_string(name_tmp, F_NAME, sizeof(name_tmp));
// get and set index
sp->engine_wash_pointer = get_engine_wash_pointer(name_tmp);
+
+ if(sp->engine_wash_pointer == NULL)
+ Warning(LOCATION,"Invalid engine wash name %s specified for subsystem %s in ship class %s", name_tmp, sp->subobj_name, sip->name);
}
parse_sound("$AliveSnd:", &sp->alive_snd, sp->subobj_name);
|
|
|
Error messages are A-1 SUPAR. But is the engine wash null-checked everywhere it should be? |
|
|
Isn't the engine wash allowed to be null? It's not a required value it just needs to be checked if it is supplied. I've never been able to get an answer on what is supposed to happen if you don't supply a value. Is it supposed to be set to Default or just no engine wash at all. |
|
|
If it's NULL it's fine, the default engine wash it applied to huge ships and the wash is skipped entirely for other ships. It's what happens when no wash is specified at all, and half the ships in retail FS2 are like that. Chain of reasoning: 1. In modelread.cpp in read_model_file(), "model_subsystem.engine_wash_pointer" becomes "thruster_bank.wash_info_pointer". 2. In shipfx.cpp in engine_wash_ship_process(), if bank.wash_info_pointer is NULL and the ship flags include SIF_HUGE_SHIP and there's any wash info, then it is set to &Engine_wash_info[0]. If the ship is not huge, then it skips the remainder of the processing for this particular thruster bank. |
|
|
I did a sweep of the codebase looking for other places where it uses "stuff_string" and then needs the result to mean something, and added warnings where appropriate. The results are in extra_error_checking.diff. |
|
|
I don't know if it needs another ticket, but aside from any fix for this bug I'd at least prefer if the other stuff_string fixing was done in a separate patch. |
|
|
That's already the case. The extra patch doesn't contain the engine-wash fix. I shall also be moving it to a separate ticket, as per conversation with Goober on the forum. |
|
|
Bah I totally missed that there were 2 files earlier :P |
|
|
Found a problem with this when dealing with tables and .tbm files. If the value is incorrect in the table but overridden by a valid value in the .tbm you get warnings. |
|
|
I added a note in 0002159 about the table/.tbm problem. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2010-03-13 08:45 | FUBAR-BDHR | New Issue | |
| 2010-03-18 04:46 | Genghis | Note Added: 0011791 | |
| 2010-03-18 04:47 | Genghis | File Added: fix_2151.diff | |
| 2010-03-19 07:06 | Goober5000 | Note Added: 0011796 | |
| 2010-03-19 07:27 | FUBAR-BDHR | Note Added: 0011797 | |
| 2010-03-19 13:34 | Genghis | Note Added: 0011799 | |
| 2010-03-20 00:47 | Genghis | Status | new => assigned |
| 2010-03-20 00:47 | Genghis | Assigned To | => Genghis |
| 2010-03-22 02:18 | Genghis | File Added: extra_error_checking.diff | |
| 2010-03-22 02:19 | Genghis | Note Added: 0011819 | |
| 2010-03-22 02:23 | Genghis | Note Edited: 0011819 | |
| 2010-03-22 12:15 | Genghis | File Deleted: extra_error_checking.diff | |
| 2010-03-22 12:15 | Genghis | File Added: extra_error_checking.diff | |
| 2010-03-22 16:25 | chief1983 | Note Added: 0011820 | |
| 2010-03-22 17:40 | Genghis | Note Added: 0011821 | |
| 2010-03-22 17:50 | Genghis | File Deleted: extra_error_checking.diff | |
| 2010-03-22 17:51 | chief1983 | Note Added: 0011822 | |
| 2010-04-02 07:15 | FUBAR-BDHR | Note Added: 0011850 | |
| 2010-04-04 15:27 | Genghis | Note Added: 0011853 | |
| 2010-05-02 05:43 | karajorma | Status | assigned => resolved |
| 2010-05-02 05:43 | karajorma | Fixed in Version | => 3.6.12 |
| 2010-05-02 05:43 | karajorma | Resolution | open => fixed |
