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

🐛 fix interrupt of sequence star/random + improve docs #249

Merged
merged 4 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions addons/beehave/nodes/composites/sequence_random.gd
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ func tick(actor: Node, blackboard: Blackboard) -> int:
c.after_run(actor, blackboard)
FAILURE:
_children_bag.erase(c)
# Interrupt any child that was RUNNING before
# but do not reset!
super.interrupt(actor, blackboard)
c.after_run(actor, blackboard)
return FAILURE
RUNNING:
Expand Down
5 changes: 4 additions & 1 deletion addons/beehave/nodes/composites/sequence_star.gd
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class_name SequenceStarComposite extends Composite
## `SUCCESS` in case all of the children report a `SUCCESS` status code.
## If at least one child reports a `FAILURE` status code, this node will also
## return `FAILURE` and tick again.
## In case a child returns `RUNNING` this node will restart.
## In case a child returns `RUNNING` this node will tick again.

var successful_index: int = 0

Expand All @@ -32,6 +32,9 @@ func tick(actor: Node, blackboard: Blackboard) -> int:
successful_index += 1
c.after_run(actor, blackboard)
FAILURE:
# Interrupt any child that was RUNNING before
# but do not reset!
super.interrupt(actor, blackboard)
c.after_run(actor, blackboard)
return FAILURE
RUNNING:
Expand Down
10 changes: 6 additions & 4 deletions docs/manual/composites.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ With composite nodes, you can craft unique and dynamic Behavior Trees for your g

## Restarting a composite

If a parent node restarts a child node, it means that the parent node will start the child node from scratch the next time it is ticked. This means that any progress made by the child node will be reset, and it will start its execution from the beginning.
When a parent node restarts, it means the whole composite node begins its evaluation again from the beginning. This does not interrupt the child nodes.
Copy link

Choose a reason for hiding this comment

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

If I'm not mistaken, there can only be one running node - that is until parallel composites are implemented. But until then, I think the docs should reflect this fact and be clear about it as not to confuse the new users. Therefore, I suggest the last sentence be rewritten as follows: This does not interrupt the running child node.


## Ticking again a composite
## Interrupting child nodes

A sequence may interrupt any `RUNNING` child node in case a child returns a `FAILURE`. The `interrupt` method will be called on all child nodes of the currently evaluated child node.
Copy link

Choose a reason for hiding this comment

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

A sequence may interrupt any RUNNING child node in case a child returns a FAILURE.

The interrupt can also happen in other cases, not just when a child returns a failure - for example, if a reactive sequence encounters a running child, that comes before the child that was running last frame, the old running child is interrupted. I'd say this part of the sentence can be left out: in case a child returns a FAILURE.

Copy link

Choose a reason for hiding this comment

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

The interrupt method will be called on all child nodes of the currently evaluated child node.

The term child nodes typically denotes only the direct children, i.e. the first-order descendants. For clarity, I'd suggest using the term descendant nodes to denote all descending nodes and say ... will be called on all descendant nodes.

Also, instead of the currently evaluated child node, I'd say the interrupted child node just to make it clear which node we are referring to.


If a parent node ticks a child node again, it means that the parent node will immediately tick the child node again on the next frame, without waiting for the current frame to finish executing. This allows the child node to continue its execution from where it left off without resetting its progress.
## Ticking again a composite

In other words, restarting a child node means that the parent node will give the child node a fresh start, while ticking it again means that the parent node will let the child node continue its execution from where it left off.
If a parent node ticks a child node again, it means that the parent node will immediately tick the child node again on the next frame, without waiting for the current frame to finish executing. This allows the child node to continue its execution from where it left off without resetting its progress.
Copy link

Choose a reason for hiding this comment

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

..., without waiting for the current frame to finish executing.

Is this correct? This sounds like the child node is ticked multiple times in the same frame. Isn't each node only ticked at most once on every frame?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Brain fog on my part. I will correct this.

Copy link

Choose a reason for hiding this comment

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

If I may add one more thing, I'd also maybe elaborate more to clarify that all that ticking again really means is that the parent composite node carries on with its execution from that given child node. Meaning, that it's not only that given child node that is ticked, it is potentially its sibling nodes following it, if the node completes and allows its siblings to be ticked too.

6 changes: 4 additions & 2 deletions test/nodes/composites/sequence_star_test.gd
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,13 @@ func test_keeps_running_child_until_failure() -> void:

assert_that(tree.tick()).is_equal(BeehaveNode.FAILURE)
assert_that(action1.count).is_equal(1)
assert_that(action2.count).is_equal(3)
# action2 will reset as it failed
assert_that(action2.count).is_equal(0)

assert_that(tree.tick()).is_equal(BeehaveNode.FAILURE)
assert_that(action1.count).is_equal(1)
assert_that(action2.count).is_equal(4)
# action2 has reset previously but sequence star will tick again
assert_that(action2.count).is_equal(1)


func test_tick_again_when_child_returns_failure() -> void:
Expand Down
Loading