another log

Also saves out the default `pc-settings.gc` file so it's less confusing
_and_ so we can request it from users to actually see what it's doing.

The fix in the last release was only to fix bad `game-size` values when
_loading_ the file. But if you don't have a file, it picks a default.

Right now it picks that default by:
1. Your largest reported resolution
2. If that fails, the one that is currently set

In reality this scenario can never really happen (if you have a set
resolution, it will be one of the reported ones). However what can
happen is for SDL to be misinformed by bad display/monitor drivers/the
OS and be given "supported" resolutions that aren't actually supported.
For example some users have a 4K resolution as their highest, despite
them using a 1080p monitor.

The solution is to not blindly assume the largest resolution is valid,
instead use the one the user already has set.

I'm also now filtering out resolutions by refresh rate, as perhaps this
also caused a problem. ie. the monitor supports a resolution if the
refresh rate is lowered, but it's currently set high (at 144hz for
example).

.github/workflows/compiler-output-check.yaml

@@ -0,0 +1,96 @@
+name: Compilation Check
+
+on:
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  build:
+    name: Compare
+    runs-on: ubuntu-20.04
+    timeout-minutes: 60
+
+    steps:
+      - name: Checkout Repository
+        uses: actions/checkout@v4
+        with:
+          ref: master
+
+      - name: Install Package Dependencies
+        run: |
+          sudo apt update
+          sudo apt install build-essential cmake \
+            clang gcc g++ lcov make nasm libxrandr-dev \
+            libxinerama-dev libxcursor-dev libpulse-dev \
+            libxi-dev zip ninja-build libgl1-mesa-dev libssl-dev
+
+      - name: Setup sccache
+        uses: hendrikmuhs/[email protected]
+        with:
+          variant: sccache
+          key: linux-ubuntu-20.04--Release-linux-clang-asan-${{ github.sha }}
+          restore-keys: linux-ubuntu-20.04--Release-linux-clang-asan
+          max-size: 1000M
+
+      - name: CMake Generation (master)
+        env:
+          CC: clang
+          CXX: clang++
+        run: |
+          cmake -B build --preset=Release-linux-clang-asan \
+            -DCMAKE_C_COMPILER_LAUNCHER=sccache \
+            -DCMAKE_CXX_COMPILER_LAUNCHER=sccache
+
+      - name: Build goalc (master)
+        run: cmake --build build --parallel $((`nproc`)) --target goalc
+
+      - name: Compile and preserve (master)
+        run: |
+          ./build/goalc/goalc --game jak1 --cmd "(make-group \"all-code\")"
+          ./build/goalc/goalc --game jak2 --cmd "(make-group \"all-code\")"
+          ./build/goalc/goalc --game jak3 --cmd "(make-group \"all-code\")"
+          mv ./out/jak1/obj ./out/jak1/obj.master
+          mv ./out/jak2/obj ./out/jak2/obj.master
+          mv ./out/jak3/obj ./out/jak3/obj.master
+
+      - name: Checkout PR
+        uses: actions/checkout@v4
+        with:
+          clean: "false"
+
+      - name: CMake Generation (PR)
+        env:
+          CC: clang
+          CXX: clang++
+        run: |
+          cmake -B build --preset=Release-linux-clang-asan \
+            -DCMAKE_C_COMPILER_LAUNCHER=sccache \
+            -DCMAKE_CXX_COMPILER_LAUNCHER=sccache
+
+      - name: Build goalc (PR)
+        run: cmake --build build --parallel $((`nproc`)) --target goalc
+
+      - name: Compile and preserve (PR)
+        run: |
+          ./build/goalc/goalc --game jak1 --cmd "(make-group \"all-code\")"
+          ./build/goalc/goalc --game jak2 --cmd "(make-group \"all-code\")"
+          ./build/goalc/goalc --game jak3 --cmd "(make-group \"all-code\")"
+          mv ./out/jak1/obj ./out/jak1/obj.pr
+          mv ./out/jak2/obj ./out/jak2/obj.pr
+          mv ./out/jak3/obj ./out/jak3/obj.pr
+
+      - name: Compare Results and Produce Report
+        run: |
+          ls -l ./out/jak1
+          ls -l ./out/jak2
+          ls -l ./out/jak3
+          set +e
+          python ./scripts/gsrc/compare-compilation-outputs.py --base "./out/jak1/obj.master,./out/jak2/obj.master,./out/jak3/obj.master" --compare "./out/jak1/obj.pr,./out/jak2/obj.pr,./out/jak3/obj.pr" --markdown
+          SCRIPT_EXIT_CODE=$?
+          cat ./comp-diff-report.md >> $GITHUB_STEP_SUMMARY
+          if [ "$SCRIPT_EXIT_CODE" -ne 0 ]; then
+            exit 1
+          fi
+
+

.gitignore

@@ -32,6 +32,7 @@ linux-default/
 *.bin
 *.log
 *.p2s
+out/
 savestate-out/
 savestate_out/
 failures/
@@ -39,6 +40,7 @@ ee-results*.json
 search-results.json
 .env
 /search-results.json
+comp-diff-report.md
 
 # graphics debug
 debug_out/
@@ -67,7 +69,6 @@ custom_assets/jak3/merc_replacements/*
 svnrev.h
 common/versions/revision.h
 ci-artifacts/
-out/build/
 __pycache__/
 
 # sqlite stuff

common/util/DgoWriter.cpp

@@ -8,6 +8,8 @@
 #include "BinaryWriter.h"
 #include "FileUtil.h"
 
+#include "common/log/log.h"
+
 void build_dgo(const DgoDescription& description, const std::string& output_prefix) {
   BinaryWriter writer;
   // dgo header
@@ -15,8 +17,14 @@ void build_dgo(const DgoDescription& description, const std::string& output_pref
   writer.add_cstr_len(description.dgo_name.c_str(), 60);
 
   for (auto& obj : description.entries) {
-    auto obj_data = file_util::read_binary_file(file_util::get_jak_project_dir() / "out" /
-                                                output_prefix / "obj" / obj.file_name);
+    // Ensure the directory exists
+    const auto file_path =
+        file_util::get_jak_project_dir() / "out" / output_prefix / "obj" / obj.file_name;
+    if (!file_util::file_exists(file_path.string())) {
+      lg::warn("tried to build a DGO with a file that doesn't exist: {}", file_path.string());
+      continue;
+    }
+    auto obj_data = file_util::read_binary_file(file_path);
     // size
     writer.add<uint32_t>(obj_data.size());
     // name
@@ -29,6 +37,9 @@ void build_dgo(const DgoDescription& description, const std::string& output_pref
     }
   }
 
-  writer.write_to_file(file_util::get_jak_project_dir() / "out" / output_prefix / "iso" /
-                       description.dgo_name);
+  // Ensure the output directory exists
+  const auto dgo_path =
+      file_util::get_jak_project_dir() / "out" / output_prefix / "iso" / description.dgo_name;
+  file_util::create_dir_if_needed_for_file(dgo_path);
+  writer.write_to_file(dgo_path);
 }

game/CMakeLists.txt

@@ -322,3 +322,6 @@ endif()
 
 add_executable(gk main.cpp)
 target_link_libraries(gk runtime)
+if(WIN32)
+    set_target_properties(gk PROPERTIES VS_DPI_AWARE "PerMonitor")
+endif()

game/settings/settings.cpp

@@ -53,7 +53,7 @@ void DebugSettings::load_settings() {
 }
 
 void DebugSettings::save_settings() {
-  // Update the version string as we are now saving it back ground
+  // Update the version string as we are now saving it back
   version = current_version;
   json data = *this;
   auto debug_settings_filename =
@@ -99,7 +99,7 @@ void DisplaySettings::load_settings() {
 }
 
 void DisplaySettings::save_settings() {
-  // Update the version string as we are now saving it back ground
+  // Update the version string as we are now saving it back
   version = current_version;
   json data = *this;
   auto file_path = file_util::get_user_settings_dir(g_game_version) / "display-settings.json";
@@ -148,7 +148,7 @@ void InputSettings::load_settings() {
 }
 
 void InputSettings::save_settings() {
-  // Update the version string as we are now saving it back ground
+  // Update the version string as we are now saving it back
   version = current_version;
   json data = *this;
   auto file_path = file_util::get_user_settings_dir(g_game_version) / "input-settings.json";

game/system/hid/display_manager.cpp

@@ -18,12 +18,14 @@ DisplayManager::DisplayManager(SDL_Window* window) : m_window(window) {
     // scales
     SetProcessDPIAware();
 #endif
+    // First, load the settings
+    m_display_settings.load_settings();
+    // then potentially move the window to a different display
+    initialize_window_position_from_settings();
+    // now that we are on that monitor, get the info and video modes for it
     update_curr_display_info();
     update_video_modes();
-    // Load from file now (after initializing current window settings)
-    m_display_settings.load_settings();
-    // Adjust window / monitor position
-    initialize_window_position_from_settings();
+    // finally, potentially change the display mode
     set_display_mode(m_display_settings.display_mode);
   }
 }
@@ -42,9 +44,12 @@ DisplayManager::~DisplayManager() {
 
 void DisplayManager::initialize_window_position_from_settings() {
   // Check that the display id is still valid
-  if (m_current_display_modes.find(m_display_settings.display_id) ==
-      m_current_display_modes.end()) {
-    lg::warn("[DISPLAY] Saved display ID is no longer valid, resetting to display 0");
+  const auto num_displays = SDL_GetNumVideoDisplays();
+  if (m_display_settings.display_id >= num_displays) {
+    lg::warn(
+        "[DISPLAY] Saved display ID: {} is no longer valid as only {} displays were detected, "
+        "resetting to display 0",
+        num_displays);
     m_display_settings.display_id = 0;
     m_display_settings.window_xpos = 50;
     m_display_settings.window_ypos = 50;
@@ -57,6 +62,8 @@ void DisplayManager::initialize_window_position_from_settings() {
     sdl_util::log_error(fmt::format("unable to get display bounds for display id {}",
                                     m_display_settings.display_id));
   } else {
+    lg::debug("[DISPLAY]: display bounds for {} is ({}, {}) | {}x{}", m_display_settings.display_id,
+              rect.x, rect.y, rect.w, rect.h);
     // Adjust the settings if they are out of bounds
     if (m_display_settings.window_xpos <= rect.x ||
         m_display_settings.window_xpos + 50 >= rect.x + rect.w) {
@@ -187,7 +194,7 @@ Resolution DisplayManager::get_resolution(int id) {
 }
 
 bool DisplayManager::is_supported_resolution(int width, int height) {
-  for (const auto resolution : m_available_resolutions) {
+  for (const auto& resolution : m_available_resolutions) {
     if (resolution.width == width && resolution.height == height) {
       return true;
     }
@@ -300,7 +307,7 @@ void DisplayManager::update_curr_display_info() {
   if (get_active_display_id() < 0) {
     sdl_util::log_error("could not retrieve current window's display index");
   }
-  lg::info("[DISPLAY] current display id is {}", m_display_settings.display_id);
+  lg::info("[DISPLAY] current display id is {}", get_active_display_id());
   SDL_GL_GetDrawableSize(m_window, &m_window_width, &m_window_height);
   SDL_GetWindowPosition(m_window, &m_window_xpos, &m_window_ypos);
   // Update the scale of the display as well
@@ -358,6 +365,10 @@ void DisplayManager::update_video_modes() {
     DisplayMode new_mode = {display_name_str, curr_mode.format,       curr_mode.w,
                             curr_mode.h,      curr_mode.refresh_rate, orient};
     m_current_display_modes[display_id] = new_mode;
+    lg::info(
+        "[DISPLAY]: Found monitor {}, currently set to {}x{}@{}hz. Format: {}, Orientation: {}",
+        new_mode.display_name, new_mode.screen_width, new_mode.screen_height, new_mode.refresh_rate,
+        new_mode.sdl_pixel_format, static_cast<int>(new_mode.orientation));
   }
   update_resolutions();
 }
@@ -365,17 +376,30 @@ void DisplayManager::update_video_modes() {
 void DisplayManager::update_resolutions() {
   lg::info("[DISPLAY] Enumerating resolutions");
   // Enumerate display's display modes to get the resolutions
-  auto num_display_modes = SDL_GetNumDisplayModes(get_active_display_id());
+  const auto active_display_id = get_active_display_id();
+  const auto active_refresh_rate = m_current_display_modes[active_display_id].refresh_rate;
+  auto num_display_modes = SDL_GetNumDisplayModes(active_display_id);
   SDL_DisplayMode curr_mode;
   for (int i = 0; i < num_display_modes; i++) {
-    auto ok = SDL_GetDisplayMode(get_active_display_id(), i, &curr_mode);
+    auto ok = SDL_GetDisplayMode(active_display_id, i, &curr_mode);
     if (ok != 0) {
-      sdl_util::log_error(fmt::format("unable to get display mode for display {}, index {}",
-                                      get_active_display_id(), i));
+      sdl_util::log_error(
+          fmt::format("unable to get display mode for display {}, index {}", active_display_id, i));
+      continue;
+    }
+    // Skip resolutions that aren't using the current refresh rate, they won't work.
+    // For example if your monitor is currently set to `60hz` and the monitor _could_ support
+    // resolution X but only at `30hz`...then there's no reason for us to consider it as an option.
+    if (curr_mode.refresh_rate != active_refresh_rate) {
+      lg::debug(
+          "[DISPLAY]: Skipping {}x{} as it requires {}hz but the monitor is currently set to {}hz",
+          curr_mode.w, curr_mode.h, curr_mode.refresh_rate, active_refresh_rate);
       continue;
     }
     Resolution new_res = {curr_mode.w, curr_mode.h,
                           static_cast<float>(curr_mode.w) / static_cast<float>(curr_mode.h)};
+    lg::info("[DISPLAY]: {}x{}@{}hz is supported", new_res.width, new_res.height,
+             curr_mode.refresh_rate);
     m_available_resolutions.push_back(new_res);
   }
 

game/system/hid/display_manager.h

@@ -80,7 +80,9 @@ class DisplayManager {
   game_settings::DisplaySettings::DisplayMode get_display_mode() {
     return m_display_settings.display_mode;
   }
-  int get_num_resolutions() { return m_available_resolutions.size(); }
+  int get_num_resolutions() {
+    return m_available_resolutions.size();
+  }
   Resolution get_resolution(int id);
   bool is_supported_resolution(int width, int height);
 

goal_src/jak1/pc/pckernel-common.gc

@@ -34,13 +34,14 @@
 (defmethod set-window-size! ((obj pc-settings) (width int) (height int))
   "sets the size of the display window"
   (let ((display-mode (pc-get-display-mode)))
-    (format 0 "Setting ~A size to ~D x ~D~%" display-mode width height)
     (cond
       ((= display-mode 'windowed)
         (set! (-> obj window-width) width)
         (set! (-> obj window-height) height)
+        (format 0 "Setting window size to ~D x ~D~%" (max PC_MIN_WIDTH (-> obj window-width)) (max PC_MIN_HEIGHT (-> obj window-height)))
         (pc-set-window-size! (max PC_MIN_WIDTH (-> obj window-width)) (max PC_MIN_HEIGHT (-> obj window-height))))
       (else
+        (format 0 "Setting borderless/fullscreen size to ~D x ~D~%" width height)
         (set! (-> obj width) width)
         (set! (-> obj height) height))))
   (none))
@@ -642,7 +643,11 @@
      (unless (read-from-file obj *pc-temp-string-1*)
        (format 0 "[PC] PC Settings found at '~S' but could not be loaded, using defaults!~%" *pc-temp-string-1*)
        (reset obj #t)))
-    (else (format 0 "[PC] PC Settings not found at '~S'...initializing with defaults!~%" *pc-temp-string-1*) (reset obj #t)))
+    (else
+      (format 0 "[PC] PC Settings not found at '~S'...initializing with defaults!~%" *pc-temp-string-1*)
+      (reset obj #t)
+      ;; save the file so users aren't confused, and so we can debug what the game is producing for them
+      (commit-to-file obj)))
   0)
 
 (defmethod initialize ((obj pc-settings))

goal_src/jak1/pc/pckernel-h.gc

@@ -318,18 +318,21 @@
   (set! (-> obj aspect-ratio-auto?) #t)
   (set! (-> obj vsync?) #t)
   (set! (-> obj letterbox?) #t)
-  ;; get screen size and set to borderless at screen res (if retail)
-  (cond
-    ((> (pc-get-num-resolutions) 0)
-     ;; pick first available resolution by default
-     (pc-get-resolution 0 (&-> obj width) (&-> obj height)))
-    (else
-     ;; somehow didn't get a single resolution. this is pretty bad.
-     (pc-get-active-display-size (&-> obj width) (&-> obj height))
-     (max! (-> obj width) PC_MIN_WIDTH)
-     (max! (-> obj height) PC_MIN_HEIGHT)
-     (format 0 "!!!! failed to get any screen resolutions !!!!")))
-  (format 0 "fullscreen resolution defaulted to ~D x ~D~%" (-> obj width) (-> obj height))
+  ;; default to the displays currently chosen resolution
+  (pc-get-active-display-size (&-> obj width) (&-> obj height))
+  (max! (-> obj width) PC_MIN_WIDTH)
+  (max! (-> obj height) PC_MIN_HEIGHT)
+  ;; ensure the default resolution was a valid choice
+  (let* ((saved-width (-> obj width))
+         (saved-height (-> obj height))
+         (supported-resolution? (pc-is-supported-resolution? saved-width saved-height)))
+   (cond
+     (supported-resolution?
+      (format 0 "[PC Settings]: Valid default game-size resolution set ~D x ~D~%" saved-width saved-height))
+     (else
+      (pc-get-active-display-size (&-> obj width) (&-> obj height))
+      (format 0 "[PC Settings]: Invalid game-size resolution ~D x ~D, defaulting to ~D x ~D~%" saved-width saved-height (-> obj width) (-> obj height)))))
+  (format 0 "[PC Settings]: fullscreen resolution defaulted to ~D x ~D~%" (-> obj width) (-> obj height))
   (set! (-> obj gfx-msaa) PC_DEFAULT_MSAA) ;; default msaa
   0)
 

out/jak1/fr3/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!hash.md5

out/jak1/iso/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!hash.md5

out/jak1/obj/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!hash.md5

out/jak2/fr3/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!hash.md5

out/jak2/iso/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!hash.md5

out/jak2/obj/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!hash.md5

out/jak3/fr3/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!hash.md5

out/jak3/iso/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!hash.md5

out/jak3/obj/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!hash.md5

scripts/gsrc/compare-compilation-outputs.py

@@ -4,6 +4,19 @@
 
 import os
 import hashlib
+import argparse
+
+parser = argparse.ArgumentParser("compare-compilation-outputs")
+parser.add_argument("--base", help="The base branch directories", type=str)
+parser.add_argument("--compare", help="The potentially modified directories to compare", type=str)
+parser.add_argument("--markdown", help="The format to output results as a markdown file './comp-diff-report.md'", action="store_true")
+args = parser.parse_args()
+
+to_markdown_file = False
+if args.markdown:
+    to_markdown_file = True
+
+markdown_lines = []
 
 def hash_file(filepath):
     """Returns the MD5 hash of the file."""
@@ -19,8 +32,13 @@ def compare_directories(base_dir, compare_dir):
     missing_files = []
 
     # Iterate through files in the base directory
+    total_files = 0
     for root, _, files in os.walk(base_dir):
         for file in files:
+            if file == ".gitignore":
+                continue
+            total_files = total_files + 1
+
             base_file_path = os.path.join(root, file)
             relative_path = os.path.relpath(base_file_path, base_dir)
             compare_file_path = os.path.join(compare_dir, relative_path)
@@ -34,20 +52,54 @@ def compare_directories(base_dir, compare_dir):
                 missing_files.append(relative_path)
 
     # Report results
+    print(f'Comparing {base_dir} with {compare_dir}')
+    markdown_lines.append(f'### Comparing `{base_dir}` with `{compare_dir}`\n\n')
     if not mismatched_files and not missing_files:
         print("All files matched successfully.")
+        markdown_lines.append(f'All `{total_files}` files matched successfully ✅\n\n')
+        return 0
     else:
+        markdown_lines.append(f'Found potential problems ❌\n')
+        markdown_lines.append(f'- {len(mismatched_files)} different file(s)\n')
+        markdown_lines.append(f'- {len(missing_files)} missing file(s)\n\n')
+        markdown_lines.append("| file | result |\n")
+        markdown_lines.append("|------|--------|\n")
         if mismatched_files:
             print("Mismatched files:")
+            markdown_printed_already = 0
             for file in mismatched_files:
                 print(f" - {file}")
+                if markdown_printed_already < 25:
+                    markdown_lines.append(f"| `{file}` | different |\n")
+                    markdown_printed_already = markdown_printed_already + 1
+            if len(mismatched_files) > 25:
+                markdown_lines.append(f"| ...and {len(mismatched_files) - 25} other files | different |\n")
         if missing_files:
             print("Missing files:")
+            markdown_printed_already = 0
             for file in missing_files:
                 print(f" - {file}")
+                if markdown_printed_already < 25:
+                    markdown_lines.append(f"| `{file}` | missing |\n")
+                    markdown_printed_already = markdown_printed_already + 1
+            if len(missing_files) > 25:
+                markdown_lines.append(f"| ...and {len(missing_files) - 25} other files | missing |\n")
+            markdown_lines.append("\n")
+        return 1
 
-# Usage example
-base_directory = './out/jak1/obj'
-compare_directory = './out/jak1/obj_master'
-print(f'Comparing {base_directory} with {compare_directory}')
-compare_directories(base_directory, compare_directory)
+print(f"base: {args.base}")
+print(f"compare: {args.compare}")
+base_directory_list = args.base.split(",")
+compare_directory_list = args.compare.split(",")
+result = 0
+for base_dir, comp_dir in zip(base_directory_list, compare_directory_list):
+    latest_result = compare_directories(base_dir, comp_dir)
+    if latest_result != 0:
+        result = latest_result
+
+if to_markdown_file:
+    with open('./comp-diff-report.md', 'w', encoding='utf-8') as md_file:
+        md_file.writelines(markdown_lines)
+        print("Wrote results to ./comp-diff-report.md")
+
+exit(result)

scripts/gsrc/jak1-sprite-adjustments.py

@@ -276,4 +276,3 @@ for adjust in adjustments:
         function_string += f"   ((fequal-epsilon? aspect-ratio {aspect_ratio} 0.01) {value:.1f})\n"
     function_string += f"   (else\n    (+ {coefficients[0]}\n     (* {coefficients[1]} aspect-ratio)\n     (* {coefficients[2]} aspect-ratio aspect-ratio)\n     (* {coefficients[3]} aspect-ratio aspect-ratio aspect-ratio)))))\n"
     print(function_string)
-