GenieClient / Genie4

.NET 6 Update of Genie
GNU General Public License v3.0
20 stars 14 forks source link

Fix for ParseQueue returning before parsing the full Section assigned to it #159

Closed mj-colonel-panic closed 9 months ago

mj-colonel-panic commented 10 months ago

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 commented 9 months ago

This fix is causing errors. Unfortunately, this is a deeply nested issue and will take a heavy rewrite to fix. Decision not to fix at this time. As a workaround, perens around the function evaluation will force it to recognize it as an evaluation and parse it appropriately.