Skip to content

Commit 45bbfad

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

File tree

1 file changed

+101
-109
lines changed

1 file changed

+101
-109
lines changed

‎src/xml_parsing.cpp‎

Lines changed: 101 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -429,153 +429,145 @@ 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-
}
445-
}
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-
}
438+
ThrowError(line_number, std::string("The tag <") + name +
439+
"> must have the attribute [ID]");
459440
}
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");
461+
ThrowError(line_number, "<SubTree> with ID '" + ID + "' should not have any child");
499462
}
500463
if(registered_nodes.count(ID) != 0)
501464
{
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");
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)
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())
531475
{
532-
ThrowError(line_number, std::string("Node not recognized: ") + name);
533-
}
534-
535-
if(search->second == NodeType::DECORATOR)
536-
{
537-
if(children_count != 1)
476+
if(is_builtin)
477+
{
478+
ThrowError(line_number, std::string("ID '") + ID + "' is not a registered node");
479+
}
480+
else
538481
{
539-
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID +
540-
"' must have exactly 1 child");
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+
if(node_type == NodeType::DECORATOR)
546489
{
547-
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID +
548-
"' must have 1 or more children");
490+
if(children_count != 1)
491+
{
492+
if(is_builtin)
493+
{
494+
ThrowError(line_number, std::string("The Decorator node '") + ID + "' must have exactly 1 child");
495+
}
496+
else
497+
{
498+
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID + "' must have exactly 1 child");
499+
}
500+
}
549501
}
550-
if(name == "ReactiveSequence")
502+
else if(node_type == NodeType::CONTROL)
551503
{
552-
size_t async_count = 0;
553-
for(auto child = node->FirstChildElement(); child != nullptr;
554-
child = child->NextSiblingElement())
504+
if(children_count == 0)
555505
{
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())
506+
if(is_builtin)
507+
{
508+
ThrowError(line_number, std::string("The Control node '") + ID + "' must have 1 or more children");
509+
}
510+
else
559511
{
560-
ThrowError(child->GetLineNum(),
561-
std::string("Unknown node type: ") + child_name);
512+
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID + "' must have 1 or more children");
562513
}
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")))
514+
}
515+
if(lookup_key == "ReactiveSequence")
516+
{
517+
size_t async_count = 0;
518+
for(auto child = node->FirstChildElement(); child != nullptr;
519+
child = child->NextSiblingElement())
568520
{
569-
++async_count;
570-
if(async_count > 1)
521+
const std::string child_name = child->Name();
522+
const auto child_search = registered_nodes.find(child_name);
523+
if(child_search == registered_nodes.end())
524+
{
525+
ThrowError(child->GetLineNum(), std::string("Unknown node type: ") + child_name);
526+
}
527+
const auto child_type = child_search->second;
528+
if(child_type == NodeType::CONTROL &&
529+
((child_name == "ThreadedAction") ||
530+
(child_name == "StatefulActionNode") ||
531+
(child_name == "CoroActionNode") || (child_name == "AsyncSequence")))
571532
{
572-
ThrowError(line_number, std::string("A ReactiveSequence with ID '" + ID +
573-
"' cannot have more "
574-
"than one async child."));
533+
++async_count;
534+
if(async_count > 1)
535+
{
536+
ThrowError(line_number, std::string("A ReactiveSequence with ID '") + ID +
537+
"' cannot have more than one async child.");
538+
}
575539
}
576540
}
577541
}
578542
}
543+
else if(node_type == NodeType::ACTION)
544+
{
545+
if(children_count != 0)
546+
{
547+
if(is_builtin)
548+
{
549+
ThrowError(line_number, std::string("The Action node '") + ID + "' must not have any child");
550+
}
551+
else
552+
{
553+
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID + "' must not have any child");
554+
}
555+
}
556+
}
557+
else if(node_type == NodeType::CONDITION)
558+
{
559+
if(children_count != 0)
560+
{
561+
if(is_builtin)
562+
{
563+
ThrowError(line_number, std::string("The Condition node '") + ID + "' must not have any child");
564+
}
565+
else
566+
{
567+
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID + "' must not have any child");
568+
}
569+
}
570+
}
579571
}
580572
}
581573
//recursion

0 commit comments

Comments
 (0)