Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
umbravi opened this issue Jan 21, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@umbravi
Copy link

umbravi commented Jan 21, 2023

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
Copy link
Member

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.

@mj-colonel-panic mj-colonel-panic self-assigned this Nov 16, 2023
@mj-colonel-panic mj-colonel-panic added the bug Something isn't working label Nov 16, 2023
@umbravi
Copy link
Author

umbravi commented Nov 23, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants