Source Code Project Mantis - FSSCP
View Issue Details
0002240FSSCPsoundpublic2010-06-26 18:372010-07-22 00:07
ReporterThe_E 
Assigned Toiss_mneur 
PrioritynormalSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version3.6.12 RC3 
Target VersionFixed in Version 
Summary0002240: sounds.tbl indices must be sequential
DescriptionIn order to work around an issue with a hardcoded sound index, Fury assigned indices starting with 300 for our custom sounds. This leads to a parsing error at gamesnd.cpp, line 81, as the game uses num_game_sounds to check for an entries' validity. This works, as long as the entries are in sequential order, but breaks down once a gap is introduced. This seems pretty bad to me, as it introduces yet another instance of undocumented magic-number type behaviour.
TagsNo tags attached.
Attached Filespatch mantis_2240_v2.patch (12,673) 2010-07-19 21:57
http://scp.indiegames.us/mantis/file_download.php?file_id=1542&type=bug

Notes
(0012146)
Goober5000   
2010-06-27 03:08   
Why, exactly, is this a problem? This isn't magic number behavior, it's violating the game's assumption that the sounds listing is continuous.

And what was the issue that caused the sounds to begin at 300?
(0012148)
The_E   
2010-06-27 10:51   
Well, Fury put the sounds there to avoid any collisions between BP's sounds and FSO's fixed indices. The problem was that this behaviour wasn't documented anywhere (which I have fixed).
Still, if at some point someone wants to tbm-ify this tbl, provisions have to be made for non-continuous indices, IMHO.
(0012215)
Echelon9   
2010-07-10 08:08   
This really should become a STL type to remove the need to separately track a num_game_sounds variable.
(0012239)
iss_mneur   
2010-07-19 01:14   
I have written a patch that changes the Snds, Snds_iface, and Snds_iface_handle to be SCP_vectors. It turns the sounds number into just a magic number that refers to the sound that is needed, which can be used anywhere.

The attached patch does not deal with changes to the set of hardcoded sound indexes that the engine uses. That is, sounds in sounds.tbl must be in the same contiguous order that they are now up to the last hardcoded index (thats 201 for general sounds, and 61 for interface sounds). After that the numbers do not have to be contiguous.

I realize that is this not the most complete solution to the problem, but it does remove the necessity of the entries after the hardcoded entries being in sequential order.
(0012240)
chief1983   
2010-07-19 12:28   
The general solution to this was just to fill in the gaps with empty entries that can be filled in later, that's what FotG was doing. It's working for the short term at least.
(0012242)
iss_mneur   
2010-07-19 12:41   
Yes, I realize that. This patch removes the need to do that for any sound.tbl entry after the Hardcoded entries (201 for general sounds, 61 for interface sounds).
(0012243)
chief1983   
2010-07-19 12:50   
Awesome. That would make FotG's sound guys very happy I think.
(0012249)
iss_mneur   
2010-07-19 21:56   
I have attached another version of the patch that deals with some issues in lua.cpp and sexp.cpp to deal with any cases where a tbler can use an index that are not part of the contiguous set from the retail sounds.tbl.
(0012252)
iss_mneur   
2010-07-21 22:28   
Setting ticket to resolved, as primary issue that prompted the bug has been fixed with the second patch provided. As the second patch has not caused further issues it looks to be a satisfactory resolution.
(0012253)
chief1983   
2010-07-22 00:07   
I would normally have recommended waiting to resolve until the RC gets thorough testing, but this is recent enough I'm not likely to forget about it :P

Issue History
2010-06-26 18:37The_ENew Issue
2010-06-27 03:08Goober5000Note Added: 0012146
2010-06-27 10:51The_ENote Added: 0012148
2010-07-10 08:08Echelon9Note Added: 0012215
2010-07-19 01:14iss_mneurNote Added: 0012239
2010-07-19 01:14iss_mneurAssigned To => iss_mneur
2010-07-19 01:14iss_mneurStatusnew => feedback
2010-07-19 01:16iss_mneurFile Added: mantis_2240.patch
2010-07-19 12:28chief1983Note Added: 0012240
2010-07-19 12:41iss_mneurNote Added: 0012242
2010-07-19 12:50chief1983Note Added: 0012243
2010-07-19 21:56iss_mneurNote Added: 0012249
2010-07-19 21:57iss_mneurFile Added: mantis_2240_v2.patch
2010-07-19 21:57iss_mneurFile Deleted: mantis_2240.patch
2010-07-21 22:28iss_mneurNote Added: 0012252
2010-07-21 22:28iss_mneurStatusfeedback => resolved
2010-07-21 22:28iss_mneurResolutionopen => fixed
2010-07-22 00:07chief1983Note Added: 0012253