View Issue Details

IDProjectCategoryView StatusLast Update
0002695FSSCPsoundpublic2012-09-11 10:02
ReporterMjnMixael Assigned Tom_m  
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.6.13 
Fixed in Version3.7.2 
Summary0002695: Mainhall sound search by filename causes runtime error
DescriptionIn the mainhall.tbl, you can specify sounds by sound name (from sounds.tbl) or by the sound filename.

With the addition of modular sounds, specifying a sound by filename causes a runtime error.
Steps To ReproduceOpen mainhall.tbl and replace any sound index (+Door Sounds: ## ##) with a sound filename. (+Door Sounds: sound1.wav sound2.wav) Note: These sounds still have to exist in the sounds.tbl.
Additional InformationSearching for sounds by filename was added to the mainhall.tbl's abilities to ease the need for working with the previously limited sounds.tbl.

Now that sounds.tbl is modular this feature is not necessary. An easy fix is to remove that ability and just require the sound's name from sounds.tbl
TagsNo tags attached.

Activities

Echelon9

2012-08-11 04:30

developer   ~0013901

What is the runtime error, or call stack, when this happens?

MjnMixael

2012-08-11 04:36

manager   ~0013902

Runtime error may have been the wrong term. FSO ends and Windows tells me that it has ended in an unusual way. I get no further info and there is nothing in the log.

Valathil

2012-08-19 10:43

developer   ~0013924

adding a reproducable testcase would help narrowing it down

MjnMixael

2012-08-22 00:28

manager  

mainhall.tbl (18,127 bytes)

MjnMixael

2012-08-22 00:28

manager   ~0013931

Use this mainhall.tbl. Run FSO and try to open the Ready Room door.

m_m

2012-08-25 12:47

developer   ~0013946

The mainhall code never checks if the sound it wants to play is actually a valid sound index and therefore uses garbage data when the sound index was set to -1 in when the sounds are parsed.
Also the modular sound table change removed the ability to parse sounds from file names for interface sounds which would most likely break backwards compatibility.
I uploaded a patch which adds checks to ensure that the sound index that is about to be played is valid and the patch also adds back the file name parsing for interface sound which I accidentally removed in the modular table change.

m_m

2012-08-25 12:48

developer  

mainhallSounds.patch (3,925 bytes)   
Index: code/gamesnd/gamesnd.cpp
===================================================================
--- code/gamesnd/gamesnd.cpp	(revision 9143)
+++ code/gamesnd/gamesnd.cpp	(working copy)
@@ -99,7 +99,33 @@
 	Assert( Snds_iface.size() <= INT_MAX );
 	Assert( Snds_iface.size() == Snds_iface_handle.size() );
 	
-	return gamesnd_lookup_name(name, Snds_iface);
+	int index = gamesnd_lookup_name(name, Snds_iface);
+
+	if (index < 0)
+	{
+		int i = 0;
+		for(SCP_vector<game_snd>::iterator snd = Snds_iface.begin(); snd != Snds_iface.end(); ++snd)
+		{
+			char *p = strrchr( snd->filename, '.' );
+			if(p == NULL)
+			{
+				if(!stricmp(snd->filename, name))
+				{
+					index = i;
+					break;
+				}
+			}
+			else if(!strnicmp(snd->filename, name, p-snd->filename))
+			{
+				index = i;
+				break;
+			}
+
+			i++;
+		}
+	}
+
+	return index;
 }
 
 int gamesnd_get_by_tbl_index(int index)
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp	(revision 9143)
+++ code/menuui/mainhallmenu.cpp	(working copy)
@@ -1200,8 +1200,14 @@
 							snd_stop(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx));
 						}
 
-						// play the sound
-						snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan.at(idx));
+						int sound = Main_hall->misc_anim_special_sounds.at(idx).at(s_idx);
+
+						// Check if the sound is valid
+						if (sound >= 0)
+						{
+							// play the sound
+							snd_play(&Snds_iface[sound],Main_hall->misc_anim_sound_pan.at(idx));
+						}
 						break;
 					}
 				}
@@ -1331,8 +1337,14 @@
 		if (Main_hall_door_sound_handles.at(region) != -1) {
 			snd_stop(Main_hall_door_sound_handles.at(region));
 		}
-		Main_hall_door_sound_handles.at(region) = snd_play(&Snds_iface[Main_hall->door_sounds.at(region).at(1)], Main_hall->door_sound_pan.at(region));
 
+		int sound = Main_hall->door_sounds.at(region).at(1);
+
+		if (sound >= 0)
+		{
+			Main_hall_door_sound_handles.at(region) = snd_play(&Snds_iface[sound], Main_hall->door_sound_pan.at(region));
+		}
+
 		//TODO: track current frame
 		snd_set_pos(Main_hall_door_sound_handles.at(region), &Snds_iface[SND_MAIN_HALL_DOOR_CLOSE], 
 			(float)((Main_hall_door_anim.at(region).keyframe) ? Main_hall_door_anim.at(region).keyframe : 
@@ -1369,8 +1381,14 @@
 	if (Main_hall_door_sound_handles.at(region) != -1) {
 		snd_stop(Main_hall_door_sound_handles.at(region));
 	}
-	Main_hall_door_sound_handles.at(region) = snd_play(&Snds_iface[Main_hall->door_sounds.at(region).at(0)],Main_hall->door_sound_pan.at(region));
 
+	int sound = Main_hall->door_sounds.at(region).at(0);
+
+	if (sound >= 0)
+	{
+		Main_hall_door_sound_handles.at(region) = snd_play(&Snds_iface[sound],Main_hall->door_sound_pan.at(region));
+	}
+
 	// start the sound playing at the right spot relative to the completion of the animation
 	if ( (Main_hall_door_anim.at(region).num_frames > 0) && (Main_hall_door_anim.at(region).current_frame != -1) ) {
 		snd_set_pos(Main_hall_door_sound_handles.at(region),&Snds_iface[SND_MAIN_HALL_DOOR_OPEN], 
@@ -1458,11 +1476,17 @@
 
 		// if the timestamp has popped, play a sound
 		if ( (Main_hall_next_intercom_sound_stamp != -1) && (timestamp_elapsed(Main_hall_next_intercom_sound_stamp)) ) {
-			// play the sound
-			Main_hall_intercom_sound_handle = snd_play(&Snds_iface.at(Main_hall->intercom_sounds.at(Main_hall_next_intercom_sound)));			
+			int sound = Main_hall->intercom_sounds.at(Main_hall_next_intercom_sound);
+			
+			// Check if the sound is valid
+			if (sound >= 0)
+			{
+				// play the sound
+				Main_hall_intercom_sound_handle = snd_play(&Snds_iface.at(sound));			
 
-			// unset the timestamp
-			Main_hall_next_intercom_sound_stamp = -1;
+				// unset the timestamp
+				Main_hall_next_intercom_sound_stamp = -1;
+			}
 		}
 	}
 	// if the sound is playing
mainhallSounds.patch (3,925 bytes)   

MjnMixael

2012-08-28 18:07

manager   ~0013950

attached patch fixes the issue.

iss_mneur

2012-09-11 03:23

developer   ~0013959

The patch looks fine to me.

niffiwan

2012-09-11 10:02

developer   ~0013964

Committed in r9194

Issue History

Date Modified Username Field Change
2012-08-10 19:15 MjnMixael New Issue
2012-08-11 04:30 Echelon9 Note Added: 0013901
2012-08-11 04:36 MjnMixael Note Added: 0013902
2012-08-19 10:43 Valathil Note Added: 0013924
2012-08-22 00:28 MjnMixael File Added: mainhall.tbl
2012-08-22 00:28 MjnMixael Note Added: 0013931
2012-08-25 12:47 m_m Note Added: 0013946
2012-08-25 12:48 m_m File Added: mainhallSounds.patch
2012-08-26 00:48 Echelon9 Status new => code review
2012-08-28 18:07 MjnMixael Note Added: 0013950
2012-09-11 03:23 iss_mneur Note Added: 0013959
2012-09-11 03:23 iss_mneur Assigned To => iss_mneur
2012-09-11 03:23 iss_mneur Status code review => assigned
2012-09-11 10:02 niffiwan Note Added: 0013964
2012-09-11 10:02 niffiwan Status assigned => resolved
2012-09-11 10:02 niffiwan Fixed in Version => 3.7.2
2012-09-11 10:02 niffiwan Resolution open => fixed
2012-09-11 10:02 niffiwan Assigned To iss_mneur => m_m