Merge pull request #357 from klingbolt/track_top_level_categories

Added tracking for the top level categories in each pack.

xml_converter/integration_tests/run_tests.py

@@ -20,6 +20,7 @@ def run_xml_converter(
     input_proto: Optional[List[str]] = None,
     output_proto: Optional[List[str]] = None,
     split_output_proto: Optional[str] = None,
+    allow_duplicates: Optional[bool] = None,
 ) -> Tuple[str, str, int]:
 
     # Build the command to execute the C++ program with the desired function and arguments
@@ -35,6 +36,8 @@ def run_xml_converter(
         cmd += ["--output-guildpoint-path"] + output_proto
     if split_output_proto:
         cmd += ["--output-split-guildpoint-path"] + [split_output_proto]
+    if allow_duplicates:
+        cmd += ["--allow-duplicates"]
 
     # Run the C++ program and capture its output
     result = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
@@ -174,6 +177,7 @@ def main() -> bool:
             input_proto=testcase.proto_input_paths,
             output_xml=[xml_output_dir_path],
             output_proto=[proto_output_dir_path],
+            allow_duplicates=testcase.allow_duplicates
         )
 
         # Sanitize and denoise the lines
@@ -206,8 +210,10 @@ def main() -> bool:
         if testcase.expected_returncode is not None and testcase.expected_returncode != returncode:
             print(f"Expected a return code of {testcase.expected_returncode} for {testcase.name} but got {returncode}")
 
-        testcase_passed &= diff_dirs(xml_output_dir_path, testcase.expected_output_xml_path)
-        testcase_passed &= diff_dirs(proto_output_dir_path, testcase.expected_output_proto_path)
+        if os.path.exists(testcase.expected_output_xml_path):
+            testcase_passed &= diff_dirs(xml_output_dir_path, testcase.expected_output_xml_path)
+        if os.path.exists(testcase.expected_output_proto_path):
+            testcase_passed &= diff_dirs(proto_output_dir_path, testcase.expected_output_proto_path)
 
         if testcase_passed:
             print(f"Success: test {testcase.name}")

xml_converter/integration_tests/src/testcase_loader.py

@@ -21,6 +21,7 @@ class Testcase:
     expected_stdout: List[str]
     expected_stderr: List[str]
     expected_returncode: int
+    allow_duplicates: bool
 
 
 ################################################################################
@@ -58,6 +59,7 @@ def load_testcase(path: str) -> Optional[Testcase]:
     # Load all of the input paths into either xml input or proto inputs
     xml_input_paths: List[str] = []
     proto_input_paths: List[str] = []
+    allow_duplicates: bool = False
     for pack_name, pack_type in data["input_paths"].items():
         if not isinstance(pack_name, str):
             print(f"Invalid pack name, expecting a string but got {pack_name}")
@@ -104,6 +106,13 @@ def load_testcase(path: str) -> Optional[Testcase]:
         print(f"Invalid Test, expecting string value for 'expected_returncode' in {path}")
         return None
 
+    if "allow_duplicates" in data:
+        if not isinstance(data["allow_duplicates"], bool):
+            print(f"Invalid Test, expecting bool value for 'allow_duplicates' in {path}")
+            return None
+        else:
+            allow_duplicates = data["allow_duplicates"]
+
     return Testcase(
         name=os.path.basename(path),
         xml_input_paths=xml_input_paths,
@@ -112,7 +121,8 @@ def load_testcase(path: str) -> Optional[Testcase]:
         expected_output_proto_path=os.path.join(path, "output_proto"),
         expected_stdout=to_lines(data["expected_stdout"]),
         expected_stderr=to_lines(data["expected_stderr"]),
-        expected_returncode=data["expected_returncode"]
+        expected_returncode=data["expected_returncode"],
+        allow_duplicates=allow_duplicates
     )
 
 

xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack/xml_file.xml

@@ -0,0 +1,8 @@
+<OverlayData>
+    <MarkerCategory DisplayName="MyCategory" Name="Mycategory">
+    </MarkerCategory>
+
+    <POIs>
+        <POI Type="mycategory" XPos="169.81" YPos="210.65" ZPos="215.83" MapID="50" />
+    </POIs>
+</OverlayData>

xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack2/markers.bin

@@ -0,0 +1,4 @@
+
++
+
+MyCategory 2B\�)Cf�RC{�WCB(��“^�

xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack3/xml_file.xml

@@ -0,0 +1,8 @@
+<OverlayData>
+    <MarkerCategory DisplayName="MyCategory" Name="Mycategory">
+    </MarkerCategory>
+
+    <POIs>
+        <POI Type="mycategory" XPos="169.81" YPos="210.65" ZPos="215.83" MapID="50" />
+    </POIs>
+</OverlayData>

xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/output_proto/markers.bin

@@ -0,0 +1,4 @@
+
+U
+
+MyCategory 2B\�)Cf�RC{�WC 2B\�)Cf�RC{�WC 2B\�)Cf�RC{�WCB(��“^�

xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/output_xml/xml_file.xml

@@ -0,0 +1,10 @@
+<OverlayData>
+<MarkerCategory  DisplayName="MyCategory" ID="KOjMBsKTXpY=" Name="mycategory">
+</MarkerCategory>
+
+<POIs>
+<POI  Type="mycategory" MapID="50" XPos="169.809998" YPos="210.649994" ZPos="215.830002"/>
+<POI  Type="mycategory" MapID="50" XPos="169.809998" YPos="210.649994" ZPos="215.830002"/>
+<POI  Type="mycategory" MapID="50" XPos="169.809998" YPos="210.649994" ZPos="215.830002"/>
+</POIs>
+</OverlayData>

xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/testcase.yaml

@@ -0,0 +1,8 @@
+input_paths: 
+    "pack": "xml"
+    "pack2": "proto"
+    "pack3": "xml"
+expected_stdout: |
+expected_stderr: |
+expected_returncode: 0
+allow_duplicates: true

xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack/xml_file.xml

@@ -0,0 +1,8 @@
+<OverlayData>
+    <MarkerCategory DisplayName="MyCategory" Name="Mycategory">
+    </MarkerCategory>
+
+    <POIs>
+        <POI Type="mycategory" XPos="169.81" YPos="210.65" ZPos="215.83" MapID="50" />
+    </POIs>
+</OverlayData>

xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack2/markers.bin

@@ -0,0 +1,4 @@
+
++
+
+MyCategory 2B\�)Cf�RC{�WCB(��“^�

xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack3/xml_file.xml

@@ -0,0 +1,8 @@
+<OverlayData>
+    <MarkerCategory DisplayName="MyCategory" Name="Mycategory">
+    </MarkerCategory>
+
+    <POIs>
+        <POI Type="mycategory" XPos="169.81" YPos="210.65" ZPos="215.83" MapID="50" />
+    </POIs>
+</OverlayData>

xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml

@@ -0,0 +1,16 @@
+input_paths: 
+    "pack": "xml"
+    "pack2": "proto"
+    "pack3": "xml"
+expected_stdout: |
+    Did not write due to duplicates in categories.
+    This commonly occurs when attempting to read the same pack multiple times or when separate packs coincidentally have the same name.
+    Please remove one of the packs or edit the name of the packs' top level category before running the program again.
+    If you want to bypass this stop, use '--allow-duplicates'.
+    The following top level categories were found in more than one pack:
+        "mycategory" in files:
+            pack/xml_file.xml
+            pack2/markers.bin
+            pack3/xml_file.xml
+expected_stderr: |
+expected_returncode: 0

xml_converter/src/packaging_protobin.cpp

@@ -23,7 +23,7 @@ using namespace std;
 ////////////////////////////////////////////////////////////////////////////////
 //
 ////////////////////////////////////////////////////////////////////////////////
-void parse_guildpoint_categories(
+string parse_guildpoint_categories(
     string full_category_name,
     ::guildpoint::Category proto_category,
     map<string, Category>* marker_categories,
@@ -57,25 +57,28 @@ void parse_guildpoint_categories(
     for (int i = 0; i < proto_category.children_size(); i++) {
         parse_guildpoint_categories(full_category_name + ".", proto_category.children(i), &(this_category->children), parsed_pois, state);
     }
+    return category_name;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 ////////////////////////////////////////////////////////////////////////////////
-void read_protobuf_file(string proto_filepath, const string marker_pack_root_directory, map<string, Category>* marker_categories, vector<Parseable*>* parsed_pois) {
+set<string> read_protobuf_file(string proto_filepath, const string marker_pack_root_directory, map<string, Category>* marker_categories, vector<Parseable*>* parsed_pois) {
     fstream infile;
     guildpoint::Guildpoint proto_message;
     ProtoReaderState state;
     state.marker_pack_root_directory = marker_pack_root_directory;
+    set<string> category_names;
 
     infile.open(proto_filepath, ios::in | ios::binary);
     proto_message.ParseFromIstream(&infile);
 
     state.textures = proto_message.textures();
 
     for (int i = 0; i < proto_message.category_size(); i++) {
-        parse_guildpoint_categories("", proto_message.category(i), marker_categories, parsed_pois, &state);
+        category_names.insert(parse_guildpoint_categories("", proto_message.category(i), marker_categories, parsed_pois, &state));
     }
+    return category_names;
 }
 
 ////////////////////////////////////////////////////////////////////////////////

xml_converter/src/packaging_protobin.hpp

@@ -2,6 +2,7 @@
 
 #include <fstream>
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -10,7 +11,7 @@
 #include "parseable.hpp"
 #include "string_hierarchy.hpp"
 
-void read_protobuf_file(
+std::set<std::string> read_protobuf_file(
     std::string proto_filepath,
     const std::string marker_pack_root_directory,
     std::map<std::string, Category>* marker_categories,

xml_converter/src/packaging_xml.cpp

@@ -1,5 +1,6 @@
 #include "packaging_xml.hpp"
 
+#include <set>
 #include <utility>
 
 #include "hash_helpers.hpp"
@@ -16,43 +17,54 @@ using namespace std;
 ////////////////////////////////////////////////////////////////////////////////
 
 unsigned int UNKNOWN_CATEGORY_COUNTER = 0;
-void parse_marker_categories(
+OptionalString parse_marker_categories(
     rapidxml::xml_node<>* node,
     map<string, Category>* marker_categories,
     Category* parent,
     vector<XMLError*>* errors,
     XMLReaderState* state,
     int depth = 0) {
+    OptionalString name = {
+        "",  // value
+        false,  // is_null
+    };
     if (get_node_name(node) == "MarkerCategory") {
-        string name;
-
         rapidxml::xml_attribute<>* name_attribute = find_attribute(node, "name");
         if (name_attribute == 0) {
             // TODO: This error should really be for the entire node not just the name
             errors->push_back(new XMLNodeNameError("Category attribute 'name' is missing so it cannot be properly referenced", node));
 
             // TODO: Maybe fall back on display name slugification.
-            name = "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER);
+            name = {
+                "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER),
+                false,
+            };
             UNKNOWN_CATEGORY_COUNTER++;
         }
         else {
-            name = lowercase(get_attribute_value(name_attribute));
+            name = {
+                lowercase(get_attribute_value(name_attribute)),
+                false,
+            };
         }
 
-        if (name == "") {
+        if (name.value == "") {
             errors->push_back(new XMLNodeNameError("Category attribute 'name' is an empty string so it cannot be properly referenced", node));
             // TODO: Maybe fall back on display name slugification.
-            name = "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER);
+            name = {
+                "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER),
+                false,
+            };
             UNKNOWN_CATEGORY_COUNTER++;
         }
 
         // Build the new category
         Category* category;
 
         // Create and initialize a new category if this one does not exist
-        auto existing_category_search = marker_categories->find(name);
+        auto existing_category_search = marker_categories->find(name.value);
         if (existing_category_search == marker_categories->end()) {
-            category = &(*marker_categories)[name];
+            category = &(*marker_categories)[name.value];
             category->parent = parent;
         }
         else {
@@ -68,7 +80,7 @@ void parse_marker_categories(
         // based on the hashes of its name and its parents names.
         if (!category->menu_id_is_set) {
             Hash128 new_id;
-            new_id.update(name);
+            new_id.update(name.value);
 
             Category* next_node = parent;
             while (next_node != nullptr) {
@@ -82,9 +94,15 @@ void parse_marker_categories(
         for (rapidxml::xml_node<>* child_node = node->first_node(); child_node; child_node = child_node->next_sibling()) {
             parse_marker_categories(child_node, &(category->children), category, errors, state, depth + 1);
         }
+        return name;
     }
     else {
         errors->push_back(new XMLNodeNameError("Unknown MarkerCategory Tag", node));
+        name = {
+            "",
+            true,
+        };
+        return name;
     }
 }
 
@@ -189,12 +207,13 @@ vector<Parseable*> parse_pois(rapidxml::xml_node<>* root_node, map<string, Categ
 //
 // A function which parses a single XML file into their corrisponding classes.
 ////////////////////////////////////////////////////////////////////////////////
-void parse_xml_file(string xml_filepath, const string marker_pack_root_directory, map<string, Category>* marker_categories, vector<Parseable*>* parsed_pois) {
+set<string> parse_xml_file(string xml_filepath, const string marker_pack_root_directory, map<string, Category>* marker_categories, vector<Parseable*>* parsed_pois) {
     vector<XMLError*> errors;
     rapidxml::xml_document<> doc;
     rapidxml::xml_node<>* root_node;
     XMLReaderState state;
     state.marker_pack_root_directory = marker_pack_root_directory;
+    set<string> category_names;
 
     rapidxml::file<> xml_file(xml_filepath.c_str());
     doc.parse<rapidxml::parse_non_destructive | rapidxml::parse_no_data_nodes>(xml_file.data(), xml_filepath.c_str());
@@ -210,7 +229,10 @@ void parse_xml_file(string xml_filepath, const string marker_pack_root_directory
 
     for (rapidxml::xml_node<>* node = root_node->first_node(); node; node = node->next_sibling()) {
         if (get_node_name(node) == "MarkerCategory") {
-            parse_marker_categories(node, marker_categories, nullptr, &errors, &state);
+            OptionalString name = parse_marker_categories(node, marker_categories, nullptr, &errors, &state);
+            if (name.is_null == false) {
+                category_names.insert(name.value);
+            }
         }
         else if (get_node_name(node) == "POIs") {
             vector<Parseable*> temp_vector = parse_pois(node, marker_categories, &errors, &state);
@@ -224,6 +246,7 @@ void parse_xml_file(string xml_filepath, const string marker_pack_root_directory
     for (auto error : errors) {
         error->print_error();
     }
+    return category_names;
 }
 
 ////////////////////////////////////////////////////////////////////////////////

xml_converter/src/packaging_xml.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -10,7 +11,7 @@
 #include "rapidxml-1.13/rapidxml_utils.hpp"
 #include "state_structs/xml_reader_state.hpp"
 
-void parse_xml_file(
+std::set<std::string> parse_xml_file(
     std::string xml_filepath,
     const std::string marker_pack_root_directory,
     std::map<std::string, Category>* marker_categories,

xml_converter/src/string_helper.cpp

@@ -3,6 +3,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <iostream>
+#include <set>
 #include <string>
 #include <vector>
 

xml_converter/src/string_helper.hpp

@@ -2,9 +2,15 @@
 
 #include <cstdint>
 #include <initializer_list>
+#include <set>
 #include <string>
 #include <vector>
 
+struct OptionalString {
+    std::string value;
+    bool is_null;
+};
+
 bool matches_any(std::string test, std::initializer_list<std::string> list);
 bool normalized_matches_any(std::string test, std::initializer_list<std::string> list);
 bool normalized_matches_any(std::string test, std::vector<std::string> list);