View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0003132 | FSSCP | SEXPs | public | 2014-12-05 03:33 | 2014-12-10 01:43 |
Reporter | niffiwan | Assigned To | Goober5000 | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.7.2 RC4 | ||||
Summary | 0003132: is-iff returns true when the target is destroyed | ||||
Description | See the summary. It's minor because you can work around the issue by adding: ( and ( not ( is-destroyed-delay ... or something similar. | ||||
Additional Information | From a look through the code I think the issue is that sexp_get_object_ship_wing_point_team() returns OSWPT_TYPE_EXITED for ships/wings that are destroyed/departed. sexp_is_iff() doesn't handle that case and falls through to SEXP_TRUE. I think it should probably continue to return SEXP_FALSE in these cases. | ||||
Tags | No tags attached. | ||||
|
Or SEXP_KNOWN_FALSE, given that the ship won't spontaneously become un-destroyed... |
|
Interestingly, even before the OSWPT code was added, the sexp would have exhibited this behavior, because the destroyed-or-departed check didn't exit the function. Nevertheless, this is clearly a bug. I'm a little bit wary of changing anything because it's been around since 2006, but I'm going to assume that FREDders would have been smart enough to work around this if they had encountered a problem at the time. |
|
Mantis3132-sexp.cpp.patch (999 bytes)
Index: code/parse/sexp.cpp =================================================================== --- code/parse/sexp.cpp (revision 11187) +++ code/parse/sexp.cpp (working copy) @@ -9187,6 +9187,7 @@ } case OSWPT_TYPE_WING: + case OSWPT_TYPE_WING_NOT_PRESENT: { for (i = 0; i < oswpt.wingp->current_count; i++) { @@ -9197,7 +9198,27 @@ break; } - + + case OSWPT_TYPE_EXITED: + { + // see if we can find information about the exited ship (if it is a ship) + int exited_index = ship_find_exited_ship_by_name(CTEXT(n)); + if (exited_index >= 0) + { + // if the team doesn't match the team specified, return false immediately + if (Ships_exited[exited_index].team != team) + return SEXP_KNOWN_FALSE; + } + else + { + // it's probably an exited wing, which we don't store information about + return SEXP_NAN_FOREVER; + } + } + + // we don't handle the other cases + default: + return SEXP_NAN; } } |
|
Patch attached for evaluation. |
|
thanks. Patch works for my test case, I tested both destroying the ship and changing its IFF. Codewise, I'm not 100% sure what SEXP_NAN and SEXP_NAN_FOREVER do, but everything else looks fine. |
|
Here's what the code says: #define SEXP_NAN -32764 //-4 // not a number -- used when ships/wing part of boolean and haven't arrived yet #define SEXP_NAN_FOREVER -32763 //-5 // not a number and will never change -- used to falsify boolean sexpressions If something is NAN, it is in an indeterminate state and will be re-evaluated later... unless it's NAN_FOREVER, in which case the sexp is invalidated and not processed. |
|
Fix committed to trunk@11193. |
Date Modified | Username | Field | Change |
---|---|---|---|
2014-12-05 03:33 | niffiwan | New Issue | |
2014-12-05 06:18 | MageKing17 | Note Added: 0016409 | |
2014-12-06 22:46 | Goober5000 | Assigned To | => Goober5000 |
2014-12-06 22:46 | Goober5000 | Status | new => assigned |
2014-12-06 22:56 | Goober5000 | Note Added: 0016410 | |
2014-12-06 23:12 | Goober5000 | File Added: Mantis3132-sexp.cpp.patch | |
2014-12-06 23:12 | Goober5000 | Note Added: 0016411 | |
2014-12-09 05:08 | niffiwan | Note Added: 0016417 | |
2014-12-10 01:41 | Goober5000 | Note Added: 0016418 | |
2014-12-10 01:43 | Goober5000 | Changeset attached | => fs2open trunk r11193 |
2014-12-10 01:43 | Goober5000 | Note Added: 0016419 | |
2014-12-10 01:43 | Goober5000 | Status | assigned => resolved |
2014-12-10 01:43 | Goober5000 | Resolution | open => fixed |