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

Discussing Behavior Trees #12

Open
davebaol opened this issue Oct 5, 2014 · 143 comments
Open

Discussing Behavior Trees #12

davebaol opened this issue Oct 5, 2014 · 143 comments

Comments

@davebaol
Copy link
Member

davebaol commented Oct 5, 2014

I've opened this to discuss behavior trees API enhancements.

@implicit-invocation
Resuming discussion #4 (comment) ...
Not sure why you don't like the XML format, but I think that it has some advantages:

  • comments are supported which can be really useful
  • it's more powerful than json or any inline tab-based format.
  • being it a standard format, external tools like editors (written in any language) can easily read/write a behavior tree.

So I'm playing with the XML format just to see what can be done.
Currently I can successfully load a behavior tree from this file:

<BehaviorTree>
  <Import task="com.badlogic.gdx.ai.tests.btree.dogtasks.BarkTask" as="Bark"/>
  <Import task="com.badlogic.gdx.ai.tests.btree.dogtasks.CareTask" as="Care"/>
  <Import task="com.badlogic.gdx.ai.tests.btree.dogtasks.MarkTask" as="Mark"/>
  <Import task="com.badlogic.gdx.ai.tests.btree.dogtasks.RestTask" as="Rest"/>
  <Import task="com.badlogic.gdx.ai.tests.btree.dogtasks.WalkTask" as="Walk"/>
  <Root>
    <Selector>
      <Parallel>
        <com.badlogic.gdx.ai.tests.btree.dogtasks.CareTask urgentProb="0.8"/>
        <AlwaysFail>
          <com.badlogic.gdx.ai.tests.btree.dogtasks.RestTask/>
        </AlwaysFail>
      </Parallel>
      <Sequence>
        <Bark times="3"/>
        <Walk/>
        <Bark/> <!-- times defaults to 1, see BarkTask source code -->
        <com.badlogic.gdx.ai.tests.btree.dogtasks.MarkTask/>
      </Sequence>
    </Selector>
  </Root>
</BehaviorTree>

I added the "Import" tag to improve readability. It allows you to use the given alias in place of the fully qualified class name of the task. Actually Selector, Parallel, etc.. are predefined imports. Also, the "as" attribute is optional, meaning that the simple class name is used as the alias, i.e.

  <Import task="com.badlogic.gdx.ai.tests.btree.dogtasks.BarkTask"/>

creates the task alias "BarkTask".
Also, I added task parameters, see urgentProb in CareTask and times in BarkTask.
The attribute value is parsed according to the type of the corresponding field of the task class. For example, urgentProb is a float and times is an int. Supported types are: int, Integer, float, Float, boolean, Boolean, long, Long, double, Double, short, Short, char, Character, byte, Byte, and String.

Of course, we can maintain both formalisms as long as they have the same structural features. I mean, unlike task parameters, imports are just a syntactic sugar so they are not mandatory for the inline tab-based formalism.

I think we can use a "btree" branch in order to experiment with BT improvements while keeping the master branch clean.

@davebaol
Copy link
Member Author

davebaol commented Oct 5, 2014

Ok pushed the btree branch.

I'm not fully convinced by the METADATA static field, but it's what I've come up with so far.
METADATA contains task information for loaders and editors such as the minimum/maximum number of children and the set of attributes. When writing a new task the developer must declare a METADATA field only if the min/max number of children or the attributes change with respect to the task superclass.
Having to add/remove attributes manually in METADATA may look annoying but gives you total control, preventing the user (the one that is creating the tree) from changing task fields that should not even be accessed.

@davebaol
Copy link
Member Author

davebaol commented Oct 5, 2014

Now all branch nodes have an optional attribute "deterministic" which defaults to true.
I'm not sure it makes sense for Parallel. Should we remove it from Parallel METADATA?
We can do it like that:

// Copy BranchNode.METADATA, add no attribute and remove deterministic attribute
public static final Metadata METADATA = new Metadata(BranchNode.METADATA, new String[]{}, "deterministic");

or like that:

// Create a new METADATA with no limit on the number of children and with no attributes
public static final Metadata METADATA = new Metadata(-1);

@implicit-invocation
Copy link

If we are thinking about making an editor, XML will be fine with me.
I just don't want anyone to write XML by hand 😄
If XML is generated by tools and we just have to edit some numbers or move some lines manually, I think everyone will be happy.

If we are using an XML syntax here, I don't think we should do

<com.badlogic.gdx.ai.tests.btree.dogtasks.RestTask />

or even

<bark />

How can we validate our XML then?
It should be

<task name="bark" />
<task class="com.badlogic.gdx.ai.tests.btree.dogtasks.RestTask" />

for attribute

<repeat>
  <param name="time" value="3" />
  <tasks>
      <task name="bark" />
  </tasks>
</repeat>

With XML, everything is string, so how we can tell it's a "3" string and not a number with value of 3. Writing something like

<param name="title" value="3" type="string" />

is ugly 😄
We can do it more easily with JSON or our own format because not every value must be in quotes.
And if not for formalization and validation, I don't think XML is the best choice here.

About METADATA, I'm not sure what you are using it for here and why it is static.
If we need some configurations for all the sequence or parallel nodes, we can put it in the tree instance or in your BehaviorTreeLibrary static members. The configurations can be loaded from a file by methods.
If we need parameters for 1 node, a map is enough
For example, we have

<param name="time" value="3" type="int" />

The node will have a parameter map and set get method

private Map<String, Object> paramters;
public <E> void set(String name, E value, Class<E> type);
public <E> E get(String name, Class<E> type);
// some shortcut methods
public void setInt(String name, int value);
public int getInt(String name);
...

Parser will call setInt("time", 3) after create the node.
The node will refer to the parameter getInt("time")

The time this node will be repeated is init param but the time the node did run is state.
I think state can be stored in a map like that too, a different map, called data maybe.
The get, set for parameters will be called setParam and getParam then.
And the get, set for data will be call setData and getData.
It will make the code harder to read but will make the serialization easier (I want lag compensation on a network use case too).

When we clone the tree, we clone only the params, not the data.
When we store the tree state (to rewind in lag compensation use case) we store both params and data.

@davebaol
Copy link
Member Author

davebaol commented Oct 6, 2014

@implicit-invocation

How can we validate our XML then?

Yes, we miss XML validation (which is always optional, anyways) but we can still verify if the document is well-formed.
Despite being less formal, using class name or alias as a tag and parameters as attributes is less verbose and more readable. Even Ant uses the same technique.

With XML, everything is string, so how we can tell it's a "3" string and not a number with value of 3. Writing something like <param name="title" value="3" type="string" /> is ugly 😄

Yep, it's really terrible :)
Anyways, in "3" quotes are part of the XML syntax, so it's just 3. The actual type can be inferred from the class or the primitive type of the corresponding field/setter, just like it happens with JSON.

We can do it more easily with JSON or our own format because not every value must be in quotes.
And if not for formalization and validation, I don't think XML is the best choice here.

I think that the problem with JSON, is that you have to give everything a name and you're forced to use arrays. For example in

{task:selector, deterministic:false, children: [
  {task:com.badlogic.gdx.ai.tests.btree.dogtasks.BarkTask},
  {task:com.badlogic.gdx.ai.tests.btree.dogtasks.CareTask}
]}

you must create an array named children and all those [{...}, ..., {..}] are more error prone and less readable IMO. With XML, children are simply nested tags, which looks to me like a more natural representation for a tree structure.

"Unfortunately" I have a meeting now. I'm going to reply the other stuff later.

Anyways, at the moment my major intent is to find the best compromise among simplicity, usability and performance for such a transitory where users have to write trees by hand since no editor is (yet) available.

And thanks for your valuable considerations, really. :)
Later

@implicit-invocation
Copy link

The actual type can be inferred from the class or the primitive type of the corresponding field/setter, just like it happens with JSON.

With JSON, {"key": "value"} means string, {"key": true} means boolean and {"key": 3} means number. To do it with XML, it will become <sequence title="\"some random title\""> and even uglier.

I agree with you about the array notation being error prone and less readable. But it isn't true that you have to give everything a name

{
  "import": {
    "bark": "com.badlogic.gdx.ai.tests.btree.dogtasks.BarkTask",
    "care": "com.badlogic.gdx.ai.tests.btree.dogtasks.CareTask"
  },
  "root": {
    "sequence": {
      "title": "test",
      "children": [
        "bark",
        "com.badlogic.gdx.ai.tests.btree.dogtasks.WalkTask",
        {
          "repeat": {
            "time": 3,
            "children": [
              "bark"
            ]
          }
        }
      ]
    }
  }
}

I still prefer something we can write by hands

import
  some_registered_task:"packageName.className"
root
  type:sequence
    task:fromClass className:"packageName.className"
    task:fromScript scriptFile:"script_file_path"
    type:limit count:3
      type:some_registered_task
    type:include file:"path_to_file" lazy:true

I think it is easy enough to read, to write a parser or even to export from tools.

@davebaol
Copy link
Member Author

davebaol commented Oct 6, 2014

Well, yes I meant that with JSON you're forced to use an array when the order is relevant since properties are unordered.
Also, I was assuming that the XML tree is backed up by the Java classes of the nodes at runtime, so we can use reflection to infer the actual type of the attributes. This is always true when you run the tree form inside your Java application, but you're right, it's no longer true for external tools that parse the XML without having node classes in the classpath or if they are written with any other non Java language. Good point! 👍

Back to the inline format then!
Maybe we might make it a bit simpler and less verbose. I mean, what's the point of all those "type:" and "task:" ? Couldn't it just be something as simple as

import
  care:"packageName.CareTask"
root
  sequence
    care urgentProb:0.8
    script file:"script_file_path"
    limit count:3
      packageName.BarkTask
    include file:"path_to_file" lazy:true

By the way, the "times" attribute in BarkTask was just an example. I know it's recommended to use a limit decorator for things like that. :)

@implicit-invocation
Copy link

I'm not sure about the "deterministic" attribute you use for the branch nodes.
Sequences and selectors will always pick the next child to run until one child fail or succeed.
So if the node list is ordered, they will always be deterministic.
Are you talking about a RandomSelector?
If it is a random selector, it will be non-deterministic by default, unless we choose a deterministic random algorithm.

@davebaol
Copy link
Member Author

davebaol commented Oct 7, 2014

@implicit-invocation
Did you look into the implementation?
https://github.com/libgdx/gdx-ai/blob/btree/gdx-ai/src/com/badlogic/gdx/ai/btree/BranchNode.java#L56

If the branch is non-deterministic the actualTask is chosen randomly among the next children.

EDIT:
Note that children are swapped.

@implicit-invocation
Copy link

I did look at the implementation.
But I think it would be better if we make a RandomSelector and not make everything have a random behavior like that.
A sequence that is not sequential is semantically wrong.

@davebaol
Copy link
Member Author

davebaol commented Oct 7, 2014

Hmmm.... why? Suppose you want to burn something.

sequence deterministic:false
  task:GetMatches
  task:GetGasoline

The order you get matches and gasoline is irrelevant but you need both. Am I missing something?

@implicit-invocation
Copy link

Oh, I missed that case.
But I'm more familiar with the concept provided by aigamedev.com where

a sequence respects its order, it implicitly expresses dependencies between its child behaviors.

I usually use sequence in cases that the entity checks for a condition before executing an action. (I use task for condition)

And in your case where the order is less relevant, I'm thinking of a Probability selector

Probability selectors pick one of their child nodes randomly based on the weights provided by the designer.

We can even rank the node for a soft order, GetMatches and GetGasoline will have a same weight then.

@davebaol
Copy link
Member Author

davebaol commented Oct 7, 2014

Yeah the use of a deterministic sequence whose first child is a condition task in the most common case.
However when the order is irrelevant and the children have equal probability a non deterministic sequence might come in handy. Also, the overhead for deterministic branches is just an "if" per child.

Anyways, in order to make the file format easier for both the parser and the user I'm thinking of something like that:

import care:"packageName.CareTask"
import bark:"packageName.BarkTask"
root
  sequence
    care urgentProb:0.8
    script file:"script_file_path"
    limit count:3
      bark
    include file:"path_to_file" lazy:true

This way all the lines of the file have the same format:

indent name attr1:val1 attr2:val2 ...

Currently I'm rewriting the parser with Ragel which is a bit hard to learn when you're new to it, but it's really cool because is very fast and most importantly is cross-platform, meaning that external non Java tools can easily use similar code to load behavior trees as long as Ragel supports that particular programming language.

@davebaol
Copy link
Member Author

davebaol commented Oct 8, 2014

@implicit-invocation
Ok just pushed the new parser written with Ragel.
Lines in the file have the following syntax:

[[indent] [name [attr:value ...]] [comment]]

where:

  • name is in the form of a Java fully qualified class name
  • attr is in the form of Java identifier
  • value must be a boolean, a number or a quoted string accepting JSON-like escape sequences
  • comment starts with a # and extends up to the newline

Sample tree: https://github.com/libgdx/gdx-ai/blob/btree/tests/data/dog.tree

Currently METADATA is still used, but I'm open to use a different approach. :)

@implicit-invocation
Copy link

Wow, you are fast.
I am also experimenting with something I have in mind (mostly stuffs related to the traversal).
Will discuss with you about them soon.

@davebaol
Copy link
Member Author

davebaol commented Oct 8, 2014

Eh I had just to learn how to use Ragel and add JSON-like values, the rest is a mix of your old parser and my xml parser.
Looking forward to discussing your new stuff :)

@davebaol
Copy link
Member Author

davebaol commented Oct 9, 2014

Added clone capability.

@davebaol
Copy link
Member Author

davebaol commented Oct 9, 2014

Added behavior tree library, subtree reference nodes and tests.

@implicit-invocation
I created two distinct reference nodes instead of one with a lazy parameter, because they extend different classes: Task for the eager reference and Decorator for the lazy reference. Of course they can be merged in one class with the lazy parameter by extending Node. I'm not sure it's worth though.

@JesseTG
Copy link

JesseTG commented Oct 9, 2014

Ooh, Ragel looks fun to use. I should keep it in my back pocket.

@implicit-invocation
Copy link

@davebaol
So you choose the name "Reference" instead of "Include", the java class name is not that important but I still think "include" or "import" should look more natural in the tree definition file.
And why extending Task for eager reference? Decorator is fine for both eager and lazy reference to extend. You don't run the reference anyway. And code-wise there is almost no difference between a Task and a Node.
And I still don't get why Metadata is static. You don't want every sequence to act the same way, do you?

@davebaol
Copy link
Member Author

@implicit-invocation
ok, it makes sense. Lazy and eager subtree inclusion is now implemented as a single decorator.

And I still don't get why Metadata is static. You don't want every sequence to act the same way, do you?

Yes, METADATA is static because contains no runtime information. Actually, it is only used by the parser to emit errors in case an attribute doesn't exist or the number of children in a node is wrong.
For instance, most decorators must have exactly one child. However Include is an anomalous decorator because must have no children at all when you define the tree. If it's lazy it will have 1 child at runtime though. If it's eager will never run. So include's METADATA is new Metadata("subtree", "lazy") which says no children and two attributes.

@implicit-invocation
Copy link

Oh, I misunderstood it. So there will be no logic concerning the METADATA.
METADATA is just... METADATA after all

@davebaol
Copy link
Member Author

Yeah, METADATA contains the name of the attributes, not their value. So it's static final and conceptually immutable too (no setters, and its fields are package private).
Unfortunately it can not be accessed by external tools, unless they are written in Java and have nodes in the classpath. Maybe we can export such information somehow for external tools, or use a completely different technique.

@davebaol
Copy link
Member Author

@implicit-invocation
Just pushed some javadoc, code clean up, and minor fixes.
I'd like to release gdx-ai 1.4.0 on sunday (old releases were part of the libgdx project, so we have to start with 1.4.0 due to maven artifacts).
I'm going to merge the btree branch before releasing. Please let me know if you want to add or change something before we release. :)

@Tom-Ski
Copy link
Member

Tom-Ski commented Oct 11, 2014

Last release for gdx-ai was 1.3.1, so you'd only need to go to 1.3.2.

@davebaol
Copy link
Member Author

Yeah @Tom-Ski, but a lot of stuff has been added since 1.3.1, namely Steering Behaviors and Behavior Trees.
So it will be 1.4.0 :)

@implicit-invocation
Copy link

@davebaol please go ahead, I don't have anything to add yet.

@davebaol
Copy link
Member Author

@implicit-invocation
Ok thanks, no hurry.

BTW, I'm thinking of renaming Node to Task which is the typical superclass name in all the API I've seen in the past. It makes sense because everything is a task in a behavior tree. Actually, the entire behavior tree can be seen as a task made of subtask. So I'm going to make the following renaming:

  • Task -> LeafTask
  • BranchNode -> BranchTask
  • Node -> Task

Hope you don't mind :)

@implicit-invocation
Copy link

Well, I think renaming Node to Task is not a good idea.
People who want to create their trees will only use Sequence, Parallel, Include... and extend Task, they will never touch Node or BrachNode. Our documentations can tell them: "extend Task, not Node and do not add child to Task, they are leaves".
There is no point making people call something LeafTask when they never see BranchTask or any other kind of Task.
And Node and Task will create the boundary between our internal implementation and the API.

@davebaol
Copy link
Member Author

TBH I don't see the problem. Usually the one who creates a tree (not necessarily the developer) is used to think in term of actions and conditions which are the leaf tasks. The same is true for the developer.
Task and LeafTask seem to me more natural names and most of the behavior tree literature calls them like that. Why should our API use a different terminology? It might be confusing for the user.

@davebaol
Copy link
Member Author

Also if I got it right the static priority

staticPriority
  [guard1] task1
  [guard2] task2
  [guard3] task3

acts like the more verbose tree below

selector
  sequence
    guard1
    alwaysSucceed
      task1
  sequence
    guard2
    alwaysSucceed
      task2
  sequence
    guard3
    alwaysSucceed
      task3

with the difference that the latter always returns SUCCEEDED when its active child task finishes, instead of returning the active child task's termination status.

@davebaol
Copy link
Member Author

I've just realized that, at the API level, we can take into consideration the idea to add a guard to the Task base class, meaning that any task in any part of the tree can be conditioned.
In normal situations, if the task's guard succeeds the task is evaluated otherwise FAILURE status is returned.
On the other hand, if the task is a child of a priority branch, either dynamic or static, its guard is evaluated according to its parent policy.

For instance, in absence of a priority branch parent, the tree below

    sequence
      random success:0.1
      piss

can be expressed simply by

    [random success:0.1]  piss

Also, task's guards are optional, meaning that they implicity evaluate to SUCCESS.
So, the tasks piss, [] piss and [success] piss are unconditioned and totally equivalent

What do you think?

@implicit-invocation
I'd like to ear your opinion too :)

@davebaol
Copy link
Member Author

BTW, since guards are nothing more than good old tasks, guarding the guard would come for free.
I mean something like

[[[guard0] guard1] guard2] task

or maybe

[guard0] [guard1] [guard2] task

This would provide a simple inline syntax for guard sequences, which are a viable alternative to a guard tree whose root is a sequence. As long as you use short guard sequences (for instance, 0 to 4 guards) this construct remains pretty readable. But there's nothing - apart from common-sense, of course - stopping you from guarding dozens of guards.

To keep the logic simple for the user (and the implementation simple for me LOL) I'd just impose the limitation that guards can't run over multiple ticks, meaning that they must terminate with SUCCEEDED or FAILED immediately while RUNNING is an illegal status (should likely cause a runtime exception).

@davebaol
Copy link
Member Author

I got task guards and dynamic priorities working at the API level just like described above.
Now the most boring part will be writing the parser. 😑

Also, since any task can have a guard, I think that a specific task for staticPriority is no longer necessary because now you can easily get the same result with selector by guarding its children. In text format it would be something like that

selector
  [guard1] task1
  [guard2] task2
  [guard3] task3

Since guards can be either something simple like a single task or something complex like a whole tree, I'm convinced that AI designers and developers will really benefit from such a feature. :bowtie:

@MartinSojka
Copy link

Dropping by from the Reddit thread ...

Interesting that you got the guards working already. I'm not sure I explained the priorities idea properly though, so let's try again, this time with an example.

Boring old Java code stuff. First, a leaf task which supplies its own priority.

class PlayHappyAnimation extends LeafTask<MyBlackboard> {
    @Override public Status execute() {
        // Run the animation and so on
        // Always succeed
        return success();
    }
    // This one is new, defaults to returning 0
    @Override public int getPriority() {
        // Needs return values 1-10
        return 100 / getObject().needForFood() / getObject().needForShelter() - 10;
    }
}

This one calculates some priority for another task (or branch)

class NeedPriority implements TaskPrioritizer<MyBlackboard> {
    @TaskAttribute
    public String need;

    @Override public int getPriority(Task task, MyBlackboard blackboard) {
        switch( need ) {
            case "food": return blackboard.needForFood();
            case "shelter": return blackboard.needForShelter();
            default: return Integer.MIN_VALUE;
        }
    }
}

Let's put it all together. Priorities are in parentheses before the tasks. Default priority is 0.

prioritySelector type:static runUntil:success
    (NeedPriority need:food) sequence
        # Search for food sequence here
    (NeedPriority need:shelter) sequence
        # Search for shelter sequence here
    PlayHappyAnmation # returns its own priority

Now, if the priorities for food or shelter search are above those for playing a happy animation, the one with the bigger need (= priority) gets used. If the need for both isn't that great, the happy animation plays, and the whole task succeeds. If they are great, but fail, PlayHappyAnimation gets played anyway. The AI might be hungry and without a roof above their head, but it can at least still smile.

@davebaol
Copy link
Member Author

I see your point and I think that my proposal is a superset of yours :)
For instance, you should be able to implement the desired behavior like that

class CheckPriorityCondition extends LeafTask<MyBlackboard> {

    public enum Need {
       Food, Shelter
    };

    @TaskAttribute(required=true) public Need need;

    public CheckPriorityCondition () {
    } 

    @Override public Status execute() {
        return needs(getObject(), need) ? Status.SUCCEEDED : Status.FAILED;
    }

    // No @Override; this method only exists for this task
    public boolean needs(MyBlackboard blackboard, Need need)  {
        // Use whatever formula you want and determine whether the given need is actually needed or not
    }
}
dynamicGuardSelector
    [checkPriority need:"food"] sequence
        # Search for food sequence here
    [checkPriority need:"shelter"] sequence
        # Search for shelter sequence here
    PlayHappyAnimation

What do you think?

@MartinSojka
Copy link

Ok, so ...

The idea with the guards is roughly the same for a static tree, though the needs(...) method gets quite a bit more complicated (it has to check for all the needs and compare them, not just the one passed).

And when you add another need, the formula needs to get updated as well, to pick whichever need is the strongest and if it is the one we're checking for, and if all the needs together are strong enough to not rather play a happy animation instead.

And then you try to add a need at runtime, which means that you suddenly have to write needs(...) to be able to deal with a variable number of needs, not just an enum of them.

And then you decide to add another bunch of potential subtrees, each of them evaluating the needs and other information to decide on priority, and each needing to know the exact formula for priority all the other, potentially many and changing at runtime branches use.

Essentially: Replacing priorities (which each subtree can calculate for itself not caring what other subtrees are doing, or which subtrees there are in the first place) with guards requires that the guards are way more complicated and tightly coupled with each other.

@MartinSojka
Copy link

Here's an idea how guards and priorities can work together to their strengths: A "planning selector".

Assume the selector has a bunch (more than 10, possibly more than 100) different possible branches. Assume we can also estimate the cost of executing each branch (either by querying each of them for the estimated cost, or via some external cost estimation class). The algorithm is thus:

  1. Pre-filter the branches: Make a list of all where the guards evaluate to "true".
  2. Estimate the costs of each branch in the list (that doesn't run them!).
  3. Assign the negative of those costs as priorities (least costly = highest priority).
  4. Pick and run the one with the highest priority.
  5. (If runUntil:success) Repeat until a branch returns with a success or running out of branches. Else (if runUntil:finish) just return the result of the first branch.

@implicit-invocation
Copy link

To deal with changes at runtime, should we just allow referencing callbacks/functors for attributes (with an additional paramater in the annotation as well)?

@davebaol great work 👍 I've been through some kind of crisis 😄 fortunately I'm alive again

@davebaol
Copy link
Member Author

@implicit-invocation

To deal with changes at runtime, should we just allow referencing callbacks/functors for attributes (with an additional paramater in the annotation as well)?

Sounds like a nice idea for future extensions.

I've been through some kind of crisis 😄 fortunately I'm alive again

Nice to hear it. Welcome back! 😃

@MartinSojka
TBH I'm not sure I've fully understood your use case. Maybe I just need a more concrete example.
Anyways, I've used enum just for the sake of clarity. There's nothing stopping you from using a HashMap or any other data structure (which you usually access to through the blackboard).
Also, there are some aspects I really like in this solution:

  • Being guards regular tasks, they can be entire subtrees and guarding guards is allowed. The only limitation (introduced to keep things simple and clear) is that guards must complete in 1 tick.
  • Any task at any point of the tree can be guarded. This looks generally useful to me and effectively helps to reduce the depth of the tree, which as a result improves the readability.
  • There's no specific assumption or restriction, like an integer priority value. Rather condition guard evaluation yields a boolean value (SUCCEDED and FAILED i.e. true and false respectively). And, since tasks are written in code, there's virtually no limit to what you can do. You might even use a scripting language like Lua to let the AI designer specify the guard's logic (well, this goes for all tasks, not just guards).
  • This approach provides a comfortable solution to what is likely the most annoying issue in behavior tree usage: they make it difficult to think and design in terms of states. See Combining Behavior Trees and State Machines

@davebaol
Copy link
Member Author

Done!
Here is a working tree using guards, DynamicGuardSelector and subtree references.
I'll update the wiki page after the end of the LibGDX Jam, I guess. 😃

@scooterman
Copy link

@davebaol, sorry to hijack the thread, but how's the best way to do a "while" loop using behavior trees? I want to iterate over a list of possible targets and apply a selector over them. UntilFail and UntilSuccess won't resolve the problem since they can possibly end on a infinite loop.

@davebaol
Copy link
Member Author

davebaol commented Feb 3, 2016

@scooterman
I don't see what's the problem with UntilFail/UntilSuccess.

untilFail
  sequence
    next
    processItem

where

  • the task next advances the iterator and stores current item into the blackboard. It fails if there is not a next element.
  • processItem does something with the current item taken from the blackboard. This is your selector or whatever.

@scooterman
Copy link

@davebaol thanks for the reply. If processItem does run and I return success (meaning that, for example, I can execute an attack on the selected item, hence making sequence return true) untilFail will exit? I can let the iteration end but the ideal (in my case) would be to stop right after a successful sequence.

@davebaol
Copy link
Member Author

davebaol commented Feb 3, 2016

@scooterman
No, untilFail will exit only if the child sequence fails. You can decorate processItem with ìnvert, alwaysFail or alwaysSucceed based on your needs.

@scooterman
Copy link

great, thanks for the clarification.

@scooterman
Copy link

in fact, it won't work @davebaol. take a look on this snippet:

subtree name:"seekAndDestroy"
    sequence
        untilSuccess # loop through every entity in range
            sequence
                nextEnemyInRange
                canAttack?
                attackEnemy # try to attack

Suppose I have iterated over all my enemies in range and haven't attacked any of them. How I'm supposed to exit this loop since selectEnemyInRange will return false and the condition too?

Also it would be interesting have something like a "nop" task that wouldn't interfere in the current processing result, so I could use nextEnemyInRange on both selectors and sequences without having to invert it according to the logic.

@davebaol
Copy link
Member Author

davebaol commented Feb 4, 2016

Well, you have to distinguish between end-of-iteration condition and in-range condition:

subtree name:"seekAndDestroy"
  untilFail    # loop through every entity
    sequence
      nextEntity   # only fails if there's no next entity
      alwaysSucceed  
        sequence
          isEnemyInRange?
          canAttack?
          attackEnemy # try to attack

BTW, in your tree the first sequence (the root of the subtree) is useless.

@scooterman
Copy link

@davebaol sorry for annoying, if you prefere another conversation mechanism like irc just say 😄

The problem with this approach is that I lose information on one thing: if my attack succeed or not for that specific entity. My blackboard is the brain for one entity, and this tree is to check if I can attack another one in range. The ideal for this subtree was to shortcut or break the result of attackEnemy returning SUCCESS if one of it managed to attack, otherwise iterate over the next entity until exhausted. If exhausted and no one managed to attack, the subtree should return FAIL meaning that no enemy was found to attack. I could add a check after alwaysSucceed and then one after the untilFail but that's not composable, say I decide to implement a new verification for a different kind of operation. Imo this is looking like I'm trying to program procedurally which it's not desirable, but I can't see another way to do that without iteration, specially when starting to compose behaviors.

This may be related to the priority discussion above, imo.

@davebaol
Copy link
Member Author

davebaol commented Feb 4, 2016

@scooterman

subtree name:"seekAndDestroy"
  sequence
    setFlag value:false
    untilFail
      sequence
        isFlag? value:false
        nextEntity  # only fails if there's no next entity
        alwaysSucceed
          sequence
            isEnemyInRange?
            canAttack?
            attackEnemy
            setFlag value:true
    isFlag? value:true    

@scooterman
Copy link

@davebaol I ended implementing an "exhaust" branch that simplified this case. Do you have any interest on an pull request for this?

@davebaol
Copy link
Member Author

@scooterman
Sure, if it's general enough I'll merge it.

@mgsx-dev
Copy link
Contributor

I don't know if it's the right place to post but I have some suggestions :

First of all I think current API is complete specially with guards and dynamic guard selector, I personally always found a way to implement logic with provided generic tasks without hacking a re-code things (nice job BTW).

My concern right now is about pooling strategy :

  • It seams impossible to use LibGDX Poolable interface with Task because of API conflicts (both reset methods have same signature but not the same contract). The only way I found so far would be to rename Task.reset to Task.resetTask and let Task implements Poolable interface. This change the current API.
  • there is no way to remove children from task in order to recycle. Note that possibility to remove children would be a useful feature for editors as well.
  • For now I only recycle entire trees (based on archetype name) but this lead to 2 drawbacks :
    • in context of editor, pool have to be cleared to reflect changes on edited tree archetype which make pools almost useless.
    • in a "game context", it work well if you have few tree archetype and a lot of instance of them but with a lot of tree archetypes, pools may retained lot of big trees which may not be reused at all.

My conclusion is that pooling strategy is game specific : recycle whole trees, recycle tasks, don't recycle is a design choice. But we need a mechanism to allow individual tasks recycling though and I think using Poolable interface is the best choice we have.

@davebaol What do you think ? Did you faced this problem ? I could provide a PR if you don't have time to implement it but I need your approval first.

@davebaol
Copy link
Member Author

davebaol commented Jan 2, 2017

@mgsx-dev

@davebaol What do you think ? Did you faced this problem ? I could provide a PR if you don't have time to implement it but I need your approval first.

I've never needed to instantiate/destroy trees in game. I always do it during the initialization phase of the level, but I do recognize your use case. So, yes, PR is welcome. 😄

It seams impossible to use LibGDX Poolable interface with Task because of API conflicts (both reset methods have same signature but not the same contract). The only way I found so far would be to rename Task.reset to Task.resetTask and let Task implements Poolable interface. This change the current API.

Sounds good to me

there is no way to remove children from task in order to recycle. Note that possibility to remove children would be a useful feature for editors as well.

Yeah, this would be an interesting feature. Just notice that when you remove a child the parent task MUST update his own internal status accordingly. This operation is task-specific, of course.

My conclusion is that pooling strategy is game specific : recycle whole trees, recycle tasks, don't recycle is a design choice. But we need a mechanism to allow individual tasks recycling though and I think using Poolable interface is the best choice we have.

Couldn't agree more

mgsx-dev added a commit to mgsx-dev/gdx-ai that referenced this issue Jan 2, 2017
mgsx-dev added a commit to mgsx-dev/gdx-ai that referenced this issue Jan 2, 2017
mgsx-dev added a commit to mgsx-dev/gdx-ai that referenced this issue Jan 2, 2017
mgsx-dev added a commit to mgsx-dev/gdx-ai that referenced this issue Jan 2, 2017
mgsx-dev added a commit to mgsx-dev/gdx-ai that referenced this issue Jan 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests