From a7091a85596c0ab4e19a053ba091c94e2f3a143a Mon Sep 17 00:00:00 2001 From: amillionbouncyballs Date: Tue, 11 Aug 2026 12:02:12 +1000 Subject: [PATCH 1/5] Fix #675: report the real settings path, honour --cached on Windows The first-run auth warning hardcoded ~/.config/infinidream/settings.json and suggested --cached. Neither holds outside Linux: the path is wrong on Windows and macOS, and --cached was never parsed on Windows at all. Settings path ------------- JSONStorage::Initialise() already resolves the real per-platform location into m_ConfigPath, built from each client_*.h's m_AppData, but never exposed it. Add ConfigPath() to IStorageInterface / JSONStorage / CSettings and use it at all three message sites in EDreamClient.cpp -- the issue named one; the two in LoginWithMagicLinkCode() hardcoded the same literal. The messages now name the file the client actually opened, so they cannot drift from reality again. --cached on Windows ------------------- Parsing alone would not have been enough: m_CachedOnlyMode was consumed only inside the #ifdef LINUX_GNU block in client.h, so the flag would have been accepted and then silently inert -- the same class of bug. So: - WinMain parses --cached via CommandLineToArgvW. lpCmdLine is ANSI and untokenised; this gives argv semantics with the CRT's own quoting rules. - The cached-only branch moves out of the platform guard. Only the genuinely Linux console magic-link flow stays inside it. - The no-cache failure goes through g_Log->Error(), which already forwards to PlatformUtils::NotifyError() (CLog::Error, Common/Log.cpp). On Windows the client is a GUI binary with no console attached, so stderr goes nowhere and the log file is the only channel that reaches a user. Preventing recurrence --------------------- This bug was not caused by an #ifdef -- it was caused by the absence of one. Shared code stated a platform fact as a string literal, which compiles cleanly on all three platforms and is wrong on two of them, with no build-time signal. - PlatformUtils.h loses its #ifdef WIN32 API block. Win32SetMessageWindow becomes SetNativeMessageWindow, declared unconditionally, with explicit commented no-ops on Linux and Mac. Unconditional declarations preserve the property that matters: a missing implementation is a link error on that platform, whereas a missing #ifdef branch fails silently at runtime. - scripts/check_platform_paths.py fails on platform path literals in shared code, allowlisting the three client_*.h shims whose job is to define them. Wired into a new .github/workflows/lint.yml. Verified to flag the original line when reintroduced. - AGENTS.md documents the convention, the three-implementation table, and why the link error beats an #ifdef. Also corrects two stale references to e-dream.xcodeproj; the file is infinidream.xcodeproj. Verified on Linux: builds clean, and under a throwaway HOME the warning follows it, which proves the path is resolved rather than literal. Windows and macOS are unbuilt here. The WinMain parsing, the PlatformUtils_win.cpp rename and the Mac no-ops need a build on those platforms. No Mac hardware available, so the Mac path is compile-unverified. Windows test ------------ Build Release | x64 from client_generic/MSVC/e-dream.sln, or: cd client_generic\WinBuild python build.py Binary lands in client_generic\MSVC\Release\infinidream.exe. WARNING: step 1 deletes the data folder, including cached videos. Back it up first if you want to keep them, or expect a re-download. :: 1. Settings path. Clean install, run, then read the log. rmdir /s /q "%LOCALAPPDATA%\Infinidream" client_generic\MSVC\Release\infinidream.exe powershell -Command "Select-String 'settings.generator.nickname' $env:LOCALAPPDATA\Infinidream\Logs\*.log" :: expect: ...in C:\Users\\AppData\Local\Infinidream\settings.json :: bug: ...in ~/.config/infinidream/settings.json :: 2. --cached with an empty cache. Should exit without opening a window. client_generic\MSVC\Release\infinidream.exe --cached powershell -Command "Select-String 'cached videos' $env:LOCALAPPDATA\Infinidream\Logs\*.log" :: expect: --cached requested but no cached videos found. :: bug: a window opens, or the flag is ignored entirely :: 3. --cached with content. Sign in, let some dreams download, quit, then: client_generic\MSVC\Release\infinidream.exe --cached :: expect: plays offline from cache with no sign-in prompt To watch the log live during any of the above: powershell -Command "Get-Content -Wait -Tail 20 $env:LOCALAPPDATA\Infinidream\Logs\*.log" Co-Authored-By: Claude Opus 5 --- .github/workflows/lint.yml | 14 +++ AGENTS.md | 48 ++++++- client_generic/Client/PlatformUtils.h | 13 +- client_generic/Client/Player.cpp | 2 +- client_generic/Client/client.h | 21 +++- client_generic/Client/main.cpp | 18 ++- .../LinuxBuild/PlatformUtils_Linux.cpp | 7 ++ client_generic/MSVC/PlatformUtils_win.cpp | 4 +- client_generic/MacBuild/PlatformUtils_Mac.mm | 6 + client_generic/Networking/EDreamClient.cpp | 13 +- client_generic/TupleStorage/JSONStorage.h | 2 + client_generic/TupleStorage/Settings.h | 11 ++ client_generic/TupleStorage/storage.h | 5 + scripts/check_platform_paths.py | 119 ++++++++++++++++++ 14 files changed, 262 insertions(+), 21 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100644 scripts/check_platform_paths.py diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 000000000..757ded108 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,14 @@ +name: Lint +run-name: ${{ github.actor }} is running lint checks 🔍 +on: [push, pull_request] +jobs: + platform-paths: + runs-on: ubuntu-latest + steps: + - name: Check out repository code + uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.x' + - name: Shared code must not hardcode platform paths + run: python scripts/check_platform_paths.py diff --git a/AGENTS.md b/AGENTS.md index 8b80cf163..eab714890 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,7 +19,7 @@ Cross-platform native desktop application and screensaver for infinidream.ai (ma ``` client_generic/ MacBuild/ - e-dream.xcodeproj # Xcode project + infinidream.xcodeproj # Xcode project build.py # Build script (-r release, -s stage, -n notarize, -v version) release.py # Publish release with Sparkle appcast generation MSVC/ @@ -44,7 +44,7 @@ Shared setup (all platforms): clone with submodules (`git submodule update --ini ```bash brew install git-lfs && git lfs install # Required for binary assets ./vcpkg/bootstrap-vcpkg.sh && ./vcpkg/vcpkg install # Install C++ deps -open client_generic/MacBuild/e-dream.xcodeproj # Open in Xcode +open client_generic/MacBuild/infinidream.xcodeproj # Open in Xcode cd client_generic/MacBuild && ./build.py # Build app (Debug) cd client_generic/MacBuild && ./build.py -r -n # Release build with notarization cd client_generic/MacBuild && ./release.py -v X.Y.Z # Publish release (appcast) @@ -82,6 +82,50 @@ Linux has no UI sign-in: authenticate with `INFINIDREAM_API_KEY` env var or `~/. - Code signing: macOS auto-discovers Developer ID from Keychain; Windows uses Authenticode (`release.py --sign`) - Auto-update via Sparkle appcast XML — macOS only; Windows/Linux ship via GitHub releases without in-app update +## Platform-Specific Code + +**Shared code asks for platform facts; it never states them.** A hardcoded +`~/.config/infinidream/settings.json` in shared code compiles cleanly on all +three platforms and is wrong on two of them, with no build-time signal — that is +how [#675](https://github.com/e-dream-ai/client/issues/675) happened. + +The seam is `client_generic/Client/PlatformUtils.h`: one header of `static` +declarations, with exactly one implementation compiled per build system. + +| Platform | Implementation | Wired into | +|---|---|---| +| Windows | `client_generic/MSVC/PlatformUtils_win.cpp` | `MSVC/electricsheep.vcxproj` | +| macOS | `client_generic/MacBuild/PlatformUtils_Mac.mm` | `MacBuild/infinidream.xcodeproj` | +| Linux | `client_generic/LinuxBuild/PlatformUtils_Linux.cpp` | `LinuxBuild/CMakeLists.txt` | + +Because the declarations are unconditional, **adding a method and forgetting an +implementation is a link error on that platform** — loud and immediate. That is +the property an `#ifdef` cannot give you: a missing or wrong `#ifdef` branch +fails silently at runtime, usually in a user's log. + +Rules: + +- Need a platform-specific value or behaviour in shared code? Add a + `PlatformUtils` method and implement it in **all three** files. Where a + platform has nothing to do, write an explicit no-op with a comment saying why. +- Never add an `#ifdef WIN32` / `MAC` / `LINUX_GNU` to `PlatformUtils.h` itself. + Platform-private helpers go in that platform's own internal header — see + `LinuxBuild/PlatformUtils_Internal.h`. +- For the settings file location, use `g_Settings()->ConfigPath()`. It returns + the path the client actually opened, so it cannot drift from reality. +- To put an error in front of a user, call `g_Log->Error()`. It already forwards + to `PlatformUtils::NotifyError()` (see `CLog::Error` in `Common/Log.cpp`), so + calling the seam directly as well reports the same failure twice. + `NotifyError()` is currently a `TODO` stub on Windows and Mac — the log file is + the only channel that reaches a user there. +- `#ifdef` is still fine for `#include`s and for genuinely platform-shaped APIs + (a function taking an `HWND`). It is not fine for facts that shared code could + have asked for. + +`scripts/check_platform_paths.py` enforces the path half of this in CI +(`.github/workflows/lint.yml`). Run it locally with +`python scripts/check_platform_paths.py`. + ## Runtime Logs Runtime logs are written to one file per day (`YYYY_MM_DD.log`) in: diff --git a/client_generic/Client/PlatformUtils.h b/client_generic/Client/PlatformUtils.h index b345f003d..d446e1926 100644 --- a/client_generic/Client/PlatformUtils.h +++ b/client_generic/Client/PlatformUtils.h @@ -41,12 +41,13 @@ class PlatformUtils static void NotifyError(std::string_view errorMessage); static std::string CalculateFileMD5(const std::string& filepath); -#ifdef WIN32 - /// HWND for SetTimer-based delayed work and input hooks (registered from the player window). - static void Win32SetMessageWindow(void* hwnd); - /// Called from display WM_MOUSEMOVE (client coordinates). - static void NotifyMouseMoved(int x, int y); -#endif + /// Native window handle used for delayed work and input hooks, registered from + /// the player window (an HWND on Windows). Platforms that route those through + /// other means implement this as a no-op. + static void SetNativeMessageWindow(void* _nativeHandle); + /// Called from the display's mouse-move event, in client coordinates. + /// Platforms that deliver mouse moves directly implement this as a no-op. + static void NotifyMouseMoved(int _x, int _y); }; class CDelayedDispatch diff --git a/client_generic/Client/Player.cpp b/client_generic/Client/Player.cpp index bd771947e..b799b381e 100644 --- a/client_generic/Client/Player.cpp +++ b/client_generic/Client/Player.cpp @@ -346,7 +346,7 @@ void CPlayer::SetHWND(HWND _hWnd) if (m_hWnd == nullptr) { m_hWnd = _hWnd; - PlatformUtils::Win32SetMessageWindow(static_cast(_hWnd)); + PlatformUtils::SetNativeMessageWindow(static_cast(_hWnd)); } } #endif diff --git a/client_generic/Client/client.h b/client_generic/Client/client.h index 0a57f6d01..e2ff644eb 100644 --- a/client_generic/Client/client.h +++ b/client_generic/Client/client.h @@ -851,24 +851,37 @@ class CElectricSheep (CPlayer::MultiDisplayMode)g_Settings()->Get( "settings.player.MultiDisplayMode", 0)); -#ifdef LINUX_GNU - // Linux/headless: pre-check auth / cache state before the window opens. + // --cached: play locally cached videos without a session. Parsed and honoured + // on every platform, so keep this branch out of any platform guard. if (m_CachedOnlyMode) { - // --cached: play locally cached videos without a session. Cache::CacheManager& cmPre = Cache::CacheManager::getInstance(); cmPre.loadDiskCachedFromJson(); size_t cachedCount = cmPre.getCachedDreamCount(); double cachedGB = cmPre.getCacheSize(); if (cachedCount == 0 && cachedGB < 0.001) { - fprintf(stderr, "No cached videos found. Run without --cached first to download content.\n"); + // Error() hands the message to PlatformUtils::NotifyError() as well as + // the log (see CLog::Error) — that is the platform seam for putting a + // failure in front of a user: stderr on Linux, a TODO stub on Windows + // and Mac. So this one call both records the failure and delivers it to + // whatever those stubs grow into. Until they grow something, the log + // file is all a Windows user has: the client is a GUI binary there, + // with no console attached. + g_Log->Error("--cached requested but no cached videos found. " + "Run without --cached first to download content."); return false; } + + g_Log->Info("--cached: no sealed session token, cycling through %zu (%.1f GB) of cached videos.", + cachedCount, cachedGB); printf("No sealed session token — cycling through %zu (%.1f GB) of cached videos.\n", cachedCount, cachedGB); m_MultipleInstancesMode = true; // forces offline mode through the rest of Startup() } +#ifdef LINUX_GNU + // Linux/headless only: the console magic-link flow, before the window opens. + // Mac and Windows reach their own GUI sign-in wizard instead. else { std::string sealedSession = g_Settings()->Get("settings.content.sealed_session", std::string("")); diff --git a/client_generic/Client/main.cpp b/client_generic/Client/main.cpp index 79ac131e7..7eae80652 100644 --- a/client_generic/Client/main.cpp +++ b/client_generic/Client/main.cpp @@ -9,6 +9,9 @@ #ifdef WIN32 #include #include +#include // CommandLineToArgvW — must follow windows.h +#include +#pragma comment(lib, "shell32.lib") #endif #include #include @@ -35,7 +38,20 @@ typedef CElectricSheep_Linux CElectricSheepClient; int32_t APIENTRY WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow) { - bool cachedOnlyMode = false; // WIN32 does not support --cached flag parsing yet + // lpCmdLine is ANSI and untokenised. CommandLineToArgvW splits the wide command + // line with the same quoting rules the CRT gives argv, so we get argv semantics + // without hand-rolling a parser. Screensaver flags (/s, /c, /p) are parsed + // separately in client_win32.h off GetCommandLineA(); this only wants --cached. + bool cachedOnlyMode = false; + int wideArgCount = 0; + LPWSTR* wideArgs = CommandLineToArgvW(GetCommandLineW(), &wideArgCount); + if (wideArgs != nullptr) + { + for (int i = 1; i < wideArgCount; ++i) + if (wcscmp(wideArgs[i], L"--cached") == 0) { cachedOnlyMode = true; break; } + + LocalFree(wideArgs); + } #else int32_t main(int argc, char* argv[]) { diff --git a/client_generic/LinuxBuild/PlatformUtils_Linux.cpp b/client_generic/LinuxBuild/PlatformUtils_Linux.cpp index 27dfcccdc..606bc4f5c 100644 --- a/client_generic/LinuxBuild/PlatformUtils_Linux.cpp +++ b/client_generic/LinuxBuild/PlatformUtils_Linux.cpp @@ -196,6 +196,13 @@ std::function& PlatformUtils_GetMouseCallback() return s_mouseCallback; } +// No-ops on Linux: there is no HWND to register, and CDisplayVulkan invokes the +// mouse-moved callback directly from the Wayland/X11 event loop rather than +// routing through PlatformUtils. +void PlatformUtils::SetNativeMessageWindow(void* /*_nativeHandle*/) {} + +void PlatformUtils::NotifyMouseMoved(int /*_x*/, int /*_y*/) {} + // --------------------------------------------------------------------------- // Thread name // --------------------------------------------------------------------------- diff --git a/client_generic/MSVC/PlatformUtils_win.cpp b/client_generic/MSVC/PlatformUtils_win.cpp index 6b6d09ecc..9deeba9e1 100644 --- a/client_generic/MSVC/PlatformUtils_win.cpp +++ b/client_generic/MSVC/PlatformUtils_win.cpp @@ -134,9 +134,9 @@ std::string RunCommandAndGetFirstLine(const char* command) } } // namespace -void PlatformUtils::Win32SetMessageWindow(void* hwnd) +void PlatformUtils::SetNativeMessageWindow(void* _nativeHandle) { - g_win32MsgHwnd = static_cast(hwnd); + g_win32MsgHwnd = static_cast(_nativeHandle); } void PlatformUtils::NotifyMouseMoved(int x, int y) diff --git a/client_generic/MacBuild/PlatformUtils_Mac.mm b/client_generic/MacBuild/PlatformUtils_Mac.mm index b66035903..a73636250 100644 --- a/client_generic/MacBuild/PlatformUtils_Mac.mm +++ b/client_generic/MacBuild/PlatformUtils_Mac.mm @@ -128,6 +128,12 @@ } } +// No-ops on Mac: there is no HWND to register, and SetOnMouseMovedCallback above +// installs an NSEvent monitor that delivers mouse moves directly. +void PlatformUtils::SetNativeMessageWindow(void* /*_nativeHandle*/) {} + +void PlatformUtils::NotifyMouseMoved(int /*_x*/, int /*_y*/) {} + void PlatformUtils::OpenURLExternally(std::string_view _url) { NSString* str = [[NSString alloc] initWithBytes:_url.data() diff --git a/client_generic/Networking/EDreamClient.cpp b/client_generic/Networking/EDreamClient.cpp index 1a847cd8d..fb99829ee 100644 --- a/client_generic/Networking/EDreamClient.cpp +++ b/client_generic/Networking/EDreamClient.cpp @@ -599,7 +599,8 @@ bool EDreamClient::LoginWithMagicLinkCode() { if (email.empty()) g_Log->Warning("No email in settings and no tty — " - "set settings.generator.nickname in ~/.config/infinidream/settings.json"); + "set settings.generator.nickname in %s", + g_Settings()->ConfigPath().c_str()); else g_Log->Warning("No tty for magic link code entry — " "run interactively once to establish a session"); @@ -612,8 +613,9 @@ bool EDreamClient::LoginWithMagicLinkCode() fprintf(stderr, "\nWelcome to infinidream!\n" "Enter your invited email address to get started.\n" - "It will be saved to ~/.config/infinidream/settings.json for future runs.\n" - "Email: "); + "It will be saved to %s for future runs.\n" + "Email: ", + g_Settings()->ConfigPath().c_str()); fflush(stderr); if (!std::getline(std::cin, email)) return false; @@ -738,8 +740,9 @@ bool EDreamClient::Authenticate() } g_Log->Warning("No sealed session or API key found. " - "Set settings.generator.nickname in ~/.config/infinidream/settings.json " - "and run interactively, or run with --cached to play cached videos."); + "Set settings.generator.nickname in %s " + "and run interactively, or run with --cached to play cached videos.", + g_Settings()->ConfigPath().c_str()); fIsLoggedIn.exchange(false); fInitialAuthComplete.store(true); fAuthCV.notify_one(); diff --git a/client_generic/TupleStorage/JSONStorage.h b/client_generic/TupleStorage/JSONStorage.h index b002d6af1..b5739fd28 100644 --- a/client_generic/TupleStorage/JSONStorage.h +++ b/client_generic/TupleStorage/JSONStorage.h @@ -37,6 +37,8 @@ class JSONStorage : public IStorageInterface bool _bReadOnly = false) override; virtual bool Finalise() override; + virtual std::string ConfigPath() override { return m_ConfigPath; } + // Set values. virtual bool Set(std::string_view _entry, bool _val) override; virtual bool Set(std::string_view _entry, int32_t _val) override; diff --git a/client_generic/TupleStorage/Settings.h b/client_generic/TupleStorage/Settings.h index dbaf192eb..8a081aaf5 100644 --- a/client_generic/TupleStorage/Settings.h +++ b/client_generic/TupleStorage/Settings.h @@ -62,6 +62,17 @@ class CSettings : public Base::CSingleton return m_pStorage->Root(); }; + // Absolute path of the settings file, per-platform, as actually resolved + // at Init() time. Use this in any message that tells a user where their + // settings live — never hardcode a path, it differs on every platform. + std::string ConfigPath() + { + if (!m_pStorage) + return "?"; + + return m_pStorage->ConfigPath(); + }; + // Init. bool Init(std::string_view _sRoot, std::string_view _workingDir, bool _bReadOnly = false) diff --git a/client_generic/TupleStorage/storage.h b/client_generic/TupleStorage/storage.h index 83391a606..3c8bbfdaf 100644 --- a/client_generic/TupleStorage/storage.h +++ b/client_generic/TupleStorage/storage.h @@ -29,6 +29,11 @@ class IStorageInterface std::string Root() { return (m_sRoot); }; + // Absolute path of the file backing this storage, as resolved at + // Initialise() time. Prefer this over hardcoding a per-platform location: + // it is by construction the file the client actually reads and writes. + virtual std::string ConfigPath() = PureVirtual; + // virtual bool Initialise(std::string_view _sRoot, std::string_view _sWorkingDir, diff --git a/scripts/check_platform_paths.py b/scripts/check_platform_paths.py new file mode 100644 index 000000000..c1e513bc8 --- /dev/null +++ b/scripts/check_platform_paths.py @@ -0,0 +1,119 @@ +#!/usr/bin/env python +"""Fail if cross-platform sources hardcode a platform-specific path. + +Shared code must *ask* for platform facts rather than *state* them: + + g_Settings()->ConfigPath() -- where this platform's settings.json lives + PlatformUtils::GetWorkingDir(), GetAppPath(), ... + +A hardcoded "~/.config/infinidream/settings.json" compiles fine everywhere and +is wrong on two platforms out of three, with no build-time signal. That is the +failure this check exists to catch -- see issue #675 and the "Platform-specific +code" section of AGENTS.md. + +Run from anywhere: python scripts/check_platform_paths.py +""" + +import re +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# Directories compiled into every platform's build. +SHARED_DIRS = [ + "client_generic/Client", + "client_generic/Common", + "client_generic/Networking", + "client_generic/ContentDecoder", + "client_generic/ContentDownloader", + "client_generic/DisplayOutput", + "client_generic/TupleStorage", +] + +SOURCE_SUFFIXES = {".cpp", ".h", ".hpp", ".mm", ".inl"} + +# Vendored trees that live under the shared dirs; not ours to police. +VENDOR_PARTS = {"imgui", "websocketpp", "socket.io-client-cpp", "tinyXml", "rapidjson"} + +# Files that are *allowed* to name a platform path, with the reason why. +ALLOWLIST = { + # The per-platform shims: defining these paths is precisely their job. + "client_generic/Client/client_win32.h": "defines m_AppData for Windows", + "client_generic/Client/client_mac.h": "defines m_AppData for macOS", + "client_generic/Client/client_linux.h": "defines m_AppData for Linux", + # Dead code: the DEBUG_LOG macro sits inside an `#if 0` block. + "client_generic/Common/Log.h": "DEBUG_LOG macro is disabled behind #if 0", +} + +PATTERNS = [ + (re.compile(r"~/\.config/"), "Linux XDG config path"), + (re.compile(r"\.config/infinidream"), "Linux config directory"), + (re.compile(r"/Users/Shared/"), "macOS shared data path"), + (re.compile(r"%LOCALAPPDATA%", re.IGNORECASE), "Windows LocalAppData path"), + (re.compile(r"%APPDATA%", re.IGNORECASE), "Windows AppData path"), + (re.compile(r"%ProgramData%", re.IGNORECASE), "Windows ProgramData path"), + (re.compile(r"AppData[\\/]{1,2}Local"), "Windows AppData path"), +] + +COMMENT_START = ("//", "*", "/*") + + +def is_comment(line): + return line.lstrip().startswith(COMMENT_START) + + +def iter_shared_sources(): + for shared_dir in SHARED_DIRS: + root = REPO_ROOT / shared_dir + if not root.is_dir(): + continue + + for path in sorted(root.rglob("*")): + if path.suffix not in SOURCE_SUFFIXES: + continue + if VENDOR_PARTS.intersection(path.relative_to(REPO_ROOT).parts): + continue + + yield path + + +def main(): + findings = [] + + for path in iter_shared_sources(): + rel = path.relative_to(REPO_ROOT).as_posix() + if rel in ALLOWLIST: + continue + + text = path.read_text(encoding="utf-8", errors="replace") + for lineno, line in enumerate(text.splitlines(), start=1): + # A path in prose is documentation, not behaviour. + if is_comment(line): + continue + + for pattern, description in PATTERNS: + if pattern.search(line): + findings.append((rel, lineno, description, line.strip())) + break + + if not findings: + return 0 + + print("Hardcoded platform-specific paths in shared code:\n") + for rel, lineno, description, snippet in findings: + print(f" {rel}:{lineno} ({description})") + print(f" {snippet}\n") + + print( + "Shared code must not name a platform's paths. Use " + "g_Settings()->ConfigPath() for the settings file, or add a " + "PlatformUtils method and implement it for all three platforms.\n" + "If a hit is genuinely correct, add the file to ALLOWLIST in " + "scripts/check_platform_paths.py with a reason." + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) From 110805801ec376076eae48f1db263432f61d2002 Mon Sep 17 00:00:00 2001 From: amillionbouncyballs Date: Tue, 11 Aug 2026 12:10:30 +1000 Subject: [PATCH 2/5] Set --cached offline mode before the cache check, not after The --cached branch assigned m_MultipleInstancesMode last, after the empty-cache check had already returned false. That flag gates EDreamClient::InitializeClient(), which is what keeps the auth thread -- and therefore the sign-in wizard -- out of an explicitly offline session. Assigning it last meant the failure path never set it at all, so a --cached run with an empty cache still started auth during teardown: [error]: --cached requested but no cached videos found. Run without --cached first to download content. CElectricSheep::Shutdown() [info]: Starting Authentication... [warning]: No sealed session or API key found. Set settings.generator.nickname in and run interactively, or run with --cached to play cached videos. That last line tells the user to run with --cached, which is precisely what they had just done. Harmless, since the app is already exiting by then, but confusing to anyone reading the log after a failed --cached run. Hoisting the assignment to the top of the branch skips auth on both paths. The same run now ends: [error]: --cached requested but no cached videos found. Run without --cached first to download content. [info]: Disabling auth in multiple instance mode No behaviour change on the --cached success path, and none to startup without --cached, where the sign-in wizard still appears exactly as it did before on all three platforms. Suppressing the wizard is specific to --cached and predates this branch: it is the point of the flag, which is a user asking to play what is on disk without authenticating. Verified on Linux under a throwaway HOME. Windows and macOS remain unbuilt here; this change is in shared code with no platform-conditional paths. Co-Authored-By: Claude Opus 5 --- client_generic/Client/client.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/client_generic/Client/client.h b/client_generic/Client/client.h index e2ff644eb..545b9c562 100644 --- a/client_generic/Client/client.h +++ b/client_generic/Client/client.h @@ -855,6 +855,14 @@ class CElectricSheep // on every platform, so keep this branch out of any platform guard. if (m_CachedOnlyMode) { + // Set this up front rather than after the cache check below. It gates + // EDreamClient::InitializeClient(), which is what keeps the auth thread — + // and so the sign-in wizard — out of an explicitly offline session. The + // failure path returns early, so assigning it last meant a --cached run + // with an empty cache still started auth during teardown and told the + // user to "run with --cached", which is what they had just done. + m_MultipleInstancesMode = true; // forces offline mode through the rest of Startup() + Cache::CacheManager& cmPre = Cache::CacheManager::getInstance(); cmPre.loadDiskCachedFromJson(); size_t cachedCount = cmPre.getCachedDreamCount(); @@ -877,7 +885,6 @@ class CElectricSheep cachedCount, cachedGB); printf("No sealed session token — cycling through %zu (%.1f GB) of cached videos.\n", cachedCount, cachedGB); - m_MultipleInstancesMode = true; // forces offline mode through the rest of Startup() } #ifdef LINUX_GNU // Linux/headless only: the console magic-link flow, before the window opens. From a055c0b17ab82820363e52187b9bb53a0ae479ab Mon Sep 17 00:00:00 2001 From: alansley Date: Tue, 11 Aug 2026 13:16:00 +1000 Subject: [PATCH 3/5] Fix --cached on Windows: unrecognized-flag abort and window flash Manual Windows QA of a7091a85/11080580 turned up two --cached bugs and a UX side effect that the fixes above didn't cover: - client_win32.h's own legacy screensaver flag parser (/s, /p, /c, /a, /t, /x, /r) re-parses the raw command line independently of main()'s --cached handling. It only strips one leading dash, so "--cached" left it pointing at the second dash, matched none of the known flags, and left m_ScrMode at the invalid eNone. That fails the mode check right after and aborts Startup() before CElectricSheep::Startup() -- where the --cached/cache-check logic actually lives -- ever runs. Unrecognized flags now fall back to eWindowed instead of aborting. - Even with that fixed, an empty-cache --cached run still briefly created and showed a window: win32's Startup() builds the display before calling the shared Startup() that contains the cache check. Extracted that check into CheckCachedOnlyMode() (idempotent, guarded by m_CachedOnlyModeChecked) and call it from win32's Startup() right after command-line/logging setup, before any display is created, so an empty cache now exits with no window at all. Mac/Linux still hit the same check from the shared Startup() as before. - With those fixed, --cached playback showed permanently red "Busy" and "Remote" HUD indicators. Both flags legitimately reuse m_MultipleInstancesMode/skip the WebSocket to get offline behavior, which is indistinguishable, to those indicators, from "another instance is running" and "lost connection" -- so they lit up for expected, working offline playback. Suppressed both specifically for m_CachedOnlyMode, mirroring the existing m_OfflineDueToNoInternetOnly guard. Co-Authored-By: Claude Sonnet 5 --- client_generic/Client/client.h | 106 ++++++++++++++++++--------- client_generic/Client/client_win32.h | 17 +++++ 2 files changed, 87 insertions(+), 36 deletions(-) diff --git a/client_generic/Client/client.h b/client_generic/Client/client.h index 545b9c562..fae012586 100644 --- a/client_generic/Client/client.h +++ b/client_generic/Client/client.h @@ -196,6 +196,7 @@ class CElectricSheep std::string m_PreviousDlState; // Track download status bool m_MultipleInstancesMode; bool m_CachedOnlyMode = false; + bool m_CachedOnlyModeChecked = false; bool m_StartFullscreen = false; // Linux: start fullscreen (--fullscreen); default is windowed bool m_OfflineDueToNoInternetOnly = false; // true when m_MultipleInstancesMode was set only because internet was down (don't show Busy in that case) bool m_bConfigMode; @@ -827,6 +828,55 @@ class CElectricSheep } } + // --cached: play locally cached videos without a session. Parsed and honoured + // on every platform, so this lives outside any platform guard. Idempotent + // (m_CachedOnlyModeChecked) because Windows calls it early, before creating any + // window -- an empty cache should exit with no window ever appearing, but + // win32's own Startup() creates its display before reaching the shared + // Startup() below, so it calls this first and bails out ahead of that. + // Mac/Linux only reach it from Startup() below. + bool CheckCachedOnlyMode() + { + if (m_CachedOnlyModeChecked) + return true; + m_CachedOnlyModeChecked = true; + + if (!m_CachedOnlyMode) + return true; + + // Set this up front rather than after the cache check below. It gates + // EDreamClient::InitializeClient(), which is what keeps the auth thread — + // and so the sign-in wizard — out of an explicitly offline session. The + // failure path returns early, so assigning it last meant a --cached run + // with an empty cache still started auth during teardown and told the + // user to "run with --cached", which is what they had just done. + m_MultipleInstancesMode = true; // forces offline mode through the rest of Startup() + + Cache::CacheManager& cmPre = Cache::CacheManager::getInstance(); + cmPre.loadDiskCachedFromJson(); + size_t cachedCount = cmPre.getCachedDreamCount(); + double cachedGB = cmPre.getCacheSize(); + if (cachedCount == 0 && cachedGB < 0.001) + { + // Error() hands the message to PlatformUtils::NotifyError() as well as + // the log (see CLog::Error) — that is the platform seam for putting a + // failure in front of a user: stderr on Linux, a TODO stub on Windows + // and Mac. So this one call both records the failure and delivers it to + // whatever those stubs grow into. Until they grow something, the log + // file is all a Windows user has: the client is a GUI binary there, + // with no console attached. + g_Log->Error("--cached requested but no cached videos found. " + "Run without --cached first to download content."); + return false; + } + + g_Log->Info("--cached: no sealed session token, cycling through %zu (%.1f GB) of cached videos.", + cachedCount, cachedGB); + printf("No sealed session token — cycling through %zu (%.1f GB) of cached videos.\n", + cachedCount, cachedGB); + return true; + } + // virtual bool Startup() { @@ -852,39 +902,13 @@ class CElectricSheep "settings.player.MultiDisplayMode", 0)); // --cached: play locally cached videos without a session. Parsed and honoured - // on every platform, so keep this branch out of any platform guard. + // on every platform, so keep this branch out of any platform guard. Windows + // already ran this (see CheckCachedOnlyMode()) before creating its window; + // this call is then a no-op. Mac/Linux hit it here for the first time. if (m_CachedOnlyMode) { - // Set this up front rather than after the cache check below. It gates - // EDreamClient::InitializeClient(), which is what keeps the auth thread — - // and so the sign-in wizard — out of an explicitly offline session. The - // failure path returns early, so assigning it last meant a --cached run - // with an empty cache still started auth during teardown and told the - // user to "run with --cached", which is what they had just done. - m_MultipleInstancesMode = true; // forces offline mode through the rest of Startup() - - Cache::CacheManager& cmPre = Cache::CacheManager::getInstance(); - cmPre.loadDiskCachedFromJson(); - size_t cachedCount = cmPre.getCachedDreamCount(); - double cachedGB = cmPre.getCacheSize(); - if (cachedCount == 0 && cachedGB < 0.001) - { - // Error() hands the message to PlatformUtils::NotifyError() as well as - // the log (see CLog::Error) — that is the platform seam for putting a - // failure in front of a user: stderr on Linux, a TODO stub on Windows - // and Mac. So this one call both records the failure and delivers it to - // whatever those stubs grow into. Until they grow something, the log - // file is all a Windows user has: the client is a GUI binary there, - // with no console attached. - g_Log->Error("--cached requested but no cached videos found. " - "Run without --cached first to download content."); + if (!CheckCachedOnlyMode()) return false; - } - - g_Log->Info("--cached: no sealed session token, cycling through %zu (%.1f GB) of cached videos.", - cachedCount, cachedGB); - printf("No sealed session token — cycling through %zu (%.1f GB) of cached videos.\n", - cachedCount, cachedGB); } #ifdef LINUX_GNU // Linux/headless only: the console magic-link flow, before the window opens. @@ -1000,8 +1024,11 @@ class CElectricSheep if (m_MultipleInstancesMode) { g_Player().SetOfflineMode(true); - // Show busy indicator only for actual multiple instances, not when offline due to no internet - if (!IsPreview() && !m_OfflineDueToNoInternetOnly) { + // Show busy indicator only for actual multiple instances -- not when + // offline due to no internet, and not for --cached, which also forces + // m_MultipleInstancesMode true to get offline behaviour but isn't a + // second instance at all; "Busy" would misleadingly suggest one. + if (!IsPreview() && !m_OfflineDueToNoInternetOnly && !m_CachedOnlyMode) { m_BusyIndicatorEndTime = m_Timer.Time() + 30.0; } } @@ -1915,9 +1942,13 @@ class CElectricSheep // Show standalone indicator HUD only when credits overlay is hidden // and any indicator timer is active (hide all in preview mode) bool showDisk = m_DiskIndicatorEndTime > 0.0 && !IsPreview(); - bool showBusy = m_BusyIndicatorEndTime > 0.0 && !IsPreview(); + // --cached never starts the auth thread or a WebSocket connection + // (that's the point of offline mode), so it would otherwise read as + // a permanent "Busy"/"Remote" problem rather than the intended, + // expected offline state. Suppress both here too. + bool showBusy = m_BusyIndicatorEndTime > 0.0 && !IsPreview() && !m_CachedOnlyMode; bool showNet = m_NetworkIndicatorEndTime > 0.0 && !IsPreview(); - bool showRemote = m_RemoteIndicatorEndTime > 0.0 && !IsPreview() && !inInitialAuthWindow; + bool showRemote = m_RemoteIndicatorEndTime > 0.0 && !IsPreview() && !inInitialAuthWindow && !m_CachedOnlyMode; bool showUpdate = m_UpdateIndicatorEndTime > 0.0; bool shouldShowIndicatorHUD = !creditsVisible && @@ -2036,9 +2067,12 @@ class CElectricSheep bool updateAvailable = m_RuntimeDiagnostics.updateAvailable; bool showDisk = diskSpaceLow && !IsPreview(); - showBusy = m_MultipleInstancesMode && !m_OfflineDueToNoInternetOnly && !IsPreview(); + // Same --cached suppression as the standalone indicator HUD above: + // offline mode here is intentional (no auth thread, no WebSocket), + // not a stuck "Busy"/"Remote" state worth flagging to the user. + showBusy = m_MultipleInstancesMode && !m_OfflineDueToNoInternetOnly && !IsPreview() && !m_CachedOnlyMode; showNet = !internetConnected && !IsPreview(); - showRemote = !wsConnected && !IsPreview() && !inInitialAuthWindow; + showRemote = !wsConnected && !IsPreview() && !inInitialAuthWindow && !m_CachedOnlyMode; showUpdate = updateAvailable; // Disk needs spacing for: Busy (if shown) + Net (if shown) + Remote (if shown) + Update (if shown) diff --git a/client_generic/Client/client_win32.h b/client_generic/Client/client_win32.h index 9ec8d00f1..e2428db52 100644 --- a/client_generic/Client/client_win32.h +++ b/client_generic/Client/client_win32.h @@ -326,6 +326,15 @@ class CElectricSheep_Win32 : public CElectricSheep m_ScrMode = eFullScreenStandalone; m_bAllowFKey = true; } + else + { + // Unrecognized flag -- e.g. --cached, which main() already parsed + // separately via CommandLineToArgvW into m_CachedOnlyMode. Leaving + // m_ScrMode at eNone here fails the mode check below and aborts + // Startup() before CElectricSheep::Startup() (where the --cached + // logic lives) ever runs, so fall back to windowed mode instead. + m_ScrMode = eWindowed; + } } // Check for multiple instances if we're not specifically asked not @@ -430,6 +439,14 @@ class CElectricSheep_Win32 : public CElectricSheep _chdir(m_WorkingDir.c_str()); + // Check --cached against the disk cache now, before any window/display is + // created below -- an empty cache should exit with no window ever + // appearing. CElectricSheep::Startup() (called further down) re-checks + // this too, for Mac/Linux, but CheckCachedOnlyMode() is a no-op there + // once already run here. + if (m_CachedOnlyMode && !CheckCachedOnlyMode()) + return false; + // Mirror the Mac behavior: when the user-facing app launches, reassert // ourselves as the active screensaver if the preference is on. Done // here (not in the installer) so HKCU resolves to the actual user, not From c4cea86d657d7f82dbd9901b4dae704f95fe82ab Mon Sep 17 00:00:00 2001 From: amillionbouncyballs Date: Tue, 11 Aug 2026 16:58:06 +1000 Subject: [PATCH 4/5] Don't open a window when --cached finds an empty cache on Linux CElectricSheep_Linux::Startup() calls AddDisplay() before the shared CElectricSheep::Startup() where the --cached cache check lives, so a --cached run with an empty cache created a Vulkan window and tore it down again milliseconds later when the check failed. Windows already avoided that by calling CheckCachedOnlyMode() ahead of its own display creation; do the same on Linux, straight after InitStorage()/AttachLog() so the failure still reaches the log. This is what the m_CachedOnlyModeChecked guard is for: the re-check in the shared Startup() is now a no-op on Linux as well as Windows. Mac is unchanged - it still reaches the check for the first time in the shared path. Comments in client.h updated to match. Verified on Linux (Arch, Wayland) with the four steps already confirmed on Windows: 1. --cached, empty cache (fresh HOME): exits with no window created at all - zero Vulkan/Wayland init lines - reporting "--cached requested but no cached videos found" to both the log and stderr via NotifyError(). 2. --cached, populated 45.7 GB cache: plays offline, logging "cycling through 795 (45.7 GB) of cached videos" before the window opens, then "Disabling auth in multiple instance mode". No "Starting Authentication..." and no sign-in wizard. 3. Fresh state without --cached: the #675 message reports the real resolved settings path (it followed an overridden HOME) rather than a hardcoded ~/.config/infinidream/settings.json, and the GUI sign-in wizard still appears. 4. Normal online run against real settings: authenticates, saves a sealed session, connects the WebSocket and plays; settings.json unchanged and still valid. scripts/check_platform_paths.py passes. Co-Authored-By: Claude Opus 5 --- client_generic/Client/client.h | 14 +++++++------- client_generic/Client/client_linux.h | 8 ++++++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/client_generic/Client/client.h b/client_generic/Client/client.h index fae012586..cb34917f3 100644 --- a/client_generic/Client/client.h +++ b/client_generic/Client/client.h @@ -830,11 +830,11 @@ class CElectricSheep // --cached: play locally cached videos without a session. Parsed and honoured // on every platform, so this lives outside any platform guard. Idempotent - // (m_CachedOnlyModeChecked) because Windows calls it early, before creating any - // window -- an empty cache should exit with no window ever appearing, but - // win32's own Startup() creates its display before reaching the shared - // Startup() below, so it calls this first and bails out ahead of that. - // Mac/Linux only reach it from Startup() below. + // (m_CachedOnlyModeChecked) because Windows and Linux both call it early, + // before creating any window -- an empty cache should exit with no window ever + // appearing, but each of those platforms' own Startup() creates its display + // before reaching the shared Startup() below, so they call this first and bail + // out ahead of that. Mac only reaches it from Startup() below. bool CheckCachedOnlyMode() { if (m_CachedOnlyModeChecked) @@ -903,8 +903,8 @@ class CElectricSheep // --cached: play locally cached videos without a session. Parsed and honoured // on every platform, so keep this branch out of any platform guard. Windows - // already ran this (see CheckCachedOnlyMode()) before creating its window; - // this call is then a no-op. Mac/Linux hit it here for the first time. + // and Linux already ran this (see CheckCachedOnlyMode()) before creating + // their window; the call is then a no-op. Mac hits it here for the first time. if (m_CachedOnlyMode) { if (!CheckCachedOnlyMode()) diff --git a/client_generic/Client/client_linux.h b/client_generic/Client/client_linux.h index 449c43081..24873b908 100644 --- a/client_generic/Client/client_linux.h +++ b/client_generic/Client/client_linux.h @@ -51,6 +51,14 @@ class CElectricSheep_Linux : public CElectricSheep std::string tmp = "Working dir: " + m_WorkingDir; g_Log->Info(tmp.c_str()); + // Check --cached against the disk cache here, before AddDisplay() below + // creates a window -- an empty cache should exit with no window ever + // appearing. Needs InitStorage()/AttachLog() above to have run, so the + // failure reaches the log. CElectricSheep::Startup() re-checks this, but + // CheckCachedOnlyMode() is a no-op once already run. + if (m_CachedOnlyMode && !CheckCachedOnlyMode()) + return false; + // Run gui. // Start windowed by default; --fullscreen (m_StartFullscreen) opts into From bd3e4bc2c325df694ede0bcfb41cebf8c10bd4b9 Mon Sep 17 00:00:00 2001 From: amillionbouncyballs Date: Tue, 11 Aug 2026 17:22:14 +1000 Subject: [PATCH 5/5] Ignore LinuxBuild/__pycache__/ from the AppImage build build_appimage.py is on master, so anyone who runs the Linux AppImage build produces client_generic/LinuxBuild/__pycache__/ and then carries it as an untracked directory on every branch they check out. The root .gitignore already carries client_generic/WinBuild/__pycache__/ for the same reason on the Windows side; this mirrors it for Linux. Unrelated to the settings-path and --cached work on this branch, but it belongs on master and there is no other open PR to carry it. The sibling deps-cache/ and model-cache/ directories are deliberately not here: they are fetched by CMakeLists.txt on the rife branches, whose .gitignore already covers them, and nothing on master generates them. Co-Authored-By: Claude Opus 5 --- client_generic/LinuxBuild/.gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client_generic/LinuxBuild/.gitignore b/client_generic/LinuxBuild/.gitignore index 1374a2c5f..b584a0bcb 100644 --- a/client_generic/LinuxBuild/.gitignore +++ b/client_generic/LinuxBuild/.gitignore @@ -8,3 +8,7 @@ deps-src/ openssl-minimal/ ffmpeg-minimal/ *.AppImage + +# Bytecode from build_appimage.py, same as WinBuild/__pycache__/ in the root +# .gitignore. Anyone who runs the AppImage build generates it. +__pycache__/