Fix #675: cross-platform settings path + Windows --cached fixes - #677
Open
amillionbouncyballs wants to merge 5 commits into
Open
Fix #675: cross-platform settings path + Windows --cached fixes#677amillionbouncyballs wants to merge 5 commits into
amillionbouncyballs wants to merge 5 commits into
Conversation
… 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\<you>\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 <noreply@anthropic.com>
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 <path> 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 <noreply@anthropic.com>
Manual Windows QA of a7091a8/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 <noreply@anthropic.com>
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 e-dream-ai#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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #675: shared code was hardcoding
~/.config/infinidream/settings.jsonregardless of platform, so the first-run warning reported the wrong settings path on Windows and Mac.g_Settings()->ConfigPath()is now used so the reported path always matches where the client actually wrote the file. Also adds and fixes--cached(offline, local-cache-only playback) support on Windows.What changed and why
a7091a8 / 1108058 (original fix) — report the real settings path on all platforms; parse
--cachedinWinMainviaCommandLineToArgvWand honor it inStartup(), forcing offline mode before the empty-cache check runs (not after), so an empty-cache--cachedrun doesn't fall through into starting real auth during teardown.Found during manual Windows QA of the above, fixed in this branch:
client_win32.hhas its own legacy screensaver-flag parser (/s,/p,/c,/a,/t,/x,/r) that re-parses the raw command line independently of the--cachedparsing inmain(). It only strips one leading dash, so--cachedleft it pointing at the second dash, matched no recognized flag, and leftm_ScrModeat the invalideNone— which abortedStartup()entirely before the--cachedlogic ever ran. Unrecognized flags now fall back to windowed mode instead of aborting.--cachedrun still briefly created and showed a window, becauseclient_win32.h'sStartup()builds the display before calling into the sharedStartup()that contains the cache check. Extracted that check into an idempotentCheckCachedOnlyMode()and call it on Windows before any window is created, so an empty cache now exits with no window at all. Mac/Linux are unaffected — they still reach the same check from the sharedStartup()as before.--cachedplayback showed permanently red "Busy" and "Remote" HUD indicators, because both legitimately reusem_MultipleInstancesMode/ skip the WebSocket connection to get offline behavior — which is indistinguishable, to those indicators, from "another instance is running" or "connection lost". Suppressed both specifically for--cachedmode, mirroring the existingm_OfflineDueToNoInternetOnlyguard, with comments explaining why.Testing
Manually tested end-to-end on both Windows and Linux:
--cachedwith an empty cache: no window opens, process exits cleanly with a clear log message, no auth thread starts.--cachedwith a populated cache (after signing in and letting content download): plays offline from cache, no sign-in wizard, no "Busy"/"Remote" indicators.