diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index 8b9ce95a1..1d92e647d 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -428,119 +428,76 @@ void VerifyXML(const std::string& xml_text, const std::string ID = node->Attribute("ID") ? node->Attribute("ID") : ""; const int line_number = node->GetLineNum(); - if(name == "Decorator") + // Precondition: built-in XML element types must define attribute [ID] + const bool is_builtin = + (name == "Decorator" || name == "Action" || name == "Condition" || + name == "Control" || name == "SubTree"); + if(is_builtin && ID.empty()) { - if(children_count != 1) - { - ThrowError(line_number, "The tag must have exactly 1 " - "child"); - } - if(ID.empty()) - { - ThrowError(line_number, "The tag must have the " - "attribute [ID]"); - } - } - else if(name == "Action") - { - if(children_count != 0) - { - ThrowError(line_number, "The tag must not have any " - "child"); - } - if(ID.empty()) - { - ThrowError(line_number, "The tag must have the " - "attribute [ID]"); - } + ThrowError(line_number, + std::string("The tag <") + name + "> must have the attribute [ID]"); } - else if(name == "Condition") + + if(name == "BehaviorTree") { - if(children_count != 0) - { - ThrowError(line_number, "The tag must not have any " - "child"); - } - if(ID.empty()) + if(ID.empty() && behavior_tree_count > 1) { - ThrowError(line_number, "The tag must have the " - "attribute [ID]"); + ThrowError(line_number, "The tag must have the attribute [ID]"); } - } - else if(name == "Control") - { - if(children_count == 0) + if(registered_nodes.count(ID) != 0) { - ThrowError(line_number, "The tag must have at least 1 " - "child"); + ThrowError(line_number, "The attribute [ID] of tag must not use " + "the name of a registered Node"); } - if(ID.empty()) + if(children_count != 1) { - ThrowError(line_number, "The tag must have the " - "attribute [ID]"); + ThrowError(line_number, "The tag with ID '" + ID + + "' must have exactly 1 child"); } } else if(name == "SubTree") { if(children_count != 0) { - ThrowError(line_number, " should not have any child"); - } - if(ID.empty()) - { - ThrowError(line_number, "The tag must have the " - "attribute [ID]"); + ThrowError(line_number, + " with ID '" + ID + "' should not have any child"); } if(registered_nodes.count(ID) != 0) { - ThrowError(line_number, "The attribute [ID] of tag must " - "not use the name of a registered Node"); - } - } - else if(name == "BehaviorTree") - { - if(ID.empty() && behavior_tree_count > 1) - { - ThrowError(line_number, "The tag must have the " - "attribute [ID]"); - } - if(children_count != 1) - { - ThrowError(line_number, "The tag must have exactly 1 " - "child"); - } - if(registered_nodes.count(ID) != 0) - { - ThrowError(line_number, "The attribute [ID] of tag " - "must not use the name of a registered Node"); + ThrowError(line_number, "The attribute [ID] of tag must not use the " + "name of a registered Node"); } } else { - // search in the factory and the list of subtrees - const auto search = registered_nodes.find(name); + // use ID for builtin node types, otherwise use the element name + const auto lookup_name = is_builtin ? ID : name; + const auto search = registered_nodes.find(lookup_name); bool found = (search != registered_nodes.end()); if(!found) { - ThrowError(line_number, std::string("Node not recognized: ") + name); + ThrowError(line_number, std::string("Node not recognized: ") + lookup_name); } - if(search->second == NodeType::DECORATOR) + const auto node_type = search->second; + const std::string& registered_name = search->first; + + if(node_type == NodeType::DECORATOR) { if(children_count != 1) { - ThrowError(line_number, - std::string("The node <") + name + "> must have exactly 1 child"); + ThrowError(line_number, std::string("The node '") + registered_name + + "' must have exactly 1 child"); } } - else if(search->second == NodeType::CONTROL) + else if(node_type == NodeType::CONTROL) { if(children_count == 0) { - ThrowError(line_number, - std::string("The node <") + name + "> must have 1 or more children"); + ThrowError(line_number, std::string("The node '") + registered_name + + "' must have 1 or more children"); } - if(name == "ReactiveSequence") + if(registered_name == "ReactiveSequence") { size_t async_count = 0; for(auto child = node->FirstChildElement(); child != nullptr; @@ -562,13 +519,29 @@ void VerifyXML(const std::string& xml_text, ++async_count; if(async_count > 1) { - ThrowError(line_number, std::string("A ReactiveSequence cannot have more " - "than one async child.")); + ThrowError(line_number, std::string("A ReactiveSequence cannot have " + "more than one async child.")); } } } } } + else if(node_type == NodeType::ACTION) + { + if(children_count != 0) + { + ThrowError(line_number, std::string("The node '") + registered_name + + "' must not have any child"); + } + } + else if(node_type == NodeType::CONDITION) + { + if(children_count != 0) + { + ThrowError(line_number, std::string("The node '") + registered_name + + "' must not have any child"); + } + } } //recursion for(auto child = node->FirstChildElement(); child != nullptr; diff --git a/tests/gtest_factory.cpp b/tests/gtest_factory.cpp index 321d24ef5..b07835a1f 100644 --- a/tests/gtest_factory.cpp +++ b/tests/gtest_factory.cpp @@ -82,7 +82,7 @@ static const char* xml_text_subtree_part1 = R"( - + )"; @@ -93,11 +93,10 @@ static const char* xml_text_subtree_part2 = R"( - + - )";