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

Found incorrect behavior (I think). Wrote some tests. #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Gaboose
Copy link

@Gaboose Gaboose commented Dec 15, 2014

I'm kind of new to behavior trees so it'd be nice if someone confirmed whether these failures are real / tests are correct. Though I'm fairly certain repeatUntilSucceed and repeatUntilFail should execute its child more than once to operate correctly.

Here's how the tests fail:

FAIL: testParallel_DelayedFailure (__main__.OwylTests)
Can parallel fail if child fails later (all succeed)?
AssertionError: Lists differ: [True] != [False]

FAIL: testParallel_DelayedSuccess (__main__.OwylTests)
Can parallel succeed if child succeeds later (one succeeds)?
AssertionError: Lists differ: [False] != [True]

FAIL: testRepeatAlways_Count (__main__.OwylTests)
Does repeatAlways execute its child with every tick?
AssertionError: 50 != 100

FAIL: testRepeatUntilFail_Count (__main__.OwylTests)
Does repeatUntilFail execute its child with every tick?
AssertionError: 1 != 100

FAIL: testRepeatUntilSucceed_Count (__main__.OwylTests)
Does repeatUntilSucceed execute its child with every tick?
AssertionError: 1 != 100

@linas
Copy link
Contributor

linas commented Dec 15, 2014

Did you mean "more than once" or "more than zero?" Normally, if one is asked to do something until one suceeds, and one gets it on the very first try, you get a gold star, and don't have to do it again.

If precision is a requirement, then the semantics are usually "at least once, and at most once". For example: an automated banking machine: "(repeat-until-success: call bank; decrement account balance $50); then dispense cash-money $50" -- You don't want to decrement $50 twice: you want to keep trying till you contact the bank, but once you succeeed, you are done.

@Gaboose
Copy link
Author

Gaboose commented Dec 15, 2014

I get that. But here, following your example, the success-check "try call bank" gets executed only once even though the network is down (the test is rigged so the repeatUntilSucceed never succeeds) and I stepped through the tree 100 times.

@eykd
Copy link
Owner

eykd commented Dec 16, 2014

Yes, there's something wrong with parallel. Good find! I'm looking into it.

@linas
Copy link
Contributor

linas commented Dec 31, 2014

Any reason this pull request cannot be merged?

Since Gaboose and I plan to use behvaior trees more and more, would it be possible to be listed as a project participant/maintainer for this project?

@Gaboose
Copy link
Author

Gaboose commented Jan 6, 2015

@eykd I took the liberty of fixing these myself 5d73706 . All tests succeed.

@eykd
Copy link
Owner

eykd commented Jan 6, 2015

Hey guys, sorry for the radio silence. I took a couple weeks off around Christmas and the New Year, and now I'm catching up on everything. In investigating this issue I discovered #3 which is potentially a serious issue as well. I was working on a fix before the break. I'll try to have that packaged up this week, potentially incorporating your changes @Gaboose once I've reviewed them. Thanks.

@Gaboose
Copy link
Author

Gaboose commented Jan 12, 2015

Thanks for looking into this. I myself can't tell whether visit's leaky values are an issue or not.

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.

3 participants