Skip to content

Commit f021331

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

File tree

1 file changed

+77
-111
lines changed

1 file changed

+77
-111
lines changed

‎src/xml_parsing.cpp‎

Lines changed: 77 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -429,153 +429,119 @@ 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, std::string("The tag <") + name +
439+
"> 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 the name of a registered Node");
480451
}
481-
if(children_count == 0)
452+
if(children_count != 1)
482453
{
483-
ThrowError(line_number, "The tag <Control> with ID '" + ID +
484-
"' must have at least 1 "
485-
"child");
454+
ThrowError(line_number, "The tag <BehaviorTree> with ID '" + ID + "' must have exactly 1 child");
486455
}
487456
}
488457
else if(name == "SubTree")
489458
{
490-
if(ID.empty())
491-
{
492-
ThrowError(line_number, "The tag <SubTree> must have the "
493-
"attribute [ID]");
494-
}
495459
if(children_count != 0)
496460
{
497-
ThrowError(line_number,
498-
"<SubTree> with ID '" + ID + "' should not have any child");
499-
}
500-
if(registered_nodes.count(ID) != 0)
501-
{
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]");
461+
ThrowError(line_number, "<SubTree> with ID '" + ID + "' should not have any child");
512462
}
513463
if(registered_nodes.count(ID) != 0)
514464
{
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");
465+
ThrowError(line_number, "The attribute [ID] of tag <SubTree> must not use the name of a registered Node");
523466
}
467+
// no further validation for SubTree
524468
}
525469
else
526470
{
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)
531-
{
532-
ThrowError(line_number, std::string("Node not recognized: ") + name);
533-
}
534-
535-
if(search->second == NodeType::DECORATOR)
471+
// Unified lookup: use ID for builtin wrapper tags, otherwise use the element name
472+
const std::string lookup_key = (is_builtin ? ID : name);
473+
const auto search = registered_nodes.find(lookup_key);
474+
if(search == registered_nodes.end())
536475
{
537-
if(children_count != 1)
476+
if(is_builtin)
538477
{
539-
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID +
540-
"' must have exactly 1 child");
478+
ThrowError(line_number, std::string("ID '") + ID + "' is not a registered node");
479+
}
480+
else
481+
{
482+
ThrowError(line_number, std::string("Node not recognized: ") + name);
541483
}
542484
}
543-
else if(search->second == NodeType::CONTROL)
485+
else
544486
{
545-
if(children_count == 0)
487+
const auto node_type = search->second;
488+
const std::string& registered_name = search->first;
489+
490+
if(node_type == NodeType::DECORATOR)
546491
{
547-
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID +
548-
"' must have 1 or more children");
492+
if(children_count != 1)
493+
{
494+
ThrowError(line_number, std::string("The node '") + registered_name + "' must have exactly 1 child");
495+
}
549496
}
550-
if(name == "ReactiveSequence")
497+
else if(node_type == NodeType::CONTROL)
551498
{
552-
size_t async_count = 0;
553-
for(auto child = node->FirstChildElement(); child != nullptr;
554-
child = child->NextSiblingElement())
499+
if(children_count == 0)
555500
{
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")))
501+
ThrowError(line_number, std::string("The node '") + registered_name + "' must have 1 or more children");
502+
}
503+
if(registered_name == "ReactiveSequence")
504+
{
505+
size_t async_count = 0;
506+
for(auto child = node->FirstChildElement(); child != nullptr;
507+
child = child->NextSiblingElement())
568508
{
569-
++async_count;
570-
if(async_count > 1)
509+
const std::string child_name = child->Name();
510+
const auto child_search = registered_nodes.find(child_name);
511+
if(child_search == registered_nodes.end())
571512
{
572-
ThrowError(line_number, std::string("A ReactiveSequence with ID '" + ID +
573-
"' cannot have more "
574-
"than one async child."));
513+
ThrowError(child->GetLineNum(), std::string("Unknown node type: ") + child_name);
514+
}
515+
const auto child_type = child_search->second;
516+
if(child_type == NodeType::CONTROL &&
517+
((child_name == "ThreadedAction") ||
518+
(child_name == "StatefulActionNode") ||
519+
(child_name == "CoroActionNode") || (child_name == "AsyncSequence")))
520+
{
521+
++async_count;
522+
if(async_count > 1)
523+
{
524+
ThrowError(line_number, std::string("A ReactiveSequence with ID '") + ID +
525+
"' cannot have more than one async child.");
526+
}
575527
}
576528
}
577529
}
578530
}
531+
else if(node_type == NodeType::ACTION)
532+
{
533+
if(children_count != 0)
534+
{
535+
ThrowError(line_number, std::string("The node '") + registered_name + "' must not have any child");
536+
}
537+
}
538+
else if(node_type == NodeType::CONDITION)
539+
{
540+
if(children_count != 0)
541+
{
542+
ThrowError(line_number, std::string("The node '") + registered_name + "' must not have any child");
543+
}
544+
}
579545
}
580546
}
581547
//recursion

0 commit comments

Comments
 (0)