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

Added a new composite node: simple_parallel #332

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

DarkAngelZT
Copy link
Contributor

@DarkAngelZT DarkAngelZT commented Apr 18, 2024

Description

Added a composite node called simple parallel. It does the same thing like the one in Unreal Engine. This node allows you to execute multiple things at the same time, e.g moving while shooting. Nested simple parallel is also supported but not suggested

Addressed issues

N/A

Screen shots

Here's debug view of my own behavior tree in Beehave debugger:

beehave_debug

@bitbrain
Copy link
Owner

bitbrain commented Apr 21, 2024

@DarkAngelZT thank you for raising this - a few things on this:

  1. would you mind attaching the UE docs on this to the description of this PR? I am curious about the design decision of limiting it to exactly two child nodes
  2. any new node should be covered by unit tests to ensure it works as expected. Please read the contribution guidelines for more information. I am especially curious how this behaves with nested parallel nodes or a parallel node containing other composite subtrees. Another thing we probably want to test is that interrupts work as expected.
  3. the current v2.x version of beehave does not correctly test the debugger (since it is hard to test) - could you attach screenshots/gif of how the parallel node looks like inside the Beehave debugger?
  4. We also would require documentation on this new composite node. So far, we only support selectors and sequence nodes. Perhaps we can introduce a new one for 'Parallel'

Generally an okay pull request - one more thing: "SimpleParallel" suggests that there could be different types of parallel, however, the architecture of Beehave V2 does not allow for other types (e.g. it assumes that always one node at a time is in RUNNING state). Could we perhaps just call it Parallel?

@DarkAngelZT
Copy link
Contributor Author

DarkAngelZT commented Apr 22, 2024

@bitbrain
Thanks for reply. Here is the official explaination of simple parallel node on epic website:
https://dev.epicgames.com/documentation/en-us/unreal-engine/behavior-tree-in-unreal-engine---overview#concurrentbehaviors
So basically this node has concurrency features, but it still act like a regular composite node. Which makes it different from real parallel node. As you can see its state is only controlled by first child. So I think if we simply call it parallel woud cause some misunderstanding of its feature.
I will add icon and unit test later. This node is currently running in my project and I've wrote a python version for that few years ago for another engine, ant it working smoothly till now in some comercial projects, quality ensured ;)

@DarkAngelZT
Copy link
Contributor Author

@bitbrain Unit test and docs are done, debugger screen shot has updated above. Please review

Copy link
Owner

Choose a reason for hiding this comment

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

Can you remove this?

@bitbrain
Copy link
Owner

@DarkAngelZT the icon does not seem to display correctly in the debugger?

@DarkAngelZT
Copy link
Contributor Author

@DarkAngelZT the icon does not seem to display correctly in the debugger?

That's the same code in my current project, which doesn't have icon set. here's new one:
beehavess

Copy link
Owner

@bitbrain bitbrain left a comment

Choose a reason for hiding this comment

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

Thank you for this!

@bitbrain bitbrain merged commit 2e51a71 into bitbrain:godot-4.x Apr 23, 2024
4 checks passed
@bitbrain bitbrain mentioned this pull request Apr 23, 2024
6 tasks
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.

2 participants