Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Rockjack00
Copy link

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.

@Rockjack00 Rockjack00 force-pushed the recursive_behavior_trees branch from d17af90 to 0f26b68 Compare June 5, 2025 13:48
@facontidavide
Copy link
Collaborator

thanks a lot. I will review it soon

Copy link

@Copilot Copilot AI left a 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)
Copy link
Preview

Copilot AI Jun 5, 2025

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.

Suggested change
if(prefix.find(subtree_ID) != std::string::npos)
std::string subtree_delimited = subtree_ID + "/";
if(prefix == subtree_ID || prefix.find(subtree_delimited) == 0)

Copilot uses AI. Check for mistakes.

Copy link
Author

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:

import re

ancestors = re.split(r'::[0-9]*/?', prefix)
if subtree_ID in ancestors:
    raise RuntimeError( ... )

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