Skip to content

Commit b341e85

Browse files
Goober5000Goober5000
authored
fix a few SEXP tree problems with variables and containers (scp-fs2open#7203)
1. Add default strings for variables and containers, providing something more appropriate than `<new default required!>`. 2. Replace the `Assert` in `add_default_operator` with a check for a valid variable index. This prevents a crash if FRED adds an operator that takes a variable as an argument and there are no variables in the mission. 3. Add the correct variable type to `get_listing_opf_variable_names()` to fix SEXP tree construction in certain circumstances. Apply the same fixes to QtFRED as well.
1 parent c428a22 commit b341e85

2 files changed

Lines changed: 62 additions & 14 deletions

File tree

‎fred2/sexp_tree.cpp‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2965,11 +2965,10 @@ int sexp_tree::add_default_operator(int op_index, int argnum)
29652965
item_handle = h;
29662966

29672967
} else {
2968+
int sexp_var_index;
29682969
// special case for sexps that take variables
29692970
const int op_type = query_operator_argument_type(op_index, argnum);
2970-
if (op_type == OPF_VARIABLE_NAME) {
2971-
int sexp_var_index = get_index_sexp_variable_name(item.text);
2972-
Assert(sexp_var_index != -1);
2971+
if ((op_type == OPF_VARIABLE_NAME) && ((sexp_var_index = get_index_sexp_variable_name(item.text)) >= 0)) {
29732972
int type = SEXPT_VALID | SEXPT_VARIABLE;
29742973
if (Sexp_variables[sexp_var_index].type & SEXP_VARIABLE_STRING) {
29752974
type |= SEXPT_STRING;
@@ -2983,6 +2982,7 @@ int sexp_tree::add_default_operator(int op_index, int argnum)
29832982
sprintf(node_text, "%s(%s)", item.text.c_str(), Sexp_variables[sexp_var_index].text);
29842983
add_variable_data(node_text, type);
29852984
}
2985+
// special case for sexps that take containers (this covers multiple container OPF_ types)
29862986
else if (item.type & SEXPT_CONTAINER_NAME) {
29872987
Assertion(is_container_name_opf_type(op_type) || op_type == OPF_DATA_OR_STR_CONTAINER,
29882988
"Attempt to add default container name for a node of non-container type (%d). Please report!",
@@ -2997,7 +2997,7 @@ int sexp_tree::add_default_operator(int op_index, int argnum)
29972997
Assert(argnum == 1);
29982998
sexp_list_item temp_item;
29992999
get_default_value(&temp_item, buf2, op_index, 0);
3000-
int sexp_var_index = get_index_sexp_variable_name(temp_item.text);
3000+
sexp_var_index = get_index_sexp_variable_name(temp_item.text);
30013001
Assert(sexp_var_index != -1);
30023002

30033003
// from name get type
@@ -3435,7 +3435,7 @@ int sexp_tree::get_default_value(sexp_list_item *item, char *text_buf, int op, i
34353435

34363436
case OPF_ANIMATION_NAME:
34373437
str = "<Animation trigger name>";
3438-
break;
3438+
break;
34393439

34403440
case OPF_CONTAINER_VALUE:
34413441
str = "<container value>";
@@ -3445,6 +3445,22 @@ int sexp_tree::get_default_value(sexp_list_item *item, char *text_buf, int op, i
34453445
str = Builtin_messages[0].name;
34463446
break;
34473447

3448+
case OPF_VARIABLE_NAME:
3449+
str = "<variable name>";
3450+
break;
3451+
3452+
case OPF_CONTAINER_NAME:
3453+
str = "<container name>";
3454+
break;
3455+
3456+
case OPF_LIST_CONTAINER_NAME:
3457+
str = "<list container name>";
3458+
break;
3459+
3460+
case OPF_MAP_CONTAINER_NAME:
3461+
str = "<map container name>";
3462+
break;
3463+
34483464
default:
34493465
str = "<new default required!>";
34503466
break;
@@ -7318,7 +7334,15 @@ sexp_list_item *sexp_tree::get_listing_opf_variable_names()
73187334

73197335
for (i=0; i<MAX_SEXP_VARIABLES; i++) {
73207336
if (Sexp_variables[i].type & SEXP_VARIABLE_SET) {
7321-
head.add_data( Sexp_variables[i].variable_name );
7337+
int t = 0;
7338+
if (Sexp_variables[i].type & SEXP_VARIABLE_NUMBER) {
7339+
t = SEXPT_NUMBER;
7340+
} else if (Sexp_variables[i].type & SEXP_VARIABLE_STRING) {
7341+
t = SEXPT_STRING;
7342+
} else {
7343+
Assertion(false, "SEXP variable must be a string or a number!");
7344+
}
7345+
head.add_data(Sexp_variables[i].variable_name, (t | SEXPT_VALID | SEXPT_VARIABLE));
73227346
}
73237347
}
73247348

‎qtfred/src/ui/widgets/sexp_tree.cpp‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,11 +1037,10 @@ int sexp_tree::add_default_operator(int op_index, int argnum) {
10371037
add_or_replace_operator(item.op);
10381038
item_index = index;
10391039
} else {
1040+
int sexp_var_index;
10401041
// special case for sexps that take variables
10411042
const int op_type = query_operator_argument_type(op_index, argnum);
1042-
if (op_type == OPF_VARIABLE_NAME) {
1043-
int sexp_var_index = get_index_sexp_variable_name(item.text);
1044-
Assert(sexp_var_index != -1);
1043+
if ((op_type == OPF_VARIABLE_NAME) && ((sexp_var_index = get_index_sexp_variable_name(item.text)) >= 0)) {
10451044
int type = SEXPT_VALID | SEXPT_VARIABLE;
10461045
if (Sexp_variables[sexp_var_index].type & SEXP_VARIABLE_STRING) {
10471046
type |= SEXPT_STRING;
@@ -1055,21 +1054,22 @@ int sexp_tree::add_default_operator(int op_index, int argnum) {
10551054
sprintf(node_text, "%s(%s)", item.text.c_str(), Sexp_variables[sexp_var_index].text);
10561055
add_variable_data(node_text, type);
10571056
}
1057+
// special case for sexps that take containers (this covers multiple container OPF_ types)
10581058
else if (item.type & SEXPT_CONTAINER_NAME) {
10591059
Assertion(is_container_name_opf_type(op_type) || op_type == OPF_DATA_OR_STR_CONTAINER,
10601060
"Attempt to add default container name for a node of non-container type (%d). Please report!",
10611061
op_type);
10621062
add_container_name(item.text.c_str());
10631063
}
1064-
// modify-variable data type depends on type of variable being modified
1065-
// (we know this block is handling the second argument since it's not OPF_VARIABLE_NAME)
1064+
// modify-variable data type depends on type of variable being modified
1065+
// (we know this block is handling the second argument since it's not OPF_VARIABLE_NAME)
10661066
else if (Operators[op_index].value == OP_MODIFY_VARIABLE) {
10671067
// the the variable name
10681068
char buf2[256];
10691069
Assert(argnum == 1);
10701070
sexp_list_item temp_item;
10711071
get_default_value(&temp_item, buf2, op_index, 0);
1072-
int sexp_var_index = get_index_sexp_variable_name(temp_item.text);
1072+
sexp_var_index = get_index_sexp_variable_name(temp_item.text);
10731073
Assert(sexp_var_index != -1);
10741074

10751075
// from name get type
@@ -1084,7 +1084,7 @@ int sexp_tree::add_default_operator(int op_index, int argnum) {
10841084
}
10851085
add_data(item.text.c_str(), type);
10861086
}
1087-
// all other sexps and parameters
1087+
// all other sexps and parameters
10881088
else {
10891089
add_data(item.text.c_str(), item.type);
10901090
}
@@ -1481,6 +1481,22 @@ int sexp_tree::get_default_value(sexp_list_item* item, char* text_buf, int op, i
14811481
str = Builtin_messages[0].name;
14821482
break;
14831483

1484+
case OPF_VARIABLE_NAME:
1485+
str = "<variable name>";
1486+
break;
1487+
1488+
case OPF_CONTAINER_NAME:
1489+
str = "<container name>";
1490+
break;
1491+
1492+
case OPF_LIST_CONTAINER_NAME:
1493+
str = "<list container name>";
1494+
break;
1495+
1496+
case OPF_MAP_CONTAINER_NAME:
1497+
str = "<map container name>";
1498+
break;
1499+
14841500
default:
14851501
str = "<new default required!>";
14861502
break;
@@ -5187,7 +5203,15 @@ sexp_list_item* sexp_tree::get_listing_opf_variable_names() {
51875203

51885204
for (i = 0; i < MAX_SEXP_VARIABLES; i++) {
51895205
if (Sexp_variables[i].type & SEXP_VARIABLE_SET) {
5190-
head.add_data(Sexp_variables[i].variable_name);
5206+
int t = 0;
5207+
if (Sexp_variables[i].type & SEXP_VARIABLE_NUMBER) {
5208+
t = SEXPT_NUMBER;
5209+
} else if (Sexp_variables[i].type & SEXP_VARIABLE_STRING) {
5210+
t = SEXPT_STRING;
5211+
} else {
5212+
Assertion(false, "SEXP variable must be a string or a number!");
5213+
}
5214+
head.add_data(Sexp_variables[i].variable_name, (t | SEXPT_VALID | SEXPT_VARIABLE));
51915215
}
51925216
}
51935217

0 commit comments

Comments
 (0)