4.0.2.6 - Conditional Statement: not taking true path when 'contains' function is in multi-variant evaluation

[umbravi]

I just picked up Genie4 for the first time after being away from the game for a few years. A chunk of my scripts aren't working correctly because the 'if' conditional isn't evaluating correctly when there are more than one evaluation in a statement AND the contains function is one of the evaluations.

When 'contains' (or '!contains') function is part of a multi-variant evaluation, the 'if' conditional is not correctly evaluating.

Given script names "test.cmd":

debug 10

if (true && true) then echo true
if contains("put|Put|get|Get", "%1") then echo 1 is put or get
if "%2" == "" then echo 2 is empty
if (contains("put|Put|get|Get", "%1") || "%2" == "") then echo 1 should be put or get or 2 should be empty
if (contains("put", "%1") && "%2" == "") then echo 1 should be put and 2 should be empty
if (!contains("inverse", "%1")) then echo 1 is not inverse
if (!contains("inverse multi", "%1") && "%2" == "") then echo 1 should not be inverse multi and 2 should be empty

Exit:
exit

And command: .test put

output is:

test.cmd(5): [if (true && true) then]
test.cmd(5): [echo true]
true
test.cmd(6): [if contains("put|Put|get|Get", "put") then]
test.cmd(6): [echo 1 is put or get]
1 is put or get
test.cmd(7): [if "" == "" then]
test.cmd(7): [echo 2 is empty]
2 is empty
test.cmd(8): [if (contains("put|Put|get|Get", "put") || "" == "") then]
test.cmd(9): [if (contains("put", "put") && "" == "") then]
test.cmd(10): [if (!contains("inverse", "put")) then]
test.cmd(10): [echo 1 is not inverse]
1 is not inverse
test.cmd(11): [if (!contains("inverse multi", "put") && "" == "") then]
test.cmd(13): [Exit:]
test.cmd(14): [exit]

[mj-colonel-panic]

Sorry for the lack of discussion on this. I looked into this shortly after you opened the issue and was unable to spot what was going on. I've continued looking into it and today I believe I've found the problem. I'm documenting what I've found here.

Relevant code is in the class Genie.Script.Eval

The process that occurs when evaluating statements is to break them into sections and parse each section
This is done by putting each segment of the statement into a queue and processing it in sections
For example, contains("put|Put|get|Get", "put") breaks into 4 segments, an array like ["(", "put|Put|get|Get", "put", ")"]
The segments are objects though, not strings, and contains flags of their own. This is a boon, as each has a flag to indicate whether it has been parsed. That will come into play for my proposed fix. As each segment is parsed, it flags that segment as having been parsed.

The function ParseQueue is used recursively to parse each section. It starts at the base of the array and iterates through. The cause of this bug seems to be in the logic here.

One way a section is identified is by using parentheses. In the enum these are ParseType.SectionStartType and ParseType.SectionEndType.

When the parser reaches a SectionEndType, it returns. When the Section contains a Function with its own Section, this can cause it to return before fully evaluating the entire section.

The fix I am currently testing - and so far it looks good - is to break instead of return, and depend on it having marked the segment as parsed to prevent it from parsing a second time. I am going to run a script on this for the next hour, as the change is in a significantly highly trafficked area of code. If it maintains the stability that it has as I've written this, and after I've had another programmer take a look and give their thoughts, if they agree with my assessment I will commit this change and put it in the test build.

[umbravi]

I'm glad you found the issue. :-D

[monil2233]

Reviving an old thread with a data point from the Genie 5 side, in case it's useful to whoever picks this up.

We used this issue as a port-fidelity check — running umbravi's exact seven-line repro against the Genie 5 script engine to see whether the behavior had carried over. It hasn't: all seven lines take the true path, including 8, 9 and 11.

The interesting part is why, because it corroborates @mj-colonel-panic's diagnosis above from a completely different direction.

Genie 5's expression evaluator is recursive-descent over a precedence ladder (ParseOr → ParseAnd → ParseNot → ParseCmp → ParseAdd → ParseMul → ParseUnary → ParseAtom), and a function call parses its own argument list through a nested ParseOr call. That structure means a call's closing paren is always consumed by the call that opened it — it can never be mistaken for the end of the surrounding group. The failure mode described here has nowhere to exist, which is consistent with the cause being exactly the flat-queue return-on-SectionEndType behavior identified in Eval.ParseQueue.

Put differently: the "break instead of return, and lean on the parsed flag" fix sketched above is doing by hand what a recursive-descent parser gets structurally. That seems like good evidence the diagnosis was right and the fix was pointed at the right place.

Two details from our testing that might save some time if this gets picked back up:

• The trigger needs all three of: outer parens, a nested call, and at least one more operand after that call's closing paren. That matches the trace in the report — line 6 (if contains(...) then, no outer parens) and line 10 (if (!contains(...)), nothing trailing the call) both behave correctly, and only the multi-operand forms fail.
• Worth checking while heads and waiteval as well as if, if those share the evaluator. And it's worth including cases whose correct answer is false: a parser that terminates early and yields true will pass every true-expecting case, so an all-true test set can look green while proving nothing. Nine of our thirty-one cases expect false for exactly that reason.

No action needed on the Genie 4 side on our account — just closing the loop, since we'd have had to re-derive the cause ourselves if this issue and that analysis hadn't been written down. Thanks for that.