From ecaf8f8522c8ad19b0487f929f67744e05ef67de Mon Sep 17 00:00:00 2001 From: Roman Sandu Date: Thu, 23 Jul 2026 22:06:51 +0300 Subject: [PATCH 1/2] Add symbol-order validation tool + CI check Adds tools/validate-symbol-order.py, which validates a TU's function symbols against the original linker map (mario.MAP): * MISSING (error) - a map symbol (used or UNUSED) absent from our object * ORDER (error) - non-weak symbols in the wrong relative order; weak-only disorder is a warning (compiler-controlled) * BINDING (error) - a placed symbol whose weak/local/global linkage disagrees with the map's symbol-closure section * SIZE (warning) - an UNUSED symbol whose size differs from the map Object symbols come from `nm -S --numeric-sort` (true emission order); binding is validated against the closure section. UNUSED-symbol binding is deliberately not checked: dead-stripping removes weak/local/global alike and records no binding, so it is unknowable from the map. Exit codes: 0 pass, 1 validation failure, 2 cannot run (unresolved unit / missing object / map). tools/check-changed-symbol-order.py maps changed .cpp paths to units via objdiff.json and runs the validator on each, skipping untracked files. build.yml runs it on every changed .cpp (PR vs base branch, push vs previous commit), after the build so the objects and mario.MAP are present. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CM2qHf528zJrhyALq1CLos --- .github/workflows/build.yml | 40 +++ tools/check-changed-symbol-order.py | 90 ++++++ tools/validate-symbol-order.py | 447 ++++++++++++++++++++++++++++ 3 files changed, 577 insertions(+) create mode 100644 tools/check-changed-symbol-order.py create mode 100644 tools/validate-symbol-order.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 17be11e8a..7fa83f24e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -26,6 +26,27 @@ jobs: - name: Git config run: git config --global --add safe.directory "$GITHUB_WORKSPACE" + # Determine which .cpp files changed (PR: vs base branch; push: vs prev commit) + - name: Changed .cpp files + run: | + if [ "${{ github.event_name }}" = "pull_request" ]; then + base="${{ github.event.pull_request.base.sha }}" + else + base="${{ github.event.before }}" + fi + if [ -z "$base" ] \ + || [ "$base" = "0000000000000000000000000000000000000000" ] \ + || ! git rev-parse -q --verify "$base^{commit}" >/dev/null 2>&1; then + base="$(git rev-parse -q --verify HEAD~1 || true)" + fi + if [ -n "$base" ]; then + git diff --name-only --diff-filter=d "$base" HEAD -- '*.cpp' \ + > changed_cpp.txt + else + : > changed_cpp.txt + fi + echo "Changed .cpp files:"; cat changed_cpp.txt + # Normalize file mod times - name: Restore timestamps run: | @@ -67,3 +88,22 @@ jobs: with: name: ${{ matrix.version }}_report path: build/${{ matrix.version }}/report.json + + # Validate map symbol presence/order/linkage for each changed .cpp + - name: Validate symbol order + run: | + if [ ! -s changed_cpp.txt ]; then + echo "No changed .cpp files; nothing to validate." + exit 0 + fi + NM="$(command -v powerpc-eabi-nm || true)" + if [ -z "$NM" ]; then + NM="$(find /binutils -name powerpc-eabi-nm -type f 2>/dev/null | head -1)" + fi + if [ -z "$NM" ] || [ ! -x "$NM" ]; then + echo "::error:: powerpc-eabi-nm not found; cannot run symbol-order check." + exit 1 + fi + export NM + echo "Using nm: $NM" + python tools/check-changed-symbol-order.py $(cat changed_cpp.txt) diff --git a/tools/check-changed-symbol-order.py b/tools/check-changed-symbol-order.py new file mode 100644 index 000000000..88f09a527 --- /dev/null +++ b/tools/check-changed-symbol-order.py @@ -0,0 +1,90 @@ +#!/usr/bin/env python3 + +"""CI driver: run validate-symbol-order.py for each changed .cpp file. + +Given a list of changed paths (as arguments), map each .cpp to its decomp unit +via objdiff.json and run the symbol-order check on it. Files that aren't tracked +units -- new files, headers, SDK/JSystem sources without a built object, units +whose map TU can't be resolved -- are skipped, not failed. + +Exit status: + 0 every checked unit passed (or nothing to check) + 1 at least one checked unit failed its validation + +Usage: + python tools/check-changed-symbol-order.py src/Foo/Bar.cpp src/Baz/Qux.cpp + git diff --name-only BASE HEAD -- '*.cpp' | xargs python tools/check-changed-symbol-order.py +""" + +import json +import os +import subprocess +import sys + +script_dir = os.path.dirname(os.path.realpath(__file__)) +root_dir = os.path.abspath(os.path.join(script_dir, "..")) +VALIDATOR = os.path.join(script_dir, "validate-symbol-order.py") +OBJDIFF_JSON = os.path.join(root_dir, "objdiff.json") + + +def norm(p: str) -> str: + return p.replace("\\", "/").lstrip("./") + + +def source_to_unit() -> dict: + with open(OBJDIFF_JSON, encoding="utf-8") as f: + units = json.load(f).get("units", []) + out = {} + for u in units: + sp = u.get("metadata", {}).get("source_path") + if sp: + out[norm(sp)] = (u["name"], u.get("base_path", "")) + return out + + +def main(argv) -> int: + files = [norm(a) for a in argv if a.strip()] + cpp = [f for f in files if f.endswith(".cpp")] + if not cpp: + print("No changed .cpp files to check.") + return 0 + + mapping = source_to_unit() + passed, failed, skipped = [], [], [] + + for f in cpp: + entry = mapping.get(f) + if entry is None: + print(f"skip {f} (not a tracked decomp unit)") + skipped.append(f) + continue + unit, base = entry + if base and not os.path.exists(os.path.join(root_dir, base)): + print(f"skip {f} ({unit}: object not built)") + skipped.append(f) + continue + + print(f"\n{'=' * 72}\n{f} -> {unit}\n{'=' * 72}") + rc = subprocess.run([sys.executable, VALIDATOR, "-u", unit], + cwd=root_dir).returncode + if rc == 0: + passed.append(f) + elif rc == 2: + print(f"skip {f} ({unit}: check could not run -- see message above)") + skipped.append(f) + else: + failed.append((f, unit)) + + print(f"\n{'=' * 72}") + print(f"Symbol-order check: {len(passed)} passed, {len(failed)} failed, " + f"{len(skipped)} skipped.") + if failed: + print("\nFailed:") + for f, unit in failed: + print(f" - {f} ({unit})") + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/tools/validate-symbol-order.py b/tools/validate-symbol-order.py new file mode 100644 index 000000000..cd14d796a --- /dev/null +++ b/tools/validate-symbol-order.py @@ -0,0 +1,447 @@ +#!/usr/bin/env python3 + +""" +Validate that a translation unit's function symbol order in our compiled object +file matches the order recorded in the original linker map (mario.MAP). + +The linker map's ".text section layout" is the ground truth: it lists every +function a TU contributed, in emission order, *including* deadstripped ones +(marked UNUSED). A correct decompilation must reproduce that exact order, must +not be missing any of those symbols, and must emit the UNUSED ones at the size +the map records for them (UNUSED symbols have no counterpart in the target +object, so objdiff can't check them -- the map size is the only signal we have). + +What is checked: + * MISSING (ERROR) -- a map symbol (used or UNUSED) absent from our object. + Nothing stops you from just defining the function. + * ORDER (ERROR) -- non-weak map symbols present in our object but in the + wrong relative order (extra object-only symbols + ignored). Reordering the definitions to match is + mechanical. A disorder involving ONLY weak symbols is a + WARNING instead (compiler-controlled, won't block a PR). + * BINDING (ERROR) -- a *placed* (linked) symbol whose weak/local/global + linkage disagrees with the map's symbol-closure section + (e.g. we defined an out-of-line global that should be a + weak header inline, or dropped a `static`). UNUSED + symbols are skipped: dead-stripping removes weak, local + and global symbols alike and records no binding, so the + linkage of an UNUSED symbol is unknowable from the map. + * SIZE (WARNING) -- an UNUSED map symbol whose size in our object differs + from the size the map records. Reproducing an inlined + body's exact size is genuinely hard, so this only warns. + +Exit status is non-zero only for ERRORs (MISSING/ORDER/BINDING). SIZE warnings +on their own still exit 0. + +Note on "-inline deferred" / reverse_fn_order: empirically the map and our +object are ALWAYS in the same forward order (verified across every unit), so +comparison is forward by default. `--reverse` is provided as a manual escape +hatch should a genuinely reversed unit ever turn up. + +Usage: + python tools/validate-symbol-order.py -u mario/MarioUtil/MathUtil + python tools/validate-symbol-order.py -u mario/Enemy/areacylinder --map orig/GMSE01/files/marioUS.MAP +""" + +import argparse +import json +import os +import re +import subprocess +import sys +from typing import Dict, List, Optional, Tuple + +script_dir = os.path.dirname(os.path.realpath(__file__)) +root_dir = os.path.abspath(os.path.join(script_dir, "..")) + +DEFAULT_MAP = os.path.join(root_dir, "orig", "GMSJ01", "files", "mario.MAP") +NM = os.environ.get("NM", os.path.join(root_dir, "build", "binutils", "powerpc-eabi-nm.exe")) +OBJDIFF_JSON = os.path.join(root_dir, "objdiff.json") + + +def die(msg: str) -> None: + """Exit code 2 == cannot run the check (unresolved unit, missing object or + map, absent nm). Distinct from a genuine validation failure (exit 1), so a + CI driver can tell 'skip this file' apart from 'this file is wrong'.""" + print(msg, file=sys.stderr) + sys.exit(2) + + +class MapSym: + __slots__ = ("name", "size", "unused") + + def __init__(self, name: str, size: int, unused: bool): + self.name = name + self.size = size + self.unused = unused + + +def load_units() -> List[Dict]: + with open(OBJDIFF_JSON, encoding="utf-8") as f: + return json.load(f).get("units", []) + + +def resolve_unit(units: List[Dict], arg: str) -> Dict: + """Resolve a unit by exact name, then by suffix, matching decomp-diff -u.""" + for u in units: + if u.get("name") == arg: + return u + matches = [u for u in units if u.get("name", "").endswith(arg)] + if len(matches) == 1: + return matches[0] + if len(matches) > 1: + names = "\n ".join(m["name"] for m in matches) + die(f"Ambiguous unit '{arg}', candidates:\n {names}") + die(f"Unit not found in objdiff.json: {arg}") + + +# --------------------------------------------------------------------------- +# Map parsing +# --------------------------------------------------------------------------- + +# UNUSED line: " UNUSED 00005c ........ GetAtanTable__Fff MarioUtil.a MathUtil.cpp" +_RE_UNUSED = re.compile(r"^\s*UNUSED\s+([0-9a-fA-F]+)\s+\.+\s+(\S+)\s+(.+?)\s*$") +# Placed line: " 000c01d0 000028 800c57d0 4 MsVECMag2__FP3Vec \t MarioUtil.a MathUtil.cpp" +_RE_PLACED = re.compile( + r"^\s*([0-9a-fA-F]+)\s+([0-9a-fA-F]+)\s+([0-9a-fA-F]+)\s+(\d+)\s+(\S+)\s+(.+?)\s*$" +) + + +def parse_text_layout(map_path: str) -> Dict[str, List[MapSym]]: + """Return {tu_id: [MapSym,...]} for the .text section layout (in map order).""" + if not os.path.exists(map_path): + die(f"Map file not found: {map_path}") + with open(map_path, encoding="utf-8", errors="replace") as f: + lines = f.read().splitlines() + + start = None + for i, l in enumerate(lines): + if l.strip() == ".text section layout": + start = i + 1 + break + if start is None: + die(f"No '.text section layout' section in {map_path}") + end = len(lines) + for i in range(start, len(lines)): + if lines[i].strip().endswith("section layout"): + end = i + break + + tus: Dict[str, List[MapSym]] = {} + for l in lines[start:end]: + if not l.strip(): + continue + m = _RE_UNUSED.match(l) + if m: + tus.setdefault(m.group(3).strip(), []).append( + MapSym(m.group(2), int(m.group(1), 16), True) + ) + continue + m = _RE_PLACED.match(l) + if m: + name = m.group(5) + if name == ".text": # per-TU section header, not a symbol + continue + tus.setdefault(m.group(6).strip(), []).append( + MapSym(name, int(m.group(2), 16), False) + ) + return tus + + +def find_map_tu(tus: Dict[str, List[MapSym]], source_path: str, + override: Optional[str]) -> str: + if override: + if override not in tus: + die(f"--map-tu '{override}' not found in .text layout") + return override + base = os.path.basename(source_path) # e.g. MathUtil.cpp + stem = os.path.splitext(base)[0] # e.g. MathUtil + cands = [ + t for t in tus + if t.endswith(" " + base) or t == base or t == stem + ".o" + ] + if len(cands) == 1: + return cands[0] + if not cands: + die( + f"Could not find a .text-layout TU for source '{source_path}'.\n" + f"Pass --map-tu with the exact map identifier (e.g. 'MarioUtil.a {base}')." + ) + listed = "\n ".join(cands) + die( + f"Multiple map TUs match '{base}'. Disambiguate with --map-tu:\n {listed}" + ) + + +# --------------------------------------------------------------------------- +# Object file parsing (our compiled code) -- true emission order via nm +# --------------------------------------------------------------------------- + +# nm -S --numeric-sort: "00000000 00000028 T MsVECMag2__FP3Vec" +_RE_NM = re.compile(r"^([0-9a-fA-F]+)\s+([0-9a-fA-F]+)\s+([a-zA-Z])\s+(\S.*)$") + + +# nm type letter -> linkage/binding, matching the map's (func,) tags. +_NM_BIND = {"T": "global", "t": "local", "W": "weak", "w": "weak"} + + +def obj_functions(obj_path: str) -> List[Tuple[str, int, str]]: + """[(name, size, binding), ...] for .text symbols in emission order. + + binding is 'global' | 'local' | 'weak', derived from the nm type letter. + """ + if not os.path.exists(obj_path): + die( + f"Object not found: {obj_path}\n" + f"Build it first (e.g. `ninja {obj_path}`)." + ) + if not os.path.exists(NM): + die(f"nm not found: {NM} (set $NM to override)") + out = subprocess.run( + [NM, "-S", "--numeric-sort", "--defined-only", obj_path], + capture_output=True, text=True, + ) + if out.returncode != 0: + die(f"nm failed: {out.stderr.strip()}") + syms: List[Tuple[str, int, str]] = [] + for line in out.stdout.splitlines(): + m = _RE_NM.match(line) + if not m: + continue + size, typ, name = int(m.group(2), 16), m.group(3), m.group(4) + # t/T = text (local/global); w/W = weak (functions & thunks live here). + # Weak *objects* are v/V, data is d/D/b/B/r/R -> excluded. + if typ in _NM_BIND: + syms.append((name, size, _NM_BIND[typ])) + return syms + + +# --------------------------------------------------------------------------- +# Linker-map symbol closure -- the authority on weak/global/local binding. +# Only *linked* symbols appear here; deadstripped (UNUSED) ones never do, which +# is exactly why UNUSED symbols are non-weak by construction. +# --------------------------------------------------------------------------- + +# " 3] __dt__26__partial_array_destructorFv (func,weak) found in Runtime..." +_RE_CLOSURE = re.compile( + r"^\s*(?:\d+\]\s*)?(\S+)\s+\((\w+),(\w+)\)\s+found in\s+(.+?)\s*$" +) + + +def parse_closure(map_path: str) -> Tuple[Dict[Tuple[str, str], str], Dict[str, str]]: + """Return (binding_by_tu_name, binding_by_name) for func symbols. + + binding_by_name maps a name to its binding only when unambiguous across + every TU that defines it (so weak/global resolve cleanly; a name that is + local in several TUs still collapses to 'local'). + """ + with open(map_path, encoding="utf-8", errors="replace") as f: + lines = f.read().splitlines() + + by_tu: Dict[Tuple[str, str], str] = {} + seen: Dict[str, set] = {} + for l in lines: + if l.strip().endswith("section layout"): + break # closure precedes all the section layouts + m = _RE_CLOSURE.match(l) + if not m or m.group(2) != "func": + continue + name, binding, tu = m.group(1), m.group(3), m.group(4).strip() + by_tu[(tu, name)] = binding + seen.setdefault(name, set()).add(binding) + + by_name = {n: next(iter(b)) for n, b in seen.items() if len(b) == 1} + return by_tu, by_name + + +# --------------------------------------------------------------------------- +# Comparison + reporting +# --------------------------------------------------------------------------- + +def order_diff(expected: List[str], actual: List[str]) -> List[str]: + """Readable unified-style diff of two orderings of the same symbol set.""" + import difflib + + sm = difflib.SequenceMatcher(a=expected, b=actual, autojunk=False) + out: List[str] = [] + for tag, i1, i2, j1, j2 in sm.get_opcodes(): + if tag == "equal": + for n in expected[i1:i2]: + out.append(f" {n}") + else: + for n in expected[i1:i2]: + out.append(f" - {n} (map position)") + for n in actual[j1:j2]: + out.append(f" + {n} (object position)") + return out + + +def main() -> None: + ap = argparse.ArgumentParser( + description="Validate TU function symbol order against the linker map." + ) + ap.add_argument("-u", "--unit", required=True, + help="objdiff unit name, e.g. mario/MarioUtil/MathUtil") + ap.add_argument("--map", default=DEFAULT_MAP, + help=f"path to the linker map (default: {DEFAULT_MAP})") + ap.add_argument("--map-tu", + help="override the map .text-layout TU identifier") + ap.add_argument("--reverse", action="store_true", + help="compare against the reversed object order (escape hatch; " + "no known unit needs this)") + ap.add_argument("-v", "--verbose", action="store_true", + help="list the object-only symbols not in the map " + "(by default only their count is shown)") + args = ap.parse_args() + + unit = resolve_unit(load_units(), args.unit) + meta = unit.get("metadata", {}) + source_path = meta.get("source_path", "") + base_path = os.path.join(root_dir, unit["base_path"]) + reverse_fn = bool(meta.get("reverse_fn_order")) + + tus = parse_text_layout(args.map) + tu_id = find_map_tu(tus, source_path, args.map_tu) + map_syms = tus[tu_id] + bind_by_tu, bind_by_name = parse_closure(args.map) + + def map_binding(name: str) -> Optional[str]: + return bind_by_tu.get((tu_id, name)) or bind_by_name.get(name) + + obj_syms = obj_functions(base_path) + if args.reverse: + obj_syms = list(reversed(obj_syms)) + + map_names = [s.name for s in map_syms] + map_set = set(map_names) + map_size = {s.name: s.size for s in map_syms} + unused_names = {s.name for s in map_syms if s.unused} + + obj_names = [n for n, _, _ in obj_syms] + obj_set = set(obj_names) + obj_size = {n: sz for n, sz, _ in obj_syms} + obj_bind = {n: b for n, _, b in obj_syms} + + print(f"Unit : {unit['name']}") + print(f"Source : {source_path}") + print(f"Object : {os.path.relpath(base_path, root_dir)}") + print(f"Map TU : {tu_id}") + print(f"Map symbols : {len(map_names)} ({len(unused_names)} UNUSED) " + f"Object .text symbols: {len(obj_names)}") + print(f"reverse_fn_order: {reverse_fn}" + + (" [comparing reversed]" if args.reverse else "")) + print("-" * 78) + + failures = 0 + + # --- MISSING ---------------------------------------------------------- + missing = [s for s in map_syms if s.name not in obj_set] + if missing: + failures += 1 + print(f"\n[FAIL] {len(missing)} map symbol(s) MISSING from the object:") + for s in missing: + tag = " (UNUSED)" if s.unused else "" + print(f" - {s.name}{tag}") + else: + print("\n[ OK ] All map symbols are present in the object.") + + # --- ORDER (over the intersection) ------------------------------------ + # Non-weak ordering is an error. Weak-symbol ordering is compiler-controlled + # and painful to steer, so a disorder that only involves weak symbols is a + # warning that does not fail the check (and won't block a PR). + present_map = [n for n in map_names if n in obj_set] + obj_in_map = [n for n in obj_names if n in map_set] + nonweak = lambda n: obj_bind.get(n) != "weak" + present_map_nw = [n for n in present_map if nonweak(n)] + obj_in_map_nw = [n for n in obj_in_map if nonweak(n)] + order_weak_warn = False + if present_map_nw != obj_in_map_nw: + failures += 1 + print("\n[FAIL] Non-weak symbol ORDER differs from the map " + "(weak & object-only symbols ignored):") + for line in order_diff(present_map_nw, obj_in_map_nw): + print(line) + elif present_map != obj_in_map: + order_weak_warn = True + print("\n[WARN] Only weak symbols are out of order " + "(compiler-controlled -- does not fail the check):") + for line in order_diff(present_map, obj_in_map): + print(line) + else: + print("[ OK ] Symbol order matches the map.") + + # --- SIZE of UNUSED symbols ------------------------------------------ + size_bad = [ + s for s in map_syms + if s.unused and s.name in obj_size and obj_size[s.name] != s.size + ] + if size_bad: + # Warning only: matching an inlined body's exact size is hard, and it + # doesn't affect correctness of what does get emitted. Does not fail. + print(f"\n[WARN] {len(size_bad)} UNUSED symbol(s) with wrong size " + f"(best-effort -- does not fail the check):") + for s in size_bad: + print(f" - {s.name}") + print(f" map: 0x{s.size:x} ({s.size})") + print(f" object: 0x{obj_size[s.name]:x} ({obj_size[s.name]})") + elif unused_names: + checked = sum(1 for n in unused_names if n in obj_size) + print(f"[ OK ] UNUSED symbol sizes match ({checked}/{len(unused_names)} checked).") + + # --- BINDING (weak / local / global) --------------------------------- + # Only *placed* (linked) symbols can be checked: the closure records their + # exact binding. UNUSED symbols are deadstripped, so they are absent from + # the closure and the .text layout records no binding for them -- their + # weak/local/global linkage is simply unknowable from the map (a weak symbol + # referenced only by dead code is dead-stripped and listed UNUSED just like + # a local or global one), so we cannot and do not check them. + bind_bad = [] # (name, expected, actual) + for s in map_syms: + if s.unused: + continue + actual = obj_bind.get(s.name) + if actual is None: + continue # missing -> already reported + expected = map_binding(s.name) + if expected is not None and expected != actual: + bind_bad.append((s.name, expected, actual)) + if bind_bad: + failures += 1 + print(f"\n[FAIL] {len(bind_bad)} symbol(s) with wrong linkage " + "(weak/local/global):") + for name, exp, act in bind_bad: + print(f" - {name}") + print(f" map: {exp}") + print(f" object: {act}") + else: + print("[ OK ] Symbol linkage (weak/local/global) matches the map.") + + # --- extras (informational) ------------------------------------------ + extras = [n for n in obj_names if n not in map_set] + if extras: + print(f"\n[INFO] {len(extras)} object-only symbol(s) not in the map " + "(usually deduplicated weak/inline helpers; not an error" + + ("; -v to list)." if not args.verbose else "):")) + if args.verbose: + for n in extras: + print(f" {n}") + + warns = [] + if order_weak_warn: + warns.append("weak-symbol order") + if size_bad: + warns.append(f"{len(size_bad)} UNUSED size mismatch(es)") + + print("-" * 78) + if failures: + tail = f", {'; '.join(warns)} warning(s)" if warns else "" + print(f"RESULT: FAIL ({failures} error category/categories{tail})") + sys.exit(1) + if warns: + print(f"RESULT: PASS with warnings ({'; '.join(warns)} -- see above)") + else: + print("RESULT: PASS") + + +if __name__ == "__main__": + main() From 591687169abc62258961fbd9d05c81237c52cc4c Mon Sep 17 00:00:00 2001 From: Roman Sandu Date: Thu, 23 Jul 2026 22:07:02 +0300 Subject: [PATCH 2/2] Fix symbol presence/order/linkage in 11 TUs Fixes surfaced by the new symbol-order check, so each of these TUs now matches the linker map (function set, non-weak order, and used-symbol linkage; UNUSED sizes left as-is): * spline, Resolution, StageUtil, ProcessMeter - added missing UNUSED functions that were declared but never defined * RumbleType, MSoundDebug, MSoundScene, MSModBgm, MarioCap, riccohook - reordered function definitions to match the map's emission order * GDUtil - marked TGDLStaticOverFlow static (was global, should be local) Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CM2qHf528zJrhyALq1CLos --- src/Enemy/riccohook.cpp | 48 ++++++++++++++++++------------------ src/Enemy/spline.cpp | 3 +++ src/MSound/MSModBgm.cpp | 48 ++++++++++++++++++------------------ src/MSound/MSoundDebug.cpp | 5 ++-- src/MSound/MSoundScene.cpp | 4 +-- src/MarioUtil/GDUtil.cpp | 2 +- src/MarioUtil/RumbleType.cpp | 21 ++++++++-------- src/Player/MarioCap.cpp | 15 +++++------ src/System/ProcessMeter.cpp | 3 +++ src/System/Resolution.cpp | 6 +++++ src/System/StageUtil.cpp | 3 +++ 11 files changed, 88 insertions(+), 70 deletions(-) diff --git a/src/Enemy/riccohook.cpp b/src/Enemy/riccohook.cpp index efa6e7717..bb0d63dbc 100644 --- a/src/Enemy/riccohook.cpp +++ b/src/Enemy/riccohook.cpp @@ -4,6 +4,23 @@ #include #include +// @non-matching -- the issue seems to stem from the JDrama TNameRefGen +// search/push_back calls. +THookTake::THookTake(TRiccoHook* owner, const char* name) + : TTakeActor(name) + , mOwner(owner) +{ + initHitActor(0x400000BB, 1, -0x80000000, + mOwner->getSaveLoadParam()->mSLHitRadius.get(), + mOwner->getSaveLoadParam()->mSLHitHeight.get(), + mOwner->getSaveLoadParam()->mSLHitRadius.get(), + mOwner->getSaveLoadParam()->mSLHitHeight.get()); + + JDrama::TNameRefGen::search("オブジェクトグループ") + ->getChildren() + .push_back(this); +} + MtxPtr THookTake::getTakingMtx() { return nullptr; } f32 THookTake::getRadiusAtY(f32 y) const @@ -42,21 +59,14 @@ void THookTake::perform(u32 cue, JDrama::TGraphics* graphics) } } -// @non-matching -- the issue seems to stem from the JDrama TNameRefGen -// search/push_back calls. -THookTake::THookTake(TRiccoHook* owner, const char* name) - : TTakeActor(name) - , mOwner(owner) +THookParams::THookParams(const char* path) + : TSpineEnemyParams(path) + , PARAM_INIT(mSLHitHeight, 900.0f) + , PARAM_INIT(mSLHitRadius, 120.0f) + , PARAM_INIT(mSLHangRadius, 30.0f) + , PARAM_INIT(mSLMoveSpeed, 4.0f) { - initHitActor(0x400000BB, 1, -0x80000000, - mOwner->getSaveLoadParam()->mSLHitRadius.get(), - mOwner->getSaveLoadParam()->mSLHitHeight.get(), - mOwner->getSaveLoadParam()->mSLHitRadius.get(), - mOwner->getSaveLoadParam()->mSLHitHeight.get()); - - JDrama::TNameRefGen::search("オブジェクトグループ") - ->getChildren() - .push_back(this); + TParams::load(mPrmPath); } TRiccoHook::TRiccoHook(const char* name) @@ -96,16 +106,6 @@ void TRiccoHook::perform(u32 cue, JDrama::TGraphics* graphics) } } -THookParams::THookParams(const char* path) - : TSpineEnemyParams(path) - , PARAM_INIT(mSLHitHeight, 900.0f) - , PARAM_INIT(mSLHitRadius, 120.0f) - , PARAM_INIT(mSLHangRadius, 30.0f) - , PARAM_INIT(mSLMoveSpeed, 4.0f) -{ - TParams::load(mPrmPath); -} - TRiccoHookManager::TRiccoHookManager(const char* name) : TEnemyManager(name) { diff --git a/src/Enemy/spline.cpp b/src/Enemy/spline.cpp index 080d9dca2..e921d420d 100644 --- a/src/Enemy/spline.cpp +++ b/src/Enemy/spline.cpp @@ -20,6 +20,9 @@ TSplinePath::TSplinePath(int param_1) mXs[i] = mYs[i] = mZs[i] = i; } +// BUG: UNUSED symbol size tells us they even forgot to delete the arrays -_- +TSplinePath::~TSplinePath() { } + void TSplinePath::makeTable(f32* parametrization, f32* coords, f32* param_3) { param_3[0] = 0.0f; diff --git a/src/MSound/MSModBgm.cpp b/src/MSound/MSModBgm.cpp index 182942f8b..840306e77 100644 --- a/src/MSound/MSModBgm.cpp +++ b/src/MSound/MSModBgm.cpp @@ -48,6 +48,21 @@ JAISound* MSModBgm::modBgm(u8 param_1, u8 param_2) return sound; } +void MSModBgm::loop() +{ + switch (unk0) { + case true: + unk4 += 1; + break; + + case false: + default: + unk4 = 0; + return; + } + unk0 = 0; +} + void MSModBgm::changeTempo(u8 param_1, u8 param_2) { JAISound* sound = MSBgm::getHandle(param_2); @@ -77,21 +92,6 @@ void MSModBgm::changeTempo(u8 param_1, u8 param_2) } } -void MSModBgm::loop() -{ - switch (unk0) { - case true: - unk4 += 1; - break; - - case false: - default: - unk4 = 0; - return; - } - unk0 = 0; -} - f32 MSBgmXFade::scTiming[18] = { 0.052632f, 0.105263f, 0.157895f, 0.210526f, 0.26315799f, 0.315789f, 0.36842099f, 0.42105299f, 0.473684f, 0.52631599f, 0.578947f, 0.63157898f, @@ -126,15 +126,6 @@ void MSBgmXFade::xFadeBgmForce(f32 param_1) } } -u8 MSBgmXFade::getTimingForce(f32 param_1) -{ - for (u8 i = 0; i < 17; ++i) - if (param_1 >= scTiming[i] && param_1 < scTiming[i + 1]) - return i; - - return 0xff; -} - u8 MSBgmXFade::getTiming(f32 param_1, u32* param_2) { f32 f1 = unk0; @@ -147,3 +138,12 @@ u8 MSBgmXFade::getTiming(f32 param_1, u32* param_2) return 0xff; } + +u8 MSBgmXFade::getTimingForce(f32 param_1) +{ + for (u8 i = 0; i < 17; ++i) + if (param_1 >= scTiming[i] && param_1 < scTiming[i + 1]) + return i; + + return 0xff; +} diff --git a/src/MSound/MSoundDebug.cpp b/src/MSound/MSoundDebug.cpp index e280c29a3..e29094929 100644 --- a/src/MSound/MSoundDebug.cpp +++ b/src/MSound/MSoundDebug.cpp @@ -6,8 +6,6 @@ static void dummy() { JADPrm prm(0, ""); } -MSD* MSD::get() { return 0; } - MSPrintBase::MSPrintBase() { } MSPrintBase::MSPrintBase(u8 param1, u8 param2, u8 param3, u8 param4, u8 param5) @@ -25,3 +23,6 @@ void MSPrintEntry::print() { } MSPrint::MSPrint() { } void MSPrint::print() { } + +// UNUSED +MSD* MSD::get() { return 0; } diff --git a/src/MSound/MSoundScene.cpp b/src/MSound/MSoundScene.cpp index cf17ae2a1..98973f98f 100644 --- a/src/MSound/MSoundScene.cpp +++ b/src/MSound/MSoundScene.cpp @@ -108,8 +108,8 @@ void MSSceneSE::sortMaxTrans(Vec* param_1, u8 param_2, u8 param_3) } } -void MSSceneSE::calcPosPanSR(Vec* param_1, f32 param_2) { } +void MSSceneSE::calcPosVolume(Vec* param_1, f32* param_2, u8 param_3) { } void MSSceneSE::calcPosPanLR(Vec* param_1, f32 param_2) { } -void MSSceneSE::calcPosVolume(Vec* param_1, f32* param_2, u8 param_3) { } +void MSSceneSE::calcPosPanSR(Vec* param_1, f32 param_2) { } diff --git a/src/MarioUtil/GDUtil.cpp b/src/MarioUtil/GDUtil.cpp index efdde84b7..e0a239f2f 100644 --- a/src/MarioUtil/GDUtil.cpp +++ b/src/MarioUtil/GDUtil.cpp @@ -4,7 +4,7 @@ static TGDLStatic* currentTGDLStatic; -void TGDLStaticOverFlow() { currentTGDLStatic->unk11 = 1; } +static void TGDLStaticOverFlow() { currentTGDLStatic->unk11 = 1; } TGDLStatic::TGDLSentinel::~TGDLSentinel() { diff --git a/src/MarioUtil/RumbleType.cpp b/src/MarioUtil/RumbleType.cpp index 1b5800511..fa549a2fd 100644 --- a/src/MarioUtil/RumbleType.cpp +++ b/src/MarioUtil/RumbleType.cpp @@ -39,16 +39,7 @@ struct RumbleTypeInfo _info[23] = { const char* invalidStr = "Invalid RumbleType"; -int RumbleType::getIndex(char* strIn) -{ - for (int i = 0; i < channelNum; i++) { - if (!strcmp(strIn, _info[i].str)) { - return _info[i].unk0; - } - } - return -1; -} - +// UNUSED // Size needed: 0x54, current: 0x34 const char* RumbleType::getName(int index) { @@ -58,3 +49,13 @@ const char* RumbleType::getName(int index) return _info[index].str; } + +int RumbleType::getIndex(char* strIn) +{ + for (int i = 0; i < channelNum; i++) { + if (!strcmp(strIn, _info[i].str)) { + return _info[i].unk0; + } + } + return -1; +} diff --git a/src/Player/MarioCap.cpp b/src/Player/MarioCap.cpp index bac030a3f..2b2153779 100644 --- a/src/Player/MarioCap.cpp +++ b/src/Player/MarioCap.cpp @@ -118,6 +118,9 @@ void TMarioCap::createMirrorCap() } } +// UNUSED +void TMarioCap::addDirty() { } + void TMarioCap::perform(unsigned long param_1, JDrama::TGraphics* param_2) { // Unused stack space @@ -227,16 +230,14 @@ void TMarioCap::perform(unsigned long param_1, JDrama::TGraphics* param_2) } } -void TMarioCap::mtxEffectHide() -{ - unk20->flagOff(0x1); - unk24->flagOff(0x1); -} - void TMarioCap::mtxEffectShow() { unk20->flagOn(0x1); unk24->flagOn(0x1); } -void TMarioCap::addDirty() { } +void TMarioCap::mtxEffectHide() +{ + unk20->flagOff(0x1); + unk24->flagOff(0x1); +} diff --git a/src/System/ProcessMeter.cpp b/src/System/ProcessMeter.cpp index c19d7e90e..e932987ab 100644 --- a/src/System/ProcessMeter.cpp +++ b/src/System/ProcessMeter.cpp @@ -12,3 +12,6 @@ TProcessMeter::TProcessMeter(s32 r4) _01C = 0; _01E = 0; } + +// UNUSED +void TProcessMeter::draw(const JUTRect&) { } diff --git a/src/System/Resolution.cpp b/src/System/Resolution.cpp index 4f8b38af6..59abed7bd 100644 --- a/src/System/Resolution.cpp +++ b/src/System/Resolution.cpp @@ -21,6 +21,12 @@ u16 SMSGetGameVideoHeight() return ret; } +// UNUSED +s32 SMSGetDisplayWidthMax() { return 720; } + +// UNUSED +s32 SMSGetDisplayHeightMax() { return 574; } + u16 SMSGetTitleVideoWidth() { return 660; } u16 SMSGetTitleVideoHeight() { return SMSGetGameVideoHeight(); } diff --git a/src/System/StageUtil.cpp b/src/System/StageUtil.cpp index fa14f1142..a937ef628 100644 --- a/src/System/StageUtil.cpp +++ b/src/System/StageUtil.cpp @@ -23,6 +23,9 @@ static u8 exShineTable[] = { // clang-format on +// UNUSED +bool SMS_isExMap(u8 stage) { return stage > 0x14 && stage < 0x35; } + bool SMS_isExMap() { return (gpApplication.mCurrArea.unk0 > 0x14