From 6c31d9ceff488e90d3a33a76bdf0e81f64ce4857 Mon Sep 17 00:00:00 2001 From: Corey Ryan Dean Date: Tue, 26 May 2026 12:02:41 -0500 Subject: [PATCH] =?UTF-8?q?reliability(mainmenu):=20SafeWrite=20migration?= =?UTF-8?q?=20=E2=80=94=204=20client-side=20save=20sites?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the client-side counterpart to the #279 server/editor SafeWrite sweep. 4 sites in MainMenu.bb that wrote directly to production files now use the established atomic SafeWriteOpen/SafeWriteCommit pair, with .bak retention on a 1-cycle recovery window. ## Sites migrated (4) 1. **`Data\Last Username.dat`** (line 857, inside `LogIn()`) -- the username/password cache. WriteFile failure would leave the cache empty, locking the user out of the pre-fill on next login (forcing manual re-entry). 2. **`Data\Game Data\Misc.dat`** (line 1500, inside `LogIn()`) -- Music checkbox update. Empty Misc.dat would cause the next launch to read "no game name, no version" and fail project load. 3. **`Data\Game Data\Misc.dat`** (line 3694, inside `GameOptionsMenu()`) -- duplicate of site 2 in the in-game options screen. Same fix. 4. **`SaveTo$`** (line 3847, inside the HTTP downloader) -- the most-complex site. The write happens incrementally in a read- bytes loop with a content-length-completion check. The temp is only promoted on `BytesIn = BytesToRead`; partial downloads (EOF before completion, TCP drop, user cancellation) abort the temp and leave the prior on-disk file (if any) intact. This is meaningful because the downloader fetches game-update files and a corrupted partial write previously could leave a half-downloaded .dat in place; the next launch would read it and either crash or run with corrupt data. ## Lifecycle differences from server SafeWrite migration * MainMenu.bb is included only by Client.bb, which already includes Logging.bb (verified). No new include needed. * `LogIn()` and `GameOptionsMenu()` are different functions, so the `Local MMFinal$ / MMTemp$` declarations don't conflict despite both Misc.dat sites using the same local names. * The downloader path needs `SafeWriteAbort` on the partial path (BytesIn != BytesToRead) -- distinct from the success-path `SafeWriteCommit`. Pattern matches the RC Terrain Editor's `pad_or_optimize` site already migrated in PR #279. ## Verification * `compile.bat -t` clean (Client compiles). * `test.bat` -- 26/26 pass on retry. First run hit the documented OnlinePlayerChainTest CI/local intermittent flake (~30% rate per the feedback_blitzforge_test_handle_object_gc memory). ## Deferred The client-side SafeWrite migration is now complete for MainMenu.bb. Other remaining client/UI writes (if any) would be discovered by the next per-Tools or per-MainMenu audit. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Modules/MainMenu.bb | 64 ++++++++++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/src/Modules/MainMenu.bb b/src/Modules/MainMenu.bb index 4556f1b3..13a74e5a 100644 --- a/src/Modules/MainMenu.bb +++ b/src/Modules/MainMenu.bb @@ -853,11 +853,19 @@ Function LogIn() Wend ; If successful, download Actor/Attributes lists and go to character selection If Result = 1 - ; Save username/password - F = WriteFile("Data\Last Username.dat") + ; Save username/password. Atomic via SafeWriteOpen/ + ; SafeWriteCommit -- a WriteFile failure mid-write would + ; leave the cache file empty, locking the user out of + ; pre-fill on next login. The previous cache (if any) + ; is retained as .bak. + Local LUFinal$ = "Data\Last Username.dat" + Local LUTemp$ = SafeWriteOpen$(LUFinal$) + F = WriteFile(LUTemp$) + If F <> 0 WriteLine F, GY_TextFieldText$(TName) WriteLine F, Encrypt$(GY_TextFieldText$(TPass), -1) - CloseFile F + SafeWriteCommit%(LUTemp$, LUFinal$, F) + EndIf UName$ = GY_TextFieldText$(TName) : PWord$ = MD5$(GY_TextFieldText$(TPass)) ; Request actors list @@ -1496,12 +1504,20 @@ Function LogIn() ; Music update If GY_CheckBoxDown(BUpdateMusic) <> 1 - UpdateMusic UpdateMusic = 1 - GY_CheckBoxDown(BUpdateMusic) - ; Update file - F = WriteFile("Data\Game Data\Misc.dat") + ; Update file atomically. A crash mid-write would + ; leave Misc.dat empty, which the next launch would + ; interpret as "no game name, no version" -- locking + ; the user out of the project. SafeWriteCommit demotes + ; the previous Misc.dat to .bak for recovery. + Local MMFinal$ = "Data\Game Data\Misc.dat" + Local MMTemp$ = SafeWriteOpen$(MMFinal$) + F = WriteFile(MMTemp$) + If F <> 0 WriteLine F, GameName$ WriteLine F, UpdateGame$ WriteLine F, UpdateMusic - CloseFile F + SafeWriteCommit%(MMTemp$, MMFinal$, F) + EndIf EndIf EndIf @@ -3690,12 +3706,20 @@ Function GameOptionsMenu() ; Music update If GY_CheckBoxDown(BUpdateMusic) <> 1 - UpdateMusic UpdateMusic = 1 - GY_CheckBoxDown(BUpdateMusic) - ; Update file - F = WriteFile("Data\Game Data\Misc.dat") + ; Update file atomically. A crash mid-write would + ; leave Misc.dat empty, which the next launch would + ; interpret as "no game name, no version" -- locking + ; the user out of the project. SafeWriteCommit demotes + ; the previous Misc.dat to .bak for recovery. + Local MMFinal$ = "Data\Game Data\Misc.dat" + Local MMTemp$ = SafeWriteOpen$(MMFinal$) + F = WriteFile(MMTemp$) + If F <> 0 WriteLine F, GameName$ WriteLine F, UpdateGame$ WriteLine F, UpdateMusic - CloseFile F + SafeWriteCommit%(MMTemp$, MMFinal$, F) + EndIf EndIf EndIf @@ -3843,8 +3867,14 @@ Function DownloadFile(URL$, SaveTo$ = "", GYProgress = 0) If BytesToRead = 0 Then CloseTCPStream WWW : Return 0 - ; Create new file to write downloaded bytes into - Save = WriteFile(SaveTo$) + ; Create new file to write downloaded bytes into. + ; Atomic: write to .tmp, only promote to final on completion + ; (BytesIn = BytesToRead). Mid-download cancellation, EOF + ; before content-length, or any error path abort the temp + ; without touching the prior on-disk file (if any). The prior + ; download is retained as .bak. + Local DLTemp$ = SafeWriteOpen$(SaveTo$) + Save = WriteFile(DLTemp$) If Save = 0 Then CloseTCPStream(WWW) : Return 0 ; Incredibly complex download-to-file routine... @@ -3869,8 +3899,16 @@ Function DownloadFile(URL$, SaveTo$ = "", GYProgress = 0) If BytesIn = BytesToRead Then Exit Forever - ; Done - CloseFile(Save) + ; Done. Promote to production only if the full content-length + ; arrived; partial downloads abort the temp and leave the + ; previous file (if any) intact. SafeWriteCommit closes Save + ; internally on success; we close + abort on the partial path. + If BytesIn = BytesToRead + SafeWriteCommit%(DLTemp$, SaveTo$, Save) + Else + CloseFile(Save) + SafeWriteAbort(DLTemp$) + EndIf CloseTCPStream(WWW) EndIf