- Notifications
You must be signed in to change notification settings - Fork778
Recursive behavior trees#980
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
d17af90 to0f26b68Comparethanks a lot. I will review it soon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR addresses Issue#979 by adding runtime checks to catch recursive behavior tree definitions and providing test cases to verify these conditions.
- Added two test cases (RecursiveSubtree and RecursiveCycle) to validate that cycles in behavior trees trigger errors.
- Introduced a recursion check in the XML parser to detect subtree cycles during parsing.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/gtest_subtree.cpp | Added tests that ensure recursive trees and cycles are rejected. |
| src/xml_parsing.cpp | Implemented a recursion check for subtree creation in the XML parser. |
| const std::string subtree_ID = element->Attribute("ID"); | ||
| // check for recursion in behavior tree | ||
| if(prefix.find(subtree_ID) != std::string::npos) |
CopilotAIJun 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Consider using a delimiter-aware search to verify the presence of subtree_ID within prefix. This ensures that matching does not produce false positives if one subtree ID is a substring of another.
| if(prefix.find(subtree_ID) !=std::string::npos) | |
| std::string subtree_delimited = subtree_ID +"/"; | |
| if(prefix == subtree_ID || prefix.find(subtree_delimited) ==0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ensures that matching does not produce false positives if one subtree ID is a substring of another.
Good idea, but the provided solution does not achieve this because of the added::<UID> for subtree paths.
Is there a method used elsewhere in the library for extracting tokens from a path that could be used for this? Otherwise I can add a string splitting routine. I'm not quite as C++ literate, but in python it would look something like this:
importreancestors=re.split(r'::[0-9]*/?',prefix)ifsubtree_IDinancestors:raiseRuntimeError( ... )
This PR addresses Issue#979. It checks for the existence of a subtree in its own prefix while parsing and includes two test cases for recursive trees mentioned in the issue.