2019-12-08 00:29 EST


View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002151FSSCPtablespublic2010-05-02 01:43
ReporterFUBAR-BDHR 
Assigned ToGenghis 
PrioritynormalSeverityfeatureReproducibilityalways
StatusresolvedResolutionfixed 
Product Version3.6.12 RC1 
Target VersionFixed in Version3.6.12 
Summary0002151: No error checking for engine wash info.
DescriptionSet 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 Information3.6.13 r6005.
TagsNo tags attached.
Attached Files
  • diff file icon fix_2151.diff (597 bytes) 2010-03-18 00:47 -
    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);
    
    diff file icon fix_2151.diff (597 bytes) 2010-03-18 00:47 +

-Relationships
+Relationships

-Notes

~0011791

Genghis (developer)

Candidate fix uploaded as fix_2151.diff

~0011796

Goober5000 (administrator)

Error messages are A-1 SUPAR. But is the engine wash null-checked everywhere it should be?

~0011797

FUBAR-BDHR (developer)

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.

~0011799

Genghis (developer)

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.

~0011819

Genghis (developer)

Last edited: 2010-03-21 22:23

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.

~0011820

chief1983 (administrator)

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.

~0011821

Genghis (developer)

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.

~0011822

chief1983 (administrator)

Bah I totally missed that there were 2 files earlier :P

~0011850

FUBAR-BDHR (developer)

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.

~0011853

Genghis (developer)

I added a note in 0002159 about the table/.tbm problem.
+Notes

-Issue History
Date Modified Username Field Change
2010-03-13 03:45 FUBAR-BDHR New Issue
2010-03-18 00:46 Genghis Note Added: 0011791
2010-03-18 00:47 Genghis File Added: fix_2151.diff
2010-03-19 03:06 Goober5000 Note Added: 0011796
2010-03-19 03:27 FUBAR-BDHR Note Added: 0011797
2010-03-19 09:34 Genghis Note Added: 0011799
2010-03-19 20:47 Genghis Status new => assigned
2010-03-19 20:47 Genghis Assigned To => Genghis
2010-03-21 22:18 Genghis File Added: extra_error_checking.diff
2010-03-21 22:19 Genghis Note Added: 0011819
2010-03-21 22:23 Genghis Note Edited: 0011819
2010-03-22 08:15 Genghis File Deleted: extra_error_checking.diff
2010-03-22 08:15 Genghis File Added: extra_error_checking.diff
2010-03-22 12:25 chief1983 Note Added: 0011820
2010-03-22 13:40 Genghis Note Added: 0011821
2010-03-22 13:50 Genghis File Deleted: extra_error_checking.diff
2010-03-22 13:51 chief1983 Note Added: 0011822
2010-04-02 03:15 FUBAR-BDHR Note Added: 0011850
2010-04-04 11:27 Genghis Note Added: 0011853
2010-05-02 01:43 karajorma Status assigned => resolved
2010-05-02 01:43 karajorma Fixed in Version => 3.6.12
2010-05-02 01:43 karajorma Resolution open => fixed
+Issue History