Skip to content

Commit 7ea59fc

Browse files
EzraBrooksEzraBrooks
committed
WIP: refactor to not have duplicate checks
1 parent 8b50df6 commit 7ea59fc

File tree

1 file changed

+85
-109
lines changed

1 file changed

+85
-109
lines changed

‎src/xml_parsing.cpp

Lines changed: 85 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -429,153 +429,129 @@ void VerifyXML(const std::string& xml_text,
429429
const std::string ID = node->Attribute("ID") ? node->Attribute("ID") : "";
430430
const int line_number = node->GetLineNum();
431431

432-
if(name == "Decorator")
432+
// Precondition: built-in XML element types must define attribute [ID]
433+
const bool is_builtin =
434+
(name == "Decorator" || name == "Action" || name == "Condition" ||
435+
name == "Control" || name == "SubTree");
436+
if(is_builtin && ID.empty())
433437
{
434-
if(ID.empty())
435-
{
436-
ThrowError(line_number, "The tag <Decorator> must have the "
437-
"attribute [ID]");
438-
}
439-
if(children_count != 1)
440-
{
441-
ThrowError(line_number, "The tag <Decorator> with ID '" + ID +
442-
"' must have exactly 1 "
443-
"child");
444-
}
438+
ThrowError(line_number,
439+
std::string("The tag <") + name + "> must have the attribute [ID]");
445440
}
446-
else if(name == "Action")
447-
{
448-
if(ID.empty())
449-
{
450-
ThrowError(line_number, "The tag <Action> must have the "
451-
"attribute [ID]");
452-
}
453-
if(children_count != 0)
454-
{
455-
ThrowError(line_number, "The tag <Action> with ID '" + ID +
456-
"' must not have any "
457-
"child");
458-
}
459-
}
460-
else if(name == "Condition")
441+
442+
if(name == "BehaviorTree")
461443
{
462-
if(ID.empty())
463-
{
464-
ThrowError(line_number, "The tag <Condition> must have the "
465-
"attribute [ID]");
466-
}
467-
if(children_count != 0)
444+
if(ID.empty() && behavior_tree_count > 1)
468445
{
469-
ThrowError(line_number, "The tag <Condition> with ID '" + ID +
470-
"' must not have any "
471-
"child");
446+
ThrowError(line_number, "The tag <BehaviorTree> must have the attribute [ID]");
472447
}
473-
}
474-
else if(name == "Control")
475-
{
476-
if(ID.empty())
448+
if(registered_nodes.count(ID) != 0)
477449
{
478-
ThrowError(line_number, "The tag <Control> must have the "
479-
"attribute [ID]");
450+
ThrowError(line_number, "The attribute [ID] of tag <BehaviorTree> must not use "
451+
"the name of a registered Node");
480452
}
481-
if(children_count == 0)
453+
if(children_count != 1)
482454
{
483-
ThrowError(line_number, "The tag <Control> with ID '" + ID +
484-
"' must have at least 1 "
485-
"child");
455+
ThrowError(line_number, "The tag <BehaviorTree> with ID '" + ID +
456+
"' must have exactly 1 child");
486457
}
487458
}
488459
else if(name == "SubTree")
489460
{
490-
if(ID.empty())
491-
{
492-
ThrowError(line_number, "The tag <SubTree> must have the "
493-
"attribute [ID]");
494-
}
495461
if(children_count != 0)
496462
{
497463
ThrowError(line_number,
498464
"<SubTree> with ID '" + ID + "' should not have any child");
499465
}
500466
if(registered_nodes.count(ID) != 0)
501467
{
502-
ThrowError(line_number, "The attribute [ID] of tag <SubTree> must "
503-
"not use the name of a registered Node");
504-
}
505-
}
506-
else if(name == "BehaviorTree")
507-
{
508-
if(ID.empty() && behavior_tree_count > 1)
509-
{
510-
ThrowError(line_number, "The tag <BehaviorTree> must have the "
511-
"attribute [ID]");
512-
}
513-
if(registered_nodes.count(ID) != 0)
514-
{
515-
ThrowError(line_number, "The attribute [ID] of tag <BehaviorTree> "
516-
"must not use the name of a registered Node");
517-
}
518-
if(children_count != 1)
519-
{
520-
ThrowError(line_number, "The tag <BehaviorTree> with ID '" + ID +
521-
"' must have exactly 1 "
522-
"child");
468+
ThrowError(line_number, "The attribute [ID] of tag <SubTree> must not use the "
469+
"name of a registered Node");
523470
}
471+
// no further validation for SubTree
524472
}
525473
else
526474
{
527-
// search in the factory and the list of subtrees
528-
const auto search = registered_nodes.find(name);
529-
bool found = (search != registered_nodes.end());
530-
if(!found)
475+
// Unified lookup: use ID for builtin wrapper tags, otherwise use the element name
476+
const std::string lookup_key = (is_builtin ? ID : name);
477+
const auto search = registered_nodes.find(lookup_key);
478+
if(search == registered_nodes.end())
531479
{
532-
ThrowError(line_number, std::string("Node not recognized: ") + name);
533-
}
534-
535-
if(search->second == NodeType::DECORATOR)
536-
{
537-
if(children_count != 1)
480+
if(is_builtin)
481+
{
482+
ThrowError(line_number,
483+
std::string("ID '") + ID + "' is not a registered node");
484+
}
485+
else
538486
{
539-
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID +
540-
"' must have exactly 1 child");
487+
ThrowError(line_number, std::string("Node not recognized: ") + name);
541488
}
542489
}
543-
else if(search->second == NodeType::CONTROL)
490+
else
544491
{
545-
if(children_count == 0)
492+
const auto node_type = search->second;
493+
const std::string& registered_name = search->first;
494+
495+
if(node_type == NodeType::DECORATOR)
546496
{
547-
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID +
548-
"' must have 1 or more children");
497+
if(children_count != 1)
498+
{
499+
ThrowError(line_number, std::string("The node '") + registered_name +
500+
"' must have exactly 1 child");
501+
}
549502
}
550-
if(name == "ReactiveSequence")
503+
else if(node_type == NodeType::CONTROL)
551504
{
552-
size_t async_count = 0;
553-
for(auto child = node->FirstChildElement(); child != nullptr;
554-
child = child->NextSiblingElement())
505+
if(children_count == 0)
555506
{
556-
const std::string child_name = child->Name();
557-
const auto child_search = registered_nodes.find(child_name);
558-
if(child_search == registered_nodes.end())
559-
{
560-
ThrowError(child->GetLineNum(),
561-
std::string("Unknown node type: ") + child_name);
562-
}
563-
const auto child_type = child_search->second;
564-
if(child_type == NodeType::CONTROL &&
565-
((child_name == "ThreadedAction") ||
566-
(child_name == "StatefulActionNode") ||
567-
(child_name == "CoroActionNode") || (child_name == "AsyncSequence")))
507+
ThrowError(line_number, std::string("The node '") + registered_name +
508+
"' must have 1 or more children");
509+
}
510+
if(registered_name == "ReactiveSequence")
511+
{
512+
size_t async_count = 0;
513+
for(auto child = node->FirstChildElement(); child != nullptr;
514+
child = child->NextSiblingElement())
568515
{
569-
++async_count;
570-
if(async_count > 1)
516+
const std::string child_name = child->Name();
517+
const auto child_search = registered_nodes.find(child_name);
518+
if(child_search == registered_nodes.end())
519+
{
520+
ThrowError(child->GetLineNum(),
521+
std::string("Unknown node type: ") + child_name);
522+
}
523+
const auto child_type = child_search->second;
524+
if(child_type == NodeType::CONTROL &&
525+
((child_name == "ThreadedAction") ||
526+
(child_name == "StatefulActionNode") ||
527+
(child_name == "CoroActionNode") || (child_name == "AsyncSequence")))
571528
{
572-
ThrowError(line_number, std::string("A ReactiveSequence with ID '" + ID +
573-
"' cannot have more "
574-
"than one async child."));
529+
++async_count;
530+
if(async_count > 1)
531+
{
532+
ThrowError(line_number, std::string("A ReactiveSequence cannot have "
533+
"more than one async child."));
534+
}
575535
}
576536
}
577537
}
578538
}
539+
else if(node_type == NodeType::ACTION)
540+
{
541+
if(children_count != 0)
542+
{
543+
ThrowError(line_number, std::string("The node '") + registered_name +
544+
"' must not have any child");
545+
}
546+
}
547+
else if(node_type == NodeType::CONDITION)
548+
{
549+
if(children_count != 0)
550+
{
551+
ThrowError(line_number, std::string("The node '") + registered_name +
552+
"' must not have any child");
553+
}
554+
}
579555
}
580556
}
581557
//recursion

0 commit comments

Comments
 (0)