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

0 Members and 1 Guest are viewing this topic.

Offline mjn.mixael

  • Cutscene Master
  • 212
  • Chopped liver
    • Steam
    • Twitter
That's +Misc anim mode

Do you you not know that I am the mainhall king? I know all of the flags and how they are used (or not used in this case). Thou shalt not try to correct the mainhall king!   :P

EDIT: Actually, I'm fairtly certain that +Misc anim handles and +Misc anim flags (especially the latter) do nothing and/or aren't important anymore. Perhaps we might think about depreciating those flags alltogether? It would have to be confirmed exactly what they do though.
« Last Edit: December 13, 2011, 01:27:04 pm by mjn.mixael »
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, so after further consideration I've decided to proceed with vectorising the mainhall data structure before adding this new flag. I just have a feeling it'll be easier this way in the long run, and I feel that it should be done sooner rather than later. Feature-wise this doesn't mean any significant changes (apart from the potential removal of some hardcoded limits/magic numbers), but it will increase the code's flexibility and make adding new features easier (and it will go a long way towards preparing for a modular mainhall.tbl, which is after all the ultimate objective of this little project). This will likely take me some time and I'll be looking to the SCP for advice at times, but I'll make sure to keep this thread updated when I can.

So on that note, the first issue with vectorisation is the initialisation of vectors before they're accessed. Curently, only misc_anim_special_sounds and door_sounds are vectors, and we currently have a temporary hack in place (this was mentioned earlier in the thread) to initialise them before they're used. It looks like this:

Code: [Select]
...
SCP_vector<int> temp;
m->misc_anim_special_sounds.push_back(temp);
*code that uses misc_anim_special_sounds*
...

Obviously we want to initialise all the vectors in one spot if possible so we can simply use them without worrying about whether they exist or not. The first relevant question is where to do this. It obviously only has to be done once. main_hall_init isn't an option since the table can be read before that function is called, so we look to main_hall_read_table. I'd be fine with doing it here, but to be honest I'd prefer to spin the initialisation off into its own function and call that from main_hall_read_table. Thoughts/suggestions?

The second relevant question is what exactly the initialisation will do, or more importantly how many elements do we initialise into the vectors? I would argue that using Num Intercom Sounds, Num Misc Animations and Num Door Animations would be a good starting point, but that would mean we would have to wait until each of those are parsed before initialising that section. I'm not exactly sure which option is best, so I look to the SCP for suggestions. Thanks!
[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
So I had a sort of O**** moment today when I reread my earlier post and said that the bug mjn.mixael found should have been picked up earlier since it was introduced beforehand (revision 8079 to be exact). This means that trunk currently has this bug (I checked and it does. I also reverted to 8078 and that build is not affected, so it was definitely introduced in my changes in 8079). I think everyone here agrees that this is a bad thing. So I've decided to release that quick fix patch I mentioned before so it could be committed as a stopgap measure before I can get to it properly. I'm responsible for the fact that my code introduced a bug in trunk, and therefore I'm responsible for fixing it as soon as possible. The patch is simple and the changes are small. Here it is:
Code: [Select]
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8082)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -1184,18 +1184,18 @@
  else {
  for (s_idx = 0; (unsigned)s_idx < Main_hall->misc_anim_special_sounds.at(idx).size(); 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;
+ if ((Main_hall_misc_anim[idx].current_frame >= Main_hall->misc_anim_special_trigger[idx][s_idx+1]) && !Main_hall->misc_anim_sound_flag[idx][s_idx+1]) {
+ Main_hall->misc_anim_sound_flag[idx][s_idx+1] = 1;
 
  // if the sound is already playing, then kill it. This is a pretty safe thing to do since we can assume that
  // by the time we get to this point again, the sound will have been long finished
- if (snd_is_playing(Main_hall->misc_anim_sound_handles[idx][s_idx])) {
- snd_stop(Main_hall->misc_anim_sound_handles[idx][s_idx]);
- Main_hall->misc_anim_sound_handles[idx][s_idx] = -1;
+ if (snd_is_playing(Main_hall->misc_anim_sound_handles[idx][s_idx+1])) {
+ snd_stop(Main_hall->misc_anim_sound_handles[idx][s_idx+1]);
+ Main_hall->misc_anim_sound_handles[idx][s_idx+1] = -1;
  }
 
  // play the sound
- 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]);
+ Main_hall->misc_anim_sound_handles[idx][s_idx+1] = snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan[idx]);
  break;
  }
  }

I promise I will attend to the bug properly when vectorising the relevant data structures. I just haven't gotten to them yet. My sincere apologies to the SCP and anyone who's using affected trunk builds. The fault is mine and I take it very seriously. I will endeavour to test my patches much more thoroughly from now on. I hope this can be committed soon before people complain (I'm lucky we're not releasing nightlies at the moment). It has been tested by both mjn and myself.

On the bright side, I do have something to show for today's work: I've started on the vectorisation of main_hall_defines. The following patch vectorises the intercom_delay, intercom_sounds, and intercom_sound_pan data structures. I also made a function for their initialisation. I'm hoping to tackle the other sets of data structures the same way, but I'm open to the SCP's suggestions on alternatives. This patch does need to be tested by someone other than me. As always, any feedback can be posted here or you can let me know on IRC.

Code: [Select]
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8092)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -72,13 +72,15 @@
  int num_random_intercom_sounds;
 
  // random (min/max) delays between playing intercom sounds
- int intercom_delay[MAX_RANDOM_INTERCOM_SOUNDS][2];
+ SCP_vector<SCP_vector<int> > intercom_delay;
+ //int intercom_delay[MAX_RANDOM_INTERCOM_SOUNDS][2];
 
  // intercom sounds themselves
- int intercom_sounds[MAX_RANDOM_INTERCOM_SOUNDS];
+ SCP_vector<int> intercom_sounds;
+ //int intercom_sounds[MAX_RANDOM_INTERCOM_SOUNDS];
 
  // intercom sound pan values
- float intercom_sound_pan[MAX_RANDOM_INTERCOM_SOUNDS];
+ SCP_vector<float> intercom_sound_pan;
 
 
  // misc animations --------------------
@@ -1396,8 +1398,8 @@
  // if we have no timestamp for the next random sound, then set on
  if((Main_hall_next_intercom_sound_stamp == -1) && (Main_hall_intercom_sound_handle == -1)){
  Main_hall_next_intercom_sound_stamp = timestamp((int)(((float)rand()/(float)RAND_MAX) *
-                                             (float)(Main_hall->intercom_delay[Main_hall_next_intercom_sound][1]
-   - Main_hall->intercom_delay[Main_hall_intercom_sound_handle][0])) );
+                                             (float)(Main_hall->intercom_delay.at(Main_hall_next_intercom_sound).at(1)
+   - Main_hall->intercom_delay.at(Main_hall_next_intercom_sound).at(0))) );
  }
 
  // if the there is no sound playing
@@ -1409,7 +1411,7 @@
  // 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[Main_hall->intercom_sounds[Main_hall_next_intercom_sound]]);
+ Main_hall_intercom_sound_handle = snd_play(&Snds_iface.at(Main_hall->intercom_sounds.at(Main_hall_next_intercom_sound)));
 
  // unset the timestamp
  Main_hall_next_intercom_sound_stamp = -1;
@@ -1428,8 +1430,8 @@
 
  // set the timestamp
  Main_hall_next_intercom_sound_stamp = timestamp((int)(((float)rand()/(float)RAND_MAX) *
-                                             (float)(Main_hall->intercom_delay[Main_hall_next_intercom_sound][1]
-   - Main_hall->intercom_delay[Main_hall_next_intercom_sound][0])) );
+                                             (float)(Main_hall->intercom_delay.at(Main_hall_next_intercom_sound).at(1)
+   - Main_hall->intercom_delay.at(Main_hall_next_intercom_sound).at(0))) );
 
  // release the sound handle
  Main_hall_intercom_sound_handle = -1;
@@ -1595,8 +1597,39 @@
 int main_hall_id()
 {
  return (Main_hall - &Main_hall_defines[gr_screen.res][0]);
-}
+}
 
+//CommanderDJ - helper function for initialising intercom sounds vectors based on number of sounds
+//To be called after num_intercom_sounds has been parsed
+void intercom_sounds_init(main_hall_defines &m)
+{
+ if(!m.intercom_delay.empty())
+ {
+ //if one of these isn't empty, none of these should be empty
+ Assert(!m.intercom_sounds.empty());
+ Assert(!m.intercom_sound_pan.empty());
+
+ //since we could be reparsing with a different number of intercom sounds, clear these and reinitialise
+ m.intercom_delay.clear();
+ m.intercom_sounds.clear();
+ m.intercom_sound_pan.clear();
+ }
+
+ for(int idx=0; idx<m.num_random_intercom_sounds; idx++)
+ {
+ SCP_vector<int> temp;
+ m.intercom_delay.push_back(temp);
+
+ //for each delay we want space for two numbers - a min and a max
+ m.intercom_delay.at(idx).push_back(0);
+ m.intercom_delay.at(idx).push_back(0);
+
+ m.intercom_sounds.push_back(-1);
+
+ m.intercom_sound_pan.push_back(0);
+ }
+}
+
 // read in main hall table
 void main_hall_read_table()
 {
@@ -1652,20 +1685,24 @@
  Warning(LOCATION, "Num Intercom Sounds exceeds maximum!");
  m->num_random_intercom_sounds = MAX_RANDOM_INTERCOM_SOUNDS;
  }
+
+ //initialise intercom sounds vectors
+ intercom_sounds_init(*m);
+
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom delay
  required_string("+Intercom delay:");
- stuff_int(&m->intercom_delay[idx][0]);
- stuff_int(&m->intercom_delay[idx][1]);
+ stuff_int(&m->intercom_delay.at(idx).at(0));
+ stuff_int(&m->intercom_delay.at(idx).at(1));
  }
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom sound id
- parse_sound("+Intercom sound:", &m->intercom_sounds[idx], "+Intercom sound:", PARSE_SOUND_INTERFACE_SOUND);
+ parse_sound("+Intercom sound:", &m->intercom_sounds.at(idx), "+Intercom sound:", PARSE_SOUND_INTERFACE_SOUND);
  }
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom pan
  required_string("+Intercom pan:");
- stuff_float(&m->intercom_sound_pan[idx]);
+ stuff_float(&m->intercom_sound_pan.at(idx));
  }
 
  // misc animations
Index: code/menuui/mainhallmenu.h
===================================================================
--- code/menuui/mainhallmenu.h (revision 8092)
+++ code/menuui/mainhallmenu.h (working copy)
@@ -23,6 +23,7 @@
 
 // do a frame for the main hall
 void main_hall_do(float frametime);
+
 // close the main hall proper
 void main_hall_close();
 

Once again, apologies to anyone who was affected by the bug I introduced. I hope you can forgive me :).

P.S. Zacam, if you prefer my patches to be attached, you could have just told me :P.

[attachment deleted by a basterd]
[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
The intercom stuff still seems to work as I'd expect.
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
The intercom stuff still seems to work as I'd expect.

Good to hear. I'm technically not adding new features so it should work exactly as before. Unfortunately my progress may be stalled for a day or two as my system is facing imminent hard drive failure, so until I get that sorted out it's unlikely I'll be doing much coding. In the meantime, can any coder who's reading this thread please commit the bugfix above? I'm hating the idea that the latest trunk build is still affected by this.

EDIT: Stopgap fix kindly committed by Zacam in trunk revision 8093.
« Last Edit: December 15, 2011, 09:33:46 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 CommanderDJ

  • Software engineer
  • 210
    • Minecraft
Update!

I have vectorised more of the mainhall structures: all of the intercom stuff and most of the misc anim stuff.
Code: [Select]
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8095)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -72,13 +72,13 @@
  int num_random_intercom_sounds;
 
  // random (min/max) delays between playing intercom sounds
- int intercom_delay[MAX_RANDOM_INTERCOM_SOUNDS][2];
+ SCP_vector<SCP_vector<int> > intercom_delay;
 
  // intercom sounds themselves
- int intercom_sounds[MAX_RANDOM_INTERCOM_SOUNDS];
+ SCP_vector<int> intercom_sounds;
 
  // intercom sound pan values
- float intercom_sound_pan[MAX_RANDOM_INTERCOM_SOUNDS];
+ SCP_vector<float> intercom_sound_pan;
 
 
  // misc animations --------------------
@@ -90,22 +90,22 @@
  char misc_anim_name[MAX_MISC_ANIMATIONS][MAX_FILENAME_LEN];
 
  // Time until we will next play a given misc animation, min delay, and max delay
- int misc_anim_delay[MAX_MISC_ANIMATIONS][3];
+ SCP_vector<SCP_vector<int> > misc_anim_delay;
 
  // Goober5000, used in preference to the flag in generic_anim
- int misc_anim_paused[MAX_MISC_ANIMATIONS];
+ SCP_vector<int> misc_anim_paused;
 
  // Goober5000, used when we want to play one of several anims
- int misc_anim_group[MAX_MISC_ANIMATIONS];
+ SCP_vector<int> misc_anim_group;
 
  // coords of where to play the misc anim
- int misc_anim_coords[MAX_MISC_ANIMATIONS][2];
+ SCP_vector<SCP_vector<int> > misc_anim_coords;
 
  // misc anim play modes (see MISC_ANIM_MODE_* above)
- int misc_anim_modes[MAX_MISC_ANIMATIONS];
+ SCP_vector<int> misc_anim_modes;
 
  // panning values for each of the misc anims
- float misc_anim_sound_pan[MAX_MISC_ANIMATIONS];
+ SCP_vector<float> misc_anim_sound_pan;
 
  //sounds for each of the misc anims
  SCP_vector<SCP_vector<int> > misc_anim_special_sounds;
@@ -581,15 +581,15 @@
  }
  else {
  //start paused
- if(Main_hall->misc_anim_modes[idx] == MISC_ANIM_MODE_HOLD)
+ if(Main_hall->misc_anim_modes.at(idx) == MISC_ANIM_MODE_HOLD)
  Main_hall_misc_anim[idx].direction |= GENERIC_ANIM_DIRECTION_NOLOOP;
  }
 
  // null out the delay timestamps
- Main_hall->misc_anim_delay[idx][0] = -1;
+ Main_hall->misc_anim_delay.at(idx).at(0) = -1;
 
  // start paused
- Main_hall->misc_anim_paused[idx] = true;
+ Main_hall->misc_anim_paused.at(idx) = true;
  }
 
  // load up the door animations
@@ -1121,13 +1121,13 @@
  // render it
  if (Main_hall_misc_anim[idx].num_frames > 0) {
  // animation is paused
- if (Main_hall->misc_anim_paused[idx]) {
+ if (Main_hall->misc_anim_paused.at(idx)) {
  // if the timestamp is -1, then regenerate it
- if (Main_hall->misc_anim_delay[idx][0] == -1) {
+ if (Main_hall->misc_anim_delay.at(idx).at(0) == -1) {
  int regen_idx = -1;
 
  // if this is part of a group, we should do additional checking
- if (Main_hall->misc_anim_group[idx] >= 0) {
+ if (Main_hall->misc_anim_group.at(idx) >= 0) {
  // make sure we haven't already checked it
  if (!(group_anims_weve_checked & (1<<idx))) {
  int group_indexes[MAX_MISC_ANIMATIONS];
@@ -1136,12 +1136,12 @@
 
  // okay... now we need to make sure all anims in this group are paused and -1
  for (jdx = 0; jdx < Main_hall->num_misc_animations; jdx++) {
- if (Main_hall->misc_anim_group[jdx] == Main_hall->misc_anim_group[idx]) {
+ if (Main_hall->misc_anim_group.at(jdx) == Main_hall->misc_anim_group.at(idx)) {
  group_anims_weve_checked |= (1<<jdx);
  group_indexes[num_group_indexes] = jdx;
  num_group_indexes++;
 
- if (!Main_hall->misc_anim_paused[jdx] || Main_hall->misc_anim_delay[jdx][0] != -1) {
+ if (!Main_hall->misc_anim_paused.at(jdx) || Main_hall->misc_anim_delay.at(jdx).at(0) != -1) {
  all_neg1 = false;
  }
  }
@@ -1160,19 +1160,19 @@
 
  // reset it to some random value (based on MIN and MAX) and continue
  if (regen_idx >= 0) {
- int min = Main_hall->misc_anim_delay[regen_idx][1];
- int max = Main_hall->misc_anim_delay[regen_idx][2];
- Main_hall->misc_anim_delay[regen_idx][0] = timestamp(min + (int) (frand() * (max - min)));
+ int min = Main_hall->misc_anim_delay.at(regen_idx).at(1);
+ int max = Main_hall->misc_anim_delay.at(regen_idx).at(2);
+ Main_hall->misc_anim_delay.at(regen_idx).at(0) = timestamp(min + (int) (frand() * (max - min)));
  }
 
  // if the timestamp is not -1 and has popped, play the anim and make the timestamp -1
- } else if (timestamp_elapsed(Main_hall->misc_anim_delay[idx][0])) {
- Main_hall->misc_anim_paused[idx] = false;
+ } else if (timestamp_elapsed(Main_hall->misc_anim_delay.at(idx).at(0))) {
+ Main_hall->misc_anim_paused.at(idx) = false;
  Main_hall_misc_anim[idx].current_frame = 0;
  Main_hall_misc_anim[idx].anim_time = 0.0;
 
  // kill the timestamp
- Main_hall->misc_anim_delay[idx][0] = -1;
+ Main_hall->misc_anim_delay.at(idx).at(0) = -1;
 
  // reset the "should be playing" flags
  for (s_idx=1;s_idx<10;s_idx++) {
@@ -1195,23 +1195,23 @@
  }
 
  // play the sound
- Main_hall->misc_anim_sound_handles[idx][s_idx+1] = snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan[idx]);
+ Main_hall->misc_anim_sound_handles[idx][s_idx+1] = snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan.at(idx));
  break;
  }
  }
 
  // animation has reached the last frame
  if (Main_hall_misc_anim[idx].current_frame == Main_hall_misc_anim[idx].num_frames - 1) {
- Main_hall->misc_anim_delay[idx][0] = -1;
+ Main_hall->misc_anim_delay.at(idx).at(0) = -1;
 
  //this helps the above code reset the timers
  //MISC_ANIM_MODE_HOLD simply stops on the last frame, so we don't care
  //MISC_ANIM_MODE_LOOPED just loops so we don't care either
- if (Main_hall->misc_anim_modes[idx] == MISC_ANIM_MODE_TIMED) {
- Main_hall->misc_anim_paused[idx] = true;
+ if (Main_hall->misc_anim_modes.at(idx) == MISC_ANIM_MODE_TIMED) {
+ Main_hall->misc_anim_paused.at(idx) = true;
  }
  //don't reset sound for MISC_ANIM_MODE_HOLD
- if (Main_hall->misc_anim_modes[idx] != MISC_ANIM_MODE_HOLD) {
+ if (Main_hall->misc_anim_modes.at(idx) != MISC_ANIM_MODE_HOLD) {
  // reset the "should be playing" flags
  for (s_idx=1;s_idx<10;s_idx++) {
  Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
@@ -1223,7 +1223,7 @@
  if (Main_hall_frame_skip || Main_hall_paused) {
  frametime = 0;
  }
- generic_anim_render(&Main_hall_misc_anim[idx], frametime, Main_hall->misc_anim_coords[idx][0], Main_hall->misc_anim_coords[idx][1]);
+ generic_anim_render(&Main_hall_misc_anim[idx], frametime, Main_hall->misc_anim_coords.at(idx).at(0), Main_hall->misc_anim_coords.at(idx).at(1));
  }
  }
  }
@@ -1396,8 +1396,8 @@
  // if we have no timestamp for the next random sound, then set on
  if((Main_hall_next_intercom_sound_stamp == -1) && (Main_hall_intercom_sound_handle == -1)){
  Main_hall_next_intercom_sound_stamp = timestamp((int)(((float)rand()/(float)RAND_MAX) *
-                                             (float)(Main_hall->intercom_delay[Main_hall_next_intercom_sound][1]
-   - Main_hall->intercom_delay[Main_hall_intercom_sound_handle][0])) );
+                                             (float)(Main_hall->intercom_delay.at(Main_hall_next_intercom_sound).at(1)
+   - Main_hall->intercom_delay.at(Main_hall_next_intercom_sound).at(0))) );
  }
 
  // if the there is no sound playing
@@ -1409,7 +1409,7 @@
  // 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[Main_hall->intercom_sounds[Main_hall_next_intercom_sound]]);
+ Main_hall_intercom_sound_handle = snd_play(&Snds_iface.at(Main_hall->intercom_sounds.at(Main_hall_next_intercom_sound)));
 
  // unset the timestamp
  Main_hall_next_intercom_sound_stamp = -1;
@@ -1428,8 +1428,8 @@
 
  // set the timestamp
  Main_hall_next_intercom_sound_stamp = timestamp((int)(((float)rand()/(float)RAND_MAX) *
-                                             (float)(Main_hall->intercom_delay[Main_hall_next_intercom_sound][1]
-   - Main_hall->intercom_delay[Main_hall_next_intercom_sound][0])) );
+                                             (float)(Main_hall->intercom_delay.at(Main_hall_next_intercom_sound).at(1)
+   - Main_hall->intercom_delay.at(Main_hall_next_intercom_sound).at(0))) );
 
  // release the sound handle
  Main_hall_intercom_sound_handle = -1;
@@ -1595,8 +1595,96 @@
 int main_hall_id()
 {
  return (Main_hall - &Main_hall_defines[gr_screen.res][0]);
-}
+}
 
+//CommanderDJ - helper function for initialising intercom sounds vectors based on number of sounds
+//To be called after num_intercom_sounds has been parsed
+void intercom_sounds_init(main_hall_defines &m)
+{
+ if(!m.intercom_delay.empty())
+ {
+ //if one of these isn't empty, none of these should be empty
+ Assert(!m.intercom_sounds.empty());
+ Assert(!m.intercom_sound_pan.empty());
+
+ //since we could be reparsing with a different number of intercom sounds, clear these and reinitialise
+ m.intercom_delay.clear();
+ m.intercom_sounds.clear();
+ m.intercom_sound_pan.clear();
+ }
+
+ SCP_vector<int> temp;
+
+ for(int idx=0; idx<m.num_random_intercom_sounds; idx++)
+ {
+ //intercom_delay
+ m.intercom_delay.push_back(temp);
+
+ //each delay has a min and a max
+ m.intercom_delay.at(idx).push_back(0);
+ m.intercom_delay.at(idx).push_back(0);
+
+ //intercom_sounds
+ m.intercom_sounds.push_back(-1);
+
+ //intercom_sound_pan
+ m.intercom_sound_pan.push_back(0);
+ }
+}
+
+void misc_anim_init(main_hall_defines &m)
+{
+ if(!m.misc_anim_delay.empty())
+ {
+ //if one of these isn't empty, none of these should be
+ Assert(!m.misc_anim_paused.empty());
+ Assert(!m.misc_anim_group.empty());
+ Assert(!m.misc_anim_coords.empty());
+ Assert(!m.misc_anim_modes.empty());
+ Assert(!m.misc_anim_sound_pan.empty());
+
+
+ //since we could be reparsing with a different number of misc anims, clear these and reinitialise
+ m.misc_anim_delay.clear();
+ m.misc_anim_paused.clear();
+ m.misc_anim_group.clear();
+ m.misc_anim_coords.clear();
+ m.misc_anim_modes.clear();
+ m.misc_anim_sound_pan.clear();
+ }
+
+ SCP_vector<int> temp;
+
+ for(int idx=0; idx<m.num_misc_animations; idx++)
+ {
+ //misc_anim_delay
+ m.misc_anim_delay.push_back(temp);
+
+ //-1 default for the first entry, 0 for the others
+ m.misc_anim_delay.at(idx).push_back(-1);
+ m.misc_anim_delay.at(idx).push_back(0);
+ m.misc_anim_delay.at(idx).push_back(0);
+
+ //misc_anim_paused
+ m.misc_anim_paused.push_back(1); //default is paused
+
+ //misc_anim_group
+ m.misc_anim_group.push_back(-1);
+
+ //misc_anim_coords
+ m.misc_anim_coords.push_back(temp);
+
+ m.misc_anim_coords.at(idx).push_back(0);
+ m.misc_anim_coords.at(idx).push_back(0);
+
+ //misc_anim_modes
+ m.misc_anim_modes.push_back(MISC_ANIM_MODE_LOOP);
+
+ //misc_anim_sound_pan
+ m.misc_anim_sound_pan.push_back(0);
+ }
+}
+
 // read in main hall table
 void main_hall_read_table()
 {
@@ -1652,20 +1740,24 @@
  Warning(LOCATION, "Num Intercom Sounds exceeds maximum!");
  m->num_random_intercom_sounds = MAX_RANDOM_INTERCOM_SOUNDS;
  }
+
+ //initialise intercom sounds vectors
+ intercom_sounds_init(*m);
+
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom delay
  required_string("+Intercom delay:");
- stuff_int(&m->intercom_delay[idx][0]);
- stuff_int(&m->intercom_delay[idx][1]);
+ stuff_int(&m->intercom_delay.at(idx).at(0));
+ stuff_int(&m->intercom_delay.at(idx).at(1));
  }
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom sound id
- parse_sound("+Intercom sound:", &m->intercom_sounds[idx], "+Intercom sound:", PARSE_SOUND_INTERFACE_SOUND);
+ parse_sound("+Intercom sound:", &m->intercom_sounds.at(idx), "+Intercom sound:", PARSE_SOUND_INTERFACE_SOUND);
  }
  for(idx=0; idx<m->num_random_intercom_sounds; idx++){
  // intercom pan
  required_string("+Intercom pan:");
- stuff_float(&m->intercom_sound_pan[idx]);
+ stuff_float(&m->intercom_sound_pan.at(idx));
  }
 
  // misc animations
@@ -1675,6 +1767,10 @@
  Warning(LOCATION, "Num Misc Animations exceeds maximum!");
  m->num_misc_animations = MAX_MISC_ANIMATIONS;
  }
+
+ //initialise the misc anim vectors
+ misc_anim_init(*m);
+
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim names
  required_string("+Misc anim:");
@@ -1683,33 +1779,33 @@
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim groups, optionally
  if (optional_string("+Misc anim group:")){
- stuff_int(&m->misc_anim_group[idx]);
+ stuff_int(&m->misc_anim_group.at(idx));
  } else {
- m->misc_anim_group[idx] = -1;
+ m->misc_anim_group.at(idx) = -1;
  }
  }
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim delay
  required_string("+Misc anim delay:");
- stuff_int(&m->misc_anim_delay[idx][0]);
- stuff_int(&m->misc_anim_delay[idx][1]);
- stuff_int(&m->misc_anim_delay[idx][2]);
+ stuff_int(&m->misc_anim_delay.at(idx).at(0));
+ stuff_int(&m->misc_anim_delay.at(idx).at(1));
+ stuff_int(&m->misc_anim_delay.at(idx).at(2));
  }
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim coords
  required_string("+Misc anim coords:");
- stuff_int(&m->misc_anim_coords[idx][0]);
- stuff_int(&m->misc_anim_coords[idx][1]);
+ stuff_int(&m->misc_anim_coords.at(idx).at(0));
+ stuff_int(&m->misc_anim_coords.at(idx).at(1));
  }
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim mode
  required_string("+Misc anim mode:");
- stuff_int(&m->misc_anim_modes[idx]);
+ stuff_int(&m->misc_anim_modes.at(idx));
  }
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim pan
  required_string("+Misc anim pan:");
- stuff_float(&m->misc_anim_sound_pan[idx]);
+ stuff_float(&m->misc_anim_sound_pan.at(idx));
  }
  for(idx=0; idx<m->num_misc_animations; idx++){
  // anim sound id
Index: code/menuui/mainhallmenu.h
===================================================================
--- code/menuui/mainhallmenu.h (revision 8095)
+++ code/menuui/mainhallmenu.h (working copy)
@@ -23,6 +23,7 @@
 
 // do a frame for the main hall
 void main_hall_do(float frametime);
+
 // close the main hall proper
 void main_hall_close();
 

Please apply this to the latest trunk revision (note that this patch overrides the previous intercom vectorisation patch, so please do not have that one applied). This should be tested both with and without the -reparse_mainhall flag. If something goes wrong it'll most likely be pretty obvious (ie the game will assert or throw an exception and crash).

The next patch will include parsing changes (but not table syntax changes) so I'd prefer to get this one committed before the next one surfaces. Thanks to everyone for your patience and in particular to mjn.mixael for his testing!
[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
No issues found.
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 Echelon9

  • 210
Commander, regarding your specific query on min/max naming cross-platform, the issues we've seen previously only relate to the use of the min() and max() functions --- these need to be fully capitalised to work correctly cross platform.

Having variables names min or max doesn't create issues as far as I know.

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
Thank you for the quick response, Echelon9.

The vectorisation patch above has been committed to trunk by Zacam in revision 8106.

It is unlikely I will make further progress until after Christmas, but as always, any updates will be posted here.
[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
Hey guys,

Merry Christmas! Just a quick status update to show that I have not stopped working on this and a few questions. So, today I worked more on vectorising stuff, I'm almost done with the misc anim data structures. Once those are finished off I'll post a patch for testing. After that all that will be left to do are the door anim structures and the misc_anim_name structure (which is a two dimensional char array). With luck, I hope to have the vectorisation complete by the new year.

Question for mjn.mixael and others: would there be any demand/use for a Misc Anim Trigger New flag as well as a Misc Anim Sounds New? These would both remove the need for having the first entry in the list be the number of entries following. I do have plans to implement Sounds New but was wondering what the interest would be like for Trigger New.

Question for coders: either the comments in the code are incorrect or I'm not reading something right: the comments for the misc_anim_sound_handles and misc_anim_sound_flag arrays say that the first element in each is the number of handles, yet every mainhall.tbl I've seen only has one entry for each of those, and indeed the parsing code itself only parses one entry per flag. What gives? Also, if indeed we only need one entry per flag, why are two dimensional arrays used for those? Seems unnecessary to me, so I'd like that cleared up so that if there's a waste of space happening there I can rectify it.
[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
New flags aren't really very necessary, imo. the syntax of the originals isn't hard to understand. however, if you wanted to add the be flags, I'd them.
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
Update!

Several changes in this one: more vectorisation (finished off the misc anim structures, only the door anims left), but that's not the exciting part. The exciting part is that I have completely removed the need for the +Misc Anim Handles and +Misc Anim Flags table flags. Handles have been completely ripped out and aren't used at all, where as Flags are all handled internally by the code. Obviously it's backwards compatible so that retail tables will still work (the game ignores user input for those two flags now), but you no longer need to include those flags in mainhall.tbl with this patch.

Also I've tweaked some of the initialisation code to tie it more closely to the -reparse_mainhall flag (since that's the only time those parts of the code were used).

Please test! Apply straight to trunk, try it without the Flags and Handles table entries, try it with and without -reparse_mainhall. Post any results here.

Code: [Select]
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8113)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -110,16 +110,13 @@
  //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];
+ //frame number triggers for each of the misc anims
+ SCP_vector<SCP_vector<int> > misc_anim_special_trigger;
 
- // [N][0] == # of handles, [N][1-9] == sound handle num
- int misc_anim_sound_handles[MAX_MISC_ANIMATIONS][10];
+ //flags for each of the misc anim sounds
+ SCP_vector<SCP_vector<int> > misc_anim_sound_flag;
 
- // [N][0] == # of handles, [N][1-9] == sound "should" be playing
- int misc_anim_sound_flag[MAX_MISC_ANIMATIONS][10];
 
-
  // door animations --------------------
 
  // # of door animations
@@ -503,7 +500,7 @@
  return;
  }
 
- int idx,s_idx;
+ int idx;
  char temp[100], whee[100];
 
  //reparse the table here if the relevant cmdline flag is set
@@ -639,14 +636,6 @@
  Main_hall_door_sound_handles[idx] = -1;
  }
 
- // zero out the misc anim sounds
- for (idx=0;idx<Main_hall->num_misc_animations;idx++) {
- for (s_idx = 1;s_idx < 10;s_idx++) {
- Main_hall->misc_anim_sound_handles[idx][s_idx] = -1;
- Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
- }
- }
-
  // skip the first frame
  Main_hall_frame_skip = 1;
 
@@ -1009,10 +998,9 @@
 
  // stop any playing misc animation sounds
  for (idx=0;idx<Main_hall->num_misc_animations;idx++) {
- for (s_idx=1;s_idx<10;s_idx++) {
- if (snd_is_playing(Main_hall->misc_anim_sound_handles[idx][s_idx])) {
- snd_stop(Main_hall->misc_anim_sound_handles[idx][s_idx]);
- Main_hall->misc_anim_sound_handles[idx][s_idx] = -1;
+ for (s_idx=0;(unsigned)s_idx<Main_hall->misc_anim_special_sounds.at(idx).size();s_idx++) {
+ if (snd_is_playing(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx))) {
+ snd_stop(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx));
  }
  }
  }
@@ -1175,28 +1163,29 @@
  // kill the timestamp
  Main_hall->misc_anim_delay.at(idx).at(0) = -1;
 
+
  // reset the "should be playing" flags
- for (s_idx=1;s_idx<10;s_idx++) {
- Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
+ for (s_idx=0;(unsigned)s_idx<Main_hall->misc_anim_sound_flag.at(idx).size();s_idx++) {
+ Main_hall->misc_anim_sound_flag.at(idx).at(s_idx) = 0;
  }
+
  }
  }
  // animation is not paused
  else {
  for (s_idx = 0; (unsigned)s_idx < Main_hall->misc_anim_special_sounds.at(idx).size(); 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+1]) && !Main_hall->misc_anim_sound_flag[idx][s_idx+1]) {
- Main_hall->misc_anim_sound_flag[idx][s_idx+1] = 1;
+ if ((Main_hall_misc_anim[idx].current_frame >= Main_hall->misc_anim_special_trigger.at(idx).at(s_idx)) && !Main_hall->misc_anim_sound_flag.at(idx).at(s_idx)) {
+ Main_hall->misc_anim_sound_flag.at(idx).at(s_idx) = 1;
 
  // if the sound is already playing, then kill it. This is a pretty safe thing to do since we can assume that
  // by the time we get to this point again, the sound will have been long finished
- if (snd_is_playing(Main_hall->misc_anim_sound_handles[idx][s_idx+1])) {
- snd_stop(Main_hall->misc_anim_sound_handles[idx][s_idx+1]);
- Main_hall->misc_anim_sound_handles[idx][s_idx+1] = -1;
+ if (snd_is_playing(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx))) {
+ snd_stop(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx));
  }
 
  // play the sound
- Main_hall->misc_anim_sound_handles[idx][s_idx+1] = snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan.at(idx));
+ snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan.at(idx));
  break;
  }
  }
@@ -1214,8 +1203,8 @@
  //don't reset sound for MISC_ANIM_MODE_HOLD
  if (Main_hall->misc_anim_modes.at(idx) != MISC_ANIM_MODE_HOLD) {
  // reset the "should be playing" flags
- for (s_idx=1;s_idx<10;s_idx++) {
- Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
+ for (s_idx=0;(unsigned)s_idx<Main_hall->misc_anim_sound_flag.at(idx).size();s_idx++) {
+ Main_hall->misc_anim_sound_flag.at(idx).at(s_idx) = 0;
  }
  }
  }
@@ -1607,12 +1596,8 @@
 //To be called after num_intercom_sounds has been parsed
 void intercom_sounds_init(main_hall_defines &m)
 {
- if (!m.intercom_delay.empty()) {
- //if one of these isn't empty, none of these should be empty
- Assert(!m.intercom_sounds.empty());
- Assert(!m.intercom_sound_pan.empty());
-
- //since we could be reparsing with a different number of intercom sounds, clear these and reinitialise
+ if (Cmdline_reparse_mainhall) {
+ //we could be reparsing with a different number of intercom sounds, so clear these and reinitialise
  m.intercom_delay.clear();
  m.intercom_sounds.clear();
  m.intercom_sound_pan.clear();
@@ -1638,21 +1623,17 @@
 
 void misc_anim_init(main_hall_defines &m)
 {
- if (!m.misc_anim_delay.empty()) {
- //if one of these isn't empty, none of these should be
- Assert(!m.misc_anim_paused.empty());
- Assert(!m.misc_anim_group.empty());
- Assert(!m.misc_anim_coords.empty());
- Assert(!m.misc_anim_modes.empty());
- Assert(!m.misc_anim_sound_pan.empty());
-
- //since we could be reparsing with a different number of misc anims, clear these and reinitialise
+ if (Cmdline_reparse_mainhall) {
+ //we could be reparsing with a different number of intercom sounds, so clear these and reinitialise
  m.misc_anim_delay.clear();
  m.misc_anim_paused.clear();
  m.misc_anim_group.clear();
  m.misc_anim_coords.clear();
  m.misc_anim_modes.clear();
  m.misc_anim_sound_pan.clear();
+ m.misc_anim_special_sounds.clear();
+ m.misc_anim_special_trigger.clear();
+ m.misc_anim_sound_flag.clear();
  }
 
  SCP_vector<int> temp;
@@ -1683,6 +1664,17 @@
 
  //misc_anim_sound_pan
  m.misc_anim_sound_pan.push_back(0);
+
+ //misc_anim_special_sounds
+ m.misc_anim_special_sounds.push_back(temp); //parse_sound_list deals with the rest of the initialisation for this one
+
+ //misc_anim_special_trigger
+ m.misc_anim_special_trigger.push_back(temp);
+
+ m.misc_anim_special_trigger.at(idx).push_back(0);
+
+ //misc_anim_sound_flag
+ m.misc_anim_sound_flag.push_back(temp);
  }
 }
 
@@ -1819,30 +1811,38 @@
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
  // anim sound id
- SCP_vector<int> temp; //this is a short term hack and will be properly fixed when main_hall_defines is vectorised
- m->misc_anim_special_sounds.push_back(temp);
  parse_sound_list("+Misc anim sounds:", m->misc_anim_special_sounds.at(idx), "+Misc anim sounds:", PARSE_SOUND_INTERFACE_SOUND);
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
  // anim sound triggers
  required_string("+Misc anim trigger:");
- stuff_int(&m->misc_anim_special_trigger[idx][0]);
- for (s_idx=0; s_idx<m->misc_anim_special_trigger[idx][0]; s_idx++) {
- stuff_int(&m->misc_anim_special_trigger[idx][s_idx + 1]);
+ int temp = 0;
+ stuff_int(&temp);
+ for (s_idx=0; s_idx<temp; s_idx++) {
+ m->misc_anim_special_trigger.at(idx).push_back(0);
+ stuff_int(&m->misc_anim_special_trigger.at(idx).at(s_idx));
  }
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
- // anim sound handles
- required_string("+Misc anim handles:");
- stuff_int(&m->misc_anim_sound_handles[idx][0]);
+ // anim sound handles - deprecated, but deal with it just in case
+ if(optional_string("+Misc anim handles:")) {
+ advance_to_eoln(NULL);
+ }
+
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
- // anim sound flags
- required_string("+Misc anim flags:");
- stuff_int(&m->misc_anim_sound_flag[idx][0]);
+ // anim sound flags - table flag deprecated, so ignore user input
+ if(optional_string("+Misc anim flags:")) {
+ advance_to_eoln(NULL);
+ }
+
+ //we need one flag for each sound
+ for(s_idx=0; (unsigned) s_idx<m->misc_anim_special_sounds.at(idx).size(); s_idx++) {
+ m->misc_anim_sound_flag.at(idx).push_back(0);
+ }
  }
 
  // door animations
[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:
I haven't tried it, but a couple of notes:

Code: [Select]
(unsigned)s_idx<Main_hall->misc_anim_sound_flag.at(idx).size() and other similar lines should be pointless.  Size() returns a size_t which is already unsigned, so casting it again should be unnessary.  What is more likely is you need to cast it to (int) becuase s_idx is also int.

Second thing, is there a specific reason that you are still using indexes at all (especially for loops). Why not use iterators?
"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
I have fixed the casting.

Regarding iterators, there are definitely places I could use them, but I do have reasons not to. Firstly, the entire reason I've been changing all the square bracket indexing to .at() as I've been vectorising is the automatic bounds checking it includes. Switching to iterators would put us right back to square one in that regard and would allow out-of-bounds access without throwing an exception; something I strongly want to avoid as exceptions very clearly identify what's gone wrong. Secondly, in many places the same index is used to access different vectors in the same loop. Iterators would not allow us to do that, and would require a fair bit more work to get going properly. Thirdly, some of the relevant code still uses arrays (both from and not from main_hall_defines) which are indexed by square brackets. Whilst I will reduce the number of arrays as I finish off vectorising, some of them are not part of main_hall_defines and thus will remain arrays when I'm done. Switching to iterators would still require them to be indexed by square brackets, again resulting in more work for (IMHO) negligible gain.

EDIT: and here's the patch with the fixed casting:

Code: [Select]
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8113)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -110,16 +110,13 @@
  //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];
+ //frame number triggers for each of the misc anims
+ SCP_vector<SCP_vector<int> > misc_anim_special_trigger;
 
- // [N][0] == # of handles, [N][1-9] == sound handle num
- int misc_anim_sound_handles[MAX_MISC_ANIMATIONS][10];
+ //flags for each of the misc anim sounds
+ SCP_vector<SCP_vector<int> > misc_anim_sound_flag;
 
- // [N][0] == # of handles, [N][1-9] == sound "should" be playing
- int misc_anim_sound_flag[MAX_MISC_ANIMATIONS][10];
 
-
  // door animations --------------------
 
  // # of door animations
@@ -503,7 +500,7 @@
  return;
  }
 
- int idx,s_idx;
+ int idx;
  char temp[100], whee[100];
 
  //reparse the table here if the relevant cmdline flag is set
@@ -639,14 +636,6 @@
  Main_hall_door_sound_handles[idx] = -1;
  }
 
- // zero out the misc anim sounds
- for (idx=0;idx<Main_hall->num_misc_animations;idx++) {
- for (s_idx = 1;s_idx < 10;s_idx++) {
- Main_hall->misc_anim_sound_handles[idx][s_idx] = -1;
- Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
- }
- }
-
  // skip the first frame
  Main_hall_frame_skip = 1;
 
@@ -1009,10 +998,9 @@
 
  // stop any playing misc animation sounds
  for (idx=0;idx<Main_hall->num_misc_animations;idx++) {
- for (s_idx=1;s_idx<10;s_idx++) {
- if (snd_is_playing(Main_hall->misc_anim_sound_handles[idx][s_idx])) {
- snd_stop(Main_hall->misc_anim_sound_handles[idx][s_idx]);
- Main_hall->misc_anim_sound_handles[idx][s_idx] = -1;
+ for (s_idx=0;s_idx<(int)Main_hall->misc_anim_special_sounds.at(idx).size();s_idx++) {
+ if (snd_is_playing(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx))) {
+ snd_stop(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx));
  }
  }
  }
@@ -1175,28 +1163,29 @@
  // kill the timestamp
  Main_hall->misc_anim_delay.at(idx).at(0) = -1;
 
+
  // reset the "should be playing" flags
- for (s_idx=1;s_idx<10;s_idx++) {
- Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
- }
+ for (s_idx=0;s_idx<(int)Main_hall->misc_anim_sound_flag.at(idx).size();s_idx++) {
+ Main_hall->misc_anim_sound_flag.at(idx).at(s_idx) = 0;
+ }
+
  }
  }
  // animation is not paused
  else {
- for (s_idx = 0; (unsigned)s_idx < Main_hall->misc_anim_special_sounds.at(idx).size(); s_idx++) {
+ for (s_idx = 0; s_idx < (int)Main_hall->misc_anim_special_sounds.at(idx).size(); 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+1]) && !Main_hall->misc_anim_sound_flag[idx][s_idx+1]) {
- Main_hall->misc_anim_sound_flag[idx][s_idx+1] = 1;
+ if ((Main_hall_misc_anim[idx].current_frame >= Main_hall->misc_anim_special_trigger.at(idx).at(s_idx)) && !Main_hall->misc_anim_sound_flag.at(idx).at(s_idx)) {
+ Main_hall->misc_anim_sound_flag.at(idx).at(s_idx) = 1;
 
  // if the sound is already playing, then kill it. This is a pretty safe thing to do since we can assume that
  // by the time we get to this point again, the sound will have been long finished
- if (snd_is_playing(Main_hall->misc_anim_sound_handles[idx][s_idx+1])) {
- snd_stop(Main_hall->misc_anim_sound_handles[idx][s_idx+1]);
- Main_hall->misc_anim_sound_handles[idx][s_idx+1] = -1;
+ if (snd_is_playing(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx))) {
+ snd_stop(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx));
  }
 
  // play the sound
- Main_hall->misc_anim_sound_handles[idx][s_idx+1] = snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan.at(idx));
+ snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan.at(idx));
  break;
  }
  }
@@ -1214,8 +1203,8 @@
  //don't reset sound for MISC_ANIM_MODE_HOLD
  if (Main_hall->misc_anim_modes.at(idx) != MISC_ANIM_MODE_HOLD) {
  // reset the "should be playing" flags
- for (s_idx=1;s_idx<10;s_idx++) {
- Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
+ for (s_idx=0;s_idx<(int)Main_hall->misc_anim_sound_flag.at(idx).size();s_idx++) {
+ Main_hall->misc_anim_sound_flag.at(idx).at(s_idx) = 0;
  }
  }
  }
@@ -1607,12 +1596,8 @@
 //To be called after num_intercom_sounds has been parsed
 void intercom_sounds_init(main_hall_defines &m)
 {
- if (!m.intercom_delay.empty()) {
- //if one of these isn't empty, none of these should be empty
- Assert(!m.intercom_sounds.empty());
- Assert(!m.intercom_sound_pan.empty());
-
- //since we could be reparsing with a different number of intercom sounds, clear these and reinitialise
+ if (Cmdline_reparse_mainhall) {
+ //we could be reparsing with a different number of intercom sounds, so clear these and reinitialise
  m.intercom_delay.clear();
  m.intercom_sounds.clear();
  m.intercom_sound_pan.clear();
@@ -1638,21 +1623,17 @@
 
 void misc_anim_init(main_hall_defines &m)
 {
- if (!m.misc_anim_delay.empty()) {
- //if one of these isn't empty, none of these should be
- Assert(!m.misc_anim_paused.empty());
- Assert(!m.misc_anim_group.empty());
- Assert(!m.misc_anim_coords.empty());
- Assert(!m.misc_anim_modes.empty());
- Assert(!m.misc_anim_sound_pan.empty());
-
- //since we could be reparsing with a different number of misc anims, clear these and reinitialise
+ if (Cmdline_reparse_mainhall) {
+ //we could be reparsing with a different number of intercom sounds, so clear these and reinitialise
  m.misc_anim_delay.clear();
  m.misc_anim_paused.clear();
  m.misc_anim_group.clear();
  m.misc_anim_coords.clear();
  m.misc_anim_modes.clear();
  m.misc_anim_sound_pan.clear();
+ m.misc_anim_special_sounds.clear();
+ m.misc_anim_special_trigger.clear();
+ m.misc_anim_sound_flag.clear();
  }
 
  SCP_vector<int> temp;
@@ -1683,6 +1664,17 @@
 
  //misc_anim_sound_pan
  m.misc_anim_sound_pan.push_back(0);
+
+ //misc_anim_special_sounds
+ m.misc_anim_special_sounds.push_back(temp); //parse_sound_list deals with the rest of the initialisation for this one
+
+ //misc_anim_special_trigger
+ m.misc_anim_special_trigger.push_back(temp);
+
+ m.misc_anim_special_trigger.at(idx).push_back(0);
+
+ //misc_anim_sound_flag
+ m.misc_anim_sound_flag.push_back(temp);
  }
 }
 
@@ -1819,30 +1811,38 @@
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
  // anim sound id
- SCP_vector<int> temp; //this is a short term hack and will be properly fixed when main_hall_defines is vectorised
- m->misc_anim_special_sounds.push_back(temp);
  parse_sound_list("+Misc anim sounds:", m->misc_anim_special_sounds.at(idx), "+Misc anim sounds:", PARSE_SOUND_INTERFACE_SOUND);
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
  // anim sound triggers
  required_string("+Misc anim trigger:");
- stuff_int(&m->misc_anim_special_trigger[idx][0]);
- for (s_idx=0; s_idx<m->misc_anim_special_trigger[idx][0]; s_idx++) {
- stuff_int(&m->misc_anim_special_trigger[idx][s_idx + 1]);
+ int temp = 0;
+ stuff_int(&temp);
+ for (s_idx=0; s_idx<temp; s_idx++) {
+ m->misc_anim_special_trigger.at(idx).push_back(0);
+ stuff_int(&m->misc_anim_special_trigger.at(idx).at(s_idx));
  }
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
- // anim sound handles
- required_string("+Misc anim handles:");
- stuff_int(&m->misc_anim_sound_handles[idx][0]);
+ // anim sound handles - deprecated, but deal with it just in case
+ if(optional_string("+Misc anim handles:")) {
+ advance_to_eoln(NULL);
+ }
+
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
- // anim sound flags
- required_string("+Misc anim flags:");
- stuff_int(&m->misc_anim_sound_flag[idx][0]);
+ // anim sound flags - table flag deprecated, so ignore user input
+ if(optional_string("+Misc anim flags:")) {
+ advance_to_eoln(NULL);
+ }
+
+ //we need one flag for each sound
+ for(s_idx=0; s_idx<(int)m->misc_anim_special_sounds.at(idx).size(); s_idx++) {
+ m->misc_anim_sound_flag.at(idx).push_back(0);
+ }
  }
 
  // door animations
« Last Edit: December 26, 2011, 06:14:41 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 Eli2

  • 26
How can an iterator even be out-of-bounds  :confused:

 

Offline CommanderDJ

  • Software engineer
  • 210
    • Minecraft
You can do pointer arithmetic with iterators (since that's pretty much what they are). So if I had an iterator called v_iter I could do this:
Code: [Select]
std::vector<int>::iterator v_iter = array.begin();
v_iter = v_iter+3;

If the array in question had a size of less than 4, if I then tried to access the element at v_iter, I would be performing an out-of-bounds access operation.
[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:
You can do pointer arithmetic with iterators (since that's pretty much what they are). So if I had an iterator called v_iter I could do this:
Code: [Select]
std::vector<int>::iterator v_iter = array.begin();
v_iter = v_iter+3;

If the array in question had a size of less than 4, if I then tried to access the element at v_iter, I would be performing an out-of-bounds access operation.
Okay, but why would you do such a thing?

I don't recall seeing any iteration in the mainhall.tbl that requires you to do more than adding one.

As for the rest of your points they are valid reasons for not using iterators, but I would still suggest that using iterators is a good idea because it would allow the simplification of some of the code.  Remember that mainhall.tbl is just an interface, it does not need to reflect how the data is stored, processed, or rendered by the engine.

For example, the parallel arrays that are used everywhere, could be replaced by a vector of a custom stuct, doing this would remove most of the need for indexes and when combined with the named items that you want to add later the mainhall code could be free of indexes all together.  Yes, I realize this is a large amount of work but even :v: shouldn't have used the parallel arrays to start with, becuase they unnecessarily complicate working on and modifying the data structures. 

Any way, just something to consider, because after all we are trying to reduce the code smell as well as add features.
"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
I understand. You do make some good points. However, would you allow me to finish off the vectorisation, have that committed, and then perhaps look at adding iterators later? It's technically a separate feature, so I'd like to work on it on it's own rather than doing at the same time as the vectors. It may mesh well with the naming later, so I might work on it at the same time there. I'd just like to finish the vectorisation before starting work on anything else is all.
[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:
Sure, your the one writing the patches, I just don't want to see this turn into mess that is the autopilot code.
"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
I am pleased to present the final vectorisation patch for main_hall_defines - every single array within the structure has been converted into a vector. I even went and got rid of the old and gross c-style strings and replaced them with SCP_strings.

I'd like to get this evaluated and committed as soon as possible, so please test! Apply straight to trunk (this supersedes the previous patch). Remember to try with -reparse_mainhall, and remember to test that +Misc anim handles and +Misc anim flags have been deprecated - omit them from your tables!

Code: [Select]
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 8113)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -59,12 +59,12 @@
 #define NUM_REGIONS 7 // (6 + 1 for multiplayer equivalent of campaign room)
 typedef struct main_hall_defines {
  // bitmap and mask
- char bitmap[MAX_FILENAME_LEN];
- char mask[MAX_FILENAME_LEN];
+ SCP_string bitmap;
+ SCP_string mask;
 
  // music
- char music_name[MAX_FILENAME_LEN];
- char substitute_music_name[MAX_FILENAME_LEN];
+ SCP_string music_name;
+ SCP_string substitute_music_name;
 
  // intercom defines -------------------
 
@@ -87,7 +87,7 @@
  int num_misc_animations;
 
  // filenames of the misc animations
- char misc_anim_name[MAX_MISC_ANIMATIONS][MAX_FILENAME_LEN];
+ SCP_vector<SCP_string> misc_anim_name;
 
  // Time until we will next play a given misc animation, min delay, and max delay
  SCP_vector<SCP_vector<int> > misc_anim_delay;
@@ -110,39 +110,36 @@
  //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];
+ //frame number triggers for each of the misc anims
+ SCP_vector<SCP_vector<int> > misc_anim_special_trigger;
 
- // [N][0] == # of handles, [N][1-9] == sound handle num
- int misc_anim_sound_handles[MAX_MISC_ANIMATIONS][10];
+ //flags for each of the misc anim sounds
+ SCP_vector<SCP_vector<int> > misc_anim_sound_flag;
 
- // [N][0] == # of handles, [N][1-9] == sound "should" be playing
- int misc_anim_sound_flag[MAX_MISC_ANIMATIONS][10];
 
-
  // door animations --------------------
 
  // # of door animations
  int num_door_animations;
 
  // filenames of the door animations
- char door_anim_name[MAX_DOOR_ANIMATIONS][MAX_FILENAME_LEN];
+ SCP_vector<SCP_string> door_anim_name;
 
  // first pair : coords of where to play a given door anim
  // second pair : center of a given door anim in windowed mode
- int door_anim_coords[MAX_DOOR_ANIMATIONS][4];
+ SCP_vector<SCP_vector<int> > door_anim_coords;
 
  // sounds for each region (open/close)
  SCP_vector<SCP_vector<int> > door_sounds;
 
  // pan values for the door sounds
- float door_sound_pan[MAX_DOOR_ANIMATIONS];
+ SCP_vector<float> door_sound_pan;
 
 
  // region descriptions ----------------
 
  // text (tooltip) description
- char *region_descript[NUM_REGIONS];
+ SCP_vector<char*> region_descript;
 
  // y coord of where to draw tooltip text
  int region_yval;
@@ -503,7 +500,7 @@
  return;
  }
 
- int idx,s_idx;
+ int idx;
  char temp[100], whee[100];
 
  //reparse the table here if the relevant cmdline flag is set
@@ -529,19 +526,19 @@
  Main_hall = &Main_hall_defines[gr_screen.res][main_hall_num];
 
  // tooltip strings
- Main_hall->region_descript[0] = XSTR( "Exit FreeSpace 2", 353);
- Main_hall->region_descript[1] = XSTR( "Barracks - Manage your FreeSpace 2 pilots", 354);
- Main_hall->region_descript[2] = XSTR( "Ready room - Start or continue a campaign", 355);
- Main_hall->region_descript[3] = XSTR( "Tech room - View specifications of FreeSpace 2 ships and weaponry", 356);
- Main_hall->region_descript[4] = XSTR( "Options - Change your FreeSpace 2 options", 357);
- Main_hall->region_descript[5] = XSTR( "Campaign Room - View all available campaigns", 358);
- Main_hall->region_descript[6] = XSTR( "Multiplayer - Start or join a multiplayer game", 359);
+ Main_hall->region_descript.at(0) = XSTR( "Exit FreeSpace 2", 353);
+ Main_hall->region_descript.at(1) = XSTR( "Barracks - Manage your FreeSpace 2 pilots", 354);
+ Main_hall->region_descript.at(2) = XSTR( "Ready room - Start or continue a campaign", 355);
+ Main_hall->region_descript.at(3) = XSTR( "Tech room - View specifications of FreeSpace 2 ships and weaponry", 356);
+ Main_hall->region_descript.at(4) = XSTR( "Options - Change your FreeSpace 2 options", 357);
+ Main_hall->region_descript.at(5) = XSTR( "Campaign Room - View all available campaigns", 358);
+ Main_hall->region_descript.at(6) = XSTR( "Multiplayer - Start or join a multiplayer game", 359);
 
  // init tooltip shader // nearly black
  gr_create_shader(&Main_hall_tooltip_shader, 5, 5, 5, 168);
 
  // load the background bitmap
- Main_hall_bitmap = bm_load(Main_hall->bitmap);
+ Main_hall_bitmap = bm_load(const_cast<char*>(Main_hall->bitmap.c_str()));
  if (Main_hall_bitmap < 0) {
  nprintf(("General","WARNING! Couldn't load main hall background bitmap %s\n", Main_hall->bitmap));
  }
@@ -556,7 +553,7 @@
  Main_hall_mask_h = -1;
 
  // load the mask
- Main_hall_mask = bm_load(Main_hall->mask);
+ Main_hall_mask = bm_load(const_cast<char*>(Main_hall->mask.c_str()));
  if (Main_hall_mask < 0) {
  nprintf(("General","WARNING! Couldn't load main hall background mask %s\n", Main_hall->mask));
  if (gr_screen.res == 0) {
@@ -573,10 +570,10 @@
 
  // load up the misc animations, and nullify all the delay timestamps for the misc animations
  for (idx=0;idx<Main_hall->num_misc_animations;idx++) {
- generic_anim_init(&Main_hall_misc_anim[idx], Main_hall->misc_anim_name[idx]);
+ generic_anim_init(&Main_hall_misc_anim[idx], const_cast<char*>(Main_hall->misc_anim_name.at(idx).c_str()));
  Main_hall_misc_anim[idx].ani.bg_type = bg_type;
  if (generic_anim_stream(&Main_hall_misc_anim[idx]) == -1) {
- nprintf(("General","WARNING!, Could not load misc %s anim in main hall\n",Main_hall->misc_anim_name[idx]));
+ nprintf(("General","WARNING!, Could not load misc %s anim in main hall\n",Main_hall->misc_anim_name.at(idx)));
  } else {
  //start paused
  if (Main_hall->misc_anim_modes.at(idx) == MISC_ANIM_MODE_HOLD)
@@ -592,10 +589,10 @@
 
  // load up the door animations
  for (idx=0;idx<Main_hall->num_door_animations;idx++) {
- generic_anim_init(&Main_hall_door_anim[idx], Main_hall->door_anim_name[idx]);
+ generic_anim_init(&Main_hall_door_anim[idx], const_cast<char*>(Main_hall->door_anim_name.at(idx).c_str()));
  Main_hall_door_anim[idx].ani.bg_type = bg_type;
  if (generic_anim_stream(&Main_hall_door_anim[idx]) == -1) {
- nprintf(("General","WARNING!, Could not load door anim %s in main hall\n",Main_hall->door_anim_name[idx]));
+ nprintf(("General","WARNING!, Could not load door anim %s in main hall\n",Main_hall->door_anim_name.at(idx)));
  } else {
  Main_hall_door_anim[idx].direction = GENERIC_ANIM_DIRECTION_BACKWARDS | GENERIC_ANIM_DIRECTION_NOLOOP;
  }
@@ -639,14 +636,6 @@
  Main_hall_door_sound_handles[idx] = -1;
  }
 
- // zero out the misc anim sounds
- for (idx=0;idx<Main_hall->num_misc_animations;idx++) {
- for (s_idx = 1;s_idx < 10;s_idx++) {
- Main_hall->misc_anim_sound_handles[idx][s_idx] = -1;
- Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
- }
- }
-
  // skip the first frame
  Main_hall_frame_skip = 1;
 
@@ -1009,10 +998,9 @@
 
  // stop any playing misc animation sounds
  for (idx=0;idx<Main_hall->num_misc_animations;idx++) {
- for (s_idx=1;s_idx<10;s_idx++) {
- if (snd_is_playing(Main_hall->misc_anim_sound_handles[idx][s_idx])) {
- snd_stop(Main_hall->misc_anim_sound_handles[idx][s_idx]);
- Main_hall->misc_anim_sound_handles[idx][s_idx] = -1;
+ for (s_idx=0;s_idx<(int)Main_hall->misc_anim_special_sounds.at(idx).size();s_idx++) {
+ if (snd_is_playing(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx))) {
+ snd_stop(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx));
  }
  }
  }
@@ -1048,13 +1036,13 @@
  hall = &Main_hall_defines[gr_screen.res][main_hall_num];
 
  // Goober5000 - try substitute first
- index = event_music_get_spooled_music_index(hall->substitute_music_name);
+ index = event_music_get_spooled_music_index(const_cast<char*>(hall->substitute_music_name.c_str()));
  if ((index >= 0) && (Spooled_music[index].flags & SMF_VALID)) {
  return index;
  }
 
  // now try regular
- index = event_music_get_spooled_music_index(hall->music_name);
+ index = event_music_get_spooled_music_index(const_cast<char*>(hall->music_name.c_str()));
  if ((index >= 0) && (Spooled_music[index].flags & SMF_VALID)) {
  return index;
  }
@@ -1175,28 +1163,29 @@
  // kill the timestamp
  Main_hall->misc_anim_delay.at(idx).at(0) = -1;
 
+
  // reset the "should be playing" flags
- for (s_idx=1;s_idx<10;s_idx++) {
- Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
- }
+ for (s_idx=0;s_idx<(int)Main_hall->misc_anim_sound_flag.at(idx).size();s_idx++) {
+ Main_hall->misc_anim_sound_flag.at(idx).at(s_idx) = 0;
+ }
+
  }
  }
  // animation is not paused
  else {
- for (s_idx = 0; (unsigned)s_idx < Main_hall->misc_anim_special_sounds.at(idx).size(); s_idx++) {
+ for (s_idx = 0; s_idx < (int)Main_hall->misc_anim_special_sounds.at(idx).size(); 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+1]) && !Main_hall->misc_anim_sound_flag[idx][s_idx+1]) {
- Main_hall->misc_anim_sound_flag[idx][s_idx+1] = 1;
+ if ((Main_hall_misc_anim[idx].current_frame >= Main_hall->misc_anim_special_trigger.at(idx).at(s_idx)) && !Main_hall->misc_anim_sound_flag.at(idx).at(s_idx)) {
+ Main_hall->misc_anim_sound_flag.at(idx).at(s_idx) = 1;
 
  // if the sound is already playing, then kill it. This is a pretty safe thing to do since we can assume that
  // by the time we get to this point again, the sound will have been long finished
- if (snd_is_playing(Main_hall->misc_anim_sound_handles[idx][s_idx+1])) {
- snd_stop(Main_hall->misc_anim_sound_handles[idx][s_idx+1]);
- Main_hall->misc_anim_sound_handles[idx][s_idx+1] = -1;
+ if (snd_is_playing(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx))) {
+ snd_stop(Main_hall->misc_anim_special_sounds.at(idx).at(s_idx));
  }
 
  // play the sound
- Main_hall->misc_anim_sound_handles[idx][s_idx+1] = snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan.at(idx));
+ snd_play(&Snds_iface[Main_hall->misc_anim_special_sounds.at(idx).at(s_idx)],Main_hall->misc_anim_sound_pan.at(idx));
  break;
  }
  }
@@ -1214,8 +1203,8 @@
  //don't reset sound for MISC_ANIM_MODE_HOLD
  if (Main_hall->misc_anim_modes.at(idx) != MISC_ANIM_MODE_HOLD) {
  // reset the "should be playing" flags
- for (s_idx=1;s_idx<10;s_idx++) {
- Main_hall->misc_anim_sound_flag[idx][s_idx] = 0;
+ for (s_idx=0;s_idx<(int)Main_hall->misc_anim_sound_flag.at(idx).size();s_idx++) {
+ Main_hall->misc_anim_sound_flag.at(idx).at(s_idx) = 0;
  }
  }
  }
@@ -1240,7 +1229,7 @@
  if (Main_hall_door_anim[idx].num_frames > 0) {
  // first pair : coords of where to play a given door anim
  // second pair : center of a given door anim in windowed mode
- generic_anim_render(&Main_hall_door_anim[idx], frametime, Main_hall->door_anim_coords[idx][0], Main_hall->door_anim_coords[idx][1]);
+ generic_anim_render(&Main_hall_door_anim[idx], frametime, Main_hall->door_anim_coords.at(idx).at(0), Main_hall->door_anim_coords.at(idx).at(1));
  }
  }
 }
@@ -1309,7 +1298,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.at(region).at(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.at(region));
 
  //TODO: track current frame
  snd_set_pos(Main_hall_door_sound_handles[region], &Snds_iface[SND_MAIN_HALL_DOOR_CLOSE],
@@ -1338,7 +1327,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.at(region).at(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.at(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) ) {
@@ -1366,8 +1355,8 @@
  }
 
  // set the position of the mouse cursor and the newly clicked region
- int mx = Main_hall->door_anim_coords[new_region][2];
- int my = Main_hall->door_anim_coords[new_region][3];
+ int mx = Main_hall->door_anim_coords.at(new_region).at(2);
+ int my = Main_hall->door_anim_coords.at(new_region).at(3);
  gr_resize_screen_pos( &mx, &my );
  mouse_set_pos( mx, my );
 
@@ -1551,13 +1540,13 @@
  if (!help_overlay_active(Main_hall_overlay_id)) {
  int shader_y = (Main_hall->region_yval) - Main_hall_tooltip_padding[gr_screen.res]; // subtract more to pull higher
  // get the width of the string
- gr_get_string_size(&w, NULL, Main_hall->region_descript[text_index]);
+ gr_get_string_size(&w, NULL, Main_hall->region_descript.at(text_index));
 
  gr_set_shader(&Main_hall_tooltip_shader);
  gr_shade(0, shader_y, gr_screen.clip_width_unscaled, (gr_screen.clip_height_unscaled - shader_y));
 
  gr_set_color_fast(&Color_bright_white);
- gr_string((gr_screen.max_w_unscaled - w)/2, Main_hall->region_yval, Main_hall->region_descript[text_index]);
+ gr_string((gr_screen.max_w_unscaled - w)/2, Main_hall->region_yval, Main_hall->region_descript.at(text_index));
  }
 }
 
@@ -1607,12 +1596,8 @@
 //To be called after num_intercom_sounds has been parsed
 void intercom_sounds_init(main_hall_defines &m)
 {
- if (!m.intercom_delay.empty()) {
- //if one of these isn't empty, none of these should be empty
- Assert(!m.intercom_sounds.empty());
- Assert(!m.intercom_sound_pan.empty());
-
- //since we could be reparsing with a different number of intercom sounds, clear these and reinitialise
+ if (Cmdline_reparse_mainhall) {
+ //we could be reparsing with a different number of intercom sounds, so clear these and reinitialise
  m.intercom_delay.clear();
  m.intercom_sounds.clear();
  m.intercom_sound_pan.clear();
@@ -1636,28 +1621,32 @@
  }
 }
 
+//helper function for initialising misc anim vectors based on number of anims
+//to be called after num_misc_animations has been parsed
 void misc_anim_init(main_hall_defines &m)
 {
- if (!m.misc_anim_delay.empty()) {
- //if one of these isn't empty, none of these should be
- Assert(!m.misc_anim_paused.empty());
- Assert(!m.misc_anim_group.empty());
- Assert(!m.misc_anim_coords.empty());
- Assert(!m.misc_anim_modes.empty());
- Assert(!m.misc_anim_sound_pan.empty());
-
- //since we could be reparsing with a different number of misc anims, clear these and reinitialise
+ if (Cmdline_reparse_mainhall) {
+ //we could be reparsing with a different number of misc anims, so clear these and reinitialise
+ m.misc_anim_name.clear();
  m.misc_anim_delay.clear();
  m.misc_anim_paused.clear();
  m.misc_anim_group.clear();
  m.misc_anim_coords.clear();
  m.misc_anim_modes.clear();
  m.misc_anim_sound_pan.clear();
+ m.misc_anim_special_sounds.clear();
+ m.misc_anim_special_trigger.clear();
+ m.misc_anim_sound_flag.clear();
  }
 
  SCP_vector<int> temp;
+ SCP_string temp_string;
 
  for (int idx=0; idx<m.num_misc_animations; idx++) {
+
+ //misc_anim_name
+ m.misc_anim_name.push_back(temp_string);
+
  //misc_anim_delay
  m.misc_anim_delay.push_back(temp);
 
@@ -1682,15 +1671,71 @@
  m.misc_anim_modes.push_back(MISC_ANIM_MODE_LOOP);
 
  //misc_anim_sound_pan
- m.misc_anim_sound_pan.push_back(0);
+ m.misc_anim_sound_pan.push_back(0.0f);
+
+ //misc_anim_special_sounds
+ m.misc_anim_special_sounds.push_back(temp); //parse_sound_list deals with the rest of the initialisation for this one
+
+ //misc_anim_special_trigger
+ m.misc_anim_special_trigger.push_back(temp);
+
+ m.misc_anim_special_trigger.at(idx).push_back(0);
+
+ //misc_anim_sound_flag
+ m.misc_anim_sound_flag.push_back(temp);
  }
 }
 
+//helper function for initialising door anim vectors based on number of anims
+//to be called after num_door_animations has been parsed
+void door_anim_init(main_hall_defines &m)
+{
+ if (Cmdline_reparse_mainhall) {
+ //since we could be reparsing with a different number of door anims, clear these and reinitialise
+ m.door_anim_name.clear();
+ m.door_anim_coords.clear();
+ m.door_sounds.clear();
+ m.door_sound_pan.clear();
+ m.region_descript.clear();
+ }
+
+ SCP_vector<int> temp;
+ SCP_string temp_string;
+
+ for(int idx=0; idx<m.num_door_animations; idx++)
+ {
+ //door_anim_name
+ m.door_anim_name.push_back(temp_string);
+
+ //door_anim_coords
+ m.door_anim_coords.push_back(temp);
+
+ //we want two pairs of coordinates for each animation
+ m.door_anim_coords.at(idx).push_back(0);
+ m.door_anim_coords.at(idx).push_back(0);
+ m.door_anim_coords.at(idx).push_back(0);
+ m.door_anim_coords.at(idx).push_back(0);
+
+ //door_sounds
+ m.door_sounds.push_back(temp);
+
+ //door_sound_pan
+ m.door_sound_pan.push_back(0.0f);
+ }
+
+ //region_descript
+ for (int idx=0; idx<NUM_REGIONS; idx++) {
+ m.region_descript.push_back(NULL);
+ }
+
+}
+
 // read in main hall table
 void main_hall_read_table()
 {
  main_hall_defines *m, temp;
  int count, idx, s_idx, m_idx, rval;
+ char temp_string[MAX_FILENAME_LEN];
 
  if ((rval = setjmp(parse_abort)) != 0) {
  mprintf(("TABLES: Unable to parse '%s'!  Error code = %i.\n", "mainhall.tbl", rval));
@@ -1706,7 +1751,7 @@
  while (!optional_string("#end")) {
  // read in 2 resolutions
  for (m_idx=0; m_idx<GR_NUM_RESOLUTIONS; m_idx++) {
- // maybe use a temp main hall stuct
+ // maybe use a temp main hall struct
  if (count >= MAIN_HALLS_MAX) {
  m = &temp;
  Warning(LOCATION, "Number of main halls in mainhall.tbl has exceeded max of %d. All further main halls will be ignored.", MAIN_HALLS_MAX);
@@ -1721,17 +1766,21 @@
 
  // bitmap and mask
  required_string("+Bitmap:");
- stuff_string(m->bitmap, F_NAME, MAX_FILENAME_LEN);
+ stuff_string(temp_string, F_NAME, MAX_FILENAME_LEN);
+ m->bitmap = temp_string;
 
  required_string("+Mask:");
- stuff_string(m->mask, F_NAME, MAX_FILENAME_LEN);
+ stuff_string(temp_string, F_NAME, MAX_FILENAME_LEN);
+ m->mask = temp_string;
 
  required_string("+Music:");
- stuff_string(m->music_name, F_NAME, MAX_FILENAME_LEN);
+ stuff_string(temp_string, F_NAME, MAX_FILENAME_LEN);
+ m->music_name = temp_string;
 
  // Goober5000
  if (optional_string("+Substitute Music:")) {
- stuff_string(m->substitute_music_name, F_NAME, MAX_FILENAME_LEN);
+ stuff_string(temp_string, F_NAME, MAX_FILENAME_LEN);
+ m->substitute_music_name = temp_string;
  }
 
  // intercom sounds
@@ -1778,7 +1827,8 @@
  for (idx=0; idx<m->num_misc_animations; idx++) {
  // anim names
  required_string("+Misc anim:");
- stuff_string(m->misc_anim_name[idx], F_NAME, MAX_FILENAME_LEN);
+ stuff_string(temp_string, F_NAME, MAX_FILENAME_LEN);
+ m->misc_anim_name.at(idx) = (SCP_string)temp_string;
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
@@ -1819,30 +1869,38 @@
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
  // anim sound id
- SCP_vector<int> temp; //this is a short term hack and will be properly fixed when main_hall_defines is vectorised
- m->misc_anim_special_sounds.push_back(temp);
  parse_sound_list("+Misc anim sounds:", m->misc_anim_special_sounds.at(idx), "+Misc anim sounds:", PARSE_SOUND_INTERFACE_SOUND);
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
  // anim sound triggers
  required_string("+Misc anim trigger:");
- stuff_int(&m->misc_anim_special_trigger[idx][0]);
- for (s_idx=0; s_idx<m->misc_anim_special_trigger[idx][0]; s_idx++) {
- stuff_int(&m->misc_anim_special_trigger[idx][s_idx + 1]);
+ int temp = 0;
+ stuff_int(&temp);
+ for (s_idx=0; s_idx<temp; s_idx++) {
+ m->misc_anim_special_trigger.at(idx).push_back(0);
+ stuff_int(&m->misc_anim_special_trigger.at(idx).at(s_idx));
  }
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
- // anim sound handles
- required_string("+Misc anim handles:");
- stuff_int(&m->misc_anim_sound_handles[idx][0]);
+ // anim sound handles - deprecated, but deal with it just in case
+ if(optional_string("+Misc anim handles:")) {
+ advance_to_eoln(NULL);
+ }
+
  }
 
  for (idx=0; idx<m->num_misc_animations; idx++) {
- // anim sound flags
- required_string("+Misc anim flags:");
- stuff_int(&m->misc_anim_sound_flag[idx][0]);
+ // anim sound flags - table flag deprecated, so ignore user input
+ if(optional_string("+Misc anim flags:")) {
+ advance_to_eoln(NULL);
+ }
+
+ //we need one flag for each sound
+ for(s_idx=0; s_idx<(int)m->misc_anim_special_sounds.at(idx).size(); s_idx++) {
+ m->misc_anim_sound_flag.at(idx).push_back(0);
+ }
  }
 
  // door animations
@@ -1853,25 +1911,27 @@
  m->num_door_animations = MAX_DOOR_ANIMATIONS;
  }
 
+ //initialise the door anim vectors
+ door_anim_init(*m);
+
  for (idx=0; idx<m->num_door_animations; idx++) {
  // door name
  required_string("+Door anim:");
- stuff_string(m->door_anim_name[idx], F_NAME, MAX_FILENAME_LEN);
+ stuff_string(temp_string, F_NAME, MAX_FILENAME_LEN);
+ m->door_anim_name.at(idx) = (SCP_string)temp_string;
  }
 
  for (idx=0; idx<m->num_door_animations; idx++) {
  // door coords
  required_string("+Door coords:");
- stuff_int(&m->door_anim_coords[idx][0]);
- stuff_int(&m->door_anim_coords[idx][1]);
- stuff_int(&m->door_anim_coords[idx][2]);
- stuff_int(&m->door_anim_coords[idx][3]);
+ stuff_int(&m->door_anim_coords.at(idx).at(0));
+ stuff_int(&m->door_anim_coords.at(idx).at(1));
+ stuff_int(&m->door_anim_coords.at(idx).at(2));
+ stuff_int(&m->door_anim_coords.at(idx).at(3));
  }
 
  for (idx=0; idx<m->num_door_animations; idx++) {
  // door open and close sounds
- SCP_vector<int> temp; //this is a short term hack and will be properly fixed when main_hall_defines is vectorised
- m->door_sounds.push_back(temp);
  parse_sound_list("+Door sounds:", m->door_sounds.at(idx), "+Door sounds:", (parse_sound_flags)(PARSE_SOUND_INTERFACE_SOUND | PARSE_SOUND_SCP_SOUND_LIST));
  }
 
@@ -1884,9 +1944,6 @@
  // tooltip y location
  required_string("+Tooltip Y:");
  stuff_int(&m->region_yval);
- for (idx=0; idx<NUM_REGIONS; idx++) {
- m->region_descript[idx] = NULL;
- }
  }
 
  if (count < MAIN_HALLS_MAX) {
@@ -1906,11 +1963,11 @@
  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_1024][hall].door_anim_name[OPTIONS_REGION], "2_vhallheads");
+ Main_hall_defines[GR_1024][hall].door_anim_name.at(OPTIONS_REGION) = "2_vhallheads";
 
  // set the background
- strcpy_s(Main_hall_defines[GR_640][hall].bitmap, "vhallhead");
- strcpy_s(Main_hall_defines[GR_1024][hall].bitmap, "2_vhallhead");
+ Main_hall_defines[GR_640][hall].bitmap = "vhallhead";
+ Main_hall_defines[GR_1024][hall].bitmap = "2_vhallhead";
  }
 
  // free up memory from parsing the mainhall tbl
@@ -1926,7 +1983,7 @@
 int main_hall_is_vasudan()
 {
  // kind of a hack for now
- return (!stricmp(Main_hall->music_name, "Psampik") || !stricmp(Main_hall->music_name, "Psamtik"));
+ return (!stricmp(Main_hall->music_name.c_str(), "Psampik") || !stricmp(Main_hall->music_name.c_str(), "Psamtik"));
 }
 
 // silence sounds on mainhall if we hit a pause mode (ie. lost window focus, minimized, etc);

If a coder could take a look at these as soon as possible, that'd be great!
[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