cleanup gk CLI and fix issue that caused revert (#2310)

My mistake -- testing focused too much on preserving the existing
behaviour I clearly forgot to make sure the new stuff worked properly.

Just had to early out and not modify the args if they were in the new
format.

.vs/launch.vs.json

@@ -100,43 +100,22 @@
       "type": "default",
       "project": "CMakeLists.txt",
       "projectTarget": "gk.exe (bin\\gk.exe)",
-      "name": "Game - Runtime (no kernel)",
-      "args": ["-fakeiso", "-debug", "-nokernel", "-v", "-nodisplay"]
-    },
-    {
-      "type": "default",
-      "project": "CMakeLists.txt",
-      "projectTarget": "gk.exe (bin\\gk.exe)",
-      "name": "Game - Runtime (with kernel)",
-      "args": ["-fakeiso", "-debug", "-v"]
+      "name": "Game - Runtime",
+      "args": ["-v", "--", "-fakeiso", "-debug"]
     },
     {
       "type": "default",
       "project": "CMakeLists.txt",
       "projectTarget": "gk.exe (bin\\gk.exe)",
       "name": "Game - Runtime (boot)",
-      "args": ["-boot", "-fakeiso", "-debug", "-v"]
+      "args": ["-v", "--", "-boot", "-fakeiso", "-debug"]
     },
     {
       "type": "default",
       "project": "CMakeLists.txt",
       "projectTarget": "gk.exe (bin\\gk.exe)",
-      "name": "Game - Runtime (boot no debug)",
-      "args": ["-boot", "-fakeiso", "-v"]
-    },
-    {
-      "type": "default",
-      "project": "CMakeLists.txt",
-      "projectTarget": "gk.exe (bin\\gk.exe)",
-      "name": "Game - Jak 2 - Runtime (boot)",
-      "args": ["-boot", "-fakeiso", "-debug", "-v", "-jak2"]
-    },
-    {
-      "type": "default",
-      "project": "CMakeLists.txt",
-      "projectTarget": "gk.exe (bin\\gk.exe)",
-      "name": "Game - Jak 2 - Runtime (no boot)",
-      "args": ["-fakeiso", "-debug", "-v", "-jak2"]
+      "name": "Game - Jak 2 - Runtime",
+      "args": ["-v", "--game", "jak2", "--", "-fakeiso", "-debug"]
     },
     {
       "type": "default",

Taskfile.yml

@@ -45,21 +45,21 @@ tasks:
       - sh: test -f {{.GK_BIN_RELEASE_DIR}}/gk{{.EXE_FILE_EXTENSION}}
         msg: "Couldn't locate runtime executable in '{{.GK_BIN_RELEASE_DIR}}/gk'"
     cmds:
-      - "{{.GK_BIN_RELEASE_DIR}}/gk -boot -fakeiso -debug -v"
+      - "{{.GK_BIN_RELEASE_DIR}}/gk -v --game {{.GAME}} -- -boot -fakeiso -debug"
   boot-game-retail:
     desc: "Boots the game without debug mode, it will fail if it's not already compiled!"
     preconditions:
       - sh: test -f {{.GK_BIN_RELEASE_DIR}}/gk{{.EXE_FILE_EXTENSION}}
         msg: "Couldn't locate runtime executable in '{{.GK_BIN_RELEASE_DIR}}/gk'"
     cmds:
-      - "{{.GK_BIN_RELEASE_DIR}}/gk -boot -fakeiso -v"
+      - "{{.GK_BIN_RELEASE_DIR}}/gk -v --game {{.GAME}} -- -boot -fakeiso"
   run-game:
     desc: "Start the game's runtime, to start the game itself the REPL is required"
     preconditions:
       - sh: test -f {{.GK_BIN_RELEASE_DIR}}/gk{{.EXE_FILE_EXTENSION}}
         msg: "Couldn't locate runtime executable in '{{.GK_BIN_RELEASE_DIR}}/gk'"
     cmds:
-      - "{{.GK_BIN_RELEASE_DIR}}/gk -fakeiso -debug -{{.GAME}} -v"
+      - "{{.GK_BIN_RELEASE_DIR}}/gk -v --game {{.GAME}} -- -fakeiso -debug"
   # DEVELOPMENT
   repl:
     desc: "Start the REPL"
@@ -77,9 +77,6 @@ tasks:
         ignore_error: true
       - cmd: npx prettier --write ./decompiler/config/jak2/**/*.jsonc
         ignore_error: true
-  run-game-headless:
-    cmds:
-      - "{{.GK_BIN_RELEASE_DIR}}/gk -fakeiso -debug -nodisplay"
   # DECOMPILING
   decomp:
     cmds:

common/util/FileUtil.cpp

@@ -129,6 +129,7 @@ std::optional<std::string> try_get_jak_project_path() {
 
 std::optional<fs::path> try_get_data_dir() {
   fs::path my_path = get_current_executable_path();
+  lg::info("Current executable directory - {}", my_path.string());
   auto data_dir = my_path.parent_path() / "data";
   if (fs::exists(data_dir) && fs::is_directory(data_dir)) {
     return std::make_optional(data_dir);

game/common/game_common_types.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include "common/versions.h"
+
 //! Supported languages.
 enum class Language {
   English = 0,
@@ -10,4 +12,10 @@ enum class Language {
   Japanese = 5,
   UK_English = 6,
   // uk english?
-};
+};
+
+struct GameLaunchOptions {
+  GameVersion game_version = GameVersion::Jak1;
+  bool disable_display = false;
+  bool disable_debug_vm = false;
+};

game/kernel/jak2/kmachine.cpp

@@ -152,17 +152,17 @@ void InitParms(int argc, const char* const* argv) {
     // new for jak 2
     if (arg == "-user") {
       i++;
-      std::string levelName = argv[i];
-      Msg(6, "dkernel: user %s\n", levelName.c_str());
-      kstrcpy(DebugBootUser, levelName.c_str());
+      std::string userName = argv[i];
+      Msg(6, "dkernel: user %s\n", userName.c_str());
+      kstrcpy(DebugBootUser, userName.c_str());
     }
 
     // new for jak 2
     if (arg == "-art") {
       i++;
-      std::string levelName = argv[i];
-      Msg(6, "dkernel: art-group %s\n", levelName.c_str());
-      kstrcpy(DebugBootArtGroup, levelName.c_str());
+      std::string artGroupName = argv[i];
+      Msg(6, "dkernel: art-group %s\n", artGroupName.c_str());
+      kstrcpy(DebugBootArtGroup, artGroupName.c_str());
       kstrcpy(DebugBootMessage, "art-group");
     }
 

game/main.cpp

@@ -15,6 +15,10 @@
 #include "common/util/unicode_util.h"
 #include "common/versions.h"
 
+#include "game/common/game_common_types.h"
+
+#include "third-party/CLI11.hpp"
+
 #ifdef _WIN32
 extern "C" {
 __declspec(dllexport) unsigned long NvOptimusEnablement = 0x00000001;
@@ -40,51 +44,146 @@ void setup_logging(bool verbose) {
   lg::initialize();
 }
 
+std::string game_arg_documentation() {
+  // clang-format off
+  std::string output = fmt::format(fmt::emphasis::bold, "Game Args (passed through to the game runtime after '--')\n");
+  output += fmt::format(fmt::fg(fmt::color::gray), "Order matters, some args will negate others (see kmachine.cpp for details)\n");
+  output += fmt::format(fmt::fg(fmt::color::gray), "Args with `*` are not well supported\n\n");
+  // Common args
+  output += fmt::format(fmt::emphasis::bold, "Common:\n");
+  output += "  -cd          * Use the DVD drive for everything. This is how the game runs in retail\n";
+  output += "  -cddata      * Use the DVD drive for everything but IOP modules\n";
+  output += "  -deviso      * One of two modes for testing without the need for DVDs\n";
+  output += "  -fakeiso       The other of two modes for testing without the need for DVDs\n";
+  output += "  -boot          Used to set GOAL up for running the game in retail mode\n";
+  output += "  -debug         Used to set GOAL up for debugging/development\n";
+  output += "  -debug-mem     Used to set up GOAL in debug mode, but not to load debug-segments\n";
+  output += "  -nokernel      An added mode to allow booting without a KERNEL.CGO for testing\n";
+  output += "  -nosound       An added mode to allow booting without sound for testing\n";
+  output += "  -level [name]  Used to inform the game to boot a specific level the default level is `#f`\n";
+  // Jak 1 Related
+  output += fmt::format(fmt::emphasis::bold | fmt::fg(fmt::color::orange), "Jak 1:\n");
+  output += "  -demo          Used to pass the message `demo` to the gkernel in the DebugBootMessage (instead of play)\n";
+  // Jak 2 only
+  output += fmt::format(fmt::emphasis::bold | fmt::fg(fmt::color::purple), "Jak 2:\n");
+  output += "  -kiosk         A demo mode, TODO on specifics\n";
+  output += "  -preview       A demo mode, TODO on specifics\n";
+  output += "  -debug-boot    Used to boot the game in retail mode, but with debug segments\n";
+  output += "  -user [name]   Specify the debugging username, the default is `unknown`\n";
+  output += "  -art [name]    Specify the art-group name to set `DebugBootArtGroup`, there is no default\n";
+  // clang-format on
+  return output;
+}
+
 /*!
  * Entry point for the game.
  */
 int main(int argc, char** argv) {
   ArgumentGuard u8_guard(argc, argv);
 
-  // TODO - replace with CLI11 and just propagate args through
-  // - https://github.com/CLIUtils/CLI11/issues/744
+  // TODO - this is a temporary shim to convert the old arg format
+  // into the new
+  //
+  // This is needed to avoid a coupled release with the launcher and
+  // can be removed after one release cycle
+  //
+  // Normal users launch gk with _no_ args
+  //
+  // Only handling args the launcher provides, all others can be changed
+  // in this repo at the time of merge.
+  std::vector<char*> adjusted_argv_vals;
+  std::vector<char*> adjusted_argv_vals_passthru;
+  for (int i = 0; i < argc; i++) {
+    const auto& val = std::string(argv[i]);
+    // Handle all args that aren't passed through
+    if (val == "-proj-path") {
+      adjusted_argv_vals.push_back("--proj-path");
+      i++;
+      if (i > argc) {
+        return 1;
+      }
+      adjusted_argv_vals.push_back(argv[i]);
+    } else if (val == "--") {
+      // if we hit a '--' then break out, args will be matched but they are already in the new
+      // format
+      break;
+    } else if (val == "-boot") {
+      // now handle all the ones that get passed to the game
+      adjusted_argv_vals_passthru.push_back("-boot");
+    } else if (val == "-fakeiso") {
+      adjusted_argv_vals_passthru.push_back("-fakeiso");
+    } else if (val == "-debug") {
+      adjusted_argv_vals_passthru.push_back("-debug");
+    }
+  }
+
+  std::vector<char*> new_argv;
+  if (!adjusted_argv_vals.empty() || !adjusted_argv_vals_passthru.empty()) {
+    new_argv.push_back(argv[0]);
+    for (const auto& arg : adjusted_argv_vals) {
+      new_argv.push_back(arg);
+    }
+    if (!adjusted_argv_vals_passthru.empty()) {
+      new_argv.push_back("--");
+      for (const auto& arg : adjusted_argv_vals_passthru) {
+        new_argv.push_back(arg);
+      }
+    }
+    argv = new_argv.data();
+    argc = new_argv.size();
+  }
+  // --- END temporary shim
+
+  // CLI flags
+  std::string game_name = "jak1";
+  bool verbose_logging = false;
+  bool disable_avx2 = false;
+  bool disable_display = false;
+  bool disable_debug_vm = false;
+  fs::path project_path_override;
+  std::vector<std::string> game_args;
+  CLI::App app{"OpenGOAL Game Runtime"};
+  app.add_option("-g,--game", game_name, "The game name: 'jak1' or 'jak2'");
+  app.add_flag("-v,--verbose", verbose_logging, "Enable verbose logging on stdout");
+  app.add_flag("--no-avx2", verbose_logging, "Disable AVX2 for testing");
+  app.add_flag("--no-display", disable_display, "Disable video display");
+  app.add_flag("--no-vm", disable_debug_vm, "Disable debug PS2 VM (defaulted to on)");
+  app.add_option("--proj-path", project_path_override,
+                 "Specify the location of the 'data/' folder");
+  app.footer(game_arg_documentation());
+  app.add_option("Game Args", game_args,
+                 "Remaining arguments (after '--') that are passed-through to the game itself");
+  app.allow_extras();
+  CLI11_PARSE(app, argc, argv);
+
+  // Create struct with all non-kmachine handled args to pass to the runtime
+  GameLaunchOptions game_options;
+  game_options.disable_debug_vm = disable_debug_vm;
+  game_options.disable_display = disable_display;
+  game_options.game_version = game_name_to_version(game_name);
 
   // Figure out if the CPU has AVX2 to enable higher performance AVX2 versions of functions.
   setup_cpu_info();
   // If the CPU doesn't have AVX, GOAL code won't work and we exit.
   if (!get_cpu_info().has_avx) {
-    printf("Your CPU does not support AVX, which is required for OpenGOAL.\n");
+    lg::info("Your CPU does not support AVX, which is required for OpenGOAL.");
     return -1;
   }
 
-  // parse arguments
-  bool verbose = false;
-  bool disable_avx2 = false;
-  std::optional<fs::path> project_path_override = std::nullopt;
-  for (int i = 1; i < argc; i++) {
-    if (std::string("-v") == argv[i]) {
-      verbose = true;
-      break;
-    }
-
-    if (std::string("-no-avx2") == argv[i]) {
-      disable_avx2 = true;
-    }
-
-    if (std::string("-proj-path") == argv[i] && i + 1 < argc) {
-      project_path_override = std::make_optional(fs::path(argv[i + 1]));
-    }
-  }
-
   // set up file paths for resources. This is the full repository when developing, and the data
   // directory (a subset of the full repo) in release versions
-  if (!file_util::setup_project_path(project_path_override)) {
+  if (project_path_override.empty()) {
+    lg::info("No project path provided, looking for data/ folder in current directory");
+    if (!file_util::setup_project_path({})) {
+      return 1;
+    }
+  } else if (!file_util::setup_project_path(project_path_override)) {
     return 1;
   }
 
   if (disable_avx2) {
     // for debugging the non-avx2 code paths, there's a flag to manually disable.
-    printf("Note: AVX2 code has been manually disabled.\n");
+    lg::info("Note: AVX2 code has been manually disabled.");
     get_cpu_info().has_avx2 = false;
   }
 
@@ -96,38 +195,40 @@ int main(int argc, char** argv) {
 #endif
 
   if (get_cpu_info().has_avx2) {
-    printf("AVX2 mode enabled\n");
+    lg::info("AVX2 mode enabled");
   } else {
-    printf("AVX2 mode disabled\n");
+    lg::info("AVX2 mode disabled");
   }
 
   try {
-    setup_logging(verbose);
+    setup_logging(verbose_logging);
   } catch (const std::exception& e) {
     lg::error("Failed to setup logging: {}", e.what());
     return 1;
   }
 
   bool force_debug_next_time = false;
+  // always start with an empty arg, as internally kmachine starts at `1` not `0`
+  std::vector<char*> arg_ptrs = {""};
+  for (auto& str : game_args) {
+    arg_ptrs.push_back(str.data());
+  }
+
   while (true) {
-    std::vector<std::string> args;
-    for (int i = 0; i < argc; i++) {
-      args.push_back(argv[i]);
-    }
     if (force_debug_next_time) {
-      args.push_back("-boot");
-      args.push_back("-debug");
+      game_args.push_back("-boot");
+      game_args.push_back("-debug");
       force_debug_next_time = false;
-    }
-    std::vector<char*> ptrs;
-    for (auto& str : args) {
-      ptrs.push_back(str.data());
+      arg_ptrs.clear();
+      for (auto& str : game_args) {
+        arg_ptrs.push_back(str.data());
+      }
     }
 
     // run the runtime in a loop so we can reset the game and have it restart cleanly
     lg::info("OpenGOAL Runtime {}.{}", versions::GOAL_VERSION_MAJOR, versions::GOAL_VERSION_MINOR);
     try {
-      auto exit_status = exec_runtime(ptrs.size(), ptrs.data());
+      auto exit_status = exec_runtime(game_options, arg_ptrs.size(), arg_ptrs.data());
       switch (exit_status) {
         case RuntimeExitStatus::EXIT:
           return 0;

game/runtime.cpp

@@ -310,25 +310,14 @@ void dmac_runner(SystemThreadInterface& iface) {
  * Main function to launch the runtime.
  * GOAL kernel arguments are currently ignored.
  */
-RuntimeExitStatus exec_runtime(int argc, char** argv) {
+RuntimeExitStatus exec_runtime(GameLaunchOptions game_options, int argc, char** argv) {
   g_argc = argc;
   g_argv = argv;
   g_main_thread_id = std::this_thread::get_id();
 
-  // parse opengoal arguments
-  g_game_version = GameVersion::Jak1;
-  bool enable_display = true;
-  for (int i = 1; i < argc; i++) {
-    if (std::string("-nodisplay") == argv[i]) {  // disable video display
-      enable_display = false;
-    } else if (std::string("-vm") == argv[i]) {  // enable debug ps2 VM
-      VM::use = true;
-    } else if (std::string("-novm") == argv[i]) {  // disable debug ps2 VM
-      VM::use = false;
-    } else if (std::string("-jak2") == argv[i]) {
-      g_game_version = GameVersion::Jak2;
-    }
-  }
+  bool enable_display = !game_options.disable_display;
+  VM::use = !game_options.disable_debug_vm;
+  g_game_version = game_options.game_version;
 
   // set up discord stuff
   gStartTime = time(nullptr);

game/runtime.h

@@ -10,11 +10,12 @@
 #include "common/common_types.h"
 #include "common/versions.h"
 
+#include "game/common/game_common_types.h"
 #include "game/kernel/common/kboot.h"
 
 extern u8* g_ee_main_mem;
 extern GameVersion g_game_version;
 
-RuntimeExitStatus exec_runtime(int argc, char** argv);
+RuntimeExitStatus exec_runtime(GameLaunchOptions game_options, int argc, char** argv);
 
 extern std::thread::id g_main_thread_id;

goalc/main.cpp

@@ -7,6 +7,7 @@
 #include "common/util/FileUtil.h"
 #include "common/util/diff.h"
 #include "common/util/string_util.h"
+#include "common/util/unicode_util.h"
 #include "common/versions.h"
 
 #include "goalc/compiler/Compiler.h"
@@ -24,6 +25,8 @@ void setup_logging() {
 }
 
 int main(int argc, char** argv) {
+  ArgumentGuard u8_guard(argc, argv);
+
   bool auto_find_user = false;
   std::string cmd = "";
   std::string username = "#f";

scripts/batch/gk-display.bat

@@ -1,4 +1,4 @@
 @echo off
 cd ..\..
-out\build\Release\bin\gk -boot -fakeiso -debug -v
+out\build\Release\bin\gk -v -- -boot -fakeiso -debug
 pause

scripts/batch/gk-release.bat

@@ -1,4 +1,4 @@
 @echo off
 cd ..\..
-out\build\Release\bin\gk -boot -fakeiso -v
+out\build\Release\bin\gk -v -- -boot -fakeiso
 pause

scripts/batch/gk.bat

@@ -1,4 +1,4 @@
 @echo off
 cd ..\..
-out\build\Release\bin\gk -fakeiso -debug -v -nodisplay
+out\build\Release\bin\gk -v --no-display -- -fakeiso -debug
 pause

scripts/batch/gk2-display.bat

@@ -1,4 +1,4 @@
 @echo off
 cd ..\..
-out\build\Release\bin\gk -boot -fakeiso -debug -v -jak2
+out\build\Release\bin\gk -v --game jak2 -- -boot -fakeiso -debug
 pause

scripts/shell/boot_game.sh

@@ -3,4 +3,4 @@
 # Directory of this script
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 
-"${DIR}"/../../build/game/gk -boot -fakeiso -debug "$@"
+"${DIR}"/../../build/game/gk -- -boot -fakeiso -debug "$@"

scripts/shell/boot_kernel.sh

@@ -3,4 +3,4 @@
 # Directory of this script
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 
-"${DIR}"/../../build/game/gk -fakeiso -debug -nodisplay"$@"
+"${DIR}"/../../build/game/gk -- -fakeiso -debug -nodisplay"$@"

scripts/shell/gk.sh

@@ -3,4 +3,4 @@
 # Directory of this script
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 
-"${DIR}"/../../build/game/gk -fakeiso -debug -nokernel -nodisplay "$@"
+"${DIR}"/../../build/game/gk --no-display -- -fakeiso -debug -nokernel -nodisplay "$@"

test/goalc/framework/test_runner.cpp

@@ -107,34 +107,45 @@ void CompilerTestRunner::run_always_pass(const std::string& test_category,
 }
 
 void runtime_no_kernel_jak1() {
-  constexpr int argc = 6;
-  const char* argv[argc] = {"", "-fakeiso", "-debug", "-nokernel", "-nodisplay", "-nosound"};
-  exec_runtime(argc, const_cast<char**>(argv));
+  constexpr int argc = 5;
+  const char* argv[argc] = {"", "-fakeiso", "-debug", "-nokernel", "-nosound"};
+  GameLaunchOptions game_options;
+  game_options.disable_display = true;
+  exec_runtime(game_options, argc, const_cast<char**>(argv));
 }
 
 void runtime_no_kernel_jak2() {
-  constexpr int argc = 7;
-  const char* argv[argc] = {"",           "-fakeiso", "-debug", "-nokernel",
-                            "-nodisplay", "-nosound", "-jak2"};
-  exec_runtime(argc, const_cast<char**>(argv));
+  constexpr int argc = 5;
+  const char* argv[argc] = {"", "-fakeiso", "-debug", "-nokernel", "-nosound"};
+  GameLaunchOptions game_options;
+  game_options.disable_display = true;
+  game_options.game_version = GameVersion::Jak2;
+  exec_runtime(game_options, argc, const_cast<char**>(argv));
 }
 
 void runtime_with_kernel_jak1() {
-  constexpr int argc = 5;
-  const char* argv[argc] = {"", "-fakeiso", "-debug", "-nodisplay", "-nosound"};
-  exec_runtime(argc, const_cast<char**>(argv));
+  constexpr int argc = 4;
+  const char* argv[argc] = {"", "-fakeiso", "-debug", "-nosound"};
+  GameLaunchOptions game_options;
+  game_options.disable_display = true;
+  exec_runtime(game_options, argc, const_cast<char**>(argv));
 }
 
 void runtime_with_kernel_jak2() {
-  constexpr int argc = 6;
-  const char* argv[argc] = {"", "-fakeiso", "-debug", "-nodisplay", "-nosound", "-jak2"};
-  exec_runtime(argc, const_cast<char**>(argv));
+  constexpr int argc = 4;
+  const char* argv[argc] = {"", "-fakeiso", "-debug", "-nosound"};
+  GameLaunchOptions game_options;
+  game_options.disable_display = true;
+  game_options.game_version = GameVersion::Jak2;
+  exec_runtime(game_options, argc, const_cast<char**>(argv));
 }
 
 void runtime_with_kernel_no_debug_segment() {
-  constexpr int argc = 5;
-  const char* argv[argc] = {"", "-fakeiso", "-debug-mem", "-nodisplay", "-nosound"};
-  exec_runtime(argc, const_cast<char**>(argv));
+  constexpr int argc = 4;
+  const char* argv[argc] = {"", "-fakeiso", "-debug-mem", "-nosound"};
+  GameLaunchOptions game_options;
+  game_options.disable_display = true;
+  exec_runtime(game_options, argc, const_cast<char**>(argv));
 }
 
 void createDirIfAbsent(const std::string& path) {