Skip to content

Commit e8424fb

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

File tree

1 file changed

+78
-109
lines changed

1 file changed

+78
-109
lines changed

‎src/xml_parsing.cpp

Lines changed: 78 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -429,153 +429,122 @@ 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);
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);
529478
bool found = (search != registered_nodes.end());
530-
if(!found)
479+
if(found)
531480
{
532481
ThrowError(line_number, std::string("Node not recognized: ") + name);
533482
}
534-
535-
if(search->second == NodeType::DECORATOR)
536-
{
537-
if(children_count != 1)
538-
{
539-
ThrowError(line_number, std::string("The node <") + name + "> with ID '" + ID +
540-
"' must have exactly 1 child");
541-
}
542-
}
543-
else if(search->second == NodeType::CONTROL)
483+
else
544484
{
545-
if(children_count == 0)
485+
const auto node_type = search->second;
486+
const std::string& registered_name = search->first;
487+
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+
ThrowError(line_number, std::string("The node '") + registered_name +
493+
"' must have exactly 1 child");
494+
}
549495
}
550-
if(name == "ReactiveSequence")
496+
else if(node_type == NodeType::CONTROL)
551497
{
552-
size_t async_count = 0;
553-
for(auto child = node->FirstChildElement(); child != nullptr;
554-
child = child->NextSiblingElement())
498+
if(children_count == 0)
555499
{
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")))
500+
ThrowError(line_number, std::string("The node '") + registered_name +
501+
"' 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(),
514+
std::string("Unknown node type: ") + child_name);
515+
}
516+
const auto child_type = child_search->second;
517+
if(child_type == NodeType::CONTROL &&
518+
((child_name == "ThreadedAction") ||
519+
(child_name == "StatefulActionNode") ||
520+
(child_name == "CoroActionNode") || (child_name == "AsyncSequence")))
521+
{
522+
++async_count;
523+
if(async_count > 1)
524+
{
525+
ThrowError(line_number, std::string("A ReactiveSequence cannot have "
526+
"more than one async child."));
527+
}
575528
}
576529
}
577530
}
578531
}
532+
else if(node_type == NodeType::ACTION)
533+
{
534+
if(children_count != 0)
535+
{
536+
ThrowError(line_number, std::string("The node '") + registered_name +
537+
"' must not have any child");
538+
}
539+
}
540+
else if(node_type == NodeType::CONDITION)
541+
{
542+
if(children_count != 0)
543+
{
544+
ThrowError(line_number, std::string("The node '") + registered_name +
545+
"' must not have any child");
546+
}
547+
}
579548
}
580549
}
581550
//recursion

0 commit comments

Comments
 (0)