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

Precondition Node should run children to completion before checking it's if statement again #3

Merged
merged 1 commit into from
Dec 30, 2024

Conversation

dsobek
Copy link

@dsobek dsobek commented Dec 27, 2024

I ran across an issue where a Precondition's "if" statement was evaluating from true to false while it's children were still executing. This resulted in the children only executing partway before the Precondition node short circuited the tree by returning the value of the "else" port unexpectedly.

For a more concrete example, suppose I want to implement a "RunOnce" behavior as a subtree, the xml would look like the following:

<Sequence>
    <Script code="first_run := true" />
    <Precondition if="first_run == true" else="FAILURE">
        <Sequence>
            <Script code="first_run := false" />
            <SomeLongRunningAction />
        </Control>
     </Decorator>
 </Control>

Over multiple ticks of SomeLongRunningAction this will shortcircuit return FAILURE from the "else" of Precondition. This PR would allow for SomeLongRunningAction (and any other following nodes that are children of Precondition) to run to Completion.

@dsobek dsobek force-pushed the precondition-fix-picknik branch from e57b4c9 to b1c0766 Compare December 27, 2024 04:47
@dsobek dsobek self-assigned this Dec 27, 2024
@dsobek dsobek requested review from nbbrooks and dyackzan December 27, 2024 04:50
@dsobek dsobek force-pushed the precondition-fix-picknik branch from b1c0766 to c167145 Compare December 30, 2024 16:37
@dsobek dsobek requested a review from dyackzan December 30, 2024 17:34
@dsobek dsobek merged commit ddc958e into master Dec 30, 2024
13 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants