Skip to content

Added enter() and exit() methods for beehave nodes#126

Merged
bitbrain merged 10 commits into
bitbrain:godot-4.xfrom
lostptr:feat/enter-exit
Feb 15, 2023
Merged

Added enter() and exit() methods for beehave nodes#126
bitbrain merged 10 commits into
bitbrain:godot-4.xfrom
lostptr:feat/enter-exit

Conversation

@lostptr

@lostptr lostptr commented Feb 12, 2023

Copy link
Copy Markdown
Contributor

Description

Many things had to be changed:

  • Added enter() and exit() methods in BeehaveNode.
  • All composites and decorators now have to call their children's enter and exit.
  • Adjusted some composites to use those methods in their logic now:
    • SequenceStar
    • SelectorStar
    • SequenceRandom
    • SelectorRandom
  • Since it's the parent's responsability to call those methods, all tests needed to be adjusted so that a BeehaveTree is added as the root.
  • Some tests needed adjustments because now decorators will throw errors if they have no children.
  • A new test was added to validade the enter and exit methods.
    • A new test action was added to make those tests possible (MockAction)

Addressed issues

#117

- Added `enter()` and `exit()` methods in BeehaveNode.
- Adjusted some composites to use those methods now
- Since it's the parent's responsability to call those methods,
all tests needed to be adjusted so that a `BeehaveTree` is
added as the root.
- Some tests needed adjustments because now decorators **will**
throw errors if they have no children.
- A new test was added to validade the enter and exit methods.
  - A new test action was added to make those tests possible.
@bitbrain bitbrain added the 🍯 feature A new addition to this addon. label Feb 13, 2023
Comment thread addons/beehave/nodes/beehave_node.gd
@bitbrain

Copy link
Copy Markdown
Owner

Great PR! We may want to be a bit more specific about the naming of the methods, as it is currently not quite clear when enter and exit are called exactly.

Called before the first time it ticks by the parent.

This makes it sound that the enter method is called only once in its lifetime, however, we should probably be more specific here in the description.

Could you also add something to the README.md to describe the additional behaviour? Perhaps under "Actions and Conditions"

@bitbrain bitbrain added the 🟡 changes requested Changes have been requested from the contributor. label Feb 13, 2023
@lostptr

lostptr commented Feb 13, 2023

Copy link
Copy Markdown
Contributor Author

Great PR! We may want to be a bit more specific about the naming of the methods, as it is currently not quite clear when enter and exit are called exactly.

How about before_run and after_run?

Could you also add something to the README.md to describe the additional behaviour?

Oh yes! Forgot about that

@bitbrain

Copy link
Copy Markdown
Owner

@lostptr good suggestion. Mhhh. I was first thinking about before_tick and after_tick but that would technically be wrong, right?

@lostptr

lostptr commented Feb 13, 2023

Copy link
Copy Markdown
Contributor Author

Yes, that would be wrong. I was thinking of giving an example in the documentation but maybe that's too much...?

Say we have a character that will walk periodically. It must pick a destination, play the walk animation, move towards the destination and go back to an idle animation.

This is how it would look like:
image

Something along the lines...

@bitbrain

Copy link
Copy Markdown
Owner

@lostptr I think that's great! Also, sorry for the merge conflict <3

lostptr and others added 5 commits February 13, 2023 20:25
- A short explanation of those methods was also added in the README.
* Fix: Check child count before applying interrupt

So it doesn't throw an error, when:
- there's no child node inside the BeehaveTree
- the BeehaveTree is disabled

* Add duck typing check for interrupt method in child

* 🧪 Add Trees to UnitTestScene to cause errors if fix is not applied

* EmptyTree
  A tree, that is activated and has no children
* OnlyOneActionTree
  A tree with only one action in it
* DeactivatedTree
  A tree that is deactivated

---------

Co-authored-by: miguel <miguel-gonzalez@gmx.de>
@lostptr

lostptr commented Feb 14, 2023

Copy link
Copy Markdown
Contributor Author

@bitbrain ok yeah this messes everything lol

Now the beehave tree root is a weird composite that just runs everything without a care.
I can't test now. Like, I dont even know how to make the tests work again because of this:

for child in get_children():
    if child is BeehaveNode:
        status = child.tick(actor, blackboard)

Why is this needed?

Alternatively, that "execute" implementation would require very minimal changes because all of the code would be handled inside the node itself, not their parent. I substancially reduces coupling and make it much easier to create new composites since there will be no need to think where to put the before_run and after_run calls. It just calls _execute and that method would know what to do.

With the current implementation, we introduce some constraints:

  • Every behaviour tree must have a BeehaveTree as the root (many tests did not had that).
  • The BeehaveTree must have only one child.
  • Decorators must have a child (many tests where using Succeeder and Failer without children) else, an error occours.
  • The logic for calling before_run and after_run must be implemented by every Composite and Decorator (and the BeehaveTree) meaning that, for every new composite there must have logic to know when to call the appropriate method.

That's why I had to make so many changes :/

Now consider what would need to be established with the "execute" implementation:

  • Composite, Decorator and BeehaveTree would need to call _execute() instead of tick() on their children.
  • Don't ever overload _execute.

Thats it.

In fact, none even needs to know that this method exists! It is only relevant when you create Composites or Decorators and even then, all you need to know is that you have to call child._execute() instead of child.tick().
I'm insisting in this because now I'm seeing that is the simplest and most future proof solution. The chances of someone getting confused are slim if they read the docs, as is the case with any other library. ¯\_(ツ)_/¯

@bitbrain

bitbrain commented Feb 14, 2023

Copy link
Copy Markdown
Owner

@lostptr that was just an attempt to safely run its only child as it can happen that someone adds no children to a tree but disabling it may not be a good idea as folks may want to add child nodes at runtime. What you are describing though should be still correct: BeehaveTree should only ever have one child at max. Perhaps replacing the for loop with if check + get_child(0) might be better after all.

@bitbrain bitbrain linked an issue Feb 15, 2023 that may be closed by this pull request
@bitbrain bitbrain removed the 🟡 changes requested Changes have been requested from the contributor. label Feb 15, 2023
Comment thread addons/beehave/nodes/beehave_tree.gd Outdated
@bitbrain

Copy link
Copy Markdown
Owner

@lostptr thank you!

@bitbrain
bitbrain merged commit 5cdcf6f into bitbrain:godot-4.x Feb 15, 2023
bitbrain added a commit that referenced this pull request Feb 15, 2023
…118

* Added `enter()` and `exit()` methods for beehave nodes

- Added `enter()` and `exit()` methods in BeehaveNode.
- Adjusted some composites to use those methods now
- Since it's the parent's responsability to call those methods,
all tests needed to be adjusted so that a `BeehaveTree` is
added as the root.
- Some tests needed adjustments because now decorators **will**
throw errors if they have no children.
- A new test was added to validade the enter and exit methods.
  - A new test action was added to make those tests possible.

* Changed `enter` and `exit` to `before_run` and `after_run`

- A short explanation of those methods was also added in the README.

* Fix: Check child count before applying interrupt (#121)

* Fix: Check child count before applying interrupt

So it doesn't throw an error, when:
- there's no child node inside the BeehaveTree
- the BeehaveTree is disabled

* Add duck typing check for interrupt method in child

* 🧪 Add Trees to UnitTestScene to cause errors if fix is not applied

* EmptyTree
  A tree, that is activated and has no children
* OnlyOneActionTree
  A tree with only one action in it
* DeactivatedTree
  A tree that is deactivated

---------

Co-authored-by: miguel <miguel-gonzalez@gmx.de>

* 🐛 do only unregister monitor when present (#128)

* Update addons/beehave/nodes/beehave_tree.gd

---------

Co-authored-by: miguel <miguel-gonzalez@gmx.de>
Co-authored-by: Patrick Werner <paddi@tastenklopfer.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🍯 feature A new addition to this addon.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nodes with enter() and exit() methods

2 participants