Author Topic: Modular Mainhall.tbl  (Read 29324 times)

0 Members and 1 Guest are viewing this topic.

Offline mjn.mixael

  • Cutscene Master
  • 212
  • Chopped liver
    • Steam
    • Twitter
I tested the patches. They definitely load sounds by string which is great! However, I tried to have it load a sound by string that wasn't already in the sounds.tbl and it never worked. It simply played the "mouse-click beep" instead.

Here's the table entry
Code: [Select]
+Door sounds: itemdraw clrsnd
'itemdraw' exists in the sounds.tbl, but 'clrsnd' doesn't. I created a wav file with a simple 3 second tone and saved it to data/sounds as 'clrsnd'. (I also tried .ogg) However, it never played that sound file. Does this patch still require the sounds to be tabled in sounds.tbl?

I've included a log to hopefully help debug.

[attachment deleted by a basterd]
Cutscene Upgrade Project - Mainhall Remakes - Between the Ashes
Youtube Channel - P3D Model Box
Between the Ashes is looking for committed testers, PM me for details.
Freespace Upgrade Project See what's happening.

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
Yes, the code as of now still requires the sounds to be in sounds.tbl. To be honest I'm not sure what would be required to get it to search other places. A more experienced coder will have to chime in. Regardless, I'm going to deal with Misc Anim Sounds first before getting to that.

Oh, and regarding parsing mainhall.tbl once at startup, I compiled and tested a release build with the changed behaviour and it seems to work fine. I moved from different game states (loaded a mission etc) and back into the mainhall and everything worked as it should. As far as I can see there's no hidden dependencies, so if mjn.mixael or someone else would like to test and verify the patch then I think we'll be good.
« Last Edit: December 05, 2011, 12:38:05 am by CommanderDJ »
[16:57] <CommanderDJ> What prompted the decision to split WiH into acts?
[16:58] <battuta> it was long, we wanted to release something
[16:58] <battuta> it felt good to have a target to hit
[17:00] <RangerKarl> not sure if talking about strike mission, or jerking off
[17:00] <CommanderDJ> WUT
[17:00] <CommanderDJ> hahahahaha
[17:00] <battuta> hahahaha
[17:00] <RangerKarl> same thing really, if you think about it

 

Offline mjn.mixael

  • Cutscene Master
  • 212
  • Chopped liver
    • Steam
    • Twitter
Yes, the code as of now still requires the sounds to be in sounds.tbl. To be honest I'm not sure what would be required to get it to search other places. A more experienced coder will have to chime in. Regardless, I'm going to deal with Misc Anim Sounds first before getting to that.

Hmm, well the whole reason I wanted to get it to load by filename was to bypass the sounds.tbl in the first place. (Making the mainhall.tbl much more independent from another non-modular table. So I, too, would like to hear what the experienced coders have to say on this. Perhaps it'd be much too difficult or not possible at all, which would make me sad... but I'd understand.

Loading by string is still easier because modders would only need to add the sounds to the sounds.tbl and it wouldn't matter where.

Oh, and regarding parsing mainhall.tbl once at startup, I compiled and tested a release build with the changed behaviour and it seems to work fine. I moved from different game states (loaded a mission etc) and back into the mainhall and everything worked as it should. As far as I can see there's no hidden dependencies, so if mjn.mixael or someone else would like to test and verify the patch then I think we'll be good.

I can test it, just send me the patch.
Cutscene Upgrade Project - Mainhall Remakes - Between the Ashes
Youtube Channel - P3D Model Box
Between the Ashes is looking for committed testers, PM me for details.
Freespace Upgrade Project See what's happening.

 

Offline Iss Mneur

  • 210
  • TODO:
Hmm, well the whole reason I wanted to get it to load by filename was to bypass the sounds.tbl in the first place. (Making the mainhall.tbl much more independent from another non-modular table. So I, too, would like to hear what the experienced coders have to say on this. Perhaps it'd be much too difficult or not possible at all, which would make me sad... but I'd understand.

Loading by string is still easier because modders would only need to add the sounds to the sounds.tbl and it wouldn't matter where.
I was looking into this last night.  I actually thought that parse_sound implemented the direct load from file behaviour you are looking for, though having looked at it, it obviously can't.  That being said, I am sure that either weapons or ships are able to load untabled sounds so that would be where I would look.
"I love deadlines. I like the whooshing sound they make as they fly by." -Douglas Adams
wxLauncher 0.9.4 public beta (now with no config file editing for FRED) | wxLauncher 2.0 Request for Comments

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
Oh, and regarding parsing mainhall.tbl once at startup, I compiled and tested a release build with the changed behaviour and it seems to work fine. I moved from different game states (loaded a mission etc) and back into the mainhall and everything worked as it should. As far as I can see there's no hidden dependencies, so if mjn.mixael or someone else would like to test and verify the patch then I think we'll be good.
I can test it, just send me the patch.

Here it is:

Code: [Select]
Index: code/freespace2/freespace.cpp
===================================================================
--- code/freespace2/freespace.cpp (revision 8067)
+++ code/freespace2/freespace.cpp (working copy)
@@ -2025,6 +2025,11 @@
  old_alpha_colors_init();
  }
 
+#ifdef NDEBUG
+ //if this is a release build, we only need to read in mainhall.tbl once at startup
+ main_hall_read_table();
+#endif
+
  if (Cmdline_env) {
  ENVMAP = Default_env_map = bm_load("cubemap");
  }
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8067)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -369,9 +369,6 @@
 static int Main_hall_f1_text_frame = 0;
 static int F1_text_done = 0;
 
-// read in main hall table
-void main_hall_read_table();
-
 // "press f1" for help stuff
 #define MAIN_HALL_HELP_TIME 5000
 int Main_hall_help_stamp = -1;
@@ -509,8 +506,10 @@
  int idx,s_idx;
  char temp[100], whee[100];
 
+#ifndef NDEBUG
  // read in the main hall table
  main_hall_read_table();
+#endif
 
  if(!Num_main_halls) {
  Error(LOCATION, "No main halls were loaded to initialize.");
Index: code/menuui/mainhallmenu.h
===================================================================
--- code/menuui/mainhallmenu.h (revision 8067)
+++ code/menuui/mainhallmenu.h (working copy)
@@ -18,6 +18,9 @@
 // initialize the main hall proper
 void main_hall_init(int main_hall_num);
 
+// read in main hall table
+void main_hall_read_table();
+
 // do a frame for the main hall
 void main_hall_do(float frametime);
 

Make sure you test a release build though since that's the one with the changed behaviour. Thanks, hopefully once it's verified that nothing's broken this one can get committed.


Hmm, well the whole reason I wanted to get it to load by filename was to bypass the sounds.tbl in the first place. (Making the mainhall.tbl much more independent from another non-modular table. So I, too, would like to hear what the experienced coders have to say on this. Perhaps it'd be much too difficult or not possible at all, which would make me sad... but I'd understand.

Loading by string is still easier because modders would only need to add the sounds to the sounds.tbl and it wouldn't matter where.
I was looking into this last night.  I actually thought that parse_sound implemented the direct load from file behaviour you are looking for, though having looked at it, it obviously can't.  That being said, I am sure that either weapons or ships are able to load untabled sounds so that would be where I would look.

So are we intending to change parse_sound itself so that if it fails to load the sound the usual way it will search folders/VPs? Or just change it specifically for these flags? Personally I'd rather the first one, but I'm unsure of what consequences that might have.
[16:57] <CommanderDJ> What prompted the decision to split WiH into acts?
[16:58] <battuta> it was long, we wanted to release something
[16:58] <battuta> it felt good to have a target to hit
[17:00] <RangerKarl> not sure if talking about strike mission, or jerking off
[17:00] <CommanderDJ> WUT
[17:00] <CommanderDJ> hahahahaha
[17:00] <battuta> hahahaha
[17:00] <RangerKarl> same thing really, if you think about it

 

Offline mjn.mixael

  • Cutscene Master
  • 212
  • Chopped liver
    • Steam
    • Twitter
I tested the patch. I played some missions, went to the tech room, switched between multiple campaigns/mainhalls. It seems to work as expected. :yes:
Cutscene Upgrade Project - Mainhall Remakes - Between the Ashes
Youtube Channel - P3D Model Box
Between the Ashes is looking for committed testers, PM me for details.
Freespace Upgrade Project See what's happening.

 

Offline Goober5000

  • HLP Loremaster
  • Moderator
  • 214
    • Goober5000 Productions
Hmm, well the whole reason I wanted to get it to load by filename was to bypass the sounds.tbl in the first place. (Making the mainhall.tbl much more independent from another non-modular table. So I, too, would like to hear what the experienced coders have to say on this. Perhaps it'd be much too difficult or not possible at all, which would make me sad... but I'd understand.
A game sound has to be stored in sounds.tbl; there's no way around that without completely rewriting the sound code.  About the most you could hope for is being able to add sounds to sounds.tbl on-the-fly, but doing that would be approximately as hard as making sounds.tbl modular.  (The two are essentially the same thing, if you think about it.)

  

Offline mjn.mixael

  • Cutscene Master
  • 212
  • Chopped liver
    • Steam
    • Twitter
Hmm, honest question here. How are scripts and the play-sound-from-file sexp able to do it? Is that possibly applicable?
Cutscene Upgrade Project - Mainhall Remakes - Between the Ashes
Youtube Channel - P3D Model Box
Between the Ashes is looking for committed testers, PM me for details.
Freespace Upgrade Project See what's happening.

 

Offline Goober5000

  • HLP Loremaster
  • Moderator
  • 214
    • Goober5000 Productions
Scripts, I dunno.  But play-sound-from-file uses a special music (not sound) handle that's sort of suspended above the rest of the music system.

 

Offline Iss Mneur

  • 210
  • TODO:
Hmm, honest question here. How are scripts and the play-sound-from-file sexp able to do it? Is that possibly applicable?
play-sound-from-file uses the music subsystem (which is why it can only do one at a time).

I can't find where the lua loads a sound file from disk.  I have found where it interacts with the sound.tbl, but which seems to be done by loading from the tbl by index or tabled name.

I have been unable to find the code that I thought existed for weapons.tbl and/or ships.tbl... :(

So I started trawling my archive of incomplete patches, apparently I am conflating reality with a discussion (on the internal SCP board) that Fury, chief1983, and I had over a year ago about sounds.tbl as a result of mantis 2240.  In essence it discuses how to make sounds.tbl modular (at least from a tabler perspective).  I still think it is doable, but it would have to go through antipodes because of how fundamentally it would change the internals of the sound manager. It would also be a lot of work.

In the meantime, ComanderDJ, I think it would be best to finish fixing up mainhall.tbl and then tackling the issues with sound so as to avoid feature creep. The proposed format is workable in face of future extension when the sound manager supports it.  So I think you should just leave the parse_sounds behaviour as you have patched and skip the ability to dynamically load sounds that are untabled for the time being. That is, only support sounds loaded by number or by tabled name for now.

If you or anyone else would like to presue modular sounds.tbl I am willing to provide guidance, otherwise it will have to wait until my plate clears.  This feature would probably endanger sound.tbls future existence and  entail removing all hard coded indexes from the interface, including (possibly) the ability to control all "default" sounds via tbl.  Yes, this is a huge order and this is why I keep putting it off/taking so long. It is basically a rewrite on order with the HUD rewrite.
"I love deadlines. I like the whooshing sound they make as they fly by." -Douglas Adams
wxLauncher 0.9.4 public beta (now with no config file editing for FRED) | wxLauncher 2.0 Request for Comments

 

Offline jg18

  • A very happy zod
  • 210
  • can do more than spellcheck
otherwise it will have to wait until my plate clears.
When was the last time that happened? :p

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
In the meantime, ComanderDJ, I think it would be best to finish fixing up mainhall.tbl and then tackling the issues with sound so as to avoid feature creep. The proposed format is workable in face of future extension when the sound manager supports it.  So I think you should just leave the parse_sounds behaviour as you have patched and skip the ability to dynamically load sounds that are untabled for the time being. That is, only support sounds loaded by number or by tabled name for now.

That is what I was planning to do. I will get Misc Anim Sounds working first, then get to the other issues in this thread. We can deal with sounds.tbl later. Sorry mjn, but it seems your request deals with an entirely different area as Iss Mneur and Goober have explained. Perhaps I will see what I can do with his help after all this mainhall stuff is done.


I tested the patch. I played some missions, went to the tech room, switched between multiple campaigns/mainhalls. It seems to work as expected. :yes:

Since mjn.mixael and I have both tested the changed mainhall.tbl parsing, is there any chance that could be committed? Personally I think the changes to the sound parsing are ready as well, but I would want the changes to the mainhall flags to all be committed in one go, so I'd like that to wait until Misc Anim Sounds has been sorted out.
That said, I'll let the powers that be decide when this stuff is ready to go in. If there's anything else you need from me to bring the patches up to scratch I'm more than happy to help.
[16:57] <CommanderDJ> What prompted the decision to split WiH into acts?
[16:58] <battuta> it was long, we wanted to release something
[16:58] <battuta> it felt good to have a target to hit
[17:00] <RangerKarl> not sure if talking about strike mission, or jerking off
[17:00] <CommanderDJ> WUT
[17:00] <CommanderDJ> hahahahaha
[17:00] <battuta> hahahaha
[17:00] <RangerKarl> same thing really, if you think about it

 

Offline pecenipicek

  • Roast Chicken
  • 211
  • Powered by copious amounts of coffee and nicotine
    • Minecraft
    • Skype
    • Steam
    • Twitter
    • PeceniPicek's own deviantart page
Might i reccomend splitting up the differing release <-> debug behaviour into a cmdline flag? in my mind, both should behave the same, except the debug build yells at you loudly when you **** up?

My reccomendation is basically as follows:

Default behaviour, load mainhalls only once at start, if flag ticked, reparse every time?




Just a slight reccomendation.
Skype: vrganjko
Ho, ho, ho, to the bottle I go
to heal my heart and drown my woe!
Rain may fall and wind may blow,
and many miles be still to go,
but under a tall tree I will lie!

The Apocalypse Project needs YOU! - recruiting info thread.

 

Offline mjn.mixael

  • Cutscene Master
  • 212
  • Chopped liver
    • Steam
    • Twitter
Sadness, but that makes sense IssMneur.

String functionality is still a large step forward since the sounds don't have to be in any particular sound slot this way.
Cutscene Upgrade Project - Mainhall Remakes - Between the Ashes
Youtube Channel - P3D Model Box
Between the Ashes is looking for committed testers, PM me for details.
Freespace Upgrade Project See what's happening.

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
Okay, I've figured out a possible solution to Misc Anim Sounds.
Code: [Select]
required_string("+Misc anim sounds:");
stuff_int(&m->misc_anim_special_sounds[idx][0]);
parse_sound_list("", &m->misc_anim_special_sounds[idx][1], "+Misc anim sounds:", m->misc_anim_special_sounds[idx][0], 0, true);

It parses the first entry (which specifies the number of sounds) as usual, then passes the rest of the entries to parse_sound_list. The only drawback is that because I've had to use a blank tag, the warning within parse_sound_list won't specify what exactly has gone wrong if it flares up.

The only other solution I can see without writing new functions is to use parse_sound_core directly (it's part of my sound patch earlier in the thread), which would result in the correct warning message, but it would also be duplicating some code from the parse_sound function, and since parse_sound_core is meant to be a helper function I would prefer the first option. That said, I'll do what SCP feels is best.

With this solution, here's the patch containing all three flag extensions (you'll need the sound patch applied for this to work):
Code: [Select]
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8068)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -1660,8 +1660,7 @@
  }
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom sound id
- required_string("+Intercom sound:");
- stuff_int(&m->intercom_sounds[idx]);
+ parse_sound("+Intercom sound:", &m->intercom_sounds[idx], "+Intercom sound:", true);
  }
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom pan
@@ -1716,9 +1715,7 @@
  // anim sound id
  required_string("+Misc anim sounds:");
  stuff_int(&m->misc_anim_special_sounds[idx][0]);
- for(s_idx=0; s_idx<m->misc_anim_special_sounds[idx][0]; s_idx++){
- stuff_int(&m->misc_anim_special_sounds[idx][s_idx + 1]);
- }
+ parse_sound_list("", &m->misc_anim_special_sounds[idx][1], "+Misc anim sounds:", m->misc_anim_special_sounds[idx][0], 0, true);
  }
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim sound triggers
@@ -1761,9 +1758,7 @@
  }
  for(idx=0; idx<m->num_door_animations; idx++){
  // door open and close sounds
- required_string("+Door sounds:");
- stuff_int(&m->door_sounds[idx][0]);
- stuff_int(&m->door_sounds[idx][1]);
+ parse_sound_list("+Door sounds:", &m->door_sounds[idx][0], "+Door sounds:", 2, 0, true);
  }
  for(idx=0; idx<m->num_door_animations; idx++){
  // door pan value
« Last Edit: December 06, 2011, 06:16:10 pm by CommanderDJ »
[16:57] <CommanderDJ> What prompted the decision to split WiH into acts?
[16:58] <battuta> it was long, we wanted to release something
[16:58] <battuta> it felt good to have a target to hit
[17:00] <RangerKarl> not sure if talking about strike mission, or jerking off
[17:00] <CommanderDJ> WUT
[17:00] <CommanderDJ> hahahahaha
[17:00] <battuta> hahahaha
[17:00] <RangerKarl> same thing really, if you think about it

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
Sorry guys, my bad, I realised that I had made a fair few changes to the sound patch since the last time it was posted. Here it is:
Code: [Select]
Index: code/gamesnd/gamesnd.cpp
===================================================================
--- code/gamesnd/gamesnd.cpp (revision 8067)
+++ code/gamesnd/gamesnd.cpp (working copy)
@@ -52,6 +52,30 @@
  return -1;
 }
 
+int gamesnd_get_by_iface_name(char* name)
+{
+ Assert( Snds_iface.size() <= INT_MAX );
+ Assert( Snds_iface.size() == Snds_iface_handle.size() );
+ 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))
+ {
+ return i;
+ }
+ }
+ else if(!strnicmp(snd->filename, name, p-snd->filename))
+ {
+ return i;
+ }
+ i++;
+ }
+ return -1;
+}
+
 int gamesnd_get_by_tbl_index(int index)
 {
  //if we get passed -1, don't bother trying to look it up.
@@ -85,40 +109,111 @@
 }
 
 /**
- * Parse a sound
- *
+ * Helper function for parse_sound and parse_sound_list. Do not use directly.
+ *
  * @param tag Tag
  * @param idx_dest Sound index destination
  * @param object_name Object name being parsed
+ * @param is_interface_sound Whether the sound is an interface sound or not (defaults to false)
+ * @param buf Buffer holding string to be parsed
  *
- * This also means you shouldn't use optional_string or required_string,
- * just make sure the destination sound index can handle -1 if things
- * don't work out.
  */
-void parse_sound(char* tag, int *idx_dest, char* object_name)
+void parse_sound_core(char* tag, int *idx_dest, char* object_name, bool is_interface_sound, char* buf)
 {
- char buf[MAX_FILENAME_LEN];
  int idx;
 
- if(optional_string(tag))
+ if(is_interface_sound)
+ idx = gamesnd_get_by_iface_name(buf);
+ else
+ idx = gamesnd_get_by_name(buf);
+
+ if(idx != -1)
  {
- stuff_string(buf, F_NAME, MAX_FILENAME_LEN);
- idx = gamesnd_get_by_name(buf);
- if(idx != -1)
- (*idx_dest) = idx;
+ (*idx_dest) = idx;
+ }
+ else
+ {
+ if(is_interface_sound)
+ idx = gamesnd_get_by_iface_tbl_index(atoi(buf));
  else
- {
  idx = gamesnd_get_by_tbl_index(atoi(buf));
- if (idx != -1)
- (*idx_dest) = idx;
- }
 
- Assert( Snds.size() <= INT_MAX );
- //Ensure sound is in range
- if((*idx_dest) < -1 || (*idx_dest) >= (int)Snds.size())
+ if (idx != -1)
+ (*idx_dest) = idx;
+ }
+
+ int size_to_check = 0;
+
+ if(is_interface_sound)
+ {
+ size_to_check = Snds_iface.size();
+ Assert( Snds_iface.size() == Snds_iface_handle.size() );
+ }
+ else
+ {
+ size_to_check = Snds.size();
+ }
+
+ Assert( size_to_check <= INT_MAX );
+
+ //Ensure sound is in range
+ if((*idx_dest) < -1 || (*idx_dest) >= (int)size_to_check)
+ {
+ (*idx_dest) = -1;
+ Warning(LOCATION, "%s sound index out of range on '%s'. Must be between 0 and %d. Forcing to -1 (Nonexistant sound).\n", tag, object_name, size_to_check);
+ }
+}
+
+/**
+ * Parse a sound. When using this function for a table entry,
+ * required_string and optional_string aren't needed, as this function deals with
+ * that as its tag parameter, just make sure that the destination sound index can
+ * handle -1 if things don't work out.
+ *
+ * @param tag Tag
+ * @param idx_dest Sound index destination
+ * @param object_name Object name being parsed
+ * @param is_interface_sound Whether the sound is an interface sound or not (defaults to false)
+ *
+ */
+void parse_sound(char* tag, int *idx_dest, char* object_name, bool is_interface_sound)
+{
+ if(optional_string(tag))
+ {
+ char buf[MAX_FILENAME_LEN];
+ stuff_string(buf, F_NAME, MAX_FILENAME_LEN);
+
+ parse_sound_core(tag, idx_dest, object_name, is_interface_sound, buf);
+ }
+}
+
+/**
+ * CommanderDJ - Parse a list of sounds. When using this function for a table entry,
+ * required_string and optional_string aren't needed, as this function deals with
+ * that as its tag parameter, just make sure that the destination sound index(es) can
+ * handle -1 if things don't work out.
+ *
+ * @param tag Tag
+ * @param idx_dest Destination of first sound index in list
+ * @param object_name Object name being parsed
+ * @param num Number of sounds to parse (defaults to 1)
+ * @param offset Element to start filling at (defaults to 0)
+ * @param is_interface_sound Whether the sound is an interface sound or not (defaults to false)
+ *
+ */
+void parse_sound_list(char* tag, int* idx_dest, char* object_name, int num, int offset, bool is_interface_sound)
+{
+ if(optional_string(tag))
+ {
+ //sanity checks
+ Assert(num>0);
+ Assert(offset>=0);
+
+ for(int i=0; i<num; i++)
  {
- (*idx_dest) = -1;
- Warning(LOCATION, "%s sound index out of range on '%s'. Must be between 0 and %d. Forcing to -1 (Nonexistant sound).\n", tag, object_name, Snds.size());
+ char buf[MAX_FILENAME_LEN];
+ stuff_string_white(buf, MAX_FILENAME_LEN);
+ parse_sound_core(tag, &idx_dest[i+offset], object_name, is_interface_sound, buf);
  }
  }
 }
Index: code/gamesnd/gamesnd.h
===================================================================
--- code/gamesnd/gamesnd.h (revision 8067)
+++ code/gamesnd/gamesnd.h (working copy)
@@ -28,13 +28,16 @@
 void gamesnd_play_iface(int n);
 void gamesnd_play_error_beep();
 int gamesnd_get_by_name(char* name);
+int gamesnd_get_by_iface_name(char* name);
 int gamesnd_get_by_tbl_index(int index);
 int gamesnd_get_by_iface_tbl_index(int index);
 
 //This should handle NO_SOUND just fine since it doesn't directly access lowlevel code
 //Does all parsing for a sound
-void parse_sound(char* tag, int *idx_dest, char* object_name);
+void parse_sound(char* tag, int* idx_dest, char* object_name, bool is_interface_sound = false);
 
+void parse_sound_list(char* tag, int* idx_dest, char* object_name, int num = 1, int offset = 0, bool is_interface_sound = false);
+
 // this is a callback, so it needs to be a real function
 void common_play_highlight_sound();
 

[16:57] <CommanderDJ> What prompted the decision to split WiH into acts?
[16:58] <battuta> it was long, we wanted to release something
[16:58] <battuta> it felt good to have a target to hit
[17:00] <RangerKarl> not sure if talking about strike mission, or jerking off
[17:00] <CommanderDJ> WUT
[17:00] <CommanderDJ> hahahahaha
[17:00] <battuta> hahahaha
[17:00] <RangerKarl> same thing really, if you think about it

 

Offline Iss Mneur

  • 210
  • TODO:
Default behaviour, load mainhalls only once at start, if flag ticked, reparse every time?
As much as I am opposed to adding more cmdline flags, this does make sense.  If you want to debug the mainhall you use the -reparse_mainhall_everytime.

otherwise it will have to wait until my plate clears.
When was the last time that happened? :p
Its been a while, but either way there is a queue, I will get to it eventually....
"I love deadlines. I like the whooshing sound they make as they fly by." -Douglas Adams
wxLauncher 0.9.4 public beta (now with no config file editing for FRED) | wxLauncher 2.0 Request for Comments

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
WARNING: WALL OF TEXT!

I believe that flag extension of Intercom Sound, Misc Anim Sounds and Door Sounds is complete and ready for testing. However, a few significant changes have been made since the last update, so I'll go through them one by one (this next bit is primarily for coders. If you just want to test, scroll down to the bottom of the post and apply the patch in the attachment):

Changes to the sound parsing code
Now, I will be describing all the changes made in detail, even ones covered earlier in the thread, for clarity and completeness' sake. I like my work to be transparent to all involved. Unless any issues come up during testing this will be the final version of the updated sound parsing code. Let's dig right in.

  • gamesnd_get_by_iface_name has been introduced. This is an almost direct copy and paste of gamesnd_get_by_name except it accesses the Snds_iface vector.
  • A very slight optimisation was added to gamesnd_get_by_iface_tbl_index.
  • parse_sound_core was introduced. This extracts the shared functionality of parse_sound and the new parse_sound_list into a helper function. parse_sound_core was extended to be able to lookup the Snds_iface vector as well as Snds. This is controlled via an optional boolean parameter called is_interface_sound, which defaults to false so that existing code doesn't break. parse_sound_core is now also able to handle both sound indexes and file names, looking up the appropriate index in sounds.tbl.
  • parse_sound_list was introduced. This parses a list of sound entries into a vector (I'll get to the reasons for this shortly). It supports two formats: the first follows the style of retail Misc Anim Sounds, where the first entry is the number of sounds following, and the rest of the entries are the sounds themselves. The second is a new one (which if I hear correctly may or may not be used by the potentially upcoming interface.tbl) where all the entries are simply sounds.
And here is the patch for the sounds changes:
Code: [Select]
Index: code/gamesnd/gamesnd.cpp
===================================================================
--- code/gamesnd/gamesnd.cpp (revision 8068)
+++ code/gamesnd/gamesnd.cpp (working copy)
@@ -52,6 +52,30 @@
  return -1;
 }
 
+int gamesnd_get_by_iface_name(char* name)
+{
+ Assert( Snds_iface.size() <= INT_MAX );
+ Assert( Snds_iface.size() == Snds_iface_handle.size() );
+ 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))
+ {
+ return i;
+ }
+ }
+ else if(!strnicmp(snd->filename, name, p-snd->filename))
+ {
+ return i;
+ }
+ i++;
+ }
+ return -1;
+}
+
 int gamesnd_get_by_tbl_index(int index)
 {
  //if we get passed -1, don't bother trying to look it up.
@@ -71,6 +95,9 @@
 
 int gamesnd_get_by_iface_tbl_index(int index)
 {
+ //if we get passed -1, don't bother trying to look it up.
+ if (index == -1)
+ return -1;
  Assert( Snds_iface.size() <= INT_MAX );
  Assert( Snds_iface.size() == Snds_iface_handle.size() );
  int i = 0;
@@ -85,41 +112,131 @@
 }
 
 /**
- * Parse a sound
- *
+ * Helper function for parse_sound and parse_sound_list. Do not use directly.
+ *
  * @param tag Tag
  * @param idx_dest Sound index destination
  * @param object_name Object name being parsed
+ * @param is_interface_sound Whether the sound is an interface sound or not (defaults to false)
+ * @param buf Buffer holding string to be parsed
  *
- * This also means you shouldn't use optional_string or required_string,
- * just make sure the destination sound index can handle -1 if things
- * don't work out.
  */
-void parse_sound(char* tag, int *idx_dest, char* object_name)
+void parse_sound_core(char* tag, int *idx_dest, char* object_name, bool is_interface_sound, char* buf)
 {
- char buf[MAX_FILENAME_LEN];
  int idx;
 
+ if(is_interface_sound)
+ idx = gamesnd_get_by_iface_name(buf);
+ else
+ idx = gamesnd_get_by_name(buf);
+
+ if(idx != -1)
+ {
+ (*idx_dest) = idx;
+ }
+ else
+ {
+ if(is_interface_sound)
+ idx = gamesnd_get_by_iface_tbl_index(atoi(buf));
+ else
+ idx = gamesnd_get_by_tbl_index(atoi(buf));
+
+ if (idx != -1)
+ (*idx_dest) = idx;
+ }
+
+ int size_to_check = 0;
+
+ if(is_interface_sound)
+ {
+ size_to_check = Snds_iface.size();
+ Assert( Snds_iface.size() == Snds_iface_handle.size() );
+ }
+ else
+ {
+ size_to_check = Snds.size();
+ }
+
+ Assert( size_to_check <= INT_MAX );
+
+ //Ensure sound is in range
+ if((*idx_dest) < -1 || (*idx_dest) >= (int)size_to_check)
+ {
+ (*idx_dest) = -1;
+ Warning(LOCATION, "%s sound index out of range on '%s'. Must be between 0 and %d. Forcing to -1 (Nonexistent sound).\n", tag, object_name, size_to_check);
+ }
+}
+
+/**
+ * Parse a sound. When using this function for a table entry,
+ * required_string and optional_string aren't needed, as this function deals with
+ * that as its tag parameter, just make sure that the destination sound index can
+ * handle -1 if things don't work out.
+ *
+ * @param tag Tag
+ * @param idx_dest Sound index destination
+ * @param object_name Object name being parsed
+ * @param is_interface_sound Whether the sound is an interface sound or not (defaults to false)
+ *
+ */
+void parse_sound(char* tag, int *idx_dest, char* object_name, bool is_interface_sound)
+{
  if(optional_string(tag))
  {
+ char buf[MAX_FILENAME_LEN];
  stuff_string(buf, F_NAME, MAX_FILENAME_LEN);
- idx = gamesnd_get_by_name(buf);
- if(idx != -1)
- (*idx_dest) = idx;
- else
+
+ parse_sound_core(tag, idx_dest, object_name, is_interface_sound, buf);
+ }
+}
+
+/**
+ * CommanderDJ - Parse a list of sounds. When using this function for a table entry,
+ * required_string and optional_string aren't needed, as this function deals with
+ * that as its tag parameter, just make sure that the destination sound index(es) can
+ * handle -1 if things don't work out.
+ *
+ * @param destination Vector where sound indexes are to be stored
+ * @param tag Tag
+ * @param object_name Name of object being parsed
+ * @param SCP_format Format of list to be parsed (defaults to false)
+ * False: First entry in list is number of sounds (retail format).
+ * True: Every entry is a sound.
+ * @param is_interface_sound Whether the sound is an interface sound or not (defaults to false)
+ *
+ */
+void parse_sound_list(char* tag, SCP_vector<int>& destination, char* object_name, bool SCP_format, bool is_interface_sound)
+{
+ if(optional_string(tag))
+ {
+ //complain if the list is empty
+ if(!destination.empty())
  {
- idx = gamesnd_get_by_tbl_index(atoi(buf));
- if (idx != -1)
- (*idx_dest) = idx;
+ Warning(LOCATION, "%s in '%s' is not an empty list! Its values will be overwritten!", tag, object_name);
  }
 
- Assert( Snds.size() <= INT_MAX );
- //Ensure sound is in range
- if((*idx_dest) < -1 || (*idx_dest) >= (int)Snds.size())
+ int check=0;
+
+ //if we're using the old format, parse the first entry separately
+ if(!SCP_format)
  {
- (*idx_dest) = -1;
- Warning(LOCATION, "%s sound index out of range on '%s'. Must be between 0 and %d. Forcing to -1 (Nonexistant sound).\n", tag, object_name, Snds.size());
+ stuff_int(&check);
  }
+
+ //now read the rest of the entries on the line
+ for(int i=0;!check_for_eoln();i++)
+ {
+ char buf[MAX_FILENAME_LEN];
+ stuff_string_white(buf, MAX_FILENAME_LEN);
+ destination.push_back(-1);
+ parse_sound_core(tag, &destination.at(i), object_name, is_interface_sound, buf);
+ }
+
+ //if we're using the old format, double check the size)
+ if(!SCP_format && (destination.size() != (unsigned)check))
+ {
+ Warning(LOCATION, "%s in '%s' has %i entries. This does not match entered size of %i.", tag, object_name, destination.size(), check);
+ }
  }
 }
 
Index: code/gamesnd/gamesnd.h
===================================================================
--- code/gamesnd/gamesnd.h (revision 8068)
+++ code/gamesnd/gamesnd.h (working copy)
@@ -28,13 +28,16 @@
 void gamesnd_play_iface(int n);
 void gamesnd_play_error_beep();
 int gamesnd_get_by_name(char* name);
+int gamesnd_get_by_iface_name(char* name);
 int gamesnd_get_by_tbl_index(int index);
 int gamesnd_get_by_iface_tbl_index(int index);
 
 //This should handle NO_SOUND just fine since it doesn't directly access lowlevel code
 //Does all parsing for a sound
-void parse_sound(char* tag, int *idx_dest, char* object_name);
+void parse_sound(char* tag, int* idx_dest, char* object_name, bool is_interface_sound = false);
 
+void parse_sound_list(char* tag, SCP_vector<int>& destination, char* object_name, bool SCP_format, bool is_interface_sound);
+
 // this is a callback, so it needs to be a real function
 void common_play_highlight_sound();

Changes to general parsing code
  • This is only a small addition, but it was necessary for me to add a check_for_eoln() function.

Here it is:
Code: [Select]
Index: code/parse/parselo.cpp
===================================================================
--- code/parse/parselo.cpp (revision 8068)
+++ code/parse/parselo.cpp (working copy)
@@ -430,6 +430,15 @@
  return 0;
 }
 
+int check_for_eoln()
+{
+ ignore_gray_space();
+
+ if(*Mp == EOLN)
+ return 1;
+ else
+ return 0;
+}
 // similar to optional_string, but just checks if next token is a match.
 // It doesn't advance Mp except to skip past white space.
 //
Index: code/parse/parselo.h
===================================================================
--- code/parse/parselo.h (revision 8068)
+++ code/parse/parselo.h (working copy)
@@ -155,6 +155,7 @@
 extern int check_for_string(char *pstr);
 extern int check_for_string_raw(char *pstr);
 extern int check_for_eof();
+extern int check_for_eoln();
 
 // from aicode.cpp
 extern void parse_float_list(float *plist, int size);

Changes to mainhall code
Several things to go through here.
  • Extension of Intercom Sound, Misc Anim Sounds, and Door Sounds flags to accept filenames. Ah, finally a feature that was actually asked for! There's not much more to say here, but using the updated parse_sound and parse_sound_list functions, these flags now accept filenames in their entries, not just sound indexes. The files still have to be in sounds.tbl, but at least now they don't have to be in any particular slot.
  • Conversion of the misc_anim_special_sounds and door_sounds arrays into vectors. Ah yes, I mentioned this before. After extensive discussion with Iss Mneur, we decided that converting these arrays (and eventually all of the arrays in the mainhall struct) into vectors was necessary - most immediately so that parse_sound_list would play nice (and so we could support both the retail and the new formats for sound lists), but also to make modularisation of mainhall.tbl easier in the future.
Here's the code for these changes:
Code: [Select]
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8068)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -107,8 +107,8 @@
  // panning values for each of the misc anims
  float misc_anim_sound_pan[MAX_MISC_ANIMATIONS];
 
- // [N][0] == # of sounds, [N][1-9] sound index
- int misc_anim_special_sounds[MAX_MISC_ANIMATIONS][10];
+ //sounds for each of the misc anims
+ SCP_vector<SCP_vector<int>> misc_anim_special_sounds;
 
  // [N][0] == # of triggers, [N][1-9] >= frame num
  int misc_anim_special_trigger[MAX_MISC_ANIMATIONS][10];
@@ -133,7 +133,7 @@
  int door_anim_coords[MAX_DOOR_ANIMATIONS][4];
 
  // sounds for each region (open/close)
- int door_sounds[MAX_DOOR_ANIMATIONS][2];
+ SCP_vector<SCP_vector<int>> door_sounds;
 
  // pan values for the door sounds
  float door_sound_pan[MAX_DOOR_ANIMATIONS];
@@ -1182,7 +1182,7 @@
  }
  // animation is not paused
  else {
- for (s_idx = Main_hall->misc_anim_special_sounds[idx][0]; s_idx > 0; s_idx--) {
+ for (s_idx = Main_hall->misc_anim_special_sounds.at(idx).size(); s_idx > 0; s_idx--) {
  // if we've passed the trigger point, then play the sound and break out of the loop
  if ((Main_hall_misc_anim[idx].current_frame >= Main_hall->misc_anim_special_trigger[idx][s_idx]) && !Main_hall->misc_anim_sound_flag[idx][s_idx]) {
  Main_hall->misc_anim_sound_flag[idx][s_idx] = 1;
@@ -1195,7 +1195,7 @@
  }
 
  // play the sound
- Main_hall->misc_anim_sound_handles[idx][s_idx] = snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds[idx][s_idx]],Main_hall->misc_anim_sound_pan[idx]);
+ Main_hall->misc_anim_sound_handles[idx][s_idx] = snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan[idx]);
  break;
  }
  }
@@ -1308,7 +1308,7 @@
  if (Main_hall_door_sound_handles[region] != -1) {
  snd_stop(Main_hall_door_sound_handles[region]);
  }
- Main_hall_door_sound_handles[region] = snd_play(&Snds_iface[Main_hall->door_sounds[region][1]], Main_hall->door_sound_pan[region]);
+ Main_hall_door_sound_handles[region] = snd_play(&Snds_iface[Main_hall->door_sounds.at(region).at(1)], Main_hall->door_sound_pan[region]);
 
  //TODO: track current frame
  snd_set_pos(Main_hall_door_sound_handles[region], &Snds_iface[SND_MAIN_HALL_DOOR_CLOSE],
@@ -1334,7 +1334,7 @@
  if(Main_hall_door_sound_handles[region] != -1){
  snd_stop(Main_hall_door_sound_handles[region]);
  }
- Main_hall_door_sound_handles[region] = snd_play(&Snds_iface[Main_hall->door_sounds[region][0]],Main_hall->door_sound_pan[region]);
+ Main_hall_door_sound_handles[region] = snd_play(&Snds_iface[Main_hall->door_sounds.at(region).at(0)],Main_hall->door_sound_pan[region]);
 
  // start the sound playing at the right spot relative to the completion of the animation
  if( (Main_hall_door_anim[region].num_frames > 0) && (Main_hall_door_anim[region].current_frame != -1) ) {
@@ -1660,8 +1660,7 @@
  }
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom sound id
- required_string("+Intercom sound:");
- stuff_int(&m->intercom_sounds[idx]);
+ parse_sound("+Intercom sound:", &m->intercom_sounds[idx], "+Intercom sound:", true);
  }
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom pan
@@ -1714,11 +1713,9 @@
  }
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim sound id
- required_string("+Misc anim sounds:");
- stuff_int(&m->misc_anim_special_sounds[idx][0]);
- for(s_idx=0; s_idx<m->misc_anim_special_sounds[idx][0]; s_idx++){
- stuff_int(&m->misc_anim_special_sounds[idx][s_idx + 1]);
- }
+ SCP_vector<int> temp; //put *something* in the vector so we don't cause an exception when calling parse_sound_list()
+ m->misc_anim_special_sounds.push_back(temp);
+ parse_sound_list("+Misc anim sounds:", m->misc_anim_special_sounds.at(idx), "+Misc anim sounds:", false, true);
  }
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim sound triggers
@@ -1761,9 +1758,9 @@
  }
  for(idx=0; idx<m->num_door_animations; idx++){
  // door open and close sounds
- required_string("+Door sounds:");
- stuff_int(&m->door_sounds[idx][0]);
- stuff_int(&m->door_sounds[idx][1]);
+ SCP_vector<int> temp; //put *something* in the vector so we don't cause an exception when calling parse_sound_list()
+ m->door_sounds.push_back(temp);
+ parse_sound_list("+Door sounds:", m->door_sounds.at(idx), "+Door sounds:", true, true);
  }
  for(idx=0; idx<m->num_door_animations; idx++){
  // door pan value
@@ -1790,10 +1787,10 @@
  if(Vasudan_funny){
  int hall = main_hall_id();
 
- Main_hall_defines[GR_640][hall].door_sounds[OPTIONS_REGION][0] = SND_VASUDAN_BUP;
- Main_hall_defines[GR_640][hall].door_sounds[OPTIONS_REGION][1] = SND_VASUDAN_BUP;
- Main_hall_defines[GR_1024][hall].door_sounds[OPTIONS_REGION][0] = SND_VASUDAN_BUP;
- Main_hall_defines[GR_1024][hall].door_sounds[OPTIONS_REGION][1] = SND_VASUDAN_BUP;
+ Main_hall_defines[GR_640][hall].door_sounds.at(OPTIONS_REGION).at(0) = SND_VASUDAN_BUP;
+ Main_hall_defines[GR_640][hall].door_sounds.at(OPTIONS_REGION).at(1) = SND_VASUDAN_BUP;
+ Main_hall_defines[GR_1024][hall].door_sounds.at(OPTIONS_REGION).at(0) = SND_VASUDAN_BUP;
+ Main_hall_defines[GR_1024][hall].door_sounds.at(OPTIONS_REGION).at(1) = SND_VASUDAN_BUP;
 
  // set head anim. hehe
  strcpy_s(Main_hall_defines[GR_640][hall].door_anim_name[OPTIONS_REGION], "vhallheads");

But wait, there's more! More features to come in the near future! In rough order of completion, you can expect:
  • A -reparse_mainhall commandline flag. This was proposed by pecenipicek and approved by Iss Mneur: instead of changing mainhall.tbl parsing behaviour by type of build, default behaviour will be changed so that mainhall.tbl is parsed once at startup. If you're debugging the mainhall and want it to be parsed every time a hall is loaded, you just enable the -reparse_mainhall flag and away you go!
  • A new mainhall.tbl flag, Misc Anim Sounds New. This will work the same way as Misc Anim Sounds, except it will no longer require the first entry to be the number of sounds following.
  • Complete vectorisation of the main_hall_defines struct. As said previously, in order to make the transition to a modular mainhall.tbl easier, the flexibility and built-in error checking of vectors will replace the old fixed-size arrays within main_hall_defines.

That plus the other remaining features named in the thread:
Quote
3. Introducing name identifiers for mainhalls and updating the code to use these to refer to mainhalls rather than indexes, and adding the accompanying +Name flag to mainhall.tbl.
4. Changing the mainhall array to a vector and thus making mainhalls dynamic, and updating the code accordingly.
5. Modularizing mainhall.tbl.
You can expect more progress in the coming days if all goes to plan, I'm loving working on this so far!

Testers!
Want to test the newly extended mainhall.tbl flags without wading through my long-winded explanations or applying patch after patch? Well you're in luck because you only have to apply one! Simply download the attached file, which contains all the necessary code changes, apply it and build! Report back with any findings, good or bad! All feedback is read and taken into account!

EDIT: Just to be clear, no changes are needed to mainhall.tbl syntax to test these, except to replace sound indexes with filenames at the appropriate flags. Include filenames without quotes. Extensions are optional, as they will be ignored anyway.

[attachment deleted by a basterd]
« Last Edit: December 07, 2011, 07:40:28 am by CommanderDJ »
[16:57] <CommanderDJ> What prompted the decision to split WiH into acts?
[16:58] <battuta> it was long, we wanted to release something
[16:58] <battuta> it felt good to have a target to hit
[17:00] <RangerKarl> not sure if talking about strike mission, or jerking off
[17:00] <CommanderDJ> WUT
[17:00] <CommanderDJ> hahahahaha
[17:00] <battuta> hahahaha
[17:00] <RangerKarl> same thing really, if you think about it

 

Offline mjn.mixael

  • Cutscene Master
  • 212
  • Chopped liver
    • Steam
    • Twitter
Getting a crash here. Debug yielded some interesting errors. Log attached.

The crash happens when I switch campaigns/mainhalls or go to the ready room and back out. I'm guessing it'll happen with other game state changes as well. Interestingly the first mainhall loads fine, it's when the game is reloading the mainhall that seems to cause the issue.

[attachment deleted by a basterd]
Cutscene Upgrade Project - Mainhall Remakes - Between the Ashes
Youtube Channel - P3D Model Box
Between the Ashes is looking for committed testers, PM me for details.
Freespace Upgrade Project See what's happening.

 

Offline pecenipicek

  • Roast Chicken
  • 211
  • Powered by copious amounts of coffee and nicotine
    • Minecraft
    • Skype
    • Steam
    • Twitter
    • PeceniPicek's own deviantart page
Getting a crash here. Debug yielded some interesting errors. Log attached.

The crash happens when I switch campaigns/mainhalls or go to the ready room and back out. I'm guessing it'll happen with other game state changes as well. Interestingly the first mainhall loads fine, it's when the game is reloading the mainhall that seems to cause the issue.
the **** does this flag do? " -disable_di_mouse" (not related to problem, just curious)
Skype: vrganjko
Ho, ho, ho, to the bottle I go
to heal my heart and drown my woe!
Rain may fall and wind may blow,
and many miles be still to go,
but under a tall tree I will lie!

The Apocalypse Project needs YOU! - recruiting info thread.