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

Make DoUntil live up to its name #3

Merged
merged 2 commits into from Sep 5, 2020
Merged

Conversation

jschiff
Copy link
Contributor

@jschiff jschiff commented Sep 5, 2020

My understanding of DoUntil is that it should keep doing the action every frame UNTIL some condition is true. This change makes DoUntil behave that way. Currently it seems to be a clone of DoWhenTask.

My understanding of DoUntil is that it should keep doing the action every frame UNTIL some condition is true. This change makes DoUntil behave that way. Currently it seems to be a clone of DoWhenTask.
Copy link
Owner

@MartinIsla MartinIsla left a comment

Choose a reason for hiding this comment

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

I can't believe I missed this one. This is indeed the expected behaviour for DoUntilTask.

@jschiff
Copy link
Contributor Author

jschiff commented Sep 5, 2020

Don't forget to hit the merge button.

@MartinIsla MartinIsla merged commit 900ede6 into MartinIsla:master Sep 5, 2020
MartinIsla added a commit that referenced this pull request Sep 6, 2020
The behavior in DoUntilTask was actually right before. The task is executed until the condition is true.

# Conflicts:
#	Assets/STasks/Scripts/Task Variants/DoUntilTask.cs
@MartinIsla
Copy link
Owner

Sorry, I just realized the previous code was correct. The action is invoked until the condition is true. The task is completed (and terminated) once the condition is true. The proposed changes are more like a DoWhile, which doesn't exist yet, but it would be easy to implement.

This, however, made me notice another issue. The action shouldn't be invoked in the Complete() method in STask. It's a leftover from the times where the only possible task was what's now called DoTask.

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.

None yet

2 participants