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

0 Members and 1 Guest are viewing this topic.

Offline CaptJosh

  • 210
An excellent point. So, basically... (the following is pseudocode)
Code: [Select]
if (isset($name)) {
 $name = $mainhall;
} else { $mainhall == $index; }
« Last Edit: November 02, 2011, 09:27:10 am by CaptJosh »
CaptJosh

There are only 10 kinds of people in the world;
those who understand binary and those who don't.

 

Offline Goober5000

  • HLP Loremaster
  • Administrator
  • 214
    • Goober5000 Productions
See, but wouldn't referencing mainhalls by name would require a change in how campaign files work.. and likely pilot files too?
The newest pilot code (still in antipodes) references ships and weapons by name, rather than index.  So this could be introduced at the same time.

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
Bumping for great justice; I'm going to start having a look at this over the next few days and seeing what parts I can implement. Before I do however, I just want to make sure that this is what we want:

1. Dynamic mainhalls: currently the code limits us to 10 mainhalls at maximum, and 2 of those are really ever used, wasting memory. To save memory and to future-proof the code, we want the code to treat mainhalls dynamically.

2. Mainhall names: currently the code refers to mainhalls only by their index into the 2-dimensional Main_hall_defines array (someone correct me on this if I'm wrong). Once mainhalls are made dynamic (using vectors), indexes will no longer be a reliable method of identifying a particular mainhall. Thus mainhalls should have a table-defined Name property that will be used as its identifier. If no name is specified, the code should assign whatever index the mainhalls happens to be loaded at as its name.

3. Modular mainhalls (the reason this thread exists, lol): Enough said, really. After 1 and 2 we want to be able to use modular tables for mainhalls.

Discuss! Once the requirements have been established I'll see what I can do (with the SCP's help) to start implementing them.
[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
  • Anims: 420, Cutscenes: 10, Mainhalls: 7, Logos: 52
    • Steam
    • Twitter
    • Mix-Hai Productions
That all looks pretty good and what I'm hoping for.

Now, this is something else I started thinking about which may be a case of me asking for cake AND cupcakes for desert... but I won't know until I ask.

The table would still be limited in it's modular ability because of the sounds. I know there's a lot of issues to making the sounds.tbl modular... So...

Would it be possible to expand the mainhall.tbl flags "+Intercom sound:" "+Misc anim sounds:" and "+Door sounds:" to also accept a filename string? (Similar to table entries like "$Warpout Start Sound:" in the ships.tbl?
Cutscene Upgrade Project - Mainhall Remakes - MixaelANITools - Between the Ashes - MjnMixael's Render Boutique - Mix-Hai Productions
Youtube Channel - P3D Model Box - Photobucket Albums - Model Releases - Downloads
Between the Ashes is looking for committed testers, PM me for details.
Report MediaVP issues, now on the MediaVP Mantis! Read all about it Here!

 

Offline Goober5000

  • HLP Loremaster
  • Administrator
  • 214
    • Goober5000 Productions
Would it be possible to expand the mainhall.tbl flags "+Intercom sound:" "+Misc anim sounds:" and "+Door sounds:" to also accept a filename string? (Similar to table entries like "$Warpout Start Sound:" in the ships.tbl?
This would be the easiest feature in the thread. :p  All you'd have to do is perform a lookup in sounds.tbl if the stuff after the colon is text rather than a number.


One more design change would also be warranted.  Currently, I believe mainhall.tbl is parsed each and every time the mainhall screen is displayed.  It would be better to parse it once at game start.

 

Offline karajorma

  • King Louie - Jungle VIP
  • Administrator
  • 214
    • Karajorma's Freespace FAQ
Parse it every time in debug then! Anybody working on changing the main hall would prefer that. :p
Karajorma's Freespace FAQ. It's almost like asking me yourself.

[ Diaspora ] - [ Seeds Of Rebellion ] - [ Mind Games ]

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
Okay, here's the order in which I'll work on these features:

1. Expansion of "+Intercom sound:" "+Misc anim sounds:" and "+Door sounds:" flags to accept filename strings.
2. Having mainhall.tbl parsed only once at game load in release builds and every time a mainhall is displayed in debug builds (this is current behaviour for all builds).
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.

If all goes as planned, expect to see a patch for number 1 later today.
« Last Edit: December 03, 2011, 08:04:07 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 mjn.mixael

  • Cutscene Master
  • 212
  • Anims: 420, Cutscenes: 10, Mainhalls: 7, Logos: 52
    • Steam
    • Twitter
    • Mix-Hai Productions
OK, this post probably has more info than necessary, but I like to be thorough since few people have actually looked into the mainhall.tbl and it's quirks, but here is a rundown of the different sound flags and how they work/might work with string ability... as requested.

Here's how the sound stuff might work with string ability. The three different sections are all slightly different, so I'll go through each one.

Things to consider for all three: Would the file extension need to be included? (EX: SOUND or SOUND.EXT) Some of the flags list multiple sounds in one line. Should those get quotes or how would spaces be handled in filenames?

Intercom Sounds
Code: [Select]
+Num Intercom Sounds: 1
+Intercom delay: 8000 15000
+Intercom sound: 38
+Intercom pan: 0.0
The entries are fairly self explanatory. The flag in question for giving string ability will be '+Intercom sound:' where it currently gets the index to the sound from the sounds.tbl. This flag should only ever have one sound listed (but the flag itself can be listed the number of times equal to the number of total intercom sounds). An example with string ability.
Code: [Select]
+Num Intercom Sounds: 1
+Intercom delay: 8000 15000
+Intercom sound: 'SOUND.EXT'
+Intercom pan: 0.0

Misc Anim Sounds
Code: [Select]
+Num Misc Animations: 1
+Misc anim: LOWgal-m1
+Misc anim delay: -1 9000 30000
+Misc anim coords: 14 14
+Misc anim mode: 2
+Misc anim pan: -0.5
+Misc anim sounds: 3 67 68 67
+Misc anim trigger: 3 1 20 42
+Misc anim handles: 3
+Misc anim flags: 1
Misc anims are probably the most complicated sound wise. The flag you need to pay attention to is '+Misc anim sounds:'. It's current syntax has the first number being the total number of sounds followed by the index numbers for the sounds listed in the sounds.tbl. Again, tehre can be multiple of this flag matching to each misc anim. An example with string ability.
Code: [Select]
+Num Misc Animations: 1
+Misc anim: LOWgal-m1
+Misc anim delay: -1 9000 30000
+Misc anim coords: 14 14
+Misc anim mode: 2
+Misc anim pan: -0.5
+Misc anim sounds: 3 'SOUND 1.EXT' 'SOUND 2.EXT' 'SOUND 3.EXT'
+Misc anim trigger: 3 1 20 42
+Misc anim handles: 3
+Misc anim flags: 1

Door Anim Sounds
Code: [Select]
+Num Door Animations: 1
+Door anim: LOWgal-d1
+Door coords: 68 260 103 298
+Door sounds: 23 24
+Door pan: -0.7
And again for door animations. However, these will always only have two sounds listed in '+Door sounds:' (Mouse over, mouse off). An example with string ability.
Code: [Select]
+Num Door Animations: 1
+Door anim: LOWgal-d1
+Door coords: 68 260 103 298
+Door sounds: 'SOUND1.EXT' 'SOUND2.EXT'
+Door pan: -0.7
Cutscene Upgrade Project - Mainhall Remakes - MixaelANITools - Between the Ashes - MjnMixael's Render Boutique - Mix-Hai Productions
Youtube Channel - P3D Model Box - Photobucket Albums - Model Releases - Downloads
Between the Ashes is looking for committed testers, PM me for details.
Report MediaVP issues, now on the MediaVP Mantis! Read all about it Here!

 

Offline Iss Mneur

  • 210
  • TODO:
Things to consider for all three: Would the file extension need to be included? (EX: SOUND or SOUND.EXT) Some of the flags list multiple sounds in one line. Should those get quotes or how would spaces be handled in filenames?
In .tbl files, the extension of a file should not be used (if a .tbl requires an extension for a filename then that is a bug).

As far as I know, spaces are forbidden in file names in FSO.
"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
Here's a first patch for feature number 1!

Code: [Select]
Index: code/gamesnd/gamesnd.cpp
===================================================================
--- code/gamesnd/gamesnd.cpp (revision 8063)
+++ 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.
Index: code/gamesnd/gamesnd.h
===================================================================
--- code/gamesnd/gamesnd.h (revision 8063)
+++ code/gamesnd/gamesnd.h (working copy)
@@ -28,6 +28,7 @@
 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);
 
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8063)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -1595,8 +1595,41 @@
 int main_hall_id()
 {
  return (Main_hall - &Main_hall_defines[gr_screen.res][0]);
-}
+}
 
+/**
+ *CommanderDJ - stuff a sound index either directly
+ *or by looking up the filename in sounds.tbl
+ *@param to_stuff Variable to be stuffed
+ *@param is_interface_sound Whether the sound is an interface sound or not
+ *
+ *
+*/
+void stuff_sound_by_index_or_filename(int* to_stuff, bool is_interface_sound)
+{
+ char temp[MAX_FILENAME_LEN];
+ stuff_string_white(temp, MAX_FILENAME_LEN);
+
+ //is this an index (integer) or a filename (string)?
+ if(can_construe_as_integer(temp))
+ *to_stuff = atoi(temp);
+ else
+ {
+ //find the appropriate index in sounds.tbl
+ int index = 0;
+ if(is_interface_sound)
+ index = gamesnd_get_by_iface_name(temp);
+ else
+ index = gamesnd_get_by_name(temp);
+
+ //sanity check
+ if(index == -1)
+ Warning(LOCATION, "Invalid sound filename '%s' in mainhall.tbl! No sound will be played!\n", temp);
+
+ *to_stuff = index;
+ }
+}
+
 // read in main hall table
 void main_hall_read_table()
 {
@@ -1661,7 +1694,7 @@
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom sound id
  required_string("+Intercom sound:");
- stuff_int(&m->intercom_sounds[idx]);
+ stuff_sound_by_index_or_filename(&m->intercom_sounds[idx], true);
  }
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom pan
@@ -1717,7 +1750,7 @@
  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]);
+ stuff_sound_by_index_or_filename(&m->misc_anim_special_sounds[idx][s_idx + 1], true);
  }
  }
  for(idx=0; idx<m->num_misc_animations; idx++){
@@ -1762,8 +1795,8 @@
  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]);
+ stuff_sound_by_index_or_filename(&m->door_sounds[idx][0], true);
+ stuff_sound_by_index_or_filename(&m->door_sounds[idx][1], true);
  }
  for(idx=0; idx<m->num_door_animations; idx++){
  // door pan value

As IssMneur said, the code doesn't use file extensions - if they're included then they're ignored with the code above. As far as syntax goes for the patch above, quotes are optional - the way the code treats it is to use everything between quotes as a filename if they're included, and if not then it reads the name until it hits whitespace.

Quick breakdown of the code above: I put everything into a function called stuff_sound_by_index_or_filename() (I wasn't sure which file to put that in so I just left it in mainhallmenu.cpp for now. I'm sure there's a better place for it so if a coder has suggestions please give them). If the table entry has an integer this gets treated as an index, but if it's text then the code treats that text as a filename and locates the corresponding index and uses that (-1 is used if the file doesn't exist). gamesnd_get_by_iface_name() is an almost direct copy and paste of gamesnd_get_by_name() but using the Snds_iface vector as opposed to Snds.
[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
D-d-d-double post! Here's a potential patch for feature number 2! It's a very small patch, and I'm hoping I've understood everything correctly and haven't done anything inherently dangerous. But that's where coder review comes in!

Code: [Select]
Index: code/freespace2/freespace.cpp
===================================================================
--- code/freespace2/freespace.cpp (revision 8065)
+++ 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 8065)
+++ 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 8065)
+++ 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);

K. I'm done for the day. I eagerly await feedback!
[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 Eli2

  • 26
@param to_stuff Variable to be stuffed
why use a out variable if your return type is void?

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
@param to_stuff Variable to be stuffed
why use a out variable if your return type is void?

I don't understand the question. to_stuff is passed by reference and is the actual variable that needs to be given the index into sounds.tbl. Sure, I could have used a return type instead, but I thought it was best to keep the format consistent with the other stuff_... functions.
[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:
Don't mix using braces in conditionals.

stuff_sound_by_index_or_filename may already be implemented by parse_sound. If it does something different that can't or shouldn't be merged into parse_sound, it should be renamed, with comments to point out the difference and put into the same place as parse_sound.
"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 Goober5000

  • HLP Loremaster
  • Administrator
  • 214
    • Goober5000 Productions
I don't understand the question. to_stuff is passed by reference and is the actual variable that needs to be given the index into sounds.tbl. Sure, I could have used a return type instead, but I thought it was best to keep the format consistent with the other stuff_... functions.
Yes, this is the correct approach.

Also, you should check that there's not some hidden dependency that will be broken by main hall tables being parsed once at startup.  You never know.

 

Offline Iss Mneur

  • 210
  • TODO:

Yes, this is the correct approach.

Also, you should check that there's not some hidden dependency that will be broken by main hall tables being parsed once at startup.  You never know.
That is a good point.  Now that you mention it I think the transition out of the mainhall causes all of the bitmaps and sounds used to be unloaded.
"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 Goober5000

  • HLP Loremaster
  • Administrator
  • 214
    • Goober5000 Productions
That's distinct from parsing the table though.  If it isn't, it should be.

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
So after discussion with IssMneur I went and implemented a parse_sound_list function and made a useful change to parse_sound as well as some other stuff, but it turns out after all of that Misc Anim Sounds entries aren't really sound lists because of that first number which specified how many sounds follow. There are two options for this that I can see:

1. Change the syntax for mainhall.tbl. I would see this as unacceptable, given the amount of stuff we'd be breaking and the possibly massive rewrites necessary, but it's there.
2. Write a function specifically for parsing Misc Anim Sounds entries. The thing is that neither parse_sound nor parse_sound_list work for Misc Anim Sounds because of the syntax, so I may be forced to write a function just for them. I have no problem doing this, I just wanted to get more experienced input before doing so.

If there are any viable options I've missed then I'm all ears.

On the bright side however is this: the Intercom Sounds flag can be successfully expanded using parse_sound, and the Door Sounds flag can be successfully expanded using parse_sound_list.

That said, here is the patch with the new/updated sound functions!

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();
 

I have tested all of these and they work as intended without breaking existing behaviour, AFAICT. You'll notice I have extended parse_sound a little bit and extracted the functionality it shares with parse_sound_list and put it into a helper function.

And here is the patch for the extension of Intercom Sounds and Door Sounds
Code: [Select]
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8067)
+++ 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
@@ -1761,9 +1760,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

You'll notice I've just used the tag for the name parameter for now. Once mainhalls have names they will be used instead.

Feedback/review welcome/needed!
[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:
That's distinct from parsing the table though.  If it isn't, it should be.
Very true, but stranger things have been done.  Though it explain the current behaviour of parsing every time the mainhall is displayed.

So after discussion with IssMneur I went and implemented a parse_sound_list function and made a useful change to parse_sound as well as some other stuff, but it turns out after all of that Misc Anim Sounds entries aren't really sound lists because of that first number which specified how many sounds follow. There are two options for this that I can see:

1. Change the syntax for mainhall.tbl. I would see this as unacceptable, given the amount of stuff we'd be breaking and the possibly massive rewrites necessary, but it's there.
2. Write a function specifically for parsing Misc Anim Sounds entries. The thing is that neither parse_sound nor parse_sound_list work for Misc Anim Sounds because of the syntax, so I may be forced to write a function just for them. I have no problem doing this, I just wanted to get more experienced input before doing so.

If there are any viable options I've missed then I'm all ears.
Yes you did.  You parse the count first with stuff_int then pass that number to parse_sound_list so it can parse the rest of the line.
« Last Edit: December 04, 2011, 09:39:13 pm by Iss Mneur »
"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
Yes you did.  You parse the count first with stuff_int then pass that number to parse_sound_list so it can parse the rest of the line.

I tried this. The problem with it is that parse_sound_list (just like parse_sound) parses the tag before it parses the sounds. If it doesn't find the tag it doesn't parse the list.
[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