Cleanup: Pass all paths to tools rather than tools constructing them (#2017)

* Pass all paths to tools rather than tools constructing them

* fix: make --baserom-segments required

* sync with mm reviews

* --version everywhere

Makefile

@@ -596,10 +596,10 @@ setup-audio:
 setup: venv
 	$(MAKE) -C tools
 	$(PYTHON) tools/decompress_baserom.py $(VERSION)
-	$(PYTHON) tools/extract_baserom.py $(BASEROM_DIR)/baserom-decompressed.z64 --oot-version $(VERSION) -o $(EXTRACTED_DIR)/baserom
+	$(PYTHON) tools/extract_baserom.py $(BASEROM_DIR)/baserom-decompressed.z64 $(EXTRACTED_DIR)/baserom -v $(VERSION)
-	$(PYTHON) tools/extract_incbins.py $(EXTRACTED_DIR)/baserom --oot-version $(VERSION) -o $(EXTRACTED_DIR)/incbin
+	$(PYTHON) tools/extract_incbins.py $(EXTRACTED_DIR)/baserom $(EXTRACTED_DIR)/incbin -v $(VERSION)
-	$(PYTHON) tools/msgdis.py $(VERSION)
+	$(PYTHON) tools/msgdis.py $(EXTRACTED_DIR)/baserom $(EXTRACTED_DIR)/text -v $(VERSION)
-	$(PYTHON) extract_assets.py -v $(VERSION) -j$(N_THREADS)
+	$(PYTHON) extract_assets.py $(EXTRACTED_DIR)/baserom $(EXTRACTED_DIR)/assets -v $(VERSION) -j$(N_THREADS)
 	$(AUDIO_EXTRACT) -o $(EXTRACTED_DIR) -v $(VERSION) --read-xml
 
 disasm:

diff_settings.py

@@ -1,5 +1,5 @@
 def add_custom_arguments(parser):
-    parser.add_argument("-v", "--oot-version", help="OOT version", default="gc-eu-mq-dbg")
+    parser.add_argument("-v", "--version", dest="oot_version", help="OOT version", default="gc-eu-mq-dbg")
 
 def apply(config, args):
     version = args.oot_version

extract_assets.py

@@ -30,7 +30,7 @@ def ExtractFile(assetConfig: version_config.AssetConfig, outputPath: Path, outpu
     outputPath.mkdir(parents=True, exist_ok=True)
     outputSourcePath.mkdir(parents=True, exist_ok=True)
 
-    execStr = f"{zapdPath} e -eh -i {xmlPath} -b extracted/{version}/baserom -o {outputPath} -osf {outputSourcePath} -gsf 1 -rconf {configPath} --cs-float both {ZAPDArgs}"
+    execStr = f"{zapdPath} e -eh -i {xmlPath} -b {globalBaseromSegmentsDir} -o {outputPath} -osf {outputSourcePath} -gsf 1 -rconf {configPath} --cs-float both {ZAPDArgs}"
 
     if name.startswith("code/") or name.startswith("overlays/"):
         assert assetConfig.start_offset is not None
@@ -63,8 +63,7 @@ def ExtractFunc(assetConfig: version_config.AssetConfig):
     xml_path = assetConfig.xml_path
     xml_path_str = str(xml_path)
 
-    version = globalVersionConfig.version
+    outPath = globalOutputDir / objectName
-    outPath = Path("extracted") / version / "assets" / objectName
     outSourcePath = outPath
 
     if xml_path_str in globalExtractedAssetsTracker:
@@ -84,17 +83,21 @@ def ExtractFunc(assetConfig: version_config.AssetConfig):
             globalExtractedAssetsTracker[xml_path_str] = globalManager.dict()
         globalExtractedAssetsTracker[xml_path_str]["timestamp"] = currentTimeStamp
 
-def initializeWorker(versionConfig: version_config.VersionConfig, abort, unaccounted: bool, extractedAssetsTracker: dict, manager):
+def initializeWorker(versionConfig: version_config.VersionConfig, abort, unaccounted: bool, extractedAssetsTracker: dict, manager, baseromSegmentsDir: Path, outputDir: Path):
     global globalVersionConfig
     global globalAbort
     global globalUnaccounted
     global globalExtractedAssetsTracker
     global globalManager
+    global globalBaseromSegmentsDir
+    global globalOutputDir
     globalVersionConfig = versionConfig
     globalAbort = abort
     globalUnaccounted = unaccounted
     globalExtractedAssetsTracker = extractedAssetsTracker
     globalManager = manager
+    globalBaseromSegmentsDir = baseromSegmentsDir
+    globalOutputDir = outputDir
 
 def processZAPDArgs(argsZ):
     badZAPDArg = False
@@ -112,7 +115,17 @@ def processZAPDArgs(argsZ):
 
 def main():
     parser = argparse.ArgumentParser(description="baserom asset extractor")
-    parser.add_argument("-v", "--oot-version", dest="oot_version", help="OOT game version", default="gc-eu-mq-dbg")
+    parser.add_argument(
+        "baserom_segments_dir",
+        type=Path,
+        help="Directory of uncompressed ROM segments",
+    )
+    parser.add_argument(
+        "output_dir",
+        type=Path,
+        help="Output directory to place files in",
+    )
+    parser.add_argument("-v", "--version", dest="oot_version", help="OOT game version", default="gc-eu-mq-dbg")
     parser.add_argument("-s", "--single", help="Extract a single asset by name, e.g. objects/gameplay_keep")
     parser.add_argument("-f", "--force", help="Force the extraction of every xml instead of checking the touched ones (overwriting current files).", action="store_true")
     parser.add_argument("-j", "--jobs", help="Number of cpu cores to extract with.")
@@ -120,7 +133,10 @@ def main():
     parser.add_argument("-Z", help="Pass the argument on to ZAPD, e.g. `-ZWunaccounted` to warn about unaccounted blocks in XMLs. Each argument should be passed separately, *without* the leading dash.", metavar="ZAPD_ARG", action="append")
     args = parser.parse_args()
 
+    baseromSegmentsDir: Path = args.baserom_segments_dir
     version: str = args.oot_version
+    outputDir: Path = args.output_dir
+
     versionConfig = version_config.load_version_config(version)
 
     global ZAPDArgs
@@ -131,7 +147,7 @@ def main():
     manager = multiprocessing.Manager()
     signal.signal(signal.SIGINT, SignalHandler)
 
-    extraction_times_p = Path("extracted") / version / "assets_extraction_times.json"
+    extraction_times_p = outputDir / "assets_extraction_times.json"
     extractedAssetsTracker = manager.dict()
     if extraction_times_p.exists() and not args.force:
         with extraction_times_p.open(encoding='utf-8') as f:
@@ -148,7 +164,7 @@ def main():
             print(f"Error. Asset {singleAssetName} not found in config.", file=os.sys.stderr)
             exit(1)
 
-        initializeWorker(versionConfig, mainAbort, args.unaccounted, extractedAssetsTracker, manager)
+        initializeWorker(versionConfig, mainAbort, args.unaccounted, extractedAssetsTracker, manager, baseromSegmentsDir, outputDir)
         # Always extract if -s is used.
         xml_path_str = str(assetConfig.xml_path)
         if xml_path_str in extractedAssetsTracker:
@@ -167,13 +183,13 @@ def main():
                 mp_context = multiprocessing.get_context("fork")
             except ValueError as e:
                 raise CannotMultiprocessError() from e
-            with mp_context.Pool(numCores, initializer=initializeWorker, initargs=(versionConfig, mainAbort, args.unaccounted, extractedAssetsTracker, manager)) as p:
+            with mp_context.Pool(numCores, initializer=initializeWorker, initargs=(versionConfig, mainAbort, args.unaccounted, extractedAssetsTracker, manager, baseromSegmentsDir, outputDir)) as p:
                 p.map(ExtractFunc, versionConfig.assets)
         except (multiprocessing.ProcessError, TypeError, CannotMultiprocessError):
             print("Warning: Multiprocessing exception ocurred.", file=os.sys.stderr)
             print("Disabling mutliprocessing.", file=os.sys.stderr)
 
-            initializeWorker(versionConfig, mainAbort, args.unaccounted, extractedAssetsTracker, manager)
+            initializeWorker(versionConfig, mainAbort, args.unaccounted, extractedAssetsTracker, manager, baseromSegmentsDir, outputDir)
             for assetConfig in versionConfig.assets:
                 ExtractFunc(assetConfig)
 

first_diff.py

@@ -30,7 +30,7 @@ def firstDiffMain():
     parser = argparse.ArgumentParser(description="Find the first difference(s) between the built ROM and the base ROM.")
 
     parser.add_argument("-c", "--count", type=int, default=5, help="find up to this many instruction difference(s)")
-    parser.add_argument("-v", "--oot-version", help="Which version should be processed", default="gc-eu-mq-dbg")
+    parser.add_argument("-v", "--version", dest="oot_version", help="Which version should be processed", default="gc-eu-mq-dbg")
     parser.add_argument("-a", "--add-colons", action='store_true', help="Add colon between bytes" )
 
     args = parser.parse_args()

retail_progress.py

@@ -387,7 +387,11 @@ if __name__ == "__main__":
         help="find functions with diffs in the given source file (if omitted, print summary of diffs for all files)",
     )
     parser.add_argument(
-        "-v", "--version", help="version to compare", default="ntsc-1.2"
+        "-v",
+        "--version",
+        dest="oot_version",
+        help="version to compare",
+        default="ntsc-1.2",
     )
     parser.add_argument(
         "--data",
@@ -405,8 +409,8 @@ if __name__ == "__main__":
 
     if args.file is not None:
         if args.data:
-            find_data_diffs(args.version, args.file)
+            find_data_diffs(args.oot_version, args.file)
         else:
-            find_functions_with_diffs(args.version, args.file)
+            find_functions_with_diffs(args.oot_version, args.file)
     else:
-        print_summary(args.version, args.csv, args.only_not_ok)
+        print_summary(args.oot_version, args.csv, args.only_not_ok)

sym_info.py

@@ -9,7 +9,7 @@ import mapfile_parser
 def symInfoMain():
     parser = argparse.ArgumentParser(description="Display various information about a symbol or address.")
     parser.add_argument("symname", help="symbol name or VROM/VRAM address to lookup")
-    parser.add_argument("-v", "--oot-version", help="Which version should be processed", default="gc-eu-mq-dbg")
+    parser.add_argument("-v", "--version", dest="oot_version", help="Which version should be processed", default="gc-eu-mq-dbg")
     parser.add_argument("-e", "--expected", dest="use_expected", action="store_true", help="use the map file in expected/build/ instead of build/")
 
     args = parser.parse_args()

tools/check_disasm_metadata_unksyms.py

@@ -35,7 +35,7 @@ def get_ldscript_syms(ldscript_p: Path):
 
 def main():
     parser = argparse.ArgumentParser()
-    parser.add_argument("--oot-version", "-v", required=True)
+    parser.add_argument("-v", "--version", dest="oot_version", required=True)
     args = parser.parse_args()
 
     version = args.oot_version

tools/disasm/sym_info.py

@@ -39,8 +39,9 @@ def main():
     parser.add_argument("sym_or_vma")
     default_version = "ntsc-1.2"
     parser.add_argument(
-        "--version",
         "-v",
+        "--version",
+        dest="oot_version",
         default=default_version,
         help=f"oot version (default: {default_version})",
     )
@@ -89,10 +90,10 @@ def main():
 
     syms_by_section_by_file = dict[str, dict[str, list[Sym]]]()
 
-    context_csv_p = Path(f"expected/build/{args.version}/context.csv")
+    context_csv_p = Path(f"expected/build/{args.oot_version}/context.csv")
     if not context_csv_p.exists():
         print(f"Context file does not exist: {context_csv_p}")
-        print(f"Hint: run `make VERSION={args.version} disasm`")
+        print(f"Hint: run `make VERSION={args.oot_version} disasm`")
         exit(1)
 
     with context_csv_p.open() as f:

tools/extract_baserom.py

@@ -21,17 +21,16 @@ def main():
         "rom", metavar="ROM", type=Path, help="Path to uncompressed ROM"
     )
     parser.add_argument(
-        "-v",
+        "output_dir",
-        "--oot-version",
+        type=Path,
-        required=True,
+        help="Output directory for segments",
-        help="OOT version",
     )
     parser.add_argument(
-        "-o",
+        "-v",
-        "--output-dir",
+        "--version",
-        type=Path,
+        dest="oot_version",
         required=True,
-        help="Output directory for segments",
+        help="OOT version",
     )
     parser.add_argument(
         "--dmadata-start",

tools/extract_incbins.py

@@ -18,21 +18,22 @@ def main():
         description="Extract incbin pieces from an uncompressed ROM."
     )
     parser.add_argument(
-        "baserom_dir", metavar="BASEROM_DIR", type=Path, help="Directory of uncompressed ROM segments"
+        "baserom_segments_dir",
+        type=Path,
+        help="Directory of uncompressed ROM segments",
+    )
+    parser.add_argument(
+        "output_dir",
+        type=Path,
+        help="Output directory for incbin pieces",
     )
     parser.add_argument(
         "-v",
-        "--oot-version",
+        "--version",
+        dest="oot_version",
         required=True,
         help="OOT version",
     )
-    parser.add_argument(
-        "-o",
-        "--output-dir",
-        type=Path,
-        required=True,
-        help="Output directory for incbin pieces",
-    )
 
     args = parser.parse_args()
 
@@ -41,7 +42,7 @@ def main():
     args.output_dir.mkdir(parents=True, exist_ok=True)
     for incbin in config.incbins:
         incbin_path = args.output_dir / incbin.name
-        with open(args.baserom_dir / incbin.segment, "rb") as f:
+        with open(args.baserom_segments_dir / incbin.segment, "rb") as f:
             offset = incbin.vram - config.dmadata_segments[incbin.segment].vram
             f.seek(offset)
             incbin_data = f.read(incbin.size)

tools/fix_bss.py

@@ -744,8 +744,9 @@ def main():
         "the current build are due to BSS ordering."
     )
     parser.add_argument(
-        "--oot-version",
         "-v",
+        "--version",
+        dest="oot_version",
         type=str,
         required=True,
         help="OOT version",

tools/ido_block_numbers.py

@@ -528,7 +528,8 @@ def main():
     parser.add_argument("filename", metavar="FILE", type=Path, help="C source file")
     parser.add_argument(
         "-v",
-        "--oot-version",
+        "--version",
+        dest="oot_version",
         type=str,
         default="gc-eu-mq-dbg",
         help="OOT version (default: gc-eu-mq-dbg)",

tools/m2ctx.py

@@ -45,7 +45,7 @@ def main():
                                      description="Creates a ctx.c file for mips2c. "
                                      "Output will be saved as oot/ctx.c")
     parser.add_argument('filepath', help="path of c file to be processed")
-    parser.add_argument("-v", "--oot-version", dest="oot_version", required=True)
+    parser.add_argument("-v", "--version", dest="oot_version", required=True)
     args = parser.parse_args()
 
     version = args.oot_version

tools/msgdis.py

@@ -4,6 +4,7 @@
 #
 
 import argparse, re, struct
+from pathlib import Path
 from typing import Callable, Dict, List, Optional, Tuple, TypeVar
 
 import version_config
@@ -189,16 +190,6 @@ def remove_comments(text : str) -> str:
 def read4(data : bytes, p : int) -> int:
     return struct.unpack(">I", data[p:p+4])[0]
 
-def read_baserom_segment(version : str, name : str) -> bytes:
-    data = None
-    with open(f"extracted/{version}/baserom/{name}", "rb") as infile:
-        data = infile.read()
-    return data
-
-def write_output_file(version : str, name : str, contents : str):
-    with open(f"extracted/{version}/text/{name}", "w") as outfile:
-        outfile.write(contents)
-
 def read_sfx_ids():
     sfx_tables = (
         (0x0000, "playerbank_table.h"),
@@ -760,7 +751,7 @@ class MessageEntry:
 
         return out
 
-def collect_messages(message_tables : List[Optional[MessageTableDesc]], version : str,
+def collect_messages(message_tables : List[Optional[MessageTableDesc]], baserom_segments_dir : Path,
                      config : version_config.VersionConfig, code_vram : int, code_bin : bytes):
 
     messages : Dict[int,MessageEntry] = {}
@@ -771,7 +762,7 @@ def collect_messages(message_tables : List[Optional[MessageTableDesc]], version
         if desc is None:
             continue
 
-        baserom_seg = read_baserom_segment(version, desc.seg_name)
+        baserom_seg = (baserom_segments_dir / desc.seg_name).read_bytes()
         code_offset = config.variables[desc.table_name] - code_vram
 
         if desc.parent is None:
@@ -829,17 +820,35 @@ def collect_messages(message_tables : List[Optional[MessageTableDesc]], version
 
 def main():
     parser = argparse.ArgumentParser(description="Extract text from the baserom into .h files")
-    parser.add_argument("version", help="OoT version")
+    parser.add_argument(
+        "baserom_segments_dir",
+        type=Path,
+        help="Directory of uncompressed ROM segments",
+    )
+    parser.add_argument(
+        "output_dir",
+        type=Path,
+        help="Output directory to place files in",
+    )
+    parser.add_argument(
+        "-v",
+        "--version",
+        dest="oot_version",
+        required=True,
+        help="OOT version",
+    )
     args = parser.parse_args()
 
-    version : str = args.version
+    baserom_segments_dir : Path = args.baserom_segments_dir
+    version : str = args.oot_version
+    output_dir : Path = args.output_dir
 
     config = version_config.load_version_config(version)
     code_vram = config.dmadata_segments["code"].vram
 
     # print(hex(code_vram))
 
-    code_bin = read_baserom_segment(version, "code")
+    code_bin = (baserom_segments_dir / "code").read_bytes()
 
     sfx_ids = read_sfx_ids()
     jpn_decoder = MessageDecoderJPN(sfx_ids)
@@ -861,8 +870,8 @@ def main():
         message_tables[3]   = None
         message_table_staff = MessageTableDesc("sStaffMessageEntryTable", "staff_message_data_static", nes_decoder, None)
 
-    messages = collect_messages(message_tables, version, config, code_vram, code_bin)
+    messages = collect_messages(message_tables, baserom_segments_dir, config, code_vram, code_bin)
-    staff_messages = collect_messages([message_table_staff], version, config, code_vram, code_bin)
+    staff_messages = collect_messages([message_table_staff], baserom_segments_dir, config, code_vram, code_bin)
 
     message_data = []
 
@@ -875,8 +884,8 @@ def main():
     message_data = "\n".join(message_data)
     message_data_staff = "\n".join(staff_messages[text_id].decode() for text_id in sorted(staff_messages.keys()))
 
-    write_output_file(version, "message_data.h", message_data)
+    (output_dir / "message_data.h").write_text(message_data)
-    write_output_file(version, "message_data_staff.h", message_data_staff)
+    (output_dir / "message_data_staff.h").write_text(message_data_staff)
 
 if __name__ == "__main__":
     main()