Bug in BTree Selector

[maddinpsy]

Issue details

The guard in a selector is not handed correctly. The guard is checked and if it fails the run method is called anyway. In SingleRunningChildBranch line 88 is an else missing. The run method is called later on the child that succeeded. This child is called twice.
I could only see it in the debugger, when executing step by step. I used to set a breakpoint in LeafTask:42, this gets called twice for the last child.

Reproduction steps/code

root
  selector
    (care urgentProb:0) walk
    bark # bark will be called two times

Version of gdx-ai and/or relevant dependencies

gdx-ai1.8.0 and below

Solution

I solved it myself with the following code:
SingleRunningChildBranch: Line 82

                } else {
                    runningChild = children.get(currentChildIndex);
                }
                runningChild.setControl(this);
                runningChild.start();
                if (runningChild.checkGuard(this))
                    run();
                else
                    runningChild.fail();
            } else {
                // Should never happen; this case must be handled by subclasses in childXXX methods
            }
        }
    }

instead of

                } else {
                    runningChild = children.get(currentChildIndex);
                }
                runningChild.setControl(this);
                runningChild.start();
                if (!runningChild.checkGuard(this))
                    runningChild.fail();
                run();
            } else {
                // Should never happen; this case must be handled by subclasses in childXXX methods
            }
        }
    }

[davebaol]

Are you sure that this change does not break sequence?

[maddinpsy]

No I'm not sure, i haven't tested it

On July 29, 2016 11:13:02 AM GMT+02:00, davebaol notifications@github.com wrote:

Are you sure that this change does not break sequence?

---

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#72 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[maddinpsy]

It looks like it even fixes a bug in sequence. I just made a test with a
guarded Task in a sequence and got a stackOverFlow.

This AIScript throws a StackOverflow on the same code. With the new Code
it runs perfectly.

root
sequence
care urgentProb:1 #always succeeds
(care urgentProb:0) walk #always fails
selector
(care urgentProb:0) walk #always fails
bark # bark will be called two times

I also might have found a mistake in the BehaviorTreeViewer.

On 29.07.2016 11:13, davebaol wrote:

Are you sure that this change does not break sequence?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#72 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGNGzwC_xRfjn9vnQAxxZOanV9PkC9Hqks5qacQegaJpZM4JXvQw.

[davebaol]

Good catch!
The stack overflow is due to infinite recursion since the run method calls itself endlessly.
Any chance you can send a PR?

[maddinpsy]

I'm sorry to bring it up again. I think something broke when fixing this. A Sequence with a guarded Task will fail, if the guard fails. I'm not sure, but I think its not how its supposed to be.

root
  seqence
    (care urgentProb:0) walk #always fails
    bark # will never be called

I suppose the following behaviour of a guard in a sequence/selector:
Sequence Case1: Guard Succeed, Guarded Task Succeed => Sequence continues
Sequence Case2: Guard Succeed, Guarded Task Fails => Sequence fails
Sequence Case3: Guard Fails, Guarded Task not Executed => Sequence continues
Selector Case1: Guard Succeed, Guarded Task Succeed => Selector succeeds
Selector Case2: Guard Succeed, Guarded Task Fails => Selector continues
Selector Case3: Guard Fails, Guarded Task not Executed => Selector continues

[davebaol]

Sequence Case3: Guard Fails, Guarded Task not Executed => Sequence continues

No, when the guard fails the whole task, made up of the guard and the guarded task, fails.
So the expected behavior in a sequence is

 Sequence Case3: Guard Fails, Guarded Task not Executed => Sequence fails

[MeFisto94]

Sequence Case3: Guard Fails, Guarded Task not Executed => Sequence fails

I've just stumbled across this problem. I think it comes down to the definition of whether a task can be considered a failure when it didn't even run.

In my use case I have a sequence of things to do (namely to shoot at a player), and there is this line:

(RequiresReloading?) ReloadWeaponAndWait

With the current behavior such "optionals" would have to be wrapped into an alwaysSucceed, throwing away the result of ReloadWeaponAndWait or need a custom Sequence Node. I can see that this would break every existing behavior, but I wonder what the best approach would be for such conditionals part of a sequence?

Another example would be example 2:

import doorLocked?:"packageName.DoorLockedCondition"
import unlockDoor:"packageName.UnlockDoorAction"
import enterRoom:"packageName.EnterRoomAction"

root
  selector
    sequence
      doorLocked?
      unlockDoor
      enterRoom
    enterRoom

This could be simplified to

import doorLocked?:"packageName.DoorLockedCondition"
import unlockDoor:"packageName.UnlockDoorAction"
import enterRoom:"packageName.EnterRoomAction"

root
  sequence
    (doorLocked?) unlockDoor
    enterRoom

Which would be less verbose and much more intuitive.

Edit: And in the new case the old behavior could be replicated by explicitly specifying the guard, which to me seems more clear/intuitive. See

root
  sequence
    (doorLocked?) unlockDoor
     doorLocked?
     enterRoom

That way e.g. someone only enters the room when the door could be unlocked, just like the regular behavior would've been and it's obvious now that "doorLocked?" is a task in the sequence which has to be fulfilled.