Don't short circuit eval_sexp on nodes which contain the special argument.

[PhantomHoover]

This implements (a more efficient version of) the solution I outlined to the problem in this thread: https://www.hard-light.net/forums/index.php?topic=97146

Most (all?) sexps that touch the .cache element of a sexp node are careful to make sure that it isn't dependent on the special argument before doing so. This simply extends those checks to the short-circuit values in .value. Performance impact should be minimal: it's just checking a flag. Testing consisted of playing Her Finest Hour a lot and confirming it didn't break.

[Goober5000]

There appears to be some sort of confusion on how when-argument is used. The existing behavior is the correct behavior. If when-argument is in an event by itself, with no repeats or trigger counts, then it should only execute once. It should not continue to execute for all of its arguments.

In other words, the behavior described in this post...
https://www.hard-light.net/forums/index.php?topic=97146.msg1905733#msg1905733
...is correct, and is the way when-argument has behaved since it was originally implemented. If it is desired to have the event continue to evaluate its sexps even after the event has been satisfied, then every-time should be used.

[MageKing17]

If when-argument is in an event by itself, with no repeats or trigger counts, then it should only execute once.

Nobody has ever, at any point, said that there were no repeats or trigger counts.

[PhantomHoover]

It makes absolutely no sense to short-circuit a sexp node based on its result when evaluated with a different special argument. Goober, your example of 'intended' behaviour appears to have nothing at all to do with the semantics of this change.

[Goober5000]

Yeah the confusion was on my end. On further investigation this does appear to be a loose end that needs patching.

[Goober5000]

Wait, I want to put this on hold again. The new code will skip the short-circuiting if the special-arg appears in any node in the sexp tree, which is not what we want.

[Goober5000]

For this specific point:

It makes absolutely no sense to short-circuit a sexp node based on its result when evaluated with a different special argument.

It does make sense if the event is supposed to complete after a certain condition. If the event is satisfied, it doesn't matter how many other arguments remain.

Now, the sexp nodes containing the arguments are flushed upon re-evaluation in test_argument_nodes_for_condition. So, to the extent there is short-circuiting, it shouldn't apply to the argument nodes anyway. [Ok, this applies to the tree with the root at the condition node, not the trees with roots at the action nodes.]

Something else must be going on below the surface, or the interaction must be more complex than it appears.

[Goober5000]

Ok, I believe the root cause is the short-circuiting within eval_when itself. At the bottom of the function, eval_when will return SEXP_NODE_FALSE if it is short-circuiting. However, as indicated, this isn't appropriate for whens that are being evaluated for multiple arguments.

Try this:

// this is part of a *-argument tree, so return SEXP_TRUE like any normal action operator
if (Sexp_current_argument_nesting_level > 0)
{
	return SEXP_TRUE;
}
// this is a top-level "when", so short-circuit if necessary
else
{
	// thanks to MageKing17 for noticing that we need to short-circuit on the correct node!
	int short_circuit_node = (arg_handler >= 0) ? arg_handler : cond;

	if (Sexp_nodes[short_circuit_node].value == SEXP_KNOWN_FALSE || Sexp_nodes[short_circuit_node].value == SEXP_NAN_FOREVER)
		return SEXP_KNOWN_FALSE;  // no need to waste time on this anymore
}

[Goober5000]

Okay, after further analysis, with discussion on Discord, my concerns have been resolved.