diff --git a/xml_converter/integration_tests/run_tests.py b/xml_converter/integration_tests/run_tests.py index a1e8a36e..06c50c8a 100755 --- a/xml_converter/integration_tests/run_tests.py +++ b/xml_converter/integration_tests/run_tests.py @@ -20,7 +20,7 @@ def run_xml_converter( input_proto: Optional[List[str]] = None, output_proto: Optional[List[str]] = None, split_output_proto: Optional[str] = None, - additional_arguments: Optional[List[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 @@ -36,8 +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 additional_arguments: - cmd += additional_arguments + 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) @@ -177,7 +177,7 @@ def main() -> bool: input_proto=testcase.proto_input_paths, output_xml=[xml_output_dir_path], output_proto=[proto_output_dir_path], - additional_arguments=testcase.additional_arguments + allow_duplicates=testcase.allow_duplicates ) # Sanitize and denoise the lines diff --git a/xml_converter/integration_tests/src/testcase_loader.py b/xml_converter/integration_tests/src/testcase_loader.py index e73a33c3..b0d4394f 100644 --- a/xml_converter/integration_tests/src/testcase_loader.py +++ b/xml_converter/integration_tests/src/testcase_loader.py @@ -21,7 +21,7 @@ class Testcase: expected_stdout: List[str] expected_stderr: List[str] expected_returncode: int - additional_arguments: List[str] + allow_duplicates: bool ################################################################################ @@ -59,7 +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] = [] - additional_arguments: 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}") @@ -106,12 +106,12 @@ def load_testcase(path: str) -> Optional[Testcase]: print(f"Invalid Test, expecting string value for 'expected_returncode' in {path}") return None - if "additional_arguments" in data: - if not isinstance(data["additional_arguments"], str): - print(f"Invalid Test, expecting string value for 'additional_arguments' in {path}") + 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: - additional_arguments = to_lines(data["additional_arguments"]) + allow_duplicates = data["allow_duplicates"] return Testcase( name=os.path.basename(path), @@ -122,7 +122,7 @@ def load_testcase(path: str) -> Optional[Testcase]: expected_stdout=to_lines(data["expected_stdout"]), expected_stderr=to_lines(data["expected_stderr"]), expected_returncode=data["expected_returncode"], - additional_arguments=additional_arguments + allow_duplicates=allow_duplicates ) diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/testcase.yaml b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/testcase.yaml index 9d4ce889..348087c5 100644 --- a/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/testcase.yaml +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/testcase.yaml @@ -3,10 +3,6 @@ input_paths: "pack2": "proto" "pack3": "xml" expected_stdout: | - The following top level categories were found in more than one pack - mycategory - mycategory expected_stderr: | expected_returncode: 0 -additional_arguments: | - --allow-duplicates \ No newline at end of file +allow_duplicates: true \ No newline at end of file diff --git a/xml_converter/src/packaging_xml.cpp b/xml_converter/src/packaging_xml.cpp index 7f5f59b8..604ded0e 100644 --- a/xml_converter/src/packaging_xml.cpp +++ b/xml_converter/src/packaging_xml.cpp @@ -17,33 +17,37 @@ using namespace std; //////////////////////////////////////////////////////////////////////////////// unsigned int UNKNOWN_CATEGORY_COUNTER = 0; -string parse_marker_categories( +OptionalString parse_marker_categories( rapidxml::xml_node<>* node, map* marker_categories, Category* parent, vector* errors, XMLReaderState* state, int depth = 0) { + OptionalString name = { + "", // value + false, // error + }; 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)); + name.error = true; // TODO: Maybe fall back on display name slugification. - name = "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER); + name.value = "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER); UNKNOWN_CATEGORY_COUNTER++; } else { - name = lowercase(get_attribute_value(name_attribute)); + name.value = lowercase(get_attribute_value(name_attribute)); } - if (name == "") { + if (name.value == "") { errors->push_back(new XMLNodeNameError("Category attribute 'name' is an empty string so it cannot be properly referenced", node)); + name.error = true; // TODO: Maybe fall back on display name slugification. - name = "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER); + name.value = "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER); UNKNOWN_CATEGORY_COUNTER++; } @@ -51,15 +55,16 @@ string parse_marker_categories( 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 { category = &existing_category_search->second; if (category->parent != parent) { errors->push_back(new XMLNodeNameError("Category somehow has a different parent then it used to. This might be a bug in xml_converter", node)); + name.error = true; } } @@ -69,7 +74,7 @@ string 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) { @@ -87,7 +92,8 @@ string parse_marker_categories( } else { errors->push_back(new XMLNodeNameError("Unknown MarkerCategory Tag", node)); - return ""; + name.error = true; + return name; } } @@ -214,7 +220,10 @@ set parse_xml_file(string xml_filepath, const string marker_pack_root_di for (rapidxml::xml_node<>* node = root_node->first_node(); node; node = node->next_sibling()) { if (get_node_name(node) == "MarkerCategory") { - category_names.insert(parse_marker_categories(node, marker_categories, nullptr, &errors, &state)); + OptionalString name = parse_marker_categories(node, marker_categories, nullptr, &errors, &state); + if (name.error == false) { + category_names.insert(name.value); + } } else if (get_node_name(node) == "POIs") { vector temp_vector = parse_pois(node, marker_categories, &errors, &state); diff --git a/xml_converter/src/string_helper.cpp b/xml_converter/src/string_helper.cpp index 3f0056f2..ec461457 100644 --- a/xml_converter/src/string_helper.cpp +++ b/xml_converter/src/string_helper.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -314,3 +315,16 @@ std::string long_to_hex_string(uint64_t number) { return hex_string; } + +void combine_sets( + std::set* set_a, + std::set* set_b, + std::vector* duplicates) { + for (string str : *set_a) { + if (auto search = set_b->find(str); search != set_b->end()) { + duplicates->push_back(str); + } + else + set_b->insert(str); + } +} diff --git a/xml_converter/src/string_helper.hpp b/xml_converter/src/string_helper.hpp index 89fc3f93..79df0b27 100644 --- a/xml_converter/src/string_helper.hpp +++ b/xml_converter/src/string_helper.hpp @@ -2,9 +2,15 @@ #include #include +#include #include #include +struct OptionalString { + std::string value; + bool error; +}; + bool matches_any(std::string test, std::initializer_list list); bool normalized_matches_any(std::string test, std::initializer_list list); bool normalized_matches_any(std::string test, std::vector list); @@ -24,3 +30,4 @@ bool has_suffix(std::string const& fullString, std::string const& ending); std::string join_file_paths(const std::string& path_a, const std::string& path_b); std::string long_to_hex_string(uint64_t number); +void combine_sets(std::set* set_a, std::set* set_b, std::vector* duplicates); diff --git a/xml_converter/src/xml_converter.cpp b/xml_converter/src/xml_converter.cpp index b5b5e464..948005c8 100644 --- a/xml_converter/src/xml_converter.cpp +++ b/xml_converter/src/xml_converter.cpp @@ -144,7 +144,9 @@ void write_burrito_directory( void process_data( vector input_taco_paths, vector input_guildpoint_paths, - + // If multiple inputs are found to have the same top level categories, + // The program will skip writing to output unless the below is true + bool allow_duplicates, // These will eventually have additional arguments for each output path to // allow for splitting out a single markerpack vector output_taco_paths, @@ -152,8 +154,7 @@ void process_data( // This is a special output path used for burrito internal use that splits // the guildpoint protobins by map id. - string output_split_guildpoint_dir, - bool allow_duplicates) { + string output_split_guildpoint_dir) { // All of the loaded pois and categories vector parsed_pois; map marker_categories; @@ -169,12 +170,7 @@ void process_data( input_taco_paths[i], &marker_categories, &parsed_pois); - for (string category_name : category_names) { - if (find(top_level_categories.begin(), top_level_categories.end(), category_name) != top_level_categories.end()) - duplicate_categories.push_back(category_name); - else - top_level_categories.insert(category_name); - } + combine_sets(&category_names, &top_level_categories, &duplicate_categories); } auto end = chrono::high_resolution_clock::now(); auto dur = end - begin; @@ -189,23 +185,16 @@ void process_data( input_guildpoint_paths[i], &marker_categories, &parsed_pois); - for (string category_name : category_names) { - if (find(top_level_categories.begin(), top_level_categories.end(), category_name) != top_level_categories.end()) - duplicate_categories.push_back(category_name); - else - top_level_categories.insert(category_name); - } + combine_sets(&category_names, &top_level_categories, &duplicate_categories); } - if (duplicate_categories.size() > 0) { + if (duplicate_categories.size() > 0 && allow_duplicates != true) { + cout << "Did not write due to duplicates in categories. If you want to bypass this, use '--allow-duplicates'" << endl; cout << "The following top level categories were found in more than one pack" << endl; for (size_t i = 0; i < duplicate_categories.size(); i++) { cout << duplicate_categories[i] << endl; } - if (allow_duplicates != true) { - cout << "Did not write due to duplicates in categories. If you want to bypass this, use '--allow-duplicates'" << endl; - return; - } + return; } // Write all of the xml taco paths @@ -252,7 +241,7 @@ int main(int argc, char* argv[]) { vector output_taco_paths; vector input_guildpoint_paths; vector output_guildpoint_paths; - bool allow_duplicates; + bool allow_duplicates = false; // Typically "~/.local/share/godot/app_userdata/Burrito/protobins" for // converting from xml markerpacks to internal protobuf files. @@ -281,9 +270,16 @@ int main(int argc, char* argv[]) { } else if (!strcmp(argv[i], "--allow-duplicates")) { allow_duplicates = true; + arg_target = nullptr; } else { - arg_target->push_back(argv[i]); + if (arg_target != nullptr) { + arg_target->push_back(argv[i]); + } + else { + cout << "Unknown argument " << argv[i] << endl; + return -1; + } } } @@ -300,10 +296,10 @@ int main(int argc, char* argv[]) { process_data( input_taco_paths, input_guildpoint_paths, + allow_duplicates, output_taco_paths, output_guildpoint_paths, - output_split_guildpoint_dir, - allow_duplicates); + output_split_guildpoint_dir); return 0; }