Skip to content

Conversation

APickledWalrus
Copy link
Member

Problem

Delays are handled improperly by conditional sections. Specifically, the delay state is not reset properly before parsing branches. When a branch is parsed, it should use the delay state from BEFORE the start of the conditional change.

Solution

I have rewritten all of the delay logic in SecConditional. Now, before loadCode is called, the delay state is updated to use the delay state from right before the start of the conditional chain. Additionally, the logic for setting the delay state after the conditional chain has been reset. The "after" delay state is now tracked and set at each conditional (meaning it accounts for whether you stop at an "else" or an "else if").

If the delay state was true before the conditional, it will continue to be true after. Otherwise, if all of the conditional branches are false, the delay state is false. If all are true, the delay state is true. If it is mixed, the delay state is unknown.

For testing purposes, I added a simple new condition, CondHasDelayBefore, for testing the delay state. Additionally, since the test script code is delayed, it is not possible to make assertions. To circumvent this, I added an "assert unsafe" option which disables this check. The code is not actually executed at runtime and therefore there will not be a delay (maybe something to fix actually with SecParse?). Recommendations for alternative approaches would be appreciated.

Testing Completed

I have expanded SecConditional.sk with a new test case for delays. Additionally, I have added a regression test for explicitly tested the linked issue.

Supporting Information

n/a


Completes: #8139
Related: none

@APickledWalrus APickledWalrus requested a review from a team as a code owner August 22, 2025 15:27
@APickledWalrus APickledWalrus added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Aug 22, 2025
@APickledWalrus APickledWalrus requested a review from a team as a code owner August 22, 2025 15:27
@APickledWalrus APickledWalrus requested review from UnderscoreTud and TheMug06 and removed request for a team August 22, 2025 15:27
@APickledWalrus APickledWalrus moved this to In Review in 2.12 Releases Aug 22, 2025
@APickledWalrus APickledWalrus linked an issue Aug 22, 2025 that may be closed by this pull request
1 task
Copy link
Contributor

@Absolutionism Absolutionism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks good

@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.12 Releases Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue.
Projects
Status: Awaiting Merge
Development

Successfully merging this pull request may close these issues.

Wait in if statement branch causes the other branch to be delayed
3 participants