[General Battuta]

Axem and I are smaaaaaaashing into that 4096 character size limit! And while we can't pretend it's exactly urgent, it would make for some very pretty events.

How about it?

[Scotty]

I second this request.

[Angelus]

Seconded!

[The E]

... you seconded the seconding?

Is that legal?

[X3N0-Life-Form]

it is if you see it that way :

thing ---- second(thing) ---- second(second(thing)) ---- second(second(second(thing))) ---- etc.

[sigtau]

second(second(second(second(thisSuggestion))));

[karajorma]

I'm not certain how easy it would be to boost that limit (can't look at the code at the moment). What is causing the issue? Is it something we could solve by giving you a better way to do something instead?

[General Battuta]

No, not really. The problem is simply that we're making events which do a great many things at once and we run out of space. Splitting them up amongst multiple events is sometimes an option, but because you can't pass arguments between events without getting clumsy with vars, it's not always ideal.

When you start using nested conditionals and such event complexity grows rapidly and so does length.

[The E]

It's caused by the event code itself going over 4096 bytes, thus causing overflows when trying to save the mission.

Actually, this seems to be more of a fred-side limit.

[sigtau]

What is the event size limit's data type? (i.e. int, float, short, long int, etc.)?

[The E]

The problem is FRED's mission save/mission parse code. In various places, events are stored in 4096-element long char buffers.

The problem is that apparently, noone thought about #defining a constant to that effect, so that's what had to be done first.

Patch:

Code: [Select]

Index: code/fred2/campaigntreewnd.cpp
===================================================================
--- code/fred2/campaigntreewnd.cpp (revision 6703)
+++ code/fred2/campaigntreewnd.cpp (working copy)
@@ -410,7 +410,7 @@
 
 int campaign_tree_wnd::fred_check_sexp(int sexp, int type, char *msg, ...)
 {
- char buf[512], buf2[2048], buf3[4096];
+ char buf[512], buf2[MAX_SEXP_SIZE], buf3[MAX_SEXP_SIZE];
  int err = 0, z, faulty_node;
  va_list args;
 
@@ -425,7 +425,7 @@
  if (!z)
  return 0;
 
- convert_sexp_to_string(sexp, buf2, SEXP_ERROR_CHECK_MODE, 2048);
+ convert_sexp_to_string(sexp, buf2, SEXP_ERROR_CHECK_MODE, MAX_SEXP_SIZE);
  sprintf(buf3, "Error in %s: %s\n\nIn sexpression: %s\n\n(Error appears to be: %s)",
  buf, sexp_error_message(z), buf2, Sexp_nodes[faulty_node].text);
 
Index: code/fred2/fredview.cpp
===================================================================
--- code/fred2/fredview.cpp (revision 6703)
+++ code/fred2/fredview.cpp (working copy)
@@ -3271,7 +3271,7 @@
 
 int CFREDView::fred_check_sexp(int sexp, int type, char *msg, ...)
 {
- char buf[512], buf2[2048], buf3[4096];
+ char buf[512], buf2[MAX_SEXP_SIZE], buf3[MAX_SEXP_SIZE];
  int err = 0, z, faulty_node;
  va_list args;
 
@@ -3286,7 +3286,7 @@
  if (!z)
  return 0;
 
- convert_sexp_to_string(sexp, buf2, SEXP_ERROR_CHECK_MODE, 2048);
+ convert_sexp_to_string(sexp, buf2, SEXP_ERROR_CHECK_MODE, MAX_SEXP_SIZE);
  sprintf(buf3, "Error in %s: %s\n\nIn sexpression: %s\n(Error appears to be: %s)",
  buf, sexp_error_message(z), buf2, Sexp_nodes[faulty_node].text);
 
Index: code/fred2/missionsave.cpp
===================================================================
--- code/fred2/missionsave.cpp (revision 6703)
+++ code/fred2/missionsave.cpp (working copy)
@@ -722,7 +722,7 @@
 
  required_string_fred("+formula:");
  parse_comments();
- convert_sexp_to_string(The_mission.cutscenes[i].formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(The_mission.cutscenes[i].formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
  }
  }
@@ -911,7 +911,7 @@
 
  required_string_fred("$Formula:");
  parse_comments();
- convert_sexp_to_string(bs->formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(bs->formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  for ( j = 0; j < bs->num_icons; j++ ) {
@@ -1016,7 +1016,7 @@
  for (i=0; i<Debriefing->num_stages; i++) {
  required_string_fred("$Formula:");
  parse_comments(2);
- convert_sexp_to_string(Debriefing->stages[i].formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Debriefing->stages[i].formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  // XSTR
@@ -1484,7 +1484,7 @@
 
  required_string_fred("$Arrival Cue:");
  parse_comments();
- convert_sexp_to_string(Ships[i].arrival_cue, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Ships[i].arrival_cue, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  required_string_fred("$Departure Location:");
@@ -1540,7 +1540,7 @@
 
  required_string_fred("$Departure Cue:");
  parse_comments();
- convert_sexp_to_string(Ships[i].departure_cue, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Ships[i].departure_cue, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  required_string_fred("$Determination:");
@@ -2261,7 +2261,7 @@
 
  required_string_fred("$Arrival Cue:");
  parse_comments();
- convert_sexp_to_string(Wings[i].arrival_cue, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Wings[i].arrival_cue, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  required_string_fred("$Departure Location:");
@@ -2317,7 +2317,7 @@
 
  required_string_fred("$Departure Cue:");
  parse_comments();
- convert_sexp_to_string(Wings[i].departure_cue, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Wings[i].departure_cue, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  required_string_fred("$Ships:");
@@ -2434,7 +2434,7 @@
 
  required_string_fred("$Formula:");
  parse_comments();
- convert_sexp_to_string(Mission_goals[i].formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Mission_goals[i].formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  if ( Mission_goals[i].type & INVALID_GOAL ) {
@@ -3179,7 +3179,7 @@
 
 int CFred_mission_save::save_events()
 {
- char out[4096];
+ char out[MAX_SEXP_SIZE];
  int i;
 
  fred_parse_flag = 0;
@@ -3191,7 +3191,7 @@
  required_string_either_fred("$Formula:", "#Goals");
  required_string_fred("$Formula:");
  parse_comments(i ? 2 : 1);
- convert_sexp_to_string(Mission_events[i].formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Mission_events[i].formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  if (*Mission_events[i].name) {
@@ -4014,13 +4014,13 @@
 
  // if the link num is -1, then this is a end-of-campaign location
  if ( link_num != -1 ) {
- if (build_sexp_string(node, 2, SEXP_SAVE_MODE, 4096)) {
+ if (build_sexp_string(node, 2, SEXP_SAVE_MODE, MAX_SEXP_SIZE)) {
  fout("   (\n      %s\n      ( next-mission \"%s\" )\n   )\n", out, Campaign.missions[link_num].name);
  } else {
  fout("   ( %s( next-mission \"%s\" ) )\n", out, Campaign.missions[link_num].name);
  }
  } else {
- if (build_sexp_string(node, 2, SEXP_SAVE_MODE, 4096)) {
+ if (build_sexp_string(node, 2, SEXP_SAVE_MODE, MAX_SEXP_SIZE)) {
  fout("   (\n      %s\n      ( end-of-campaign )\n   )\n", out);
  } else {
  fout("   ( %s( end-of-campaign ) )\n", out );
Index: code/mission/missionparse.cpp
===================================================================
--- code/mission/missionparse.cpp (revision 6703)
+++ code/mission/missionparse.cpp (working copy)
@@ -5448,9 +5448,9 @@
  // entering this if statement will result in program termination!!!!!
  // print out an error based on the return value from check_sexp_syntax()
  if ( result ) {
- char sexp_str[4096], text[4500];
+ char sexp_str[MAX_SEXP_SIZE], text[4500];
 
- convert_sexp_to_string( i, sexp_str, SEXP_ERROR_CHECK_MODE, 4096);
+ convert_sexp_to_string( i, sexp_str, SEXP_ERROR_CHECK_MODE, MAX_SEXP_SIZE);
  sprintf(text, "%s.\n\nIn sexpression: %s\n(Error appears to be: %s)",
  sexp_error_message(result), sexp_str, Sexp_nodes[bad_node].text);
 
Index: code/parse/sexp.cpp
===================================================================
--- code/parse/sexp.cpp (revision 6703)
+++ code/parse/sexp.cpp (working copy)
@@ -19562,8 +19562,8 @@
  // make sure everything works okay
  if (arg_num == -1)
  {
- char sexp_text[4096];
- convert_sexp_to_string(cur_node, sexp_text, SEXP_ERROR_CHECK_MODE, 4096);
+ char sexp_text[MAX_SEXP_SIZE];
+ convert_sexp_to_string(cur_node, sexp_text, SEXP_ERROR_CHECK_MODE, MAX_SEXP_SIZE);
  Error(LOCATION, "Error finding sexp argument.  Received value %d for sexp:\n%s", sexp_val, sexp_text);
  }
 
Index: code/parse/sexp.h
===================================================================
--- code/parse/sexp.h (revision 6703)
+++ code/parse/sexp.h (working copy)
@@ -38,10 +38,11 @@
 #endif
 */
 
-#define MAX_SEXP_VARIABLES 100
+#define MAX_SEXP_VARIABLES 250
 
 #define MAX_SEXP_TEXT 2000
 #define MAX_OPERATORS 1024  // Yes, this is used, but not by the Sexp code.
+#define MAX_SEXP_SIZE 16384
 
 // Operator argument formats (data types of an argument)
 #define OPF_NONE 1 // argument cannot exist at this position if it's this

Test builds:
http://blueplanet.fsmods.net/E/FS2%20longer%20sexps.7z

[taylor]

How about just going ahead and making it dynamic and then not having to every worry about it again?

[General Battuta]

How about just going ahead and making it dynamic and then not having to every worry about it again?

This would be doubly orgasmic.

[Goober5000]

I'm betting the FRED stuff would be easier to convert to SCP_string than would the equivalent code in the main FSO library - as ironic as that sounds.

[The E]

Well, as I said, the main issue is the FRED mission saving code. Mission parsing seems to be able to handle events that FRED can't.

[The E]

Updated patch (No code changes, just updated to match current trunk revision):

Code: [Select]

Index: fred2/campaigntreewnd.cpp
===================================================================
--- fred2/campaigntreewnd.cpp (revision 6726)
+++ fred2/campaigntreewnd.cpp (working copy)
@@ -410,7 +410,7 @@
 
 int campaign_tree_wnd::fred_check_sexp(int sexp, int type, char *msg, ...)
 {
- char buf[512], buf2[2048], buf3[4096];
+ char buf[512], buf2[MAX_SEXP_SIZE], buf3[MAX_SEXP_SIZE];
  int err = 0, z, faulty_node;
  va_list args;
 
@@ -425,7 +425,7 @@
  if (!z)
  return 0;
 
- convert_sexp_to_string(sexp, buf2, SEXP_ERROR_CHECK_MODE, 2048);
+ convert_sexp_to_string(sexp, buf2, SEXP_ERROR_CHECK_MODE, MAX_SEXP_SIZE);
  sprintf(buf3, "Error in %s: %s\n\nIn sexpression: %s\n\n(Error appears to be: %s)",
  buf, sexp_error_message(z), buf2, Sexp_nodes[faulty_node].text);
 
Index: fred2/fredview.cpp
===================================================================
--- fred2/fredview.cpp (revision 6726)
+++ fred2/fredview.cpp (working copy)
@@ -3271,7 +3271,7 @@
 
 int CFREDView::fred_check_sexp(int sexp, int type, char *msg, ...)
 {
- char buf[512], buf2[2048], buf3[4096];
+ char buf[512], buf2[MAX_SEXP_SIZE], buf3[MAX_SEXP_SIZE];
  int err = 0, z, faulty_node;
  va_list args;
 
@@ -3286,7 +3286,7 @@
  if (!z)
  return 0;
 
- convert_sexp_to_string(sexp, buf2, SEXP_ERROR_CHECK_MODE, 2048);
+ convert_sexp_to_string(sexp, buf2, SEXP_ERROR_CHECK_MODE, MAX_SEXP_SIZE);
  sprintf(buf3, "Error in %s: %s\n\nIn sexpression: %s\n(Error appears to be: %s)",
  buf, sexp_error_message(z), buf2, Sexp_nodes[faulty_node].text);
 
Index: fred2/missionsave.cpp
===================================================================
--- fred2/missionsave.cpp (revision 6726)
+++ fred2/missionsave.cpp (working copy)
@@ -722,7 +722,7 @@
 
  required_string_fred("+formula:");
  parse_comments();
- convert_sexp_to_string(The_mission.cutscenes[i].formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(The_mission.cutscenes[i].formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
  }
  }
@@ -911,7 +911,7 @@
 
  required_string_fred("$Formula:");
  parse_comments();
- convert_sexp_to_string(bs->formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(bs->formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  for ( j = 0; j < bs->num_icons; j++ ) {
@@ -1016,7 +1016,7 @@
  for (i=0; i<Debriefing->num_stages; i++) {
  required_string_fred("$Formula:");
  parse_comments(2);
- convert_sexp_to_string(Debriefing->stages[i].formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Debriefing->stages[i].formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  // XSTR
@@ -1484,7 +1484,7 @@
 
  required_string_fred("$Arrival Cue:");
  parse_comments();
- convert_sexp_to_string(Ships[i].arrival_cue, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Ships[i].arrival_cue, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  required_string_fred("$Departure Location:");
@@ -1540,7 +1540,7 @@
 
  required_string_fred("$Departure Cue:");
  parse_comments();
- convert_sexp_to_string(Ships[i].departure_cue, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Ships[i].departure_cue, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  required_string_fred("$Determination:");
@@ -2261,7 +2261,7 @@
 
  required_string_fred("$Arrival Cue:");
  parse_comments();
- convert_sexp_to_string(Wings[i].arrival_cue, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Wings[i].arrival_cue, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  required_string_fred("$Departure Location:");
@@ -2317,7 +2317,7 @@
 
  required_string_fred("$Departure Cue:");
  parse_comments();
- convert_sexp_to_string(Wings[i].departure_cue, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Wings[i].departure_cue, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  required_string_fred("$Ships:");
@@ -2434,7 +2434,7 @@
 
  required_string_fred("$Formula:");
  parse_comments();
- convert_sexp_to_string(Mission_goals[i].formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Mission_goals[i].formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  if ( Mission_goals[i].type & INVALID_GOAL ) {
@@ -3179,7 +3179,7 @@
 
 int CFred_mission_save::save_events()
 {
- char out[4096];
+ char out[MAX_SEXP_SIZE];
  int i;
 
  fred_parse_flag = 0;
@@ -3191,7 +3191,7 @@
  required_string_either_fred("$Formula:", "#Goals");
  required_string_fred("$Formula:");
  parse_comments(i ? 2 : 1);
- convert_sexp_to_string(Mission_events[i].formula, out, SEXP_SAVE_MODE, 4096);
+ convert_sexp_to_string(Mission_events[i].formula, out, SEXP_SAVE_MODE, MAX_SEXP_SIZE);
  fout(" %s", out);
 
  if (*Mission_events[i].name) {
@@ -4014,13 +4014,13 @@
 
  // if the link num is -1, then this is a end-of-campaign location
  if ( link_num != -1 ) {
- if (build_sexp_string(node, 2, SEXP_SAVE_MODE, 4096)) {
+ if (build_sexp_string(node, 2, SEXP_SAVE_MODE, MAX_SEXP_SIZE)) {
  fout("   (\n      %s\n      ( next-mission \"%s\" )\n   )\n", out, Campaign.missions[link_num].name);
  } else {
  fout("   ( %s( next-mission \"%s\" ) )\n", out, Campaign.missions[link_num].name);
  }
  } else {
- if (build_sexp_string(node, 2, SEXP_SAVE_MODE, 4096)) {
+ if (build_sexp_string(node, 2, SEXP_SAVE_MODE, MAX_SEXP_SIZE)) {
  fout("   (\n      %s\n      ( end-of-campaign )\n   )\n", out);
  } else {
  fout("   ( %s( end-of-campaign ) )\n", out );
Index: mission/missionparse.cpp
===================================================================
--- mission/missionparse.cpp (revision 6726)
+++ mission/missionparse.cpp (working copy)
@@ -5448,9 +5448,9 @@
  // entering this if statement will result in program termination!!!!!
  // print out an error based on the return value from check_sexp_syntax()
  if ( result ) {
- char sexp_str[4096], text[4500];
+ char sexp_str[MAX_SEXP_SIZE], text[4500];
 
- convert_sexp_to_string( i, sexp_str, SEXP_ERROR_CHECK_MODE, 4096);
+ convert_sexp_to_string( i, sexp_str, SEXP_ERROR_CHECK_MODE, MAX_SEXP_SIZE);
  sprintf(text, "%s.\n\nIn sexpression: %s\n(Error appears to be: %s)",
  sexp_error_message(result), sexp_str, Sexp_nodes[bad_node].text);
 
Index: parse/sexp.cpp
===================================================================
--- parse/sexp.cpp (revision 6726)
+++ parse/sexp.cpp (working copy)
@@ -19579,8 +19579,8 @@
  // make sure everything works okay
  if (arg_num == -1)
  {
- char sexp_text[4096];
- convert_sexp_to_string(cur_node, sexp_text, SEXP_ERROR_CHECK_MODE, 4096);
+ char sexp_text[MAX_SEXP_SIZE];
+ convert_sexp_to_string(cur_node, sexp_text, SEXP_ERROR_CHECK_MODE, MAX_SEXP_SIZE);
  Error(LOCATION, "Error finding sexp argument.  Received value %d for sexp:\n%s", sexp_val, sexp_text);
  }
 
Index: parse/sexp.h
===================================================================
--- parse/sexp.h (revision 6726)
+++ parse/sexp.h (working copy)
@@ -38,10 +38,11 @@
 #endif
 */
 
-#define MAX_SEXP_VARIABLES 100
+#define MAX_SEXP_VARIABLES 250
 
 #define MAX_SEXP_TEXT 2000
 #define MAX_OPERATORS 1024  // Yes, this is used, but not by the Sexp code.
+#define MAX_SEXP_SIZE 16384
 
 // Operator argument formats (data types of an argument)
 #define OPF_NONE 1 // argument cannot exist at this position if it's this

Please review for inclusion in trunk. We've been testing it on BP for some time now without apparent adverse effects.

[Goober5000]

Have you given SCP_string a shot?

[The E]

No, not yet. Doing the conversion would, I think, take a lot of refactoring though.

EDIT:

Okay, someone who is better versed in this should try it. My attempts led to a severely broken FRED, which is not the point of the exercise.

[Goober5000]

It might help to have sprintf and a few other functions defined first.  I actually have a SCP_string conversion of my own in the works; I just need to run some tests before I commit it.