offline-tests: fail on comparison in offline-tests (#2095)

Two main problems fixed here:
- offline tests will fail on a comparison failure (a mistake from the
re-write)
- art-group-info is committed to the repo and shared with every thread
(running the tests with 1 thread, for example on the CI, and locally
were producing different results)

art files are still not provided to the jak2 offline tests:
- `*-ag` files are not being output
- `art-elts.gc` is not complete, as a handful of files claim to be
missing stuff

lastly, in jak1's offline tests we were also running `tpage` and `*-vis`
files through the decompiler. This omits that (they came from the
`all_objs.json` file) -- is this an issue?

.vs/launch.vs.json

@@ -79,7 +79,7 @@
         "--game",
         "jak2",
         "--file",
-        "nav-enemy"
+        "atoll-part"
       ]
     },
     {

Taskfile.yml

@@ -125,13 +125,10 @@ tasks:
   # TESTS
   offline-tests: # ran by jenkins
     cmds:
-      - '{{.OFFLINETEST_BIN_RELEASE_DIR}}/offline-test --iso_data_path "./iso_data/{{.GAME}}" --game {{.GAME}}'
+      - '{{.OFFLINETEST_BIN_RELEASE_DIR}}/offline-test --iso_data_path "./iso_data/{{.GAME}}" --game {{.GAME}} --fail-on-cmp'
   offline-tests-fast:
     cmds:
       - '{{.OFFLINETEST_BIN_RELEASE_DIR}}/offline-test --iso_data_path "./iso_data/{{.GAME}}" --game {{.GAME}} --pretty-print --num_threads 32 --dump_current_output --fail-on-cmp'
-  offline-test-file:
-    cmds:
-      - '{{.OFFLINETEST_BIN_RELEASE_DIR}}/offline-test --iso_data_path "./iso_data/{{.GAME}}" --game {{.GAME}} --file {{.FILE}}'
   # TODO - amalgamate offline-tests and this task, run twice if the previous step fails
   update-ref-tests:
     cmds:

decompiler/config.cpp

@@ -64,6 +64,7 @@ Config read_config_file(const fs::path& path_to_config_file, const std::string&
   config.process_game_text = cfg.at("process_game_text").get<bool>();
   config.process_game_count = cfg.at("process_game_count").get<bool>();
   config.process_art_groups = cfg.at("process_art_groups").get<bool>();
+  config.dump_art_group_info = cfg.at("dump_art_group_info").get<bool>();
   config.hexdump_code = cfg.at("hexdump_code").get<bool>();
   config.hexdump_data = cfg.at("hexdump_data").get<bool>();
   config.find_functions = cfg.at("find_functions").get<bool>();

decompiler/config.h

@@ -112,6 +112,7 @@ struct Config {
   bool process_game_text = false;
   bool process_game_count = false;
   bool process_art_groups = false;
+  bool dump_art_group_info = false;
   bool rip_levels = false;
   bool extract_collision = false;
   bool find_functions = false;

decompiler/config/jak1_jp.jsonc

@@ -40,6 +40,8 @@
   "process_game_count": true,
   // write goal imports for art groups
   "process_art_groups": true,
+  // write out a json file containing the art info mapping, run this with all objects allowed
+  "dump_art_group_info": false,
 
   ///////////////////////////
   // WEIRD OPTIONS

decompiler/config/jak1_ntsc_black_label.jsonc

@@ -40,6 +40,8 @@
   "process_game_count": true,
   // write goal imports for art groups
   "process_art_groups": true,
+  // write out a json file containing the art info mapping, run this with all objects allowed
+  "dump_art_group_info": false,
 
   ///////////////////////////
   // WEIRD OPTIONS

decompiler/config/jak1_pal.jsonc

@@ -40,6 +40,8 @@
   "process_game_count": true,
   // write goal imports for art groups
   "process_art_groups": true,
+  // write out a json file containing the art info mapping, run this with all objects allowed
+  "dump_art_group_info": false,
 
   ///////////////////////////
   // WEIRD OPTIONS

decompiler/config/jak1_sced.jsonc

@@ -1,3 +1,4 @@
+// TODO - this config file doesn't seem to match the others (missing many fields)
 {
   "game_version": 1,
 
@@ -32,6 +33,8 @@
   "process_game_text": false,
   // unpack game count to assets folder
   "process_game_count": false,
+  // write out a json file containing the art info mapping, run this with all objects allowed
+  "dump_art_group_info": false,
 
   ///////////////////////////
   // WEIRD OPTIONS

decompiler/config/jak1_us2.jsonc

@@ -40,6 +40,8 @@
   "process_game_count": true,
   // write goal imports for art groups
   "process_art_groups": true,
+  // write out a json file containing the art info mapping, run this with all objects allowed
+  "dump_art_group_info": false,
 
   ///////////////////////////
   // WEIRD OPTIONS

decompiler/config/jak2/hacks.jsonc

@@ -526,12 +526,8 @@
       [479, 1],
       [510, 1]
     ],
-    "(method 17 hud-goal)": [
-      [71, 0]
-    ],
-    "(method 15 hud-miss)": [
-      [44, 0]
-    ]
+    "(method 17 hud-goal)": [[71, 0]],
+    "(method 15 hud-miss)": [[44, 0]]
   },
 
   "mips2c_functions_by_name": [

decompiler/config/jak2/label_types.jsonc

@@ -802,7 +802,5 @@
     ["L802", "uint64", true],
     ["L803", "uint64", true]
   ],
-  "hud": [
-    ["L217", "vector"]
-  ]
+  "hud": [["L217", "vector"]]
 }

decompiler/config/jak2/type_casts.jsonc

@@ -5558,9 +5558,7 @@
     [[39, 280], "v1", "(inline-array vector)"],
     [[280, 316], "v1", "(inline-array vector4w)"]
   ],
-  "(method 9 hud-box)": [
-    [[1, 53], "v1", "(inline-array vector4w)"]
-  ],
+  "(method 9 hud-box)": [[[1, 53], "v1", "(inline-array vector4w)"]],
   "(method 10 hud-box)": [
     [[1, 8], "a2", "dma-packet"],
     [[10, 17], "a2", "gs-gif-tag"],
@@ -5600,16 +5598,12 @@
     [21, "a0", "(pointer gs-scissor)"],
     [23, "a0", "(pointer gs-reg64)"]
   ],
-  "hud-create-icon": [
-    [35, "a0", "process-drawable"]
-  ],
+  "hud-create-icon": [[35, "a0", "process-drawable"]],
   "hide-hud": [
     [11, "v1", "connection"],
     [23, "v1", "connection"]
   ],
-  "enable-hud": [
-    [17, "v1", "connection"]
-  ],
+  "enable-hud": [[17, "v1", "connection"]],
   "hide-hud-quick": [
     [11, "v1", "connection"],
     [23, "v1", "connection"]
@@ -5622,10 +5616,6 @@
     [9, "v1", "connection"],
     [[9, 13], "a0", "hud"]
   ],
-  "(method 15 hud-dark-eco-symbol)": [
-    [[9, 14], "v1", "hud-health"]
-  ],
-  "(method 15 hud-gun)": [
-    [[251, 256], "v1", "dma-packet"]
-  ]
+  "(method 15 hud-dark-eco-symbol)": [[[9, 14], "v1", "hud-health"]],
+  "(method 15 hud-gun)": [[[251, 256], "v1", "dma-packet"]]
 }

decompiler/config/jak2_ntsc_v1.jsonc

@@ -40,6 +40,8 @@
   "process_game_count": true,
   // write goal imports for art groups
   "process_art_groups": false,
+  // write out a json file containing the art info mapping, run this with all objects allowed
+  "dump_art_group_info": false,
 
   ///////////////////////////
   // WEIRD OPTIONS

decompiler/config/jak3_ntsc.jsonc

@@ -40,6 +40,8 @@
   "process_game_count": false,
   // write goal imports for art groups
   "process_art_groups": false,
+  // write out a json file containing the art info mapping, run this with all objects allowed
+  "dump_art_group_info": false,
 
   ///////////////////////////
   // WEIRD OPTIONS
@@ -118,4 +120,4 @@
   // what to patch an object to and what the patch file is
   "object_patches": {
   }
-}
+}

decompiler/main.cpp

@@ -19,6 +19,7 @@
 #include "decompiler/level_extractor/extract_level.h"
 
 #include "third-party/CLI11.hpp"
+#include "third-party/json.hpp"
 
 int main(int argc, char** argv) {
   ArgumentGuard u8_guard(argc, argv);
@@ -179,6 +180,14 @@ int main(int argc, char** argv) {
   // process art groups (used in decompilation)
   if (config.decompile_code || config.process_art_groups) {
     db.extract_art_info();
+    // dumb art info to json if requested
+    if (config.dump_art_group_info) {
+      auto file_name = out_folder / "dump" / "art-group-info.min.json";
+      nlohmann::json json = db.dts.art_group_info;
+      file_util::create_dir_if_needed_for_file(file_name);
+      file_util::write_text_file(file_name, json.dump(-1));
+      lg::info("[DUMP] Dumped art group info to {}", file_name.string());
+    }
   }
 
   // main decompile.

test/decompiler/reference/jak2/engine/common_objs/collectables_REF.gc

@@ -3376,7 +3376,3 @@
 
 ;; failed to figure out what this is:
 0
-
-
-
-

test/decompiler/reference/jak2/levels/atoll/atoll-part_REF.gc

@@ -1078,7 +1078,3 @@
     (sp-flt spt-userdata 0.0)
     )
   )
-
-
-
-

test/decompiler/reference/jak2/levels/atoll/atoll-scenes_REF.gc

@@ -2853,7 +2853,3 @@
     :save #t
     )
   )
-
-
-
-

test/decompiler/reference/jak2/levels/city/kid_escort/crocesc-h_REF.gc

@@ -93,7 +93,3 @@
 
 ;; failed to figure out what this is:
 0
-
-
-
-

test/decompiler/reference/jak2/levels/city/market/ctymark-obs_REF.gc

@@ -1605,7 +1605,3 @@
   0
   (none)
   )
-
-
-
-

test/decompiler/reference/jak2/levels/common/enemy/fodder/fodder_REF.gc

@@ -1197,7 +1197,3 @@
 (defmethod enemy-method-99 fodder ((obj fodder) (arg0 process-focusable))
   (logtest? (focus-status mech) (-> arg0 focus-status))
   )
-
-
-
-

test/decompiler/reference/jak2/levels/mars_tomb/tomb-scenes_REF.gc

@@ -2639,7 +2639,3 @@
     :save #t
     )
   )
-
-
-
-

test/decompiler/reference/jak2/levels/sewer/escort/jinx-h_REF.gc

@@ -40,7 +40,3 @@
 
 ;; failed to figure out what this is:
 0
-
-
-
-

test/offline/config/config.h

@@ -23,7 +23,7 @@ class OfflineTestConfig {
   u32 num_threads;
   bool dump_mode;
   bool fail_on_cmp = false;
-  bool fail_on_compile = false;
+  bool fail_on_compile = false;  // unused currently!
   bool pretty_print = false;
   std::vector<std::string> dgos;
   std::unordered_set<std::string> skip_compile_files;

test/offline/framework/file_management.cpp

@@ -82,64 +82,16 @@ std::vector<OfflineTestSourceFile> find_source_files(const std::string& game_nam
   return result;
 }
 
-std::vector<OfflineTestArtFile> find_art_files(const std::string& game_name,
-                                               const std::vector<std::string>& dgos) {
-  // TODO - Jak 2
-  if (game_name != "jak1") {
-    return {};
+std::unordered_map<std::string, std::unordered_map<int, std::string>> find_art_files(
+    const std::string& game_name) {
+  // Pull from the json database of all art file data
+  // this is generated via 'dump_art_group_info' in the config file
+  auto file_name = file_util::get_jak_project_dir() / "test" / "offline" / "data" / game_name /
+                   "art-group-info.min.json";
+  if (!file_util::file_exists(file_name.string())) {
+    lg::error("couldn't locate {}, exiting", file_name.string());
   }
-
-  std::vector<OfflineTestArtFile> result;
-
-  // use the all_objs.json file to place them in the correct build order
-  auto obj_json = parse_commented_json(
-      file_util::read_text_file(
-          (file_util::get_jak_project_dir() / "goal_src" / game_name / "build" / "all_objs.json")
-              .string()),
-      "all_objs.json");
-
-  for (const auto& x : obj_json) {
-    auto unique_name = x[0].get<std::string>();
-    auto version = x[2].get<int>();
-
-    std::vector<std::string> dgoList = x[3].get<std::vector<std::string>>();
-    if (version == 4) {
-      bool skip_file = false;
-
-      // Check to see if we've included atleast one of the DGO/CGOs in our hardcoded list
-      // If not BLOW UP
-      std::optional<std::string> containing_dgo = {};
-      for (int i = 0; i < (int)dgoList.size(); i++) {
-        std::string& dgo = dgoList.at(i);
-        if (dgo == "NO-XGO") {
-          skip_file = true;
-          break;
-        }
-        // can either be in the DGO or CGO folder, and can either end with .CGO or .DGO
-        // TODO - Jak 2 folder may structure will be different!
-        if (std::find(dgos.begin(), dgos.end(), fmt::format("DGO/{}.DGO", dgo)) != dgos.end() ||
-            std::find(dgos.begin(), dgos.end(), fmt::format("DGO/{}.CGO", dgo)) != dgos.end() ||
-            std::find(dgos.begin(), dgos.end(), fmt::format("CGO/{}.DGO", dgo)) != dgos.end() ||
-            std::find(dgos.begin(), dgos.end(), fmt::format("CGO/{}.CGO", dgo)) != dgos.end()) {
-          containing_dgo = dgo;
-          break;
-        }
-      }
-      if (skip_file) {
-        continue;
-      }
-      if (!containing_dgo) {
-        lg::error(
-            "File [{}] is in the following DGOs [{}], and not one of these is in our list! Add "
-            "it!",
-            unique_name, fmt::join(dgoList, ", "));
-        exit(1);
-      }
-
-      OfflineTestArtFile file(containing_dgo.value(), x[1], unique_name);
-      result.push_back(file);
-    }
-  }
-
-  return result;
+  auto art_group_info =
+      parse_commented_json(file_util::read_text_file(file_name), "art-group-info.min.json");
+  return art_group_info;
 }

test/offline/framework/file_management.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <string>
+#include <unordered_map>
 
 #include "common/util/FileUtil.h"
 
@@ -32,5 +33,5 @@ struct OfflineTestArtFile {
 std::vector<OfflineTestSourceFile> find_source_files(const std::string& game_name,
                                                      const std::vector<std::string>& dgos,
                                                      const std::string& single_file);
-std::vector<OfflineTestArtFile> find_art_files(const std::string& game_name,
-                                               const std::vector<std::string>& dgos);
+std::unordered_map<std::string, std::unordered_map<int, std::string>> find_art_files(
+    const std::string& game_name);

test/offline/framework/orchestration.cpp

@@ -40,8 +40,14 @@ OfflineTestDecompiler setup_decompiler(const OfflineTestWorkGroup& work,
   for (auto& file : work.work_collection.source_files) {
     object_files.insert(file.name_in_dgo);  // todo, make this work with unique_name
   }
-  for (auto& file : work.work_collection.art_files) {
-    object_files.insert(file.unique_name);
+  auto art_group_info = find_art_files(offline_config.game_name);
+  for (const auto& [file_name, info] : art_group_info) {
+    // Add the names of all art files to the allowed object list
+    // TODO - skip this for jak2, it's not yet emitting *-ag files! / art-elts.gc is not
+    // comprehensive
+    if (offline_config.game_name != "jak2") {
+      object_files.insert(file_name);
+    }
   }
 
   dc.config->allowed_objects = object_files;
@@ -57,6 +63,13 @@ OfflineTestDecompiler setup_decompiler(const OfflineTestWorkGroup& work,
                                                      std::vector<fs::path>{},
                                                      std::vector<fs::path>{}, *dc.config);
 
+  // Set the decompiler's art group info
+  // - NOTE - this omits *-vis files and tpage* files.  Is that a problem?
+  // TODO - skip this for jak2, it's not yet emitting *-ag files! / art-elts.gc is not comprehensive
+  if (offline_config.game_name != "jak2") {
+    dc.db->dts.art_group_info = art_group_info;
+  }
+
   std::unordered_set<std::string> db_files;
   for (auto& files_by_name : dc.db->obj_files_by_name) {
     for (auto& f : files_by_name.second) {
@@ -75,11 +88,6 @@ OfflineTestDecompiler setup_decompiler(const OfflineTestWorkGroup& work,
             file.unique_name);
       }
     }
-    for (auto& file : work.work_collection.art_files) {
-      if (!db_files.count(file.unique_name)) {
-        lg::error("didn't find {}\n", file.unique_name);
-      }
-    }
     exit(1);
   }
 
@@ -88,8 +96,7 @@ OfflineTestDecompiler setup_decompiler(const OfflineTestWorkGroup& work,
 
 std::vector<std::future<OfflineTestThreadResult>> distribute_work(
     const OfflineTestConfig& offline_config,
-    const std::vector<OfflineTestSourceFile>& files,
-    const std::vector<OfflineTestArtFile>& art_files) {
+    const std::vector<OfflineTestSourceFile>& files) {
   // First, group files by their DGO so they can be partitioned
   std::unordered_map<std::string, OfflineTestWorkCollection> work_colls = {};
 
@@ -100,13 +107,6 @@ std::vector<std::future<OfflineTestThreadResult>> distribute_work(
     work_colls[file.containing_dgo].source_files.push_back(file);
   }
 
-  for (const auto& file : art_files) {
-    if (work_colls.count(file.containing_dgo) == 0) {
-      work_colls[file.containing_dgo] = OfflineTestWorkCollection();
-    }
-    work_colls[file.containing_dgo].art_files.push_back(file);
-  }
-
   // Now partition by DGO so that threads do not consume unnecessary or duplicate resources
   //
   // TODO - additionally, if we have more threads than we can actually utilize we should not
@@ -126,21 +126,13 @@ std::vector<std::future<OfflineTestThreadResult>> distribute_work(
   // all DGOs, so assign consecutive files (likely to belong to the same dgo) to the same worker.
   int total_files = 0;
   for (const auto& [dgo, work] : work_colls) {
-    total_files += work.source_files.size() + work.art_files.size();
+    total_files += work.source_files.size();
   }
   int divisor = (total_files + work_groups.size() - 1) / work_groups.size();
 
   // Divide up the work
   int file_idx = 0;
   for (const auto& [dgo, work] : work_colls) {
-    // art files
-    for (auto& art_file : work.art_files) {
-      auto& wg = work_groups.at(file_idx / divisor);
-      wg.dgo_set.insert(dgo);
-      wg.work_collection.art_files.push_back(art_file);
-      file_idx++;
-    }
-
     // source files
     for (auto& source_file : work.source_files) {
       auto& wg = work_groups.at(file_idx / divisor);

test/offline/framework/orchestration.h

@@ -97,7 +97,6 @@ class OfflineTestThreadStatus {
 
 struct OfflineTestWorkCollection {
   std::vector<OfflineTestSourceFile> source_files;
-  std::vector<OfflineTestArtFile> art_files;
 };
 
 struct OfflineTestWorkGroup {
@@ -105,9 +104,7 @@ struct OfflineTestWorkGroup {
   OfflineTestWorkCollection work_collection;
   std::shared_ptr<OfflineTestThreadStatus> status;
 
-  int work_size() const {
-    return work_collection.source_files.size() + work_collection.art_files.size();
-  }
+  int work_size() const { return work_collection.source_files.size(); }
 };
 
 class OfflineTestThreadManager {
@@ -124,5 +121,4 @@ extern OfflineTestThreadManager g_offline_test_thread_manager;
 
 std::vector<std::future<OfflineTestThreadResult>> distribute_work(
     const OfflineTestConfig& offline_config,
-    const std::vector<OfflineTestSourceFile>& files,
-    const std::vector<OfflineTestArtFile>& art_files);
+    const std::vector<OfflineTestSourceFile>& files);

test/offline/offline_test_main.cpp

@@ -83,7 +83,6 @@ int main(int argc, char* argv[]) {
   if (max_files > 0 && max_files < (int)source_files.size()) {
     source_files.erase(source_files.begin() + max_files, source_files.end());
   }
-  auto art_files = find_art_files(game_name, config.dgos);
 
   // Figure out the number of threads, prepare their statuses and start printing it
   if (num_threads < 1) {
@@ -94,7 +93,7 @@ int main(int argc, char* argv[]) {
 
   // Distribute the work amongst the threads, partitioning by DGO
   decompiler::init_opcode_info();
-  auto workers = distribute_work(config, source_files, art_files);
+  auto workers = distribute_work(config, source_files);
 
   // summarize results:
   OfflineTestThreadResult total;