From a1724e76543c6a8314aec5014a7e3759c2e75b7b Mon Sep 17 00:00:00 2001 From: klingbolt Date: Sun, 22 Sep 2024 23:07:16 -0400 Subject: [PATCH 1/6] Added tracking for the top level categories in each pack. Stops writing if there are any duplicates between packs. Added an argument to bypass this if nessecary. Addded tests for both cases --- xml_converter/integration_tests/run_tests.py | 4 ++ .../integration_tests/src/testcase_loader.py | 12 +++- .../input/pack/xml_file.xml | 8 +++ .../input/pack2/markers.bin | 4 ++ .../input/pack3/xml_file.xml | 8 +++ .../output_proto/markers.bin | 4 ++ .../output_xml/xml_file.xml | 10 ++++ .../testcase.yaml | 12 ++++ .../input/pack/xml_file.xml | 8 +++ .../input/pack2/markers.bin | 4 ++ .../input/pack3/xml_file.xml | 8 +++ .../testcase.yaml | 11 ++++ xml_converter/src/packaging_protobin.cpp | 9 ++- xml_converter/src/packaging_protobin.hpp | 3 +- xml_converter/src/packaging_xml.cpp | 11 +++- xml_converter/src/packaging_xml.hpp | 3 +- xml_converter/src/xml_converter.cpp | 59 +++++++++++++++---- 17 files changed, 159 insertions(+), 19 deletions(-) create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack/xml_file.xml create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack2/markers.bin create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack3/xml_file.xml create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/output_proto/markers.bin create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/output_xml/xml_file.xml create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/testcase.yaml create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack/xml_file.xml create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack2/markers.bin create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack3/xml_file.xml create mode 100644 xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml diff --git a/xml_converter/integration_tests/run_tests.py b/xml_converter/integration_tests/run_tests.py index 64589d29..566f7341 100755 --- a/xml_converter/integration_tests/run_tests.py +++ b/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, + additional_arguments: Optional[List[str]] = 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-waypoint-path"] + output_proto if split_output_proto: cmd += ["--output-split-waypoint-path"] + [split_output_proto] + if additional_arguments: + cmd += additional_arguments # 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], + additional_arguments=testcase.additional_arguments ) # 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 aa85b442..e73a33c3 100644 --- a/xml_converter/integration_tests/src/testcase_loader.py +++ b/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 + additional_arguments: List[str] ################################################################################ @@ -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] = [] + additional_arguments: List[str] = [] 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 "additional_arguments" in data: + if not isinstance(data["additional_arguments"], str): + print(f"Invalid Test, expecting string value for 'additional_arguments' in {path}") + return None + else: + additional_arguments = to_lines(data["additional_arguments"]) + 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"], + additional_arguments=additional_arguments ) diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack/xml_file.xml b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack/xml_file.xml new file mode 100644 index 00000000..5cf29494 --- /dev/null +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack/xml_file.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack2/markers.bin b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack2/markers.bin new file mode 100644 index 00000000..50078921 --- /dev/null +++ b/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(èÌ“^– \ No newline at end of file diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack3/xml_file.xml b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack3/xml_file.xml new file mode 100644 index 00000000..5cf29494 --- /dev/null +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/input/pack3/xml_file.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/output_proto/markers.bin b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/output_proto/markers.bin new file mode 100644 index 00000000..40148278 --- /dev/null +++ b/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(èÌ“^– \ No newline at end of file diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/output_xml/xml_file.xml b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/output_xml/xml_file.xml new file mode 100644 index 00000000..3638e1e3 --- /dev/null +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/output_xml/xml_file.xml @@ -0,0 +1,10 @@ + + + + + + + + + + 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 new file mode 100644 index 00000000..9d4ce889 --- /dev/null +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_allow_duplicates/testcase.yaml @@ -0,0 +1,12 @@ +input_paths: + "pack": "xml" + "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 diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack/xml_file.xml b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack/xml_file.xml new file mode 100644 index 00000000..5cf29494 --- /dev/null +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack/xml_file.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack2/markers.bin b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack2/markers.bin new file mode 100644 index 00000000..50078921 --- /dev/null +++ b/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(èÌ“^– \ No newline at end of file diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack3/xml_file.xml b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack3/xml_file.xml new file mode 100644 index 00000000..5cf29494 --- /dev/null +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/input/pack3/xml_file.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml new file mode 100644 index 00000000..19554081 --- /dev/null +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml @@ -0,0 +1,11 @@ +input_paths: + "pack": "xml" + "pack2": "proto" + "pack3": "xml" +expected_stdout: | + The following top level categories were found in more than one pack + mycategory + mycategory + Did not write due to duplicates in categories. If you want to bypass this, use '--allow-duplicates' +expected_stderr: | +expected_returncode: 0 \ No newline at end of file diff --git a/xml_converter/src/packaging_protobin.cpp b/xml_converter/src/packaging_protobin.cpp index 3fe1b03a..f505612e 100644 --- a/xml_converter/src/packaging_protobin.cpp +++ b/xml_converter/src/packaging_protobin.cpp @@ -23,7 +23,7 @@ using namespace std; //////////////////////////////////////////////////////////////////////////////// // //////////////////////////////////////////////////////////////////////////////// -void parse_waypoint_categories( +string parse_waypoint_categories( string full_category_name, ::waypoint::Category proto_category, map* marker_categories, @@ -57,16 +57,18 @@ void parse_waypoint_categories( for (int i = 0; i < proto_category.children_size(); i++) { parse_waypoint_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* marker_categories, vector* parsed_pois) { +set read_protobuf_file(string proto_filepath, const string marker_pack_root_directory, map* marker_categories, vector* parsed_pois) { fstream infile; waypoint::Waypoint proto_message; ProtoReaderState state; state.marker_pack_root_directory = marker_pack_root_directory; + set category_names; infile.open(proto_filepath, ios::in | ios::binary); proto_message.ParseFromIstream(&infile); @@ -74,8 +76,9 @@ void read_protobuf_file(string proto_filepath, const string marker_pack_root_dir state.textures = proto_message.textures(); for (int i = 0; i < proto_message.category_size(); i++) { - parse_waypoint_categories("", proto_message.category(i), marker_categories, parsed_pois, &state); + category_names.insert(parse_waypoint_categories("", proto_message.category(i), marker_categories, parsed_pois, &state)); } + return category_names; } //////////////////////////////////////////////////////////////////////////////// diff --git a/xml_converter/src/packaging_protobin.hpp b/xml_converter/src/packaging_protobin.hpp index 495ef54e..d362e1d5 100644 --- a/xml_converter/src/packaging_protobin.hpp +++ b/xml_converter/src/packaging_protobin.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -10,7 +11,7 @@ #include "string_hierarchy.hpp" #include "waypoint.pb.h" -void read_protobuf_file( +std::set read_protobuf_file( std::string proto_filepath, const std::string marker_pack_root_directory, std::map* marker_categories, diff --git a/xml_converter/src/packaging_xml.cpp b/xml_converter/src/packaging_xml.cpp index b2806d37..7f5f59b8 100644 --- a/xml_converter/src/packaging_xml.cpp +++ b/xml_converter/src/packaging_xml.cpp @@ -1,5 +1,6 @@ #include "packaging_xml.hpp" +#include #include #include "hash_helpers.hpp" @@ -16,7 +17,7 @@ using namespace std; //////////////////////////////////////////////////////////////////////////////// unsigned int UNKNOWN_CATEGORY_COUNTER = 0; -void parse_marker_categories( +string parse_marker_categories( rapidxml::xml_node<>* node, map* marker_categories, Category* parent, @@ -82,9 +83,11 @@ 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)); + return ""; } } @@ -189,12 +192,13 @@ vector parse_pois(rapidxml::xml_node<>* root_node, map* marker_categories, vector* parsed_pois) { +set parse_xml_file(string xml_filepath, const string marker_pack_root_directory, map* marker_categories, vector* parsed_pois) { vector errors; rapidxml::xml_document<> doc; rapidxml::xml_node<>* root_node; XMLReaderState state; state.marker_pack_root_directory = marker_pack_root_directory; + set category_names; rapidxml::file<> xml_file(xml_filepath.c_str()); doc.parse(xml_file.data(), xml_filepath.c_str()); @@ -210,7 +214,7 @@ 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); + category_names.insert(parse_marker_categories(node, marker_categories, nullptr, &errors, &state)); } else if (get_node_name(node) == "POIs") { vector temp_vector = parse_pois(node, marker_categories, &errors, &state); @@ -224,6 +228,7 @@ void parse_xml_file(string xml_filepath, const string marker_pack_root_directory for (auto error : errors) { error->print_error(); } + return category_names; } //////////////////////////////////////////////////////////////////////////////// diff --git a/xml_converter/src/packaging_xml.hpp b/xml_converter/src/packaging_xml.hpp index a099d625..6efabdfd 100644 --- a/xml_converter/src/packaging_xml.hpp +++ b/xml_converter/src/packaging_xml.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -10,7 +11,7 @@ #include "rapidxml-1.13/rapidxml_utils.hpp" #include "state_structs/xml_reader_state.hpp" -void parse_xml_file( +std::set parse_xml_file( std::string xml_filepath, const std::string marker_pack_root_directory, std::map* marker_categories, diff --git a/xml_converter/src/xml_converter.cpp b/xml_converter/src/xml_converter.cpp index 7f081d1f..02cc3ba8 100644 --- a/xml_converter/src/xml_converter.cpp +++ b/xml_converter/src/xml_converter.cpp @@ -62,40 +62,48 @@ vector get_files_by_suffix(string directory, string suffix) { return files; } -void read_taco_directory( +set read_taco_directory( string input_path, map* marker_categories, vector* parsed_pois) { + set top_level_categories; if (!filesystem::exists(input_path)) { cout << "Error: " << input_path << " is not an existing directory or file" << endl; } else if (filesystem::is_directory(input_path)) { vector xml_files = get_files_by_suffix(input_path, ".xml"); for (const string& path : xml_files) { - parse_xml_file(path, input_path, marker_categories, parsed_pois); + set category_names = (parse_xml_file(path, input_path, marker_categories, parsed_pois)); + top_level_categories.insert(category_names.begin(), category_names.end()); } } else if (filesystem::is_regular_file(input_path)) { - parse_xml_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); + set category_names = parse_xml_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); + top_level_categories.insert(category_names.begin(), category_names.end()); } + return top_level_categories; } -void read_burrito_directory( +set read_burrito_directory( string input_path, map* marker_categories, vector* parsed_pois) { + set top_level_categories; if (!filesystem::exists(input_path)) { cout << "Error: " << input_path << " is not an existing directory or file" << endl; } else if (filesystem::is_directory(input_path)) { vector burrito_files = get_files_by_suffix(input_path, ".bin"); for (const string& path : burrito_files) { - read_protobuf_file(path, input_path, marker_categories, parsed_pois); + set category_names = read_protobuf_file(path, input_path, marker_categories, parsed_pois); + top_level_categories.insert(category_names.begin(), category_names.end()); } } else if (filesystem::is_regular_file(input_path)) { - read_protobuf_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); + set category_names = read_protobuf_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); + top_level_categories.insert(category_names.begin(), category_names.end()); } + return top_level_categories; } void write_taco_directory( @@ -144,20 +152,29 @@ void process_data( // This is a special output path used for burrito internal use that splits // the waypoint protobins by map id. - string output_split_waypoint_dir) { + string output_split_waypoint_dir, + bool allow_duplicates) { // All of the loaded pois and categories vector parsed_pois; map marker_categories; + set top_level_categories; + vector duplicate_categories; // Read in all the xml taco markerpacks auto begin = chrono::high_resolution_clock::now(); for (size_t i = 0; i < input_taco_paths.size(); i++) { cout << "Loading taco pack " << input_taco_paths[i] << endl; - read_taco_directory( + set category_names = read_taco_directory( 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); + } } auto end = chrono::high_resolution_clock::now(); auto dur = end - begin; @@ -168,10 +185,27 @@ void process_data( for (size_t i = 0; i < input_waypoint_paths.size(); i++) { cout << "Loading waypoint pack " << input_waypoint_paths[i] << endl; - read_burrito_directory( + set category_names = read_burrito_directory( input_waypoint_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); + } + } + + if (duplicate_categories.size() > 0) { + 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; + } } // Write all of the xml taco paths @@ -218,6 +252,7 @@ int main(int argc, char* argv[]) { vector output_taco_paths; vector input_waypoint_paths; vector output_waypoint_paths; + bool allow_duplicates; // Typically "~/.local/share/godot/app_userdata/Burrito/protobins" for // converting from xml markerpacks to internal protobuf files. @@ -244,6 +279,9 @@ int main(int argc, char* argv[]) { // CLI arg parsing later to properly capture this. arg_target = &output_split_waypoint_paths; } + else if (!strcmp(argv[i], "--allow-duplicates")) { + allow_duplicates = true; + } else { arg_target->push_back(argv[i]); } @@ -264,7 +302,8 @@ int main(int argc, char* argv[]) { input_waypoint_paths, output_taco_paths, output_waypoint_paths, - output_split_waypoint_dir); + output_split_waypoint_dir, + allow_duplicates); return 0; } From c6b8cafa86aadcad86aebb76b0ede856f55e5942 Mon Sep 17 00:00:00 2001 From: klingbolt Date: Sun, 22 Sep 2024 23:17:43 -0400 Subject: [PATCH 2/6] fixed error from merge conflict --- xml_converter/src/xml_converter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xml_converter/src/xml_converter.cpp b/xml_converter/src/xml_converter.cpp index c5fc221b..b5b5e464 100644 --- a/xml_converter/src/xml_converter.cpp +++ b/xml_converter/src/xml_converter.cpp @@ -152,7 +152,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) { + string output_split_guildpoint_dir, bool allow_duplicates) { // All of the loaded pois and categories vector parsed_pois; @@ -302,7 +302,7 @@ int main(int argc, char* argv[]) { input_guildpoint_paths, output_taco_paths, output_guildpoint_paths, - output_split_guildpoint_dir + output_split_guildpoint_dir, allow_duplicates); return 0; From 80b85364876f14700806123f43936f0fe4a286cc Mon Sep 17 00:00:00 2001 From: klingbolt Date: Sun, 29 Sep 2024 20:42:52 -0400 Subject: [PATCH 3/6] fixed crash if no output folder is present because no output is expected --- xml_converter/integration_tests/run_tests.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/xml_converter/integration_tests/run_tests.py b/xml_converter/integration_tests/run_tests.py index 74431305..a1e8a36e 100755 --- a/xml_converter/integration_tests/run_tests.py +++ b/xml_converter/integration_tests/run_tests.py @@ -210,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}") From afef7281174a85067879cce0b0b0367c9fb1ab8d Mon Sep 17 00:00:00 2001 From: klingbolt Date: Tue, 1 Oct 2024 23:04:57 -0400 Subject: [PATCH 4/6] addressing code review. added optional string struct, function for combining set, error handling for argument parsing --- xml_converter/integration_tests/run_tests.py | 8 ++-- .../integration_tests/src/testcase_loader.py | 14 +++--- .../testcase.yaml | 6 +-- xml_converter/src/packaging_xml.cpp | 33 +++++++++----- xml_converter/src/string_helper.cpp | 14 ++++++ xml_converter/src/string_helper.hpp | 7 +++ xml_converter/src/xml_converter.cpp | 44 +++++++++---------- 7 files changed, 74 insertions(+), 52 deletions(-) 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; } From 43acbc4b4371b73b623170a415fe2c1eab992963 Mon Sep 17 00:00:00 2001 From: klingbolt Date: Sun, 10 Nov 2024 21:56:47 -0500 Subject: [PATCH 5/6] Addressing code review. Changes to optional string class. Changed how conflicts are communicated --- .../testcase.yaml | 13 ++- xml_converter/src/packaging_xml.cpp | 27 ++++--- xml_converter/src/string_helper.cpp | 13 --- xml_converter/src/string_helper.hpp | 3 +- xml_converter/src/xml_converter.cpp | 80 ++++++++++++++----- 5 files changed, 86 insertions(+), 50 deletions(-) diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml index 19554081..d36081b1 100644 --- a/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml @@ -3,9 +3,14 @@ input_paths: "pack2": "proto" "pack3": "xml" expected_stdout: | - The following top level categories were found in more than one pack - mycategory - mycategory - Did not write due to duplicates in categories. If you want to bypass this, use '--allow-duplicates' + 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 + pack3/xml_file.xml + pack2/markers.bin expected_stderr: | expected_returncode: 0 \ No newline at end of file diff --git a/xml_converter/src/packaging_xml.cpp b/xml_converter/src/packaging_xml.cpp index 604ded0e..6675c2d3 100644 --- a/xml_converter/src/packaging_xml.cpp +++ b/xml_converter/src/packaging_xml.cpp @@ -26,28 +26,35 @@ OptionalString parse_marker_categories( int depth = 0) { OptionalString name = { "", // value - false, // error + false, // is_null }; if (get_node_name(node) == "MarkerCategory") { 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.value = "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER); + name = { + "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER), + false, + }; UNKNOWN_CATEGORY_COUNTER++; } else { - name.value = lowercase(get_attribute_value(name_attribute)); + name = { + lowercase(get_attribute_value(name_attribute)), + false, + }; } 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.value = "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER); + name = { + "UNKNOWN_CATEGORY_" + to_string(UNKNOWN_CATEGORY_COUNTER), + false, + }; UNKNOWN_CATEGORY_COUNTER++; } @@ -64,7 +71,6 @@ OptionalString parse_marker_categories( 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; } } @@ -92,7 +98,10 @@ OptionalString parse_marker_categories( } else { errors->push_back(new XMLNodeNameError("Unknown MarkerCategory Tag", node)); - name.error = true; + name = { + "", + true, + }; return name; } } @@ -221,7 +230,7 @@ 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") { OptionalString name = parse_marker_categories(node, marker_categories, nullptr, &errors, &state); - if (name.error == false) { + if (name.is_null == false) { category_names.insert(name.value); } } diff --git a/xml_converter/src/string_helper.cpp b/xml_converter/src/string_helper.cpp index ec461457..fc498b87 100644 --- a/xml_converter/src/string_helper.cpp +++ b/xml_converter/src/string_helper.cpp @@ -315,16 +315,3 @@ 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 79df0b27..81f4d529 100644 --- a/xml_converter/src/string_helper.hpp +++ b/xml_converter/src/string_helper.hpp @@ -8,7 +8,7 @@ struct OptionalString { std::string value; - bool error; + bool is_null; }; bool matches_any(std::string test, std::initializer_list list); @@ -30,4 +30,3 @@ 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 948005c8..2392d950 100644 --- a/xml_converter/src/xml_converter.cpp +++ b/xml_converter/src/xml_converter.cpp @@ -62,48 +62,62 @@ vector get_files_by_suffix(string directory, string suffix) { return files; } -set read_taco_directory( +map> read_taco_directory( string input_path, map* marker_categories, vector* parsed_pois) { - set top_level_categories; + map> category_to_filepath; if (!filesystem::exists(input_path)) { cout << "Error: " << input_path << " is not an existing directory or file" << endl; } else if (filesystem::is_directory(input_path)) { + string directory_name = filesystem::path(input_path).filename(); vector xml_files = get_files_by_suffix(input_path, ".xml"); for (const string& path : xml_files) { set category_names = (parse_xml_file(path, input_path, marker_categories, parsed_pois)); - top_level_categories.insert(category_names.begin(), category_names.end()); + string relative_path = directory_name + path.substr(input_path.size()); + for (set::iterator it = category_names.begin(); it != category_names.end(); it++) { + category_to_filepath[*it].push_back(relative_path); + } } } else if (filesystem::is_regular_file(input_path)) { set category_names = parse_xml_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); - top_level_categories.insert(category_names.begin(), category_names.end()); + string filename = filesystem::path(input_path).filename(); + for (set::iterator it = category_names.begin(); it != category_names.end(); it++) { + category_to_filepath[*it].push_back(filename); + } } - return top_level_categories; + return category_to_filepath; } -set read_burrito_directory( +map> read_burrito_directory( string input_path, map* marker_categories, vector* parsed_pois) { - set top_level_categories; + map> category_to_filepath; if (!filesystem::exists(input_path)) { cout << "Error: " << input_path << " is not an existing directory or file" << endl; } else if (filesystem::is_directory(input_path)) { + string directory_name = filesystem::path(input_path).filename(); vector burrito_files = get_files_by_suffix(input_path, ".bin"); for (const string& path : burrito_files) { set category_names = read_protobuf_file(path, input_path, marker_categories, parsed_pois); - top_level_categories.insert(category_names.begin(), category_names.end()); + string relative_path = directory_name + path.substr(input_path.size()); + for (set::iterator it = category_names.begin(); it != category_names.end(); it++) { + category_to_filepath[*it].push_back(relative_path); + } } } else if (filesystem::is_regular_file(input_path)) { set category_names = read_protobuf_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); - top_level_categories.insert(category_names.begin(), category_names.end()); + string filename = filesystem::path(input_path).filename(); + for (set::iterator it = category_names.begin(); it != category_names.end(); it++) { + category_to_filepath[*it].push_back(filename); + } } - return top_level_categories; + return category_to_filepath; } void write_taco_directory( @@ -158,19 +172,22 @@ void process_data( // All of the loaded pois and categories vector parsed_pois; map marker_categories; - set top_level_categories; - vector duplicate_categories; + map>> category_to_marker_pack_names; + map> duplicate_categories; // Read in all the xml taco markerpacks auto begin = chrono::high_resolution_clock::now(); for (size_t i = 0; i < input_taco_paths.size(); i++) { cout << "Loading taco pack " << input_taco_paths[i] << endl; - set category_names = read_taco_directory( + // swap this to map >) + map> category_names = read_taco_directory( input_taco_paths[i], &marker_categories, &parsed_pois); - combine_sets(&category_names, &top_level_categories, &duplicate_categories); + for (map>::iterator it = category_names.begin(); it != category_names.end(); it++) { + category_to_marker_pack_names[it->first].push_back(it->second); + } } auto end = chrono::high_resolution_clock::now(); auto dur = end - begin; @@ -181,18 +198,37 @@ void process_data( for (size_t i = 0; i < input_guildpoint_paths.size(); i++) { cout << "Loading guildpoint pack " << input_guildpoint_paths[i] << endl; - set category_names = read_burrito_directory( + map> category_names = read_burrito_directory( input_guildpoint_paths[i], &marker_categories, &parsed_pois); - combine_sets(&category_names, &top_level_categories, &duplicate_categories); + for (map>::iterator it = category_names.begin(); it != category_names.end(); it++) { + category_to_marker_pack_names[it->first].push_back(it->second); + } } - 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; + for (map>>::iterator it = category_to_marker_pack_names.begin(); it != category_to_marker_pack_names.end(); it++) { + if (it->second.size() != 1) { + for (size_t i = 0; i < it->second.size(); i++) { + for (size_t j = 0; j < it->second[i].size(); j++) { + duplicate_categories[it->first].push_back(it->second[i][j]); + } + } + } + } + if (duplicate_categories.size() > 0 && allow_duplicates == false) { + cout << "Did not write due to duplicates in categories." << endl; + cout << "This commonly occurs when attempting to read the same pack multiple times or when separate packs coincidentally have the same name." << endl; + // TODO: This is the current advice. Further updates could allow other + // options like selective merges or changing category names to be unique + cout << "Please remove one of the packs or edit the name of the packs' top level category before running the program again." << endl; + cout << "If you want to bypass this stop, use '--allow-duplicates'." << endl; + cout << "The following top level categories were found in more than one pack:" << endl; + for (map>::iterator it = duplicate_categories.begin(); it != duplicate_categories.end(); it++) { + cout << it->first << " in files:" << endl; + for (string str : it->second) { + cout << str << endl; + } } return; } @@ -247,7 +283,7 @@ int main(int argc, char* argv[]) { // converting from xml markerpacks to internal protobuf files. vector output_split_guildpoint_paths; - vector* arg_target = &input_taco_paths; + vector* arg_target = nullptr; for (int i = 1; i < argc; i++) { if (!strcmp(argv[i], "--input-taco-path")) { From 628e590514559f746c596692611f8c19d1a99686 Mon Sep 17 00:00:00 2001 From: klingbolt Date: Tue, 12 Nov 2024 19:21:44 -0500 Subject: [PATCH 6/6] Changed names of variables to be more descriptive. Edited search for file by suffix function to return relative paths --- .../testcase.yaml | 8 +-- xml_converter/src/xml_converter.cpp | 68 +++++++++---------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml index d36081b1..9f5d0f93 100644 --- a/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml +++ b/xml_converter/integration_tests/test_cases/proto_and_xml_input_no_duplicates/testcase.yaml @@ -8,9 +8,9 @@ expected_stdout: | 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 - pack3/xml_file.xml - pack2/markers.bin + "mycategory" in files: + pack/xml_file.xml + pack2/markers.bin + pack3/xml_file.xml expected_stderr: | expected_returncode: 0 \ No newline at end of file diff --git a/xml_converter/src/xml_converter.cpp b/xml_converter/src/xml_converter.cpp index 2392d950..18f88ddd 100644 --- a/xml_converter/src/xml_converter.cpp +++ b/xml_converter/src/xml_converter.cpp @@ -36,6 +36,7 @@ bool filename_comp(string a, string b) { return lowercase(a) < lowercase(b); } +// Searchs for files within a directory with a suffix and returns their relative paths. vector get_files_by_suffix(string directory, string suffix) { vector files; DIR* dir = opendir(directory.c_str()); @@ -49,11 +50,11 @@ vector get_files_by_suffix(string directory, string suffix) { // Default: markerpacks have all xml files in the first directory for (string subfile : subfiles) { cout << subfile << " found in subfolder" << endl; - files.push_back(subfile); + files.push_back(join_file_paths(filename, subfile)); } } else if (has_suffix(filename, suffix)) { - files.push_back(path); + files.push_back(filename); } } } @@ -66,7 +67,7 @@ map> read_taco_directory( string input_path, map* marker_categories, vector* parsed_pois) { - map> category_to_filepath; + map> top_level_category_file_locations; if (!filesystem::exists(input_path)) { cout << "Error: " << input_path << " is not an existing directory or file" << endl; } @@ -74,28 +75,28 @@ map> read_taco_directory( string directory_name = filesystem::path(input_path).filename(); vector xml_files = get_files_by_suffix(input_path, ".xml"); for (const string& path : xml_files) { - set category_names = (parse_xml_file(path, input_path, marker_categories, parsed_pois)); - string relative_path = directory_name + path.substr(input_path.size()); - for (set::iterator it = category_names.begin(); it != category_names.end(); it++) { - category_to_filepath[*it].push_back(relative_path); + set top_level_category_names = parse_xml_file(join_file_paths(input_path, path), input_path, marker_categories, parsed_pois); + string relative_path = join_file_paths(directory_name, path); + for (set::iterator it = top_level_category_names.begin(); it != top_level_category_names.end(); it++) { + top_level_category_file_locations[*it].push_back(relative_path); } } } else if (filesystem::is_regular_file(input_path)) { - set category_names = parse_xml_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); + set top_level_category_names = parse_xml_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); string filename = filesystem::path(input_path).filename(); - for (set::iterator it = category_names.begin(); it != category_names.end(); it++) { - category_to_filepath[*it].push_back(filename); + for (set::iterator it = top_level_category_names.begin(); it != top_level_category_names.end(); it++) { + top_level_category_file_locations[*it].push_back(filename); } } - return category_to_filepath; + return top_level_category_file_locations; } map> read_burrito_directory( string input_path, map* marker_categories, vector* parsed_pois) { - map> category_to_filepath; + map> top_level_category_file_locations; if (!filesystem::exists(input_path)) { cout << "Error: " << input_path << " is not an existing directory or file" << endl; } @@ -103,21 +104,21 @@ map> read_burrito_directory( string directory_name = filesystem::path(input_path).filename(); vector burrito_files = get_files_by_suffix(input_path, ".bin"); for (const string& path : burrito_files) { - set category_names = read_protobuf_file(path, input_path, marker_categories, parsed_pois); - string relative_path = directory_name + path.substr(input_path.size()); - for (set::iterator it = category_names.begin(); it != category_names.end(); it++) { - category_to_filepath[*it].push_back(relative_path); + set top_level_category_names = read_protobuf_file(join_file_paths(input_path, path), input_path, marker_categories, parsed_pois); + string relative_path = join_file_paths(directory_name, path); + for (set::iterator it = top_level_category_names.begin(); it != top_level_category_names.end(); it++) { + top_level_category_file_locations[*it].push_back(relative_path); } } } else if (filesystem::is_regular_file(input_path)) { - set category_names = read_protobuf_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); + set top_level_category_names = read_protobuf_file(input_path, get_base_dir(input_path), marker_categories, parsed_pois); string filename = filesystem::path(input_path).filename(); - for (set::iterator it = category_names.begin(); it != category_names.end(); it++) { - category_to_filepath[*it].push_back(filename); + for (set::iterator it = top_level_category_names.begin(); it != top_level_category_names.end(); it++) { + top_level_category_file_locations[*it].push_back(filename); } } - return category_to_filepath; + return top_level_category_file_locations; } void write_taco_directory( @@ -172,21 +173,20 @@ void process_data( // All of the loaded pois and categories vector parsed_pois; map marker_categories; - map>> category_to_marker_pack_names; - map> duplicate_categories; + map>> top_level_category_file_locations_by_pack; + map> duplicate_categories; // Read in all the xml taco markerpacks auto begin = chrono::high_resolution_clock::now(); for (size_t i = 0; i < input_taco_paths.size(); i++) { cout << "Loading taco pack " << input_taco_paths[i] << endl; - // swap this to map >) - map> category_names = read_taco_directory( + map> top_level_category_file_locations = read_taco_directory( input_taco_paths[i], &marker_categories, &parsed_pois); - for (map>::iterator it = category_names.begin(); it != category_names.end(); it++) { - category_to_marker_pack_names[it->first].push_back(it->second); + for (map>::iterator it = top_level_category_file_locations.begin(); it != top_level_category_file_locations.end(); it++) { + top_level_category_file_locations_by_pack[it->first].push_back(it->second); } } auto end = chrono::high_resolution_clock::now(); @@ -198,20 +198,20 @@ void process_data( for (size_t i = 0; i < input_guildpoint_paths.size(); i++) { cout << "Loading guildpoint pack " << input_guildpoint_paths[i] << endl; - map> category_names = read_burrito_directory( + map> top_level_category_file_locations = read_burrito_directory( input_guildpoint_paths[i], &marker_categories, &parsed_pois); - for (map>::iterator it = category_names.begin(); it != category_names.end(); it++) { - category_to_marker_pack_names[it->first].push_back(it->second); + for (map>::iterator it = top_level_category_file_locations.begin(); it != top_level_category_file_locations.end(); it++) { + top_level_category_file_locations_by_pack[it->first].push_back(it->second); } } - for (map>>::iterator it = category_to_marker_pack_names.begin(); it != category_to_marker_pack_names.end(); it++) { + for (map>>::iterator it = top_level_category_file_locations_by_pack.begin(); it != top_level_category_file_locations_by_pack.end(); it++) { if (it->second.size() != 1) { for (size_t i = 0; i < it->second.size(); i++) { for (size_t j = 0; j < it->second[i].size(); j++) { - duplicate_categories[it->first].push_back(it->second[i][j]); + duplicate_categories[it->first].insert(it->second[i][j]); } } } @@ -224,10 +224,10 @@ void process_data( cout << "Please remove one of the packs or edit the name of the packs' top level category before running the program again." << endl; cout << "If you want to bypass this stop, use '--allow-duplicates'." << endl; cout << "The following top level categories were found in more than one pack:" << endl; - for (map>::iterator it = duplicate_categories.begin(); it != duplicate_categories.end(); it++) { - cout << it->first << " in files:" << endl; + for (map>::iterator it = duplicate_categories.begin(); it != duplicate_categories.end(); it++) { + cout << " \"" << it->first << "\" in files:" << endl; for (string str : it->second) { - cout << str << endl; + cout << " " << str << endl; } } return;