From 8118760fddce23dd65f81f8788d264e8104bb4c3 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:26:16 +1000 Subject: [PATCH 01/23] Revert "Fix `CheatList.SaveFile` returning success without writing anything" This reverts commit a2e8eed8b6aa50d2a9784c23540e0129e99cc210. --- src/BizHawk.Client.Common/tools/CheatList.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/BizHawk.Client.Common/tools/CheatList.cs b/src/BizHawk.Client.Common/tools/CheatList.cs index c00013e1f0c..5de61df7a35 100644 --- a/src/BizHawk.Client.Common/tools/CheatList.cs +++ b/src/BizHawk.Client.Common/tools/CheatList.cs @@ -288,7 +288,6 @@ public FileWriteResult SaveFile(string path) { StreamWriter sw = new(fs); sw.Write(sb.ToString()); - sw.Flush(); }); if (!result.IsError) { From 617c80b5c9ba16d4640d5aa6f762e7ee131fb32d Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:26:31 +1000 Subject: [PATCH 02/23] Revert "Improve `DialogControllerExtensions.DoWithTryAgainBox` dialog" This reverts commit 773f300b32201c0629f02c0819b29a760e9b4637. --- .../DialogControllerExtensions.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/BizHawk.Client.Common/DialogControllerExtensions.cs b/src/BizHawk.Client.Common/DialogControllerExtensions.cs index 6499754f190..a577462755b 100644 --- a/src/BizHawk.Client.Common/DialogControllerExtensions.cs +++ b/src/BizHawk.Client.Common/DialogControllerExtensions.cs @@ -99,13 +99,12 @@ public static TryAgainResult DoWithTryAgainBox( FileWriteResult fileResult = action(); while (fileResult.IsError) { + string prefix = message ?? ""; bool? askResult = dialogParent.ModalMessageBox3( - caption: "Failed to write file", - text: $"{message} Do you want to try again?" - + $"\nError details: {fileResult.UserFriendlyErrorMessage()}" - + $"\n\nFull info for debugging:\n--------------------\n{fileResult.Exception}\n--------------------" - + "\n\n(Retry?)", - icon: EMsgBoxIcon.Warning); + text: $"{prefix} Do you want to try again?\n\nError details:" + + $"{fileResult.UserFriendlyErrorMessage()}\n{fileResult.Exception!.Message}", + caption: "Error", + icon: EMsgBoxIcon.Error); if (askResult == null) return TryAgainResult.Canceled; if (askResult == false) return TryAgainResult.IgnoredFailure; if (askResult == true) fileResult = action(); From 01b5de32ab35342148934dfe665be4b9aec676a0 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:26:36 +1000 Subject: [PATCH 03/23] Revert "Fix `FileWriter.Create` choking on relative paths" This reverts commit 3ec7c2a96a62829af4f38b876cdb12e4cbedd13c. --- src/BizHawk.Client.Common/FileWriter.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/BizHawk.Client.Common/FileWriter.cs b/src/BizHawk.Client.Common/FileWriter.cs index fdbdda72ab2..227ebb01681 100644 --- a/src/BizHawk.Client.Common/FileWriter.cs +++ b/src/BizHawk.Client.Common/FileWriter.cs @@ -92,8 +92,7 @@ public static FileWriteResult Create(string path) FileWritePaths paths = new(path, writePath); try { - var parentDir = Path.GetDirectoryName(path); - if (!string.IsNullOrWhiteSpace(parentDir)) Directory.CreateDirectory(parentDir); + Directory.CreateDirectory(Path.GetDirectoryName(path)); FileStream fs = new(writePath, FileMode.Create, FileAccess.Write); return new(new FileWriter(paths, fs), paths); } From 5c524411e9d49446a78d14e8fbb69eb6ef3576ad Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:34:58 +1000 Subject: [PATCH 04/23] revert intervening changeset 2 to `TAStudio.MenuItems.cs` --- .../tools/TAStudio/TAStudio.MenuItems.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs index 3480707f117..86278361112 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs @@ -173,11 +173,13 @@ private void SaveSelectionToMacroMenuItem_Click(object sender, EventArgs e) if (file != null) { - var selectionStart = FirstSelectedRowIndex; + var selectionStart = TasView.SelectionStartIndex!.Value; MovieZone macro = new( - CurrentTasMovie, + Emulator, + Tools, + MovieSession, start: selectionStart, - length: endIndex - selectionStart + 1); + length: TasView.SelectionEndIndex!.Value - selectionStart + 1); FileWriteResult saveResult = macro.Save(file.FullName); if (saveResult.IsError) From 5ac8cc051a23387a08f73abca575898205bbd081 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 03:10:24 +1000 Subject: [PATCH 05/23] revert intervening changes to `MovieZone.cs` --- src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs b/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs index 93faff43110..c35b4dc9f87 100644 --- a/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs +++ b/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs @@ -159,15 +159,15 @@ private void PlaceMacroInternal(IMovie movie, int start) public FileWriteResult Save(string fileName) { // Save the controller definition/LogKey - // Save the controller name and controller groups count. (Only for the user) + // Save the controller name and player count. (Only for the user.) // Save whether or not the macro should use overlay input, and/or replace return FileWriter.Write(fileName, (fs) => { using var writer = new StreamWriter(fs); writer.WriteLine(InputKey); - writer.WriteLine(_movieDefinition.Name); - writer.WriteLine(_movieDefinition.ControlsOrdered.Count.ToString()); + writer.WriteLine(_emulator.ControllerDefinition.Name); + writer.WriteLine(_emulator.ControllerDefinition.PlayerCount.ToString()); writer.WriteLine($"{Overlay},{Replace}"); foreach (string line in _log) From 1bee3a04b0b683ba793cb3dff27cb2b31697ae83 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 03:11:30 +1000 Subject: [PATCH 06/23] Revert "Handle file errors when saving a macro." This reverts commit c8af4157a24ec3daf6385a4329dfa5a9244c4344. --- .../tools/TAStudio/MovieZone.cs | 24 +++++++------------ .../tools/Macros/MacroInput.cs | 2 +- .../tools/TAStudio/TAStudio.MenuItems.cs | 18 ++++---------- 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs b/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs index c35b4dc9f87..ee3b951c17b 100644 --- a/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs +++ b/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs @@ -156,25 +156,19 @@ private void PlaceMacroInternal(IMovie movie, int start) } } - public FileWriteResult Save(string fileName) + public void Save(string fileName) { // Save the controller definition/LogKey // Save the controller name and player count. (Only for the user.) // Save whether or not the macro should use overlay input, and/or replace - - return FileWriter.Write(fileName, (fs) => - { - using var writer = new StreamWriter(fs); - writer.WriteLine(InputKey); - writer.WriteLine(_emulator.ControllerDefinition.Name); - writer.WriteLine(_emulator.ControllerDefinition.PlayerCount.ToString()); - writer.WriteLine($"{Overlay},{Replace}"); - - foreach (string line in _log) - { - writer.WriteLine(line); - } - }); + string[] header = new string[4]; + header[0] = InputKey; + header[1] = _emulator.ControllerDefinition.Name; + header[2] = _emulator.ControllerDefinition.PlayerCount.ToString(); + header[3] = $"{Overlay},{Replace}"; + + File.WriteAllLines(fileName, header); + File.AppendAllLines(fileName, _log); } public static MovieZone/*?*/ Load(string fileName, IDialogController dialogController, IMovie movie) diff --git a/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs b/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs index e17f74132e5..eae31ff16d0 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs @@ -274,7 +274,7 @@ private bool SaveMacroAs(MovieZone macro) return false; } - macro.Save(result); // ignore errors: This tool is going to be removed. + macro.Save(result); Config!.RecentMacros.Add(result); return true; diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs index 86278361112..392eb0731c5 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs @@ -174,25 +174,15 @@ private void SaveSelectionToMacroMenuItem_Click(object sender, EventArgs e) if (file != null) { var selectionStart = TasView.SelectionStartIndex!.Value; - MovieZone macro = new( + new MovieZone( Emulator, Tools, MovieSession, start: selectionStart, - length: TasView.SelectionEndIndex!.Value - selectionStart + 1); - FileWriteResult saveResult = macro.Save(file.FullName); + length: TasView.SelectionEndIndex!.Value - selectionStart + 1) + .Save(file.FullName); - if (saveResult.IsError) - { - DialogController.ShowMessageBox( - $"Failed to save macro.\n{saveResult.UserFriendlyErrorMessage()}\n{saveResult.Exception.Message}", - "Error", - EMsgBoxIcon.Error); - } - else - { - Config.RecentMacros.Add(file.FullName); - } + Config.RecentMacros.Add(file.FullName); } } From c8ab1aa504471304c86c5ae9d82c239619b21289 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:35:49 +1000 Subject: [PATCH 07/23] Revert "Handle errors when saving config file." This reverts commit 214f53a96feaf58b3a59b3f82f266eb352bc4118. --- .../config/ConfigService.cs | 13 +++++++---- src/BizHawk.Client.EmuHawk/MainForm.Events.cs | 22 ++++--------------- src/BizHawk.Client.EmuHawk/MainForm.cs | 16 ++++++-------- .../config/ControllerConfig.cs | 6 +---- 4 files changed, 21 insertions(+), 36 deletions(-) diff --git a/src/BizHawk.Client.Common/config/ConfigService.cs b/src/BizHawk.Client.Common/config/ConfigService.cs index 357d02ecc49..70286ddedb4 100644 --- a/src/BizHawk.Client.Common/config/ConfigService.cs +++ b/src/BizHawk.Client.Common/config/ConfigService.cs @@ -104,14 +104,19 @@ public static bool IsFromSameVersion(string filepath, out string msg) return config ?? new T(); } - public static FileWriteResult Save(string filepath, object config) + public static void Save(string filepath, object config) { - return FileWriter.Write(filepath, (fs) => + var file = new FileInfo(filepath); + try { - using var writer = new StreamWriter(fs); + using var writer = file.CreateText(); var w = new JsonTextWriter(writer) { Formatting = Formatting.Indented }; Serializer.Serialize(w, config); - }); + } + catch + { + /* Eat it */ + } } // movie 1.0 header stuff diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs index 944fabe23fd..eecd5bfddb3 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs @@ -987,15 +987,8 @@ private void HkOverInputMenuItem_Click(object sender, EventArgs e) private void SaveConfigMenuItem_Click(object sender, EventArgs e) { - FileWriteResult result = SaveConfig(); - if (result.IsError) - { - this.ErrorMessageBox(result); - } - else - { - AddOnScreenMessage("Saved settings"); - } + SaveConfig(); + AddOnScreenMessage("Saved settings"); } private void SaveConfigAsMenuItem_Click(object sender, EventArgs e) @@ -1007,15 +1000,8 @@ private void SaveConfigAsMenuItem_Click(object sender, EventArgs e) initFileName: file); if (result is not null) { - FileWriteResult saveResult = SaveConfig(result); - if (saveResult.IsError) - { - this.ErrorMessageBox(saveResult); - } - else - { - AddOnScreenMessage("Copied settings"); - } + SaveConfig(result); + AddOnScreenMessage("Copied settings"); } } diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index 1b9e8555252..368badcb09c 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -883,12 +883,7 @@ private void CheckMayCloseAndCleanup(object/*?*/ closingSender, CancelEventArgs StopAv(); } - TryAgainResult configSaveResult = this.DoWithTryAgainBox(() => SaveConfig(), "Failed to save config file."); - if (configSaveResult == TryAgainResult.Canceled) - { - closingArgs.Cancel = true; - return; - } + SaveConfig(); // TODO: Handle failure. if (!CloseGame()) { @@ -2394,7 +2389,7 @@ public ISettingsAdapter GetSettingsAdapterForLoadedCore() public SettingsAdapter GetSettingsAdapterForLoadedCoreUntyped() => new(Emulator, static () => true, HandlePutCoreSettings, MayPutCoreSyncSettings, HandlePutCoreSyncSettings); - private FileWriteResult SaveConfig(string path = "") + private void SaveConfig(string path = "") { if (Config.SaveWindowPosition) { @@ -2421,7 +2416,7 @@ private FileWriteResult SaveConfig(string path = "") } CommitCoreSettingsToConfig(); - return ConfigService.Save(path, Config); + ConfigService.Save(path, Config); } private void ToggleFps() @@ -3927,7 +3922,10 @@ private bool CloseGame(bool clearSram = false) } // There is a cheats tool, but cheats can be active while the "cheats tool" is not. And have auto-save option. TryAgainResult cheatSaveResult = this.DoWithTryAgainBox(CheatList.SaveOnClose, "Failed to save cheats."); - if (cheatSaveResult == TryAgainResult.Canceled) return false; + if (cheatSaveResult == TryAgainResult.Canceled) + { + return false; + } // If TAStudio is open, we already asked about saving the movie. if (!Tools.IsLoaded()) diff --git a/src/BizHawk.Client.EmuHawk/config/ControllerConfig.cs b/src/BizHawk.Client.EmuHawk/config/ControllerConfig.cs index 483b3db3b47..5adcee66746 100644 --- a/src/BizHawk.Client.EmuHawk/config/ControllerConfig.cs +++ b/src/BizHawk.Client.EmuHawk/config/ControllerConfig.cs @@ -470,11 +470,7 @@ private void ButtonSaveDefaults_Click(object sender, EventArgs e) SaveToDefaults(cd); - FileWriteResult saveResult = ConfigService.Save(Config.ControlDefaultPath, cd); - if (saveResult.IsError) - { - this.ErrorMessageBox(saveResult); - } + ConfigService.Save(Config.ControlDefaultPath, cd); } } From 1f196354624f58a8b939ce9faf3751b7ac8c1560 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:44:46 +1000 Subject: [PATCH 08/23] revert intervening changes to `LuaConsole.cs` --- .../tools/Lua/LuaConsole.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs b/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs index 4d2d35e0461..e110c9a9a20 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs @@ -690,7 +690,7 @@ private FileWriteResult SaveSessionAs() var file = GetSaveFileFromUser(); if (file != null) { - FileWriteResult saveResult = _openedFiles.Save(file.FullName); + FileWriteResult saveResult = LuaImp.ScriptList.Save(file.FullName); if (saveResult.IsError) { this.ErrorMessageBox(saveResult); @@ -725,7 +725,7 @@ private void LoadSessionFromRecent(string path) public override bool AskSaveChanges() { - if (!_openedFiles.Changes || string.IsNullOrEmpty(_openedFiles.Filename)) return true; + if (!LuaImp.ScriptList.Changes || string.IsNullOrEmpty(LuaImp.ScriptList.Filename)) return true; var result = DialogController.DoWithTempMute(() => this.ModalMessageBox3( caption: "Closing with Unsaved Changes", icon: EMsgBoxIcon.Question, @@ -736,7 +736,7 @@ public override bool AskSaveChanges() TryAgainResult saveResult = this.DoWithTryAgainBox(SaveOrSaveAs, "Failed to save Lua session."); return saveResult != TryAgainResult.Canceled; } - else _openedFiles.Changes = false; + else LuaImp.ScriptList.Changes = false; return true; } @@ -752,12 +752,12 @@ private void UpdateRegisteredFunctionsDialog() private FileWriteResult SaveOrSaveAs() { - if (!string.IsNullOrWhiteSpace(_openedFiles.Filename)) + if (!string.IsNullOrWhiteSpace(LuaImp.ScriptList.Filename)) { - FileWriteResult result = _openedFiles.Save(_openedFiles.Filename); + FileWriteResult result = LuaImp.ScriptList.Save(LuaImp.ScriptList.Filename); if (!result.IsError) { - Config.RecentLuaSession.Add(_openedFiles.Filename); + Config.RecentLuaSession.Add(LuaImp.ScriptList.Filename); } return result; } @@ -804,17 +804,17 @@ private void OpenSessionMenuItem_Click(object sender, EventArgs e) private void SaveSessionMenuItem_Click(object sender, EventArgs e) { - if (_openedFiles.Changes) + if (LuaImp.ScriptList.Changes) { FileWriteResult result = SaveOrSaveAs(); if (result.IsError) { this.ErrorMessageBox(result, "Failed to save Lua session."); - OutputMessages.Text = $"Failed to save {Path.GetFileName(_openedFiles.Filename)}"; + OutputMessages.Text = $"Failed to save {Path.GetFileName(LuaImp.ScriptList.Filename)}"; } else { - OutputMessages.Text = $"{Path.GetFileName(_openedFiles.Filename)} saved."; + OutputMessages.Text = $"{Path.GetFileName(LuaImp.ScriptList.Filename)} saved."; } } } From 59b558d6732f60234293ed42206b8ece79804960 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:44:50 +1000 Subject: [PATCH 09/23] Revert "Handle file writing errors for movies and all tools." This reverts commit 02573e69a41d1c1d23ddf81f516b524aefb927d4. --- .../DialogControllerExtensions.cs | 15 +-- src/BizHawk.Client.Common/FileWriter.cs | 17 ---- src/BizHawk.Client.Common/lua/LuaFileList.cs | 18 +--- .../movie/MovieSession.cs | 2 - src/BizHawk.Client.Common/tools/CheatList.cs | 95 +++++++++---------- .../tools/Watch/WatchList/WatchList.cs | 39 ++++---- src/BizHawk.Client.EmuHawk/MainForm.cs | 31 +++--- src/BizHawk.Client.EmuHawk/tools/CDL.cs | 38 +++----- .../tools/Cheats/Cheats.cs | 19 +--- .../tools/Lua/LuaConsole.cs | 48 +++------- .../tools/Macros/MacroInput.cs | 1 - .../tools/TAStudio/TAStudio.IToolForm.cs | 11 ++- .../tools/Watch/RamSearch.cs | 35 ++----- .../tools/Watch/RamWatch.cs | 70 +++++--------- 14 files changed, 156 insertions(+), 283 deletions(-) diff --git a/src/BizHawk.Client.Common/DialogControllerExtensions.cs b/src/BizHawk.Client.Common/DialogControllerExtensions.cs index a577462755b..37543c98da4 100644 --- a/src/BizHawk.Client.Common/DialogControllerExtensions.cs +++ b/src/BizHawk.Client.Common/DialogControllerExtensions.cs @@ -6,13 +6,6 @@ namespace BizHawk.Client.Common { - public enum TryAgainResult - { - Saved, - IgnoredFailure, - Canceled, - } - public static class DialogControllerExtensions { public static void AddOnScreenMessage( @@ -91,7 +84,7 @@ public static void ErrorMessageBox( /// The user will be repeatedly asked if they want to try again until either success or the user says no. /// /// Returns true on success or if the user said no. Returns false if the user said cancel. - public static TryAgainResult DoWithTryAgainBox( + public static bool DoWithTryAgainBox( this IDialogParent dialogParent, Func action, string message) @@ -105,12 +98,12 @@ public static TryAgainResult DoWithTryAgainBox( $"{fileResult.UserFriendlyErrorMessage()}\n{fileResult.Exception!.Message}", caption: "Error", icon: EMsgBoxIcon.Error); - if (askResult == null) return TryAgainResult.Canceled; - if (askResult == false) return TryAgainResult.IgnoredFailure; + if (askResult == null) return false; + if (askResult == false) return true; if (askResult == true) fileResult = action(); } - return TryAgainResult.Saved; + return true; } /// Creates and shows a System.Windows.Forms.OpenFileDialog or equivalent with the receiver () as its parent diff --git a/src/BizHawk.Client.Common/FileWriter.cs b/src/BizHawk.Client.Common/FileWriter.cs index 227ebb01681..8a0a83c22d5 100644 --- a/src/BizHawk.Client.Common/FileWriter.cs +++ b/src/BizHawk.Client.Common/FileWriter.cs @@ -56,22 +56,6 @@ public static FileWriteResult Write(string path, byte[] bytes, string? backupPat return createResult.Value.CloseAndDispose(backupPath); } - public static FileWriteResult Write(string path, Action writeCallback, string? backupPath = null) - { - FileWriteResult createResult = Create(path); - if (createResult.IsError) return createResult; - - try - { - writeCallback(createResult.Value!.Stream); - } - catch (Exception ex) - { - return new(FileWriteEnum.FailedDuringWrite, createResult.Value!.Paths, ex); - } - - return createResult.Value.CloseAndDispose(backupPath); - } /// /// Create a FileWriter instance, or return an error if unable to access the file. @@ -92,7 +76,6 @@ public static FileWriteResult Create(string path) FileWritePaths paths = new(path, writePath); try { - Directory.CreateDirectory(Path.GetDirectoryName(path)); FileStream fs = new(writePath, FileMode.Create, FileAccess.Write); return new(new FileWriter(paths, fs), paths); } diff --git a/src/BizHawk.Client.Common/lua/LuaFileList.cs b/src/BizHawk.Client.Common/lua/LuaFileList.cs index fceb0620c44..5cc579ffd86 100644 --- a/src/BizHawk.Client.Common/lua/LuaFileList.cs +++ b/src/BizHawk.Client.Common/lua/LuaFileList.cs @@ -102,8 +102,9 @@ public bool Load(string path, bool disableOnLoad, Action onFunctionListChange) return true; } - public FileWriteResult Save(string path) + public void Save(string path) { + using var sw = new StreamWriter(path); var sb = new StringBuilder(); var saveDirectory = Path.GetDirectoryName(Path.GetFullPath(path)); foreach (var file in this) @@ -122,19 +123,10 @@ public FileWriteResult Save(string path) } } - FileWriteResult result = FileWriter.Write(path, (fs) => - { - using var sw = new StreamWriter(fs); - sw.Write(sb.ToString()); - }); - - if (!result.IsError) - { - Filename = path; - Changes = false; - } + sw.Write(sb.ToString()); - return result; + Filename = path; + Changes = false; } } } diff --git a/src/BizHawk.Client.Common/movie/MovieSession.cs b/src/BizHawk.Client.Common/movie/MovieSession.cs index 52055e4ce9b..d63501bf4a9 100644 --- a/src/BizHawk.Client.Common/movie/MovieSession.cs +++ b/src/BizHawk.Client.Common/movie/MovieSession.cs @@ -385,8 +385,6 @@ private void HandlePlaybackEnd() switch (Settings.MovieEndAction) { case MovieEndAction.Stop: - // Technically this can save the movie, but it'd be weird to be in that situation. - // Do we want that? StopMovie(); break; case MovieEndAction.Record: diff --git a/src/BizHawk.Client.Common/tools/CheatList.cs b/src/BizHawk.Client.Common/tools/CheatList.cs index 5de61df7a35..3f1fcd8b438 100644 --- a/src/BizHawk.Client.Common/tools/CheatList.cs +++ b/src/BizHawk.Client.Common/tools/CheatList.cs @@ -210,7 +210,7 @@ public void DisableAll() public bool IsActive(MemoryDomain domain, long address) => _cheatList.Exists(cheat => !cheat.IsSeparator && cheat.Enabled && cheat.Domain == domain && cheat.Contains(address)); - public FileWriteResult SaveOnClose() + public void SaveOnClose() { if (_config.AutoSaveOnClose) { @@ -221,27 +221,17 @@ public FileWriteResult SaveOnClose() CurrentFileName = _defaultFileName; } - return SaveFile(CurrentFileName); + SaveFile(CurrentFileName); } else if (_cheatList.Count is 0 && !string.IsNullOrWhiteSpace(CurrentFileName)) { - try - { - File.Delete(CurrentFileName); - } - catch (Exception ex) - { - return new(FileWriteEnum.FailedToDeleteGeneric, new(CurrentFileName, ""), ex); - } + File.Delete(CurrentFileName); _config.Recent.Remove(CurrentFileName); - return new(); } } - - return new(); } - public FileWriteResult Save() + public bool Save() { if (string.IsNullOrWhiteSpace(CurrentFileName)) { @@ -251,51 +241,54 @@ public FileWriteResult Save() return SaveFile(CurrentFileName); } - public FileWriteResult SaveFile(string path) + public bool SaveFile(string path) { - var sb = new StringBuilder(); - - foreach (var cheat in _cheatList) + try { - if (cheat.IsSeparator) - { - sb.AppendLine("----"); - } - else + new FileInfo(path).Directory?.Create(); + var sb = new StringBuilder(); + + foreach (var cheat in _cheatList) { - // Set to hex for saving - var tempCheatType = cheat.Type; - - cheat.SetType(WatchDisplayType.Hex); - - sb - .Append(cheat.AddressStr).Append('\t') - .Append(cheat.ValueStr).Append('\t') - .Append(cheat.Compare is null ? "N" : cheat.CompareStr).Append('\t') - .Append(cheat.Domain != null ? cheat.Domain.Name : "").Append('\t') - .Append(cheat.Enabled ? '1' : '0').Append('\t') - .Append(cheat.Name).Append('\t') - .Append(cheat.SizeAsChar).Append('\t') - .Append(cheat.TypeAsChar).Append('\t') - .Append(cheat.BigEndian is true ? '1' : '0').Append('\t') - .Append(cheat.ComparisonType).Append('\t') - .AppendLine(); - - cheat.SetType(tempCheatType); + if (cheat.IsSeparator) + { + sb.AppendLine("----"); + } + else + { + // Set to hex for saving + var tempCheatType = cheat.Type; + + cheat.SetType(WatchDisplayType.Hex); + + sb + .Append(cheat.AddressStr).Append('\t') + .Append(cheat.ValueStr).Append('\t') + .Append(cheat.Compare is null ? "N" : cheat.CompareStr).Append('\t') + .Append(cheat.Domain != null ? cheat.Domain.Name : "").Append('\t') + .Append(cheat.Enabled ? '1' : '0').Append('\t') + .Append(cheat.Name).Append('\t') + .Append(cheat.SizeAsChar).Append('\t') + .Append(cheat.TypeAsChar).Append('\t') + .Append(cheat.BigEndian is true ? '1' : '0').Append('\t') + .Append(cheat.ComparisonType).Append('\t') + .AppendLine(); + + cheat.SetType(tempCheatType); + } } - } - FileWriteResult result = FileWriter.Write(path, (fs) => - { - StreamWriter sw = new(fs); - sw.Write(sb.ToString()); - }); - if (!result.IsError) - { + + File.WriteAllText(path, sb.ToString()); + CurrentFileName = path; _config.Recent.Add(CurrentFileName); Changes = false; + return true; + } + catch + { + return false; } - return result; } public bool Load(IMemoryDomains domains, string path, bool append) diff --git a/src/BizHawk.Client.Common/tools/Watch/WatchList/WatchList.cs b/src/BizHawk.Client.Common/tools/Watch/WatchList/WatchList.cs index eb5afeb1ded..0a3e12d913c 100644 --- a/src/BizHawk.Client.Common/tools/Watch/WatchList/WatchList.cs +++ b/src/BizHawk.Client.Common/tools/Watch/WatchList/WatchList.cs @@ -1,6 +1,5 @@ using System.Collections; using System.Collections.Generic; -using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; @@ -339,37 +338,39 @@ public void Reload() } } - public FileWriteResult Save() + public bool Save() { if (string.IsNullOrWhiteSpace(CurrentFileName)) { - return new(); + return false; } - var sb = new StringBuilder(); - sb.Append("SystemID ").AppendLine(_systemId); - - foreach (var watch in _watchList) + using (var sw = new StreamWriter(CurrentFileName)) { - sb.AppendLine(watch.ToString()); - } + var sb = new StringBuilder(); + sb.Append("SystemID ").AppendLine(_systemId); + + foreach (var watch in _watchList) + { + sb.AppendLine(watch.ToString()); + } - FileWriteResult result = FileWriter.Write(CurrentFileName, (fs) => - { - using var sw = new StreamWriter(fs); sw.WriteLine(sb.ToString()); - }); + } - if (!result.IsError) Changes = false; - return result; + Changes = false; + return true; } - public FileWriteResult SaveAs(FileInfo file) + public bool SaveAs(FileInfo file) { - Debug.Assert(file != null, "Cannot save as without a file name."); + if (file != null) + { + CurrentFileName = file.FullName; + return Save(); + } - CurrentFileName = file.FullName; - return Save(); + return false; } private bool LoadFile(string path, bool append) diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index 368badcb09c..b5fb4f2674f 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -3921,17 +3921,18 @@ private bool CloseGame(bool clearSram = false) return false; } // There is a cheats tool, but cheats can be active while the "cheats tool" is not. And have auto-save option. - TryAgainResult cheatSaveResult = this.DoWithTryAgainBox(CheatList.SaveOnClose, "Failed to save cheats."); - if (cheatSaveResult == TryAgainResult.Canceled) - { - return false; - } + CheatList.SaveOnClose(); - // If TAStudio is open, we already asked about saving the movie. - if (!Tools.IsLoaded()) + FileWriteResult saveResult = MovieSession.StopMovie(); + if (saveResult.IsError) { - TryAgainResult saveMovieResult = this.DoWithTryAgainBox(() => MovieSession.StopMovie(), "Failed to save movie."); - if (saveMovieResult == TryAgainResult.Canceled) return false; + if (!this.ModalMessageBox2( + caption: "Quit anyway?", + icon: EMsgBoxIcon.Question, + text: "The currently playing movie could not be saved. Continue and quit anyway? All unsaved changes will be lost.")) + { + return false; + } } if (clearSram) @@ -3939,7 +3940,7 @@ private bool CloseGame(bool clearSram = false) var path = Config.PathEntries.SaveRamAbsolutePath(Game, MovieSession.Movie); if (File.Exists(path)) { - TryAgainResult clearResult = this.DoWithTryAgainBox(() => { + bool clearResult = this.DoWithTryAgainBox(() => { try { File.Delete(path); @@ -3951,7 +3952,7 @@ private bool CloseGame(bool clearSram = false) return new(FileWriteEnum.FailedToDeleteGeneric, new(path, ""), ex); } }, "Failed to clear SRAM."); - if (clearResult == TryAgainResult.Canceled) + if (!clearResult) { return false; } @@ -3959,14 +3960,14 @@ private bool CloseGame(bool clearSram = false) } else if (Emulator.HasSaveRam()) { - TryAgainResult flushResult = this.DoWithTryAgainBox( + bool flushResult = this.DoWithTryAgainBox( () => FlushSaveRAM(), "Failed flushing the game's Save RAM to your disk."); - if (flushResult == TryAgainResult.Canceled) return false; + if (!flushResult) return false; } - TryAgainResult stateSaveResult = this.DoWithTryAgainBox(AutoSaveStateIfConfigured, "Failed to auto-save state."); - if (stateSaveResult == TryAgainResult.Canceled) return false; + bool stateSaveResult = this.DoWithTryAgainBox(AutoSaveStateIfConfigured, "Failed to auto-save state."); + if (!stateSaveResult) return false; StopAv(); diff --git a/src/BizHawk.Client.EmuHawk/tools/CDL.cs b/src/BizHawk.Client.EmuHawk/tools/CDL.cs index 3061b6223f3..d0cc8754472 100644 --- a/src/BizHawk.Client.EmuHawk/tools/CDL.cs +++ b/src/BizHawk.Client.EmuHawk/tools/CDL.cs @@ -214,23 +214,19 @@ public override bool AskSaveChanges() { if (_currentFilename != null) { - bool saveResult2 = RunSave(); + RunSave(); ShutdownCDL(); - return saveResult2; + return true; } } - var result = DialogController.ShowMessageBox3("Save changes to CDL session?", "CDL Save", EMsgBoxIcon.Question); - if (result == false) + // TODO - I don't like this system. It's hard to figure out how to use it. It should be done in multiple passes. + var result = DialogController.ShowMessageBox2("Save changes to CDL session?", "CDL Auto Save", EMsgBoxIcon.Question); + if (!result) { ShutdownCDL(); return true; } - else if (result == null) - { - ShutdownCDL(); - return false; - } if (string.IsNullOrWhiteSpace(_currentFilename)) { @@ -244,9 +240,9 @@ public override bool AskSaveChanges() return false; } - bool saveResult = RunSave(); + RunSave(); ShutdownCDL(); - return saveResult; + return true; } private bool _autoloading; @@ -345,20 +341,11 @@ private void OpenMenuItem_Click(object sender, EventArgs e) LoadFile(file.FullName); } - /// - /// returns false if the operation was canceled - /// - private bool RunSave() + private void RunSave() { - TryAgainResult result = this.DoWithTryAgainBox( - () => FileWriter.Write(_currentFilename, _cdl.Save), - "Failed to save CDL session."); - if (result == TryAgainResult.Saved) - { - _recent.Add(_currentFilename); - return true; - } - return result != TryAgainResult.Canceled; + _recent.Add(_currentFilename); + using var fs = new FileStream(_currentFilename, FileMode.Create, FileAccess.Write); + _cdl.Save(fs); } private void SaveMenuItem_Click(object sender, EventArgs e) @@ -399,7 +386,8 @@ private bool RunSaveAs() return false; SetCurrentFilename(file.FullName); - return RunSave(); + RunSave(); + return true; } private void SaveAsMenuItem_Click(object sender, EventArgs e) diff --git a/src/BizHawk.Client.EmuHawk/tools/Cheats/Cheats.cs b/src/BizHawk.Client.EmuHawk/tools/Cheats/Cheats.cs index 558165dc793..775f009f789 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Cheats/Cheats.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Cheats/Cheats.cs @@ -142,7 +142,7 @@ private void LoadFile(FileSystemInfo file, bool append) } } - private FileWriteResult SaveAs() + private bool SaveAs() { var fileName = MainForm.CheatList.CurrentFileName; if (string.IsNullOrWhiteSpace(fileName)) @@ -156,8 +156,7 @@ private FileWriteResult SaveAs() CheatsFSFilterSet, this); - if (file == null) return new(); - else return MainForm.CheatList.SaveFile(file.FullName); + return file != null && MainForm.CheatList.SaveFile(file.FullName); } private void Cheats_Load(object sender, EventArgs e) @@ -362,12 +361,7 @@ private void SaveMenuItem_Click(object sender, EventArgs e) { if (MainForm.CheatList.Changes) { - FileWriteResult result = MainForm.CheatList.Save(); - if (result.IsError) - { - this.ErrorMessageBox(result); - } - else + if (MainForm.CheatList.Save()) { UpdateMessageLabel(saved: true); } @@ -380,12 +374,7 @@ private void SaveMenuItem_Click(object sender, EventArgs e) private void SaveAsMenuItem_Click(object sender, EventArgs e) { - FileWriteResult result = SaveAs(); - if (result.IsError) - { - this.ErrorMessageBox(result); - } - else + if (SaveAs()) { UpdateMessageLabel(saved: true); } diff --git a/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs b/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs index e110c9a9a20..901a492c332 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs @@ -685,25 +685,15 @@ private FileInfo GetSaveFileFromUser() return result is not null ? new FileInfo(result) : null; } - private FileWriteResult SaveSessionAs() + private void SaveSessionAs() { var file = GetSaveFileFromUser(); if (file != null) { - FileWriteResult saveResult = LuaImp.ScriptList.Save(file.FullName); - if (saveResult.IsError) - { - this.ErrorMessageBox(saveResult); - OutputMessages.Text = $"Lua session could not be saved to {file.Name}"; - } - else - { - Config.RecentLuaSession.Add(file.FullName); - OutputMessages.Text = $"{file.Name} saved."; - } - return saveResult; + LuaImp.ScriptList.Save(file.FullName); + Config.RecentLuaSession.Add(file.FullName); + OutputMessages.Text = $"{file.Name} saved."; } - return new(); } private void LoadSessionFromRecent(string path) @@ -731,11 +721,7 @@ public override bool AskSaveChanges() icon: EMsgBoxIcon.Question, text: $"Save {WindowTitleStatic} session?")); if (result is null) return false; - if (result.Value) - { - TryAgainResult saveResult = this.DoWithTryAgainBox(SaveOrSaveAs, "Failed to save Lua session."); - return saveResult != TryAgainResult.Canceled; - } + if (result.Value) SaveOrSaveAs(); else LuaImp.ScriptList.Changes = false; return true; } @@ -750,20 +736,16 @@ private void UpdateRegisteredFunctionsDialog() } } - private FileWriteResult SaveOrSaveAs() + private void SaveOrSaveAs() { if (!string.IsNullOrWhiteSpace(LuaImp.ScriptList.Filename)) { - FileWriteResult result = LuaImp.ScriptList.Save(LuaImp.ScriptList.Filename); - if (!result.IsError) - { - Config.RecentLuaSession.Add(LuaImp.ScriptList.Filename); - } - return result; + LuaImp.ScriptList.Save(LuaImp.ScriptList.Filename); + Config.RecentLuaSession.Add(LuaImp.ScriptList.Filename); } else { - return SaveSessionAs(); + SaveSessionAs(); } } @@ -806,16 +788,8 @@ private void SaveSessionMenuItem_Click(object sender, EventArgs e) { if (LuaImp.ScriptList.Changes) { - FileWriteResult result = SaveOrSaveAs(); - if (result.IsError) - { - this.ErrorMessageBox(result, "Failed to save Lua session."); - OutputMessages.Text = $"Failed to save {Path.GetFileName(LuaImp.ScriptList.Filename)}"; - } - else - { - OutputMessages.Text = $"{Path.GetFileName(LuaImp.ScriptList.Filename)} saved."; - } + SaveOrSaveAs(); + OutputMessages.Text = $"{Path.GetFileName(LuaImp.ScriptList.Filename)} saved."; } } diff --git a/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs b/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs index eae31ff16d0..9b71a4a2843 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs @@ -112,7 +112,6 @@ public override bool AskSaveChanges() return true; } - // Intentionally not updating this to use FileWriter because this tool is going to be removed later. foreach (var zone in _unsavedZones) { SaveMacroAs(_zones[zone]); diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs index 8ea5d6cb7d6..ba5db1d5209 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs @@ -163,8 +163,15 @@ public override bool AskSaveChanges() text: $"Save {WindowTitleStatic} project?")); if (shouldSaveResult == true) { - TryAgainResult saveResult = this.DoWithTryAgainBox(() => SaveTas(), "Failed to save movie."); - return saveResult != TryAgainResult.Canceled; + FileWriteResult saveResult = SaveTas(); + while (saveResult.IsError && shouldSaveResult != true) + { + shouldSaveResult = this.ModalMessageBox3( + $"Failed to save movie. {saveResult.UserFriendlyErrorMessage()}\n{saveResult.Exception.Message}\n\nTry again?", + "Error", + EMsgBoxIcon.Error); + if (shouldSaveResult == true) saveResult = SaveTas(); + } } if (shouldSaveResult is null) return false; else CurrentTasMovie.ClearChanges(); diff --git a/src/BizHawk.Client.EmuHawk/tools/Watch/RamSearch.cs b/src/BizHawk.Client.EmuHawk/tools/Watch/RamSearch.cs index d1cef842326..163b6153609 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Watch/RamSearch.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Watch/RamSearch.cs @@ -1008,13 +1008,7 @@ private void SaveMenuItem_Click(object sender, EventArgs e) if (!string.IsNullOrWhiteSpace(watches.CurrentFileName)) { - FileWriteResult saveResult = watches.Save(); - if (saveResult.IsError) - { - this.ErrorMessageBox(saveResult); - MessageLabel.Text = $"Failed to save {Path.GetFileName(_currentFileName)}"; - } - else + if (watches.Save()) { _currentFileName = watches.CurrentFileName; MessageLabel.Text = $"{Path.GetFileName(_currentFileName)} saved"; @@ -1023,20 +1017,11 @@ private void SaveMenuItem_Click(object sender, EventArgs e) } else { - FileInfo/*?*/ file = GetWatchSaveFileFromUser(CurrentFileName()); - if (file != null) + var result = watches.SaveAs(GetWatchSaveFileFromUser(CurrentFileName())); + if (result) { - var result = watches.SaveAs(file); - if (result.IsError) - { - this.ErrorMessageBox(result); - MessageLabel.Text = $"Failed to save {Path.GetFileName(_currentFileName)}"; - } - else - { - MessageLabel.Text = $"{Path.GetFileName(_currentFileName)} saved"; - Settings.RecentSearches.Add(watches.CurrentFileName); - } + MessageLabel.Text = $"{Path.GetFileName(_currentFileName)} saved"; + Settings.RecentSearches.Add(watches.CurrentFileName); } } } @@ -1050,15 +1035,7 @@ private void SaveAsMenuItem_Click(object sender, EventArgs e) watches.Add(_searches[i]); } - FileInfo/*?*/ file = GetWatchSaveFileFromUser(CurrentFileName()); - if (file == null) return; - FileWriteResult result = watches.SaveAs(file); - if (result.IsError) - { - this.ErrorMessageBox(result); - MessageLabel.Text = $"Failed to save {Path.GetFileName(_currentFileName)}"; - } - else + if (watches.SaveAs(GetWatchSaveFileFromUser(CurrentFileName()))) { _currentFileName = watches.CurrentFileName; MessageLabel.Text = $"{Path.GetFileName(_currentFileName)} saved"; diff --git a/src/BizHawk.Client.EmuHawk/tools/Watch/RamWatch.cs b/src/BizHawk.Client.EmuHawk/tools/Watch/RamWatch.cs index 86288fda74a..f0f7a9b3d8d 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Watch/RamWatch.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Watch/RamWatch.cs @@ -213,8 +213,15 @@ public override bool AskSaveChanges() if (result is null) return false; if (result.Value) { - TryAgainResult saveResult = this.DoWithTryAgainBox(Save, "Failed to save watch list."); - return saveResult != TryAgainResult.Canceled; + if (string.IsNullOrWhiteSpace(_watches.CurrentFileName)) + { + SaveAs(); + } + else + { + _watches.Save(); + Config.RecentWatches.Add(_watches.CurrentFileName); + } } else { @@ -580,45 +587,13 @@ private string CurrentFileName() : Game.FilesystemSafeName(); } - private FileWriteResult SaveAs() + private void SaveAs() { - FileInfo/*?*/ file = GetWatchSaveFileFromUser(CurrentFileName()); - if (file == null) return new(); - - FileWriteResult result = _watches.SaveAs(file); - if (result.IsError) - { - MessageLabel.Text = $"Failed to save {Path.GetFileName(_watches.CurrentFileName)}"; - } - else + var result = _watches.SaveAs(GetWatchSaveFileFromUser(CurrentFileName())); + if (result) { - MessageLabel.Text = $"{Path.GetFileName(_watches.CurrentFileName)} saved"; - Config.RecentWatches.Add(_watches.CurrentFileName); UpdateStatusBar(saved: true); - } - return result; - } - - private FileWriteResult Save() - { - if (string.IsNullOrWhiteSpace(_watches.CurrentFileName)) - { - return SaveAs(); - } - else - { - FileWriteResult saveResult = _watches.Save(); - if (saveResult.IsError) - { - MessageLabel.Text = $"Failed to save {Path.GetFileName(_watches.CurrentFileName)}"; - } - else - { - MessageLabel.Text = $"{Path.GetFileName(_watches.CurrentFileName)} saved"; - Config.RecentWatches.Add(_watches.CurrentFileName); - UpdateStatusBar(saved: true); - } - return saveResult; + Config.RecentWatches.Add(_watches.CurrentFileName); } } @@ -748,20 +723,23 @@ private void OpenMenuItem_Click(object sender, EventArgs e) private void SaveMenuItem_Click(object sender, EventArgs e) { - FileWriteResult saveResult = Save(); - if (saveResult.IsError) + if (!string.IsNullOrWhiteSpace(_watches.CurrentFileName)) { - this.ErrorMessageBox(saveResult, "Failed to save watch list."); + if (_watches.Save()) + { + Config.RecentWatches.Add(_watches.CurrentFileName); + UpdateStatusBar(saved: true); + } + } + else + { + SaveAs(); } } private void SaveAsMenuItem_Click(object sender, EventArgs e) { - FileWriteResult saveResult = SaveAs(); - if (saveResult.IsError) - { - this.ErrorMessageBox(saveResult, "Failed to save watch list."); - } + SaveAs(); } private void RecentSubMenu_DropDownOpened(object sender, EventArgs e) From 8f8d592bf0f731eb6a0e117ff75735dc252998cb Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:45:10 +1000 Subject: [PATCH 10/23] Revert "Fix: CheatList was getting forcibly autosaved, after the method that properly checks the autosave config option." This reverts commit 229b500f4b0874c59be006a6cc28d030ef91a458. --- src/BizHawk.Client.Common/tools/CheatList.cs | 12 +++++++++++- src/BizHawk.Client.EmuHawk/MainForm.cs | 8 ++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/BizHawk.Client.Common/tools/CheatList.cs b/src/BizHawk.Client.Common/tools/CheatList.cs index 3f1fcd8b438..a9cca688a9a 100644 --- a/src/BizHawk.Client.Common/tools/CheatList.cs +++ b/src/BizHawk.Client.Common/tools/CheatList.cs @@ -88,10 +88,20 @@ public bool AttemptToLoadCheatFile(IMemoryDomains domains) return file.Exists && Load(domains, file.FullName, false); } - public void NewList(string defaultFileName) + public void NewList(string defaultFileName, bool autosave = false) { _defaultFileName = defaultFileName; + if (autosave && _changes && _cheatList.Count is not 0) + { + if (string.IsNullOrEmpty(CurrentFileName)) + { + CurrentFileName = _defaultFileName; + } + + Save(); + } + _cheatList.Clear(); CurrentFileName = ""; Changes = false; diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index b5fb4f2674f..a0b0ce10ded 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -3794,7 +3794,7 @@ private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFr if (previousRom != CurrentlyOpenRom) { - CheatList.NewList(Tools.GenerateDefaultCheatFilename()); + CheatList.NewList(Tools.GenerateDefaultCheatFilename(), autosave: true); if (Config.Cheats.LoadFileByGame && Emulator.HasMemoryDomains()) { if (CheatList.AttemptToLoadCheatFile(Emulator.AsMemoryDomains())) @@ -3811,7 +3811,7 @@ private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFr } else { - CheatList.NewList(Tools.GenerateDefaultCheatFilename()); + CheatList.NewList(Tools.GenerateDefaultCheatFilename(), autosave: true); } } @@ -3854,7 +3854,7 @@ private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFr DisplayManager.UpdateGlobals(Config, Emulator); DisplayManager.Blank(); ExtToolManager.BuildToolStrip(); - CheatList.NewList(""); + CheatList.NewList("", autosave: true); OnRomChanged(); return false; } @@ -4014,7 +4014,7 @@ public void LoadNullRom(bool clearSram = false) PauseOnFrame = null; CurrentlyOpenRom = null; CurrentlyOpenRomArgs = null; - CheatList.NewList(""); + CheatList.NewList("", autosave: true); OnRomChanged(); } } From 4c48610ec7732fac130bb846cb20e7a7ab6b6534 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:45:18 +1000 Subject: [PATCH 11/23] Revert "Handle SRAM saving failure." This reverts commit 3dba90f8b3524ece7a765f9029407b98e4bc85cc. --- .../Api/Classes/EmuClientApi.cs | 1 - .../DialogControllerExtensions.cs | 31 +--- src/BizHawk.Client.Common/FileWriter.cs | 17 --- src/BizHawk.Client.Common/IMainFormForApi.cs | 2 +- src/BizHawk.Client.EmuHawk/MainForm.Events.cs | 2 +- src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs | 2 +- src/BizHawk.Client.EmuHawk/MainForm.cs | 141 +++++++++++------- 7 files changed, 90 insertions(+), 106 deletions(-) diff --git a/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs index 152bdcedcf1..aaafc27af0c 100644 --- a/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs @@ -160,7 +160,6 @@ public bool OpenRom(string path) public void RebootCore() => _mainForm.RebootCore(); - // TODO: Change return type to FileWriteResult. public void SaveRam() => _mainForm.FlushSaveRAM(); // TODO: Change return type to FileWriteResult. diff --git a/src/BizHawk.Client.Common/DialogControllerExtensions.cs b/src/BizHawk.Client.Common/DialogControllerExtensions.cs index 37543c98da4..63e0a386779 100644 --- a/src/BizHawk.Client.Common/DialogControllerExtensions.cs +++ b/src/BizHawk.Client.Common/DialogControllerExtensions.cs @@ -72,40 +72,13 @@ public static void ErrorMessageBox( { Debug.Assert(fileResult.IsError && fileResult.Exception != null, "Error box must have an error."); - string prefix = prefixMessage ?? ""; + string prefix = prefixMessage == null ? "" : prefixMessage + "\n"; dialogParent.ModalMessageBox( - text: $"{prefix}\n{fileResult.UserFriendlyErrorMessage()}\n{fileResult.Exception!.Message}", + text: $"{prefix}{fileResult.UserFriendlyErrorMessage()}\n{fileResult.Exception!.Message}", caption: "Error", icon: EMsgBoxIcon.Error); } - /// - /// If the action fails, asks the user if they want to try again. - /// The user will be repeatedly asked if they want to try again until either success or the user says no. - /// - /// Returns true on success or if the user said no. Returns false if the user said cancel. - public static bool DoWithTryAgainBox( - this IDialogParent dialogParent, - Func action, - string message) - { - FileWriteResult fileResult = action(); - while (fileResult.IsError) - { - string prefix = message ?? ""; - bool? askResult = dialogParent.ModalMessageBox3( - text: $"{prefix} Do you want to try again?\n\nError details:" + - $"{fileResult.UserFriendlyErrorMessage()}\n{fileResult.Exception!.Message}", - caption: "Error", - icon: EMsgBoxIcon.Error); - if (askResult == null) return false; - if (askResult == false) return true; - if (askResult == true) fileResult = action(); - } - - return true; - } - /// Creates and shows a System.Windows.Forms.OpenFileDialog or equivalent with the receiver () as its parent /// OpenFileDialog.RestoreDirectory (isn't this useless when specifying ? keeping it for backcompat) /// OpenFileDialog.Filter diff --git a/src/BizHawk.Client.Common/FileWriter.cs b/src/BizHawk.Client.Common/FileWriter.cs index 8a0a83c22d5..a6e019b9e0b 100644 --- a/src/BizHawk.Client.Common/FileWriter.cs +++ b/src/BizHawk.Client.Common/FileWriter.cs @@ -39,23 +39,6 @@ private FileWriter(FileWritePaths paths, FileStream stream) _stream = stream; } - public static FileWriteResult Write(string path, byte[] bytes, string? backupPath = null) - { - FileWriteResult createResult = Create(path); - if (createResult.IsError) return createResult; - - try - { - createResult.Value!.Stream.Write(bytes); - } - catch (Exception ex) - { - return new(FileWriteEnum.FailedDuringWrite, createResult.Value!.Paths, ex); - } - - return createResult.Value.CloseAndDispose(backupPath); - } - /// /// Create a FileWriter instance, or return an error if unable to access the file. diff --git a/src/BizHawk.Client.Common/IMainFormForApi.cs b/src/BizHawk.Client.Common/IMainFormForApi.cs index 7f14768cc4b..72946b19b81 100644 --- a/src/BizHawk.Client.Common/IMainFormForApi.cs +++ b/src/BizHawk.Client.Common/IMainFormForApi.cs @@ -56,7 +56,7 @@ public interface IMainFormForApi void EnableRewind(bool enabled); /// only referenced from - FileWriteResult FlushSaveRAM(bool autosave = false); + bool FlushSaveRAM(bool autosave = false); /// only referenced from void FrameAdvance(bool discardApiHawkSurfaces = true); diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs index eecd5bfddb3..ff147f665e8 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs @@ -315,7 +315,7 @@ private bool LoadstateCurrentSlot() private void FlushSaveRAMMenuItem_Click(object sender, EventArgs e) { - ShowMessageIfError(() => FlushSaveRAM(), "Failed to flush saveram!"); + FlushSaveRAM(); } private void ReadonlyMenuItem_Click(object sender, EventArgs e) diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs b/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs index 6d28e5da85b..e76b8a6d21c 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs @@ -126,7 +126,7 @@ void SelectAndLoadFromSlot(int slot) LoadMostRecentROM(); break; case "Flush SaveRAM": - FlushSaveRAMMenuItem_Click(null, EventArgs.Empty); + FlushSaveRAM(); break; case "Display FPS": ToggleFps(); diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index a0b0ce10ded..dd562ff2c50 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -1735,7 +1735,6 @@ public bool RunLibretroCoreChooser() // countdown for saveram autoflushing public int AutoFlushSaveRamIn { get; set; } - private bool AutoFlushSaveRamFailed; private void SetStatusBar() { @@ -1938,7 +1937,7 @@ private void LoadSaveRam() } } - public FileWriteResult FlushSaveRAM(bool autosave = false) + public bool FlushSaveRAM(bool autosave = false) { if (Emulator.HasSaveRam()) { @@ -1952,13 +1951,53 @@ public FileWriteResult FlushSaveRAM(bool autosave = false) path = Config.PathEntries.SaveRamAbsolutePath(Game, MovieSession.Movie); } + var file = new FileInfo(path); + var newPath = $"{path}.new"; + var newFile = new FileInfo(newPath); + var backupPath = $"{path}.bak"; + var backupFile = new FileInfo(backupPath); + var saveram = Emulator.AsSaveRam().CloneSaveRam(); if (saveram == null) - return new(); - return FileWriter.Write(path, saveram, $"{path}.bak"); + return true; + + try + { + Directory.CreateDirectory(file.DirectoryName!); + using (var fs = File.Create(newPath)) + { + fs.Write(saveram, 0, saveram.Length); + fs.Flush(flushToDisk: true); + } + + if (file.Exists) + { + if (Config.BackupSaveram) + { + if (backupFile.Exists) + { + backupFile.Delete(); + } + + file.MoveTo(backupPath); + } + else + { + file.Delete(); + } + } + + newFile.MoveTo(path); + } + catch (IOException e) + { + AddOnScreenMessage("Failed to flush saveram!"); + Console.Error.WriteLine(e); + return false; + } } - return new(); + return true; } private void RewireSound() @@ -2983,22 +3022,8 @@ private void StepRunLoop_Core(bool force = false) AutoFlushSaveRamIn--; if (AutoFlushSaveRamIn <= 0) { - FileWriteResult result = FlushSaveRAM(true); - if (result.IsError) - { - // For autosave, allow one failure before bothering the user. - if (AutoFlushSaveRamFailed) - { - this.ErrorMessageBox(result, "Failed to flush saveram!"); - } - AutoFlushSaveRamFailed = true; - AutoFlushSaveRamIn = Math.Min(600, Config.FlushSaveRamFrames); - } - else - { - AutoFlushSaveRamFailed = false; - AutoFlushSaveRamIn = Config.FlushSaveRamFrames; - } + FlushSaveRAM(true); + AutoFlushSaveRamIn = Config.FlushSaveRamFrames; } } // why not skip audio if the user doesn't want sound @@ -3940,34 +3965,41 @@ private bool CloseGame(bool clearSram = false) var path = Config.PathEntries.SaveRamAbsolutePath(Game, MovieSession.Movie); if (File.Exists(path)) { - bool clearResult = this.DoWithTryAgainBox(() => { - try - { - File.Delete(path); - AddOnScreenMessage("SRAM cleared."); - return new(); - } - catch (Exception ex) - { - return new(FileWriteEnum.FailedToDeleteGeneric, new(path, ""), ex); - } - }, "Failed to clear SRAM."); - if (!clearResult) - { - return false; - } + File.Delete(path); + AddOnScreenMessage("SRAM cleared."); } } else if (Emulator.HasSaveRam()) { - bool flushResult = this.DoWithTryAgainBox( - () => FlushSaveRAM(), - "Failed flushing the game's Save RAM to your disk."); - if (!flushResult) return false; + while (true) + { + if (FlushSaveRAM()) break; + + var result = ShowMessageBox3( + owner: this, + "Failed flushing the game's Save RAM to your disk.\n" + + "Do you want to try again?", + "IOError while writing SaveRAM", + EMsgBoxIcon.Error); + + if (result is false) break; + if (result is null) return false; + } } - bool stateSaveResult = this.DoWithTryAgainBox(AutoSaveStateIfConfigured, "Failed to auto-save state."); - if (!stateSaveResult) return false; + bool? tryAgain = null; + do + { + FileWriteResult stateSaveResult = AutoSaveStateIfConfigured(); + if (stateSaveResult.IsError) + { + tryAgain = this.ShowMessageBox3( + $"Failed to auto-save state. Do you want to try again?\n\nError details:\n{stateSaveResult.UserFriendlyErrorMessage()}\n{stateSaveResult.Exception.Message}", + "IOError while writing savestate", + EMsgBoxIcon.Error); + if (tryAgain == null) return false; + } + } while (tryAgain == true); StopAv(); @@ -4247,7 +4279,11 @@ public FileWriteResult SaveQuickSave(int slot, bool suppressOSD = false) /// private void SaveQuickSaveAndShowError(int slot) { - ShowMessageIfError(() => SaveQuickSave(slot), "Quick save failed."); + FileWriteResult result = SaveQuickSave(slot); + if (result.IsError) + { + this.ErrorMessageBox(result, "Quick save failed."); + } } public bool EnsureCoreIsAccurate() @@ -4317,9 +4353,11 @@ private void SaveStateAs() initFileName: $"{SaveStatePrefix()}.QuickSave0.State"); if (shouldSaveResult is not null) { - ShowMessageIfError( - () => SaveState(path: shouldSaveResult, userFriendlyStateName: shouldSaveResult), - "Unable to save state."); + FileWriteResult saveResult = SaveState(path: shouldSaveResult, userFriendlyStateName: shouldSaveResult); + if (saveResult.IsError) + { + this.ErrorMessageBox(saveResult, "Unable to save."); + } } if (Tools.IsLoaded()) @@ -4633,15 +4671,6 @@ public bool ShowMessageBox2( _ => null, }; - public void ShowMessageIfError(Func action, string message) - { - FileWriteResult result = action(); - if (result.IsError) - { - this.ErrorMessageBox(result, message); - } - } - public void StartSound() => Sound.StartSound(); public void StopSound() => Sound.StopSound(); From 228708182fcf7f97203d8f55f613675574b06a60 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 01:45:32 +1000 Subject: [PATCH 12/23] Revert "Move stuff around to make more sense for ask-user-about-saving-or-canceling flow even though not all asking and error handling is implemented yet. Also, fix: respect the cancel button." This reverts commit 479e28181f914f90bac6eba8517ce83f556daa9c. --- src/BizHawk.Client.EmuHawk/MainForm.cs | 74 +++++++++++++------------- 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index dd562ff2c50..837c4046dfc 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -879,18 +879,31 @@ private void CheckMayCloseAndCleanup(object/*?*/ closingSender, CancelEventArgs closingArgs.Cancel = true; return; } - // StopAv would be handled in CloseGame, but since we've asked the user about it, best to handle it now. StopAv(); } - SaveConfig(); // TODO: Handle failure. - - if (!CloseGame()) + if (!Tools.AskSave()) { closingArgs.Cancel = true; return; } + Tools.Close(); + FileWriteResult saveResult = MovieSession.StopMovie(); + if (saveResult.IsError) + { + if (!this.ModalMessageBox2( + caption: "Quit anyway?", + icon: EMsgBoxIcon.Question, + text: "The currently playing movie could not be saved. Continue and quit anyway? All unsaved changes will be lost.")) + { + closingArgs.Cancel = true; + return; + } + } + + CloseGame(); + SaveConfig(); } private readonly bool _suppressSyncSettingsWarning; @@ -2111,7 +2124,7 @@ private void LoadRomFromRecent(string rom) if (!LoadRom(romPath, new LoadRomArgs(ioa), out var failureIsFromAskSave)) { - if (failureIsFromAskSave) AddOnScreenMessage("ROM loading cancelled due to unsaved changes"); + if (failureIsFromAskSave) AddOnScreenMessage("ROM loading cancelled; a tool had unsaved changes"); else if (ioa is OpenAdvanced_LibretroNoGame || File.Exists(romPath)) AddOnScreenMessage("ROM loading failed"); else Config.RecentRoms.HandleLoadError(this, romPath, rom); } @@ -3604,12 +3617,6 @@ public bool LoadRom(string path, LoadRomArgs args, out bool failureIsFromAskSave private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFromAskSave) { failureIsFromAskSave = false; - if (!CloseGame()) - { - failureIsFromAskSave = true; - return false; - } - if (path == null) throw new ArgumentNullException(nameof(path)); if (args == null) @@ -3637,6 +3644,12 @@ private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFr // it is then up to the core itself to override its own local DeterministicEmulation setting bool deterministic = args.Deterministic ?? MovieSession.NewMovieQueued; + if (!Tools.AskSave()) + { + failureIsFromAskSave = true; + return false; + } + var loader = new RomLoader(Config, this) { ChooseArchive = LoadArchiveChooser, @@ -3650,6 +3663,8 @@ private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFr loader.OnLoadSettings += CoreSettings; loader.OnLoadSyncSettings += CoreSyncSettings; + CloseGame(); + var nextComm = CreateCoreComm(); IOpenAdvanced ioa = args.OpenAdvanced; @@ -3936,30 +3951,8 @@ private void CommitCoreSettingsToConfig() /// This closes the game but does not set things up for using the client with the new null emulator. /// This method should only be called (outside of ) if the caller is about to load a new game with no user interaction between close and load. /// - /// True if the game was closed. False if the user cancelled due to unsaved changes. - private bool CloseGame(bool clearSram = false) + private void CloseGame(bool clearSram = false) { - CommitCoreSettingsToConfig(); // Must happen before stopping the movie, since it checks for active movie. - - if (!Tools.AskSave()) - { - return false; - } - // There is a cheats tool, but cheats can be active while the "cheats tool" is not. And have auto-save option. - CheatList.SaveOnClose(); - - FileWriteResult saveResult = MovieSession.StopMovie(); - if (saveResult.IsError) - { - if (!this.ModalMessageBox2( - caption: "Quit anyway?", - icon: EMsgBoxIcon.Question, - text: "The currently playing movie could not be saved. Continue and quit anyway? All unsaved changes will be lost.")) - { - return false; - } - } - if (clearSram) { var path = Config.PathEntries.SaveRamAbsolutePath(Game, MovieSession.Movie); @@ -3983,7 +3976,7 @@ private bool CloseGame(bool clearSram = false) EMsgBoxIcon.Error); if (result is false) break; - if (result is null) return false; + if (result is null) return; } } @@ -3997,16 +3990,23 @@ private bool CloseGame(bool clearSram = false) $"Failed to auto-save state. Do you want to try again?\n\nError details:\n{stateSaveResult.UserFriendlyErrorMessage()}\n{stateSaveResult.Exception.Message}", "IOError while writing savestate", EMsgBoxIcon.Error); - if (tryAgain == null) return false; + if (tryAgain == null) return; } } while (tryAgain == true); StopAv(); + CommitCoreSettingsToConfig(); DisableRewind(); + if (MovieSession.Movie.IsActive()) // Note: this must be called after CommitCoreSettingsToConfig() because it checks if we have an active movie. + { + StopMovie(); + } + RA?.Stop(); + CheatList.SaveOnClose(); Emulator.Dispose(); // This stuff might belong in LoadNullRom. @@ -4017,8 +4017,6 @@ private bool CloseGame(bool clearSram = false) InputManager.SyncControls(Emulator, MovieSession, Config); RewireSound(); RebootStatusBarIcon.Visible = false; - - return true; } private FileWriteResult AutoSaveStateIfConfigured() From 399214db936369dc601f03cb5f9bc13a98316103 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:04:36 +1000 Subject: [PATCH 13/23] revert intervening changeset 2 to `IMainFormForTools.cs` --- src/BizHawk.Client.Common/IMainFormForTools.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/BizHawk.Client.Common/IMainFormForTools.cs b/src/BizHawk.Client.Common/IMainFormForTools.cs index 798f6e181ea..5212a46cfdd 100644 --- a/src/BizHawk.Client.Common/IMainFormForTools.cs +++ b/src/BizHawk.Client.Common/IMainFormForTools.cs @@ -20,6 +20,9 @@ public interface IMainFormForTools : IDialogController /// only referenced from PlaybackBox bool HoldFrameAdvance { get; set; } + /// only referenced from BasicBot + bool InvisibleEmulation { get; set; } + /// only referenced from LuaConsole bool IsTurboing { get; } From d82cc7e44ea928bcf1188c5d671405118cef79bd Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:07:42 +1000 Subject: [PATCH 14/23] revert intervening changeset 1 to `IMainFormForTools.cs` --- .../IMainFormForTools.cs | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/src/BizHawk.Client.Common/IMainFormForTools.cs b/src/BizHawk.Client.Common/IMainFormForTools.cs index 5212a46cfdd..520fe9b8873 100644 --- a/src/BizHawk.Client.Common/IMainFormForTools.cs +++ b/src/BizHawk.Client.Common/IMainFormForTools.cs @@ -1,7 +1,8 @@ using BizHawk.Bizware.Graphics; +using BizHawk.Client.Common; using BizHawk.Emulation.Common; -namespace BizHawk.Client.Common +namespace BizHawk.Client.EmuHawk { public interface IMainFormForTools : IDialogController { @@ -11,95 +12,95 @@ public interface IMainFormForTools : IDialogController /// referenced by 3 or more tools string CurrentlyOpenRom { get; } - /// referenced from HexEditor and RetroAchievements + /// referenced from and RetroAchievements LoadRomArgs CurrentlyOpenRomArgs { get; } - /// only referenced from TAStudio + /// only referenced from bool EmulatorPaused { get; } - /// only referenced from PlaybackBox + /// only referenced from bool HoldFrameAdvance { get; set; } - /// only referenced from BasicBot + /// only referenced from bool InvisibleEmulation { get; set; } - /// only referenced from LuaConsole + /// only referenced from bool IsTurboing { get; } - /// only referenced from TAStudio + /// only referenced from bool IsFastForwarding { get; } - /// referenced from PlayMovie and TAStudio + /// referenced from and int? PauseOnFrame { get; set; } - /// only referenced from PlaybackBox + /// only referenced from bool PressRewind { get; set; } - /// referenced from BookmarksBranchesBox and VideoWriterChooserForm + /// referenced from and BitmapBuffer CaptureOSD(); - /// only referenced from TAStudio + /// only referenced from void DisableRewind(); - /// only referenced from TAStudio + /// only referenced from void EnableRewind(bool enabled); - /// only referenced from TAStudio + /// only referenced from bool EnsureCoreIsAccurate(); - /// only referenced from TAStudio + /// only referenced from void FrameAdvance(bool discardApiHawkSurfaces = true); - /// only referenced from LuaConsole + /// only referenced from /// Override void FrameBufferResized(bool forceWindowResize = false); - /// only referenced from BasicBot + /// only referenced from bool LoadQuickSave(int slot, bool suppressOSD = false); - /// referenced from MultiDiskBundler and RetroAchievements + /// referenced from and RetroAchievements bool LoadRom(string path, LoadRomArgs args); - /// only referenced from BookmarksBranchesBox + /// only referenced from BitmapBuffer MakeScreenshotImage(); - /// referenced from ToolFormBase + /// referenced from void MaybePauseFromMenuOpened(); - /// referenced from ToolFormBase + /// referenced from void MaybeUnpauseFromMenuClosed(); /// referenced by 3 or more tools void PauseEmulator(); - /// only referenced from TAStudio + /// only referenced from bool BlockFrameAdvance { get; set; } - /// referenced from PlaybackBox and TAStudio + /// referenced from and void SetMainformMovieInfo(); /// referenced by 3 or more tools bool StartNewMovie(IMovie movie, bool newMovie); - /// only referenced from BasicBot + /// only referenced from void Throttle(); - /// only referenced from TAStudio + /// only referenced from void TogglePause(); /// referenced by 3 or more tools void UnpauseEmulator(); - /// only referenced from BasicBot + /// only referenced from void Unthrottle(); - /// only referenced from LogWindow + /// only referenced from void UpdateDumpInfo(RomStatus? newStatus = null); - /// only referenced from BookmarksBranchesBox + /// only referenced from void UpdateStatusSlots(); - /// only referenced from TAStudio + /// referenced from and RetroAchievements void UpdateWindowTitle(); } } From 9b170eca4f29e99b6ce85b701ae58d7a4ea048dc Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:07:54 +1000 Subject: [PATCH 15/23] Revert "Update comments, rename method, and remove obsolete things." This reverts commit 9fd91957df27ada8eb25289560aa9aa25ea33e1c. --- .../Api/Classes/EmuClientApi.cs | 2 +- src/BizHawk.Client.Common/FileWriter.cs | 3 +- src/BizHawk.Client.Common/IMainFormForApi.cs | 2 +- .../IMainFormForTools.cs | 3 ++ src/BizHawk.Client.EmuHawk/MainForm.Events.cs | 4 +-- src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs | 2 +- src/BizHawk.Client.EmuHawk/MainForm.cs | 32 ++++++++++--------- .../TAStudio/TAStudio.IControlMainForm.cs | 11 ++++--- 8 files changed, 34 insertions(+), 25 deletions(-) diff --git a/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs index aaafc27af0c..5805a8aa347 100644 --- a/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs @@ -88,7 +88,7 @@ private void CallStateSaved(object sender, StateSavedEventArgs args) public void CloseEmulator(int? exitCode = null) => _mainForm.CloseEmulator(exitCode); - public void CloseRom() => _mainForm.LoadNullRom(); + public void CloseRom() => _mainForm.CloseRom(); public void DisplayMessages(bool value) => _config.DisplayMessages = value; diff --git a/src/BizHawk.Client.Common/FileWriter.cs b/src/BizHawk.Client.Common/FileWriter.cs index a6e019b9e0b..039c8f47693 100644 --- a/src/BizHawk.Client.Common/FileWriter.cs +++ b/src/BizHawk.Client.Common/FileWriter.cs @@ -167,7 +167,8 @@ public void Dispose() if (_dispoed) return; _dispoed = true; - _stream!.Dispose(); + _stream!.Flush(flushToDisk: true); + _stream.Dispose(); _stream = null; // The caller should call CloseAndDispose and handle potential failure. diff --git a/src/BizHawk.Client.Common/IMainFormForApi.cs b/src/BizHawk.Client.Common/IMainFormForApi.cs index 72946b19b81..83484d5c630 100644 --- a/src/BizHawk.Client.Common/IMainFormForApi.cs +++ b/src/BizHawk.Client.Common/IMainFormForApi.cs @@ -47,7 +47,7 @@ public interface IMainFormForApi void CloseEmulator(int? exitCode = null); /// only referenced from - void LoadNullRom(bool clearSram = false); + void CloseRom(bool clearSram = false); /// only referenced from IDecodeResult DecodeCheatForAPI(string code, out MemoryDomain/*?*/ domain); diff --git a/src/BizHawk.Client.Common/IMainFormForTools.cs b/src/BizHawk.Client.Common/IMainFormForTools.cs index 520fe9b8873..417fabaccac 100644 --- a/src/BizHawk.Client.Common/IMainFormForTools.cs +++ b/src/BizHawk.Client.Common/IMainFormForTools.cs @@ -18,6 +18,9 @@ public interface IMainFormForTools : IDialogController /// only referenced from bool EmulatorPaused { get; } + /// only referenced from + bool GameIsClosing { get; } + /// only referenced from bool HoldFrameAdvance { get; set; } diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs index ff147f665e8..9deec800e43 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs @@ -264,7 +264,7 @@ private void OpenAdvancedMenuItem_Click(object sender, EventArgs e) private void CloseRomMenuItem_Click(object sender, EventArgs e) { Console.WriteLine($"Closing rom clicked Frame: {Emulator.Frame} Emulator: {Emulator.GetType().Name}"); - LoadNullRom(); + CloseRom(); Console.WriteLine($"Closing rom clicked DONE Frame: {Emulator.Frame} Emulator: {Emulator.GetType().Name}"); } @@ -1400,7 +1400,7 @@ private void UndoSavestateContextMenuItem_Click(object sender, EventArgs e) private void ClearSramContextMenuItem_Click(object sender, EventArgs e) { - LoadNullRom(clearSram: true); + CloseRom(clearSram: true); } private void ShowMenuContextMenuItem_Click(object sender, EventArgs e) diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs b/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs index e76b8a6d21c..6903825285c 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs @@ -120,7 +120,7 @@ void SelectAndLoadFromSlot(int slot) OpenRom(); break; case "Close ROM": - LoadNullRom(); + CloseRom(); break; case "Load Last ROM": LoadMostRecentROM(); diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index 837c4046dfc..cf9b33e00c2 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -901,7 +901,7 @@ private void CheckMayCloseAndCleanup(object/*?*/ closingSender, CancelEventArgs return; } } - + // zero 03-nov-2015 - close game after other steps. tools might need to unhook themselves from a core. CloseGame(); SaveConfig(); } @@ -3663,6 +3663,10 @@ private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFr loader.OnLoadSettings += CoreSettings; loader.OnLoadSyncSettings += CoreSyncSettings; + // this also happens in CloseGame(). But it needs to happen here since if we're restarting with the same core, + // any settings changes that we made need to make it back to config before we try to instantiate that core with + // the new settings objects + CommitCoreSettingsToConfig(); // adelikat: I Think by reordering things, this isn't necessary anymore CloseGame(); var nextComm = CreateCoreComm(); @@ -3947,12 +3951,13 @@ private void CommitCoreSettingsToConfig() } } - /// - /// This closes the game but does not set things up for using the client with the new null emulator. - /// This method should only be called (outside of ) if the caller is about to load a new game with no user interaction between close and load. - /// + // whats the difference between these two methods?? + // its very tricky. rename to be more clear or combine them. + // This gets called whenever a core related thing is changed. + // Like reboot core. private void CloseGame(bool clearSram = false) { + GameIsClosing = true; if (clearSram) { var path = Config.PathEntries.SaveRamAbsolutePath(Game, MovieSession.Movie); @@ -3999,7 +4004,7 @@ private void CloseGame(bool clearSram = false) CommitCoreSettingsToConfig(); DisableRewind(); - if (MovieSession.Movie.IsActive()) // Note: this must be called after CommitCoreSettingsToConfig() because it checks if we have an active movie. + if (MovieSession.Movie.IsActive()) // Note: this must be called after CommitCoreSettingsToConfig() { StopMovie(); } @@ -4008,15 +4013,12 @@ private void CloseGame(bool clearSram = false) CheatList.SaveOnClose(); Emulator.Dispose(); - - // This stuff might belong in LoadNullRom. - // However, Emulator.IsNull is used all over and at least one use (in LoadRomInternal) appears to depend on this code being here. - // Some refactoring is needed if these things are to be actually moved to LoadNullRom. Emulator = new NullEmulator(); Game = GameInfo.NullInstance; InputManager.SyncControls(Emulator, MovieSession, Config); RewireSound(); RebootStatusBarIcon.Visible = false; + GameIsClosing = false; } private FileWriteResult AutoSaveStateIfConfigured() @@ -4029,12 +4031,12 @@ private FileWriteResult AutoSaveStateIfConfigured() return new(); } - /// - /// This closes the current ROM, closes tools that require emulator services, and sets things up for the user to interact with the client having no loaded ROM. - /// - /// True if SRAM should be deleted instead of saved. - public void LoadNullRom(bool clearSram = false) + public bool GameIsClosing { get; private set; } // Lets tools make better decisions when being called by CloseGame + + public void CloseRom(bool clearSram = false) { + // This gets called after Close Game gets called. + // Tested with NESHawk and SMB3 (U) if (Tools.AskSave()) { CloseGame(clearSram); diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IControlMainForm.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IControlMainForm.cs index 6455b24610f..fc5192a0f72 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IControlMainForm.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IControlMainForm.cs @@ -59,10 +59,13 @@ public void ToggleReadOnly() public void StopMovie(bool suppressSave) { - Activate(); - _suppressAskSave = suppressSave; - StartNewTasMovie(); - _suppressAskSave = false; + if (!MainForm.GameIsClosing) + { + Activate(); + _suppressAskSave = suppressSave; + StartNewTasMovie(); + _suppressAskSave = false; + } } public bool WantsToControlRewind { get; private set; } = true; From d26da27bb5bdca4787e60cc00d11aecd8f26c898 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:08:02 +1000 Subject: [PATCH 16/23] Revert "Handle most errors related to savestate files. Autosave last slot is still broken on close and on TAStudio open." This reverts commit 199ae5e2e306184a3ceddb8c678edcfb77f2e549. --- .../Api/Classes/EmuClientApi.cs | 11 +-- .../Api/Classes/SaveStateApi.cs | 17 +--- .../DialogControllerExtensions.cs | 15 --- src/BizHawk.Client.Common/FileWriteResult.cs | 24 +---- src/BizHawk.Client.Common/IMainFormForApi.cs | 12 +-- src/BizHawk.Client.Common/SaveSlotManager.cs | 60 +++--------- .../savestates/SavestateFile.cs | 5 +- src/BizHawk.Client.EmuHawk/MainForm.Events.cs | 17 +--- src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs | 2 +- src/BizHawk.Client.EmuHawk/MainForm.cs | 97 ++++++------------- 10 files changed, 56 insertions(+), 204 deletions(-) diff --git a/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs index 5805a8aa347..f0f4614f8d9 100644 --- a/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs @@ -162,16 +162,7 @@ public bool OpenRom(string path) public void SaveRam() => _mainForm.FlushSaveRAM(); - // TODO: Change return type to FileWriteResult. - // We may wish to change more than that, since we have a mostly-dupicate ISaveStateApi.Save, neither has documentation indicating what the differences are. - public void SaveState(string name) - { - FileWriteResult result = _mainForm.SaveState(Path.Combine(_config.PathEntries.SaveStateAbsolutePath(Game.System), $"{name}.State"), name); - if (result.Exception != null && result.Exception is not UnlessUsingApiException) - { - throw result.Exception; - } - } + public void SaveState(string name) => _mainForm.SaveState(Path.Combine(_config.PathEntries.SaveStateAbsolutePath(Game.System), $"{name}.State"), name, fromLua: false); public int ScreenHeight() => _displayManager.GetPanelNativeSize().Height; diff --git a/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs b/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs index 271f036ed20..9bbe860e9c5 100644 --- a/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs @@ -39,17 +39,8 @@ public bool LoadSlot(int slotNum, bool suppressOSD) return _mainForm.LoadQuickSave(slotNum, suppressOSD: suppressOSD); } - // TODO: Change return type FileWriteResult. - public void Save(string path, bool suppressOSD) - { - FileWriteResult result = _mainForm.SaveState(path, path, suppressOSD); - if (result.Exception != null && result.Exception is not UnlessUsingApiException) - { - throw result.Exception; - } - } + public void Save(string path, bool suppressOSD) => _mainForm.SaveState(path, path, true, suppressOSD); - // TODO: Change return type to FileWriteResult. public void SaveSlot(int slotNum, bool suppressOSD) { if (slotNum is < 0 or > 10) throw new ArgumentOutOfRangeException(paramName: nameof(slotNum), message: ERR_MSG_NOT_A_SLOT); @@ -58,11 +49,7 @@ public void SaveSlot(int slotNum, bool suppressOSD) LogCallback(ERR_MSG_USE_SLOT_10); slotNum = 10; } - FileWriteResult result = _mainForm.SaveQuickSave(slotNum, suppressOSD: suppressOSD); - if (result.Exception != null && result.Exception is not UnlessUsingApiException) - { - throw result.Exception; - } + _mainForm.SaveQuickSave(slotNum, suppressOSD: suppressOSD, fromLua: true); } } } diff --git a/src/BizHawk.Client.Common/DialogControllerExtensions.cs b/src/BizHawk.Client.Common/DialogControllerExtensions.cs index 63e0a386779..3c90b47f097 100644 --- a/src/BizHawk.Client.Common/DialogControllerExtensions.cs +++ b/src/BizHawk.Client.Common/DialogControllerExtensions.cs @@ -1,7 +1,6 @@ #nullable enable using System.Collections.Generic; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; namespace BizHawk.Client.Common @@ -65,20 +64,6 @@ public static bool ModalMessageBox2( EMsgBoxIcon? icon = null) => dialogParent.DialogController.ShowMessageBox3(owner: dialogParent, text: text, caption: caption, icon: icon); - public static void ErrorMessageBox( - this IDialogParent dialogParent, - FileWriteResult fileResult, - string? prefixMessage = null) - { - Debug.Assert(fileResult.IsError && fileResult.Exception != null, "Error box must have an error."); - - string prefix = prefixMessage == null ? "" : prefixMessage + "\n"; - dialogParent.ModalMessageBox( - text: $"{prefix}{fileResult.UserFriendlyErrorMessage()}\n{fileResult.Exception!.Message}", - caption: "Error", - icon: EMsgBoxIcon.Error); - } - /// Creates and shows a System.Windows.Forms.OpenFileDialog or equivalent with the receiver () as its parent /// OpenFileDialog.RestoreDirectory (isn't this useless when specifying ? keeping it for backcompat) /// OpenFileDialog.Filter diff --git a/src/BizHawk.Client.Common/FileWriteResult.cs b/src/BizHawk.Client.Common/FileWriteResult.cs index c11193e347c..633efbd7a3c 100644 --- a/src/BizHawk.Client.Common/FileWriteResult.cs +++ b/src/BizHawk.Client.Common/FileWriteResult.cs @@ -7,17 +7,12 @@ namespace BizHawk.Client.Common public enum FileWriteEnum { Success, - // Failures during a FileWriter write. FailedToOpen, FailedDuringWrite, FailedToDeleteOldBackup, FailedToMakeBackup, FailedToDeleteOldFile, FailedToRename, - Aborted, - // Failures from other sources - FailedToDeleteGeneric, - FailedToMoveForSwap, } /// @@ -31,7 +26,7 @@ public class FileWriteResult public bool IsError => Error != FileWriteEnum.Success; - public FileWriteResult(FileWriteEnum error, FileWritePaths writer, Exception? exception) + internal FileWriteResult(FileWriteEnum error, FileWritePaths writer, Exception? exception) { Error = error; Exception = exception; @@ -70,15 +65,6 @@ public string UserFriendlyErrorMessage() return $"The file \"{Paths.Final}\" could not be created."; case FileWriteEnum.FailedDuringWrite: return $"An error occurred while writing the file."; // No file name here; it should be deleted. - case FileWriteEnum.Aborted: - return "The operation was aborted."; - - case FileWriteEnum.FailedToDeleteGeneric: - return $"The file \"{Paths.Final}\" could not be deleted."; - //case FileWriteEnum.FailedToDeleteForSwap: - // return $"Failed to swap files. Unable to write to \"{Paths.Final}\""; - case FileWriteEnum.FailedToMoveForSwap: - return $"Failed to swap files. Unable to rename \"{Paths.Temp}\" to \"{Paths.Final}\""; } string success = $"The file was created successfully at \"{Paths.Temp}\" but could not be moved"; @@ -120,12 +106,4 @@ internal FileWriteResult(T value, FileWritePaths paths) : base(FileWriteEnum.Suc public FileWriteResult(FileWriteResult other) : base(other.Error, other.Paths, other.Exception) { } } - - /// - /// This only exists as a way to avoid changing the API behavior. - /// - public class UnlessUsingApiException : Exception - { - public UnlessUsingApiException(string message) : base(message) { } - } } diff --git a/src/BizHawk.Client.Common/IMainFormForApi.cs b/src/BizHawk.Client.Common/IMainFormForApi.cs index 83484d5c630..63105920310 100644 --- a/src/BizHawk.Client.Common/IMainFormForApi.cs +++ b/src/BizHawk.Client.Common/IMainFormForApi.cs @@ -100,17 +100,11 @@ public interface IMainFormForApi /// only referenced from bool RestartMovie(); - FileWriteResult SaveQuickSave(int slot, bool suppressOSD = false); + /// only referenced from + void SaveQuickSave(int slot, bool suppressOSD = false, bool fromLua = false); - /// - /// Creates a savestate and writes it to a file. - /// - /// The path of the file to write. - /// The name to use for the state on the client's on-screen display. - /// If true, the client will not show a success message. - /// Returns a value indicating if there was an error and (if there was) why. /// referenced from and - FileWriteResult SaveState(string path, string userFriendlyStateName, bool suppressOSD = false); + void SaveState(string path, string userFriendlyStateName, bool fromLua = false, bool suppressOSD = false); /// only referenced from void StepRunLoop_Throttle(); diff --git a/src/BizHawk.Client.Common/SaveSlotManager.cs b/src/BizHawk.Client.Common/SaveSlotManager.cs index 774e9821034..ea95e684f86 100644 --- a/src/BizHawk.Client.Common/SaveSlotManager.cs +++ b/src/BizHawk.Client.Common/SaveSlotManager.cs @@ -69,65 +69,31 @@ public void ToggleRedo(IMovie movie, int slot) public bool IsRedo(IMovie movie, int slot) => slot is >= 1 and <= 10 && movie is not ITasMovie && _redo[slot - 1]; - /// - /// Takes the .state and .bak files and swaps them - /// - public FileWriteResult SwapBackupSavestate(IMovie movie, string path, int currentSlot) + public void SwapBackupSavestate(IMovie movie, string path, int currentSlot) { - string backupPath = $"{path}.bak"; - string tempPath = $"{path}.bak.tmp"; - + // Takes the .state and .bak files and swaps them var state = new FileInfo(path); - var backup = new FileInfo(backupPath); + var backup = new FileInfo($"{path}.bak"); + var temp = new FileInfo($"{path}.bak.tmp"); if (!state.Exists || !backup.Exists) { - return new(); + return; } - // Delete old temp file if it exists. - try - { - if (File.Exists(tempPath)) File.Delete(tempPath); - } - catch (Exception ex) + if (temp.Exists) { - return new(FileWriteEnum.FailedToDeleteGeneric, new(tempPath, ""), ex); + temp.Delete(); } - // Move backup to temp. - try - { - backup.MoveTo(tempPath); - } - catch (Exception ex) - { - return new(FileWriteEnum.FailedToMoveForSwap, new(tempPath, backupPath), ex); - } - // Move current to backup. - try - { - state.MoveTo(backupPath); - } - catch (Exception ex) - { - // Attempt to restore the backup - try { backup.MoveTo(backupPath); } catch { /* eat? unlikely to fail here */ } - return new(FileWriteEnum.FailedToMoveForSwap, new(backupPath, path), ex); - } - // Move backup to current. - try - { - backup.MoveTo(path); - } - catch (Exception ex) - { - // Should we attempt to restore? Unlikely to fail here since we've already touched all files. - return new(FileWriteEnum.FailedToMoveForSwap, new(path, tempPath), ex); - } + backup.CopyTo($"{path}.bak.tmp"); + backup.Delete(); + state.CopyTo($"{path}.bak"); + state.Delete(); + temp.CopyTo(path); + temp.Delete(); ToggleRedo(movie, currentSlot); - return new(); } } } diff --git a/src/BizHawk.Client.Common/savestates/SavestateFile.cs b/src/BizHawk.Client.Common/savestates/SavestateFile.cs index 966ca04c234..393f8ac76ca 100644 --- a/src/BizHawk.Client.Common/savestates/SavestateFile.cs +++ b/src/BizHawk.Client.Common/savestates/SavestateFile.cs @@ -58,7 +58,7 @@ public SavestateFile( _userBag = userBag; } - public FileWriteResult Create(string filename, SaveStateConfig config, bool makeBackup) + public FileWriteResult Create(string filename, SaveStateConfig config) { FileWriteResult createResult = ZipStateSaver.Create(filename, config.CompressionLevelNormal); if (createResult.IsError) return createResult; @@ -132,8 +132,7 @@ public FileWriteResult Create(string filename, SaveStateConfig config, bool make bs.PutLump(BinaryStateLump.LagLog, tw => tasMovie.LagLog.Save(tw), zstdCompress: true); } - makeBackup = makeBackup && config.MakeBackups; - return bs.CloseAndDispose(makeBackup ? $"{filename}.bak" : null); + return bs.CloseAndDispose(); } public bool Load(string path, IDialogParent dialogParent) diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs index 9deec800e43..a2eef27097d 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs @@ -269,7 +269,7 @@ private void CloseRomMenuItem_Click(object sender, EventArgs e) } private void QuickSavestateMenuItem_Click(object sender, EventArgs e) - => SaveQuickSaveAndShowError(int.Parse(((ToolStripMenuItem) sender).Text)); + => SaveQuickSave(int.Parse(((ToolStripMenuItem) sender).Text)); private void SaveNamedStateMenuItem_Click(object sender, EventArgs e) => SaveStateAs(); @@ -305,7 +305,7 @@ private void SaveToCurrentSlotMenuItem_Click(object sender, EventArgs e) => SavestateCurrentSlot(); private void SavestateCurrentSlot() - => SaveQuickSaveAndShowError(Config.SaveSlot); + => SaveQuickSave(Config.SaveSlot); private void LoadCurrentSlotMenuItem_Click(object sender, EventArgs e) => LoadstateCurrentSlot(); @@ -1387,15 +1387,8 @@ private void ViewCommentsContextMenuItem_Click(object sender, EventArgs e) private void UndoSavestateContextMenuItem_Click(object sender, EventArgs e) { var slot = Config.SaveSlot; - FileWriteResult swapResult = _stateSlots.SwapBackupSavestate(MovieSession.Movie, $"{SaveStatePrefix()}.QuickSave{slot % 10}.State", slot); - if (swapResult.IsError) - { - this.ErrorMessageBox(swapResult, "Failed to swap state files."); - } - else - { - AddOnScreenMessage($"Save slot {slot} restored."); - } + _stateSlots.SwapBackupSavestate(MovieSession.Movie, $"{SaveStatePrefix()}.QuickSave{slot % 10}.State", slot); + AddOnScreenMessage($"Save slot {slot} restored."); } private void ClearSramContextMenuItem_Click(object sender, EventArgs e) @@ -1465,7 +1458,7 @@ private void SlotStatusButtons_MouseUp(object sender, MouseEventArgs e) if (sender == Slot9StatusButton) slot = 9; if (sender == Slot0StatusButton) slot = 10; - if (e.Button is MouseButtons.Right) SaveQuickSaveAndShowError(slot); + if (e.Button is MouseButtons.Right) SaveQuickSave(slot); else if (e.Button is MouseButtons.Left && HasSlot(slot)) _ = LoadQuickSave(slot); } diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs b/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs index 6903825285c..da6fcf4245f 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs @@ -10,7 +10,7 @@ private bool CheckHotkey(string trigger) { void SelectAndSaveToSlot(int slot) { - SaveQuickSaveAndShowError(slot); + SaveQuickSave(slot); Config.SaveSlot = slot; UpdateStatusSlots(); } diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index cf9b33e00c2..de0dc6e9c3b 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -3985,21 +3985,8 @@ private void CloseGame(bool clearSram = false) } } - bool? tryAgain = null; - do - { - FileWriteResult stateSaveResult = AutoSaveStateIfConfigured(); - if (stateSaveResult.IsError) - { - tryAgain = this.ShowMessageBox3( - $"Failed to auto-save state. Do you want to try again?\n\nError details:\n{stateSaveResult.UserFriendlyErrorMessage()}\n{stateSaveResult.Exception.Message}", - "IOError while writing savestate", - EMsgBoxIcon.Error); - if (tryAgain == null) return; - } - } while (tryAgain == true); - StopAv(); + AutoSaveStateIfConfigured(); CommitCoreSettingsToConfig(); DisableRewind(); @@ -4021,14 +4008,9 @@ private void CloseGame(bool clearSram = false) GameIsClosing = false; } - private FileWriteResult AutoSaveStateIfConfigured() + private void AutoSaveStateIfConfigured() { - if (Config.AutoSaveLastSaveSlot && Emulator.HasSavestates()) - { - return SaveQuickSave(Config.SaveSlot); - } - - return new(); + if (Config.AutoSaveLastSaveSlot && Emulator.HasSavestates()) SavestateCurrentSlot(); } public bool GameIsClosing { get; private set; } // Lets tools make better decisions when being called by CloseGame @@ -4184,30 +4166,27 @@ public bool LoadQuickSave(int slot, bool suppressOSD = false) return LoadState(path: path, userFriendlyStateName: quickSlotName, suppressOSD: suppressOSD); } - private FileWriteResult SaveStateInternal(string path, string userFriendlyStateName, bool suppressOSD, bool makeBackup) + public void SaveState(string path, string userFriendlyStateName, bool fromLua = false, bool suppressOSD = false) { if (!Emulator.HasSavestates()) { - return new(FileWriteEnum.Aborted, new("", ""), new UnlessUsingApiException("The current emulator does not support savestates.")); + return; } if (ToolControllingSavestates is { } tool) { tool.SaveState(); - // assume success by the tool: state was created, but not as a file. So no path. - return new(); + return; } if (MovieSession.Movie.IsActive() && Emulator.Frame > MovieSession.Movie.FrameCount) { - const string errmsg = "Cannot savestate after movie end!"; - AddOnScreenMessage(errmsg); - // Failed to create state due to limitations of our movie handling code. - return new(FileWriteEnum.Aborted, new("", ""), new UnlessUsingApiException(errmsg)); + AddOnScreenMessage("Cannot savestate after movie end!"); + return; } FileWriteResult result = new SavestateFile(Emulator, MovieSession, MovieSession.UserBag) - .Create(path, Config.Savestates, makeBackup); + .Create(path, Config.Savestates); if (result.IsError) { AddOnScreenMessage($"Unable to save state {path}"); @@ -4221,32 +4200,25 @@ private FileWriteResult SaveStateInternal(string path, string userFriendlyStateN } RA?.OnSaveState(path); - if (Tools.Has()) - { - Tools.LuaConsole.CallStateSaveCallbacks(userFriendlyStateName); - } - if (!suppressOSD) { AddOnScreenMessage($"Saved state: {userFriendlyStateName}"); } } - return result; - } - - public FileWriteResult SaveState(string path, string userFriendlyStateName, bool suppressOSD = false) - { - return SaveStateInternal(path, userFriendlyStateName, suppressOSD, false); + if (!fromLua) + { + UpdateStatusSlots(); + } } - public FileWriteResult SaveQuickSave(int slot, bool suppressOSD = false) + // TODO: should backup logic be stuffed in into Client.Common.SaveStateManager? + public void SaveQuickSave(int slot, bool suppressOSD = false, bool fromLua = false) { if (!Emulator.HasSavestates()) { - return new(FileWriteEnum.Aborted, new("", ""), new UnlessUsingApiException("The current emulator does not support savestates.")); + return; } - var quickSlotName = $"QuickSave{slot % 10}"; var handled = false; if (QuicksaveSave is not null) @@ -4257,33 +4229,27 @@ public FileWriteResult SaveQuickSave(int slot, bool suppressOSD = false) } if (handled) { - // I suppose this is a success? But we have no path. - return new(); + return; } if (ToolControllingSavestates is { } tool) { tool.SaveQuickSave(slot); - // assume success by the tool: state was created, but not as a file. So no path. - return new(); + return; } var path = $"{SaveStatePrefix()}.{quickSlotName}.State"; - var ret = SaveStateInternal(path, quickSlotName, suppressOSD, true); - UpdateStatusSlots(); - return ret; - } + new FileInfo(path).Directory?.Create(); - /// - /// Runs and displays a pop up message if there was an error. - /// - private void SaveQuickSaveAndShowError(int slot) - { - FileWriteResult result = SaveQuickSave(slot); - if (result.IsError) + // Make backup first + if (Config.Savestates.MakeBackups) { - this.ErrorMessageBox(result, "Quick save failed."); + Util.TryMoveBackupFile(path, $"{path}.bak"); } + + SaveState(path, quickSlotName, fromLua, suppressOSD); + + if (Tools.Has()) Tools.LuaConsole.CallStateSaveCallbacks(quickSlotName); } public bool EnsureCoreIsAccurate() @@ -4346,19 +4312,12 @@ private void SaveStateAs() var path = Config.PathEntries.SaveStateAbsolutePath(Game.System); new FileInfo(path).Directory?.Create(); - var shouldSaveResult = this.ShowFileSaveDialog( + var result = this.ShowFileSaveDialog( fileExt: "State", filter: EmuHawkSaveStatesFSFilterSet, initDir: path, initFileName: $"{SaveStatePrefix()}.QuickSave0.State"); - if (shouldSaveResult is not null) - { - FileWriteResult saveResult = SaveState(path: shouldSaveResult, userFriendlyStateName: shouldSaveResult); - if (saveResult.IsError) - { - this.ErrorMessageBox(saveResult, "Unable to save."); - } - } + if (result is not null) SaveState(path: result, userFriendlyStateName: result); if (Tools.IsLoaded()) { From 5c348c034c3adff4b4f669ce5ae8ac5b932a75ad Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:08:10 +1000 Subject: [PATCH 17/23] Revert "Add support for backup files in FileWriter and better error reporting." This reverts commit ebbb693d68f1c406430e0888a6274e1b6a01d096. --- src/BizHawk.Client.Common/FileWriteResult.cs | 47 ++---- src/BizHawk.Client.Common/FileWriter.cs | 155 +++++------------- .../FrameworkZipWriter.cs | 6 +- src/BizHawk.Client.Common/IZipWriter.cs | 3 +- .../movie/MovieSession.cs | 2 +- .../movie/bk2/Bk2Movie.IO.cs | 4 +- .../savestates/ZipStateSaver.cs | 5 +- .../tools/TAStudio/TAStudio.cs | 2 +- .../Movie/FakeMovieSession.cs | 2 +- 9 files changed, 67 insertions(+), 159 deletions(-) diff --git a/src/BizHawk.Client.Common/FileWriteResult.cs b/src/BizHawk.Client.Common/FileWriteResult.cs index 633efbd7a3c..1c5913326e8 100644 --- a/src/BizHawk.Client.Common/FileWriteResult.cs +++ b/src/BizHawk.Client.Common/FileWriteResult.cs @@ -1,6 +1,7 @@ #nullable enable using System.Diagnostics; +using System.IO; namespace BizHawk.Client.Common { @@ -9,8 +10,6 @@ public enum FileWriteEnum Success, FailedToOpen, FailedDuringWrite, - FailedToDeleteOldBackup, - FailedToMakeBackup, FailedToDeleteOldFile, FailedToRename, } @@ -22,19 +21,17 @@ public class FileWriteResult { public readonly FileWriteEnum Error = FileWriteEnum.Success; public readonly Exception? Exception; - internal readonly FileWritePaths Paths; + public readonly string WritePath; public bool IsError => Error != FileWriteEnum.Success; - internal FileWriteResult(FileWriteEnum error, FileWritePaths writer, Exception? exception) + public FileWriteResult(FileWriteEnum error, string path, Exception? exception) { Error = error; Exception = exception; - Paths = writer; + WritePath = path; } - public FileWriteResult() : this(FileWriteEnum.Success, new("", ""), null) { } - /// /// Converts this instance to a different generic type. /// The new instance will take the value given only if this instance has no error. @@ -42,42 +39,32 @@ internal FileWriteResult(FileWriteEnum error, FileWritePaths writer, Exception? /// The value of the new instance. Ignored if this instance has an error. public FileWriteResult Convert(T value) where T : class { - if (Error == FileWriteEnum.Success) return new(value, Paths); + if (Error == FileWriteEnum.Success) return new(value, WritePath); else return new(this); } - public FileWriteResult(FileWriteResult other) : this(other.Error, other.Paths, other.Exception) { } + public FileWriteResult(FileWriteResult other) : this(other.Error, other.WritePath, other.Exception) { } public string UserFriendlyErrorMessage() { - Debug.Assert(!IsError || (Exception != null), "FileWriteResult with an error should have an exception."); - switch (Error) { - // We include the full path since the user may not have explicitly given a directory and may not know what it is. case FileWriteEnum.Success: - return $"The file \"{Paths.Final}\" was written successfully."; + return $"The file {WritePath} was written successfully."; case FileWriteEnum.FailedToOpen: - if (Paths.Final != Paths.Temp) + if (WritePath.Contains(".saving")) { - return $"The temporary file \"{Paths.Temp}\" could not be opened."; + return $"The temporary file {WritePath} already exists and could not be deleted."; } - return $"The file \"{Paths.Final}\" could not be created."; + return $"The file {WritePath} could not be created."; case FileWriteEnum.FailedDuringWrite: return $"An error occurred while writing the file."; // No file name here; it should be deleted. - } - - string success = $"The file was created successfully at \"{Paths.Temp}\" but could not be moved"; - switch (Error) - { - case FileWriteEnum.FailedToDeleteOldBackup: - return $"{success}. Unable to remove old backup file \"{Paths.Backup}\"."; - case FileWriteEnum.FailedToMakeBackup: - return $"{success}. Unable to create backup. Failed to move \"{Paths.Final}\" to \"{Paths.Backup}\"."; case FileWriteEnum.FailedToDeleteOldFile: - return $"{success}. Unable to remove the old file \"{Paths.Final}\"."; + string fileWithoutPath = Path.GetFileName(WritePath); + return $"The file {WritePath} was created successfully, but the old file could not be deleted. You may manually rename the temporary file {fileWithoutPath}."; case FileWriteEnum.FailedToRename: - return $"{success} to \"{Paths.Final}\"."; + fileWithoutPath = Path.GetFileName(WritePath); + return $"The file {WritePath} was created successfully, but could not be renamed. You may manually rename the temporary file {fileWithoutPath}."; default: return "unreachable"; } @@ -96,14 +83,14 @@ public class FileWriteResult : FileWriteResult where T : class // Note: "clas /// public readonly T? Value = default; - internal FileWriteResult(FileWriteEnum error, FileWritePaths paths, Exception? exception) : base(error, paths, exception) { } + public FileWriteResult(FileWriteEnum error, string path, Exception? exception) : base(error, path, exception) { } - internal FileWriteResult(T value, FileWritePaths paths) : base(FileWriteEnum.Success, paths, null) + public FileWriteResult(T value, string path) : base(FileWriteEnum.Success, path, null) { Debug.Assert(value != null, "Should not give a null value on success. Use the non-generic type if there is no value."); Value = value; } - public FileWriteResult(FileWriteResult other) : base(other.Error, other.Paths, other.Exception) { } + public FileWriteResult(FileWriteResult other) : base(other.Error, other.WritePath, other.Exception) { } } } diff --git a/src/BizHawk.Client.Common/FileWriter.cs b/src/BizHawk.Client.Common/FileWriter.cs index 039c8f47693..4486d5bc098 100644 --- a/src/BizHawk.Client.Common/FileWriter.cs +++ b/src/BizHawk.Client.Common/FileWriter.cs @@ -2,69 +2,57 @@ using System.Diagnostics; using System.IO; -using System.Threading; using BizHawk.Common.StringExtensions; namespace BizHawk.Client.Common { - public class FileWritePaths(string final, string temp) - { - public readonly string Final = final; - public readonly string Temp = temp; - public string? Backup; - } - - /// - /// Provides a mechanism for safely overwriting files, by using a temporary file that only replaces the original after writing has been completed. - /// Optionally makes a backup of the original file. - /// public class FileWriter : IDisposable { - private FileStream? _stream; // is never null until this.Dispose() public FileStream Stream { get => _stream ?? throw new ObjectDisposedException("Cannot access a disposed FileStream."); } - public FileWritePaths Paths; + public string FinalPath; + public string TempPath; - public bool UsingTempFile => Paths.Temp != Paths.Final; + public bool UsingTempFile => TempPath != FinalPath; private bool _finished = false; - private FileWriter(FileWritePaths paths, FileStream stream) + private FileWriter(string final, string temp, FileStream stream) { - Paths = paths; + FinalPath = final; + TempPath = temp; _stream = stream; } - - /// - /// Create a FileWriter instance, or return an error if unable to access the file. - /// + // There is no public constructor. This is the only way to create an instance. public static FileWriteResult Create(string path) { string writePath = path; - // If the file already exists, we will write to a temporary location first and preserve the old one until we're done. - if (File.Exists(path)) - { - writePath = path.InsertBeforeLast('.', ".saving", out bool inserted); - if (!inserted) writePath = $"{path}.saving"; - - // The file might already exist, if a prior file write failed. - // Maybe the user should have dealt with this on the previously failed save. - // But we want to support plain old "try again", so let's ignore that. - } - FileWritePaths paths = new(path, writePath); try { + // If the file already exists, we will write to a temporary location first and preserve the old one until we're done. + if (File.Exists(path)) + { + writePath = path.InsertBeforeLast('.', ".saving", out bool inserted); + if (!inserted) writePath = $"{path}.saving"; + + if (File.Exists(writePath)) + { + // The user should probably have dealt with this on the previously failed save. + // But maybe we should support plain old "try again", so let's delete it. + File.Delete(writePath); + } + } FileStream fs = new(writePath, FileMode.Create, FileAccess.Write); - return new(new FileWriter(paths, fs), paths); + return new(new FileWriter(path, writePath, fs), writePath); } catch (Exception ex) // There are many exception types that file operations might raise. { - return new(FileWriteEnum.FailedToOpen, paths, ex); + return new(FileWriteEnum.FailedToOpen, writePath, ex); } } @@ -72,9 +60,8 @@ public static FileWriteResult Create(string path) /// This method must be called after writing has finished and must not be called twice. /// Dispose will be called regardless of the result. /// - /// If not null, renames the original file to this path. /// If called twice. - public FileWriteResult CloseAndDispose(string? backupPath = null) + public FileWriteResult CloseAndDispose() { // In theory it might make sense to allow the user to try again if we fail inside this method. // If we implement that, it is probably best to make a static method that takes a FileWriteResult. @@ -84,63 +71,29 @@ public FileWriteResult CloseAndDispose(string? backupPath = null) _finished = true; Dispose(); - Paths.Backup = backupPath; - if (!UsingTempFile) + if (!UsingTempFile) return new(FileWriteEnum.Success, FinalPath, null); + + if (File.Exists(FinalPath)) { - // The chosen file did not already exist, so there is nothing to back up and nothing to rename. - return new(FileWriteEnum.Success, Paths, null); + try + { + File.Delete(FinalPath); + } + catch (Exception ex) + { + return new(FileWriteEnum.FailedToDeleteOldFile, TempPath, ex); + } } - try { - // When everything goes right, this is all we need. - File.Replace(Paths.Temp, Paths.Final, backupPath); - return new(FileWriteEnum.Success, Paths, null); + File.Move(TempPath, FinalPath); } - catch + catch (Exception ex) { - // When things go wrong, we have to do a lot of work in order to - // figure out what went wrong and tell the user. - return FindTheError(); - } - } - - private FileWriteResult FindTheError() - { - // It is an unfortunate reality that .NET provides horrible exception messages - // when using File.Replace(source, destination, backup). They are not only - // unhelpful by not telling which file operation failed, but can also be a lie. - // File.Move isn't great either. - // So, we will split this into multiple parts and subparts. - - // 1) Handle backup file, if necessary - // a) Delete the old backup, if it exists. We check existence here to avoid DirectoryNotFound errors. - // If this fails, return that failure. - // If it succeeded but the file somehow still exists, report that error. - // b) Ensure the target directory exists. - // Rename the original file, and similarly report any errors. - // 2) Handle renaming of temp file, the same way renaming of original for backup was done. - - if (Paths.Backup != null) - { - try { DeleteIfExists(Paths.Backup); } - catch (Exception ex) { return new(FileWriteEnum.FailedToDeleteOldBackup, Paths, ex); } - if (!TryWaitForFileToVanish(Paths.Backup)) return new(FileWriteEnum.FailedToDeleteOldBackup, Paths, new Exception("The file was supposedly deleted but is still there.")); - - try { MoveFile(Paths.Final, Paths.Backup); } - catch (Exception ex) { return new(FileWriteEnum.FailedToMakeBackup, Paths, ex); } - if (!TryWaitForFileToVanish(Paths.Final)) return new(FileWriteEnum.FailedToMakeBackup, Paths, new Exception("The file was supposedly moved but is still in the orignal location.")); + return new(FileWriteEnum.FailedToRename, TempPath, ex); } - try { DeleteIfExists(Paths.Final); } - catch (Exception ex) { return new(FileWriteEnum.FailedToDeleteOldFile, Paths, ex); } - if (!TryWaitForFileToVanish(Paths.Final)) return new(FileWriteEnum.FailedToDeleteOldFile, Paths, new Exception("The file was supposedly deleted but is still there.")); - - try { MoveFile(Paths.Temp, Paths.Final); } - catch (Exception ex) { return new(FileWriteEnum.FailedToRename, Paths, ex); } - if (!TryWaitForFileToVanish(Paths.Temp)) return new(FileWriteEnum.FailedToRename, Paths, new Exception("The file was supposedly moved but is still in the orignal location.")); - - return new(FileWriteEnum.Success, Paths, null); + return new(FileWriteEnum.Success, FinalPath, null); } /// @@ -156,7 +109,7 @@ public void Abort() try { // Delete because the file is almost certainly useless and just clutter. - File.Delete(Paths.Temp); + File.Delete(TempPath); } catch { /* eat? this is probably not very important */ } } @@ -174,35 +127,5 @@ public void Dispose() // The caller should call CloseAndDispose and handle potential failure. Debug.Assert(_finished, $"{nameof(FileWriteResult)} should not be disposed before calling {nameof(CloseAndDispose)}"); } - - - private static void DeleteIfExists(string path) - { - if (File.Exists(path)) - { - File.Delete(path); - } - } - - private static void MoveFile(string source, string destination) - { - FileInfo file = new(destination); - file.Directory.Create(); - File.Move(source, destination); - } - - /// - /// Supposedly it is possible for File.Delete to return before the file has actually been deleted. - /// And File.Move too, I guess. - /// - private static bool TryWaitForFileToVanish(string path) - { - for (var i = 25; i != 0; i--) - { - if (!File.Exists(path)) return true; - Thread.Sleep(10); - } - return false; - } } } diff --git a/src/BizHawk.Client.Common/FrameworkZipWriter.cs b/src/BizHawk.Client.Common/FrameworkZipWriter.cs index a8c9fcd6521..a0e18891b83 100644 --- a/src/BizHawk.Client.Common/FrameworkZipWriter.cs +++ b/src/BizHawk.Client.Common/FrameworkZipWriter.cs @@ -47,7 +47,7 @@ public static FileWriteResult Create(string path, int compre return fs.Convert(ret); } - public FileWriteResult CloseAndDispose(string? backupPath = null) + public FileWriteResult CloseAndDispose() { if (_archive == null || _fs == null) throw new ObjectDisposedException("Cannot use disposed ZipWriter."); @@ -58,11 +58,11 @@ public FileWriteResult CloseAndDispose(string? backupPath = null) FileWriteResult ret; if (_writeException == null) { - ret = _fs.CloseAndDispose(backupPath); + ret = _fs.CloseAndDispose(); } else { - ret = new(FileWriteEnum.FailedDuringWrite, _fs.Paths, _writeException); + ret = new(FileWriteEnum.FailedDuringWrite, _fs.TempPath, _writeException); _fs.Abort(); } diff --git a/src/BizHawk.Client.Common/IZipWriter.cs b/src/BizHawk.Client.Common/IZipWriter.cs index 98a3762daf3..e02768448d5 100644 --- a/src/BizHawk.Client.Common/IZipWriter.cs +++ b/src/BizHawk.Client.Common/IZipWriter.cs @@ -12,8 +12,7 @@ public interface IZipWriter : IDisposable /// This method must be called after writing has finished and must not be called twice. /// Dispose will be called regardless of the result. /// - /// If not null, renames the original file to this path. - FileWriteResult CloseAndDispose(string? backupPath = null); + FileWriteResult CloseAndDispose(); /// /// Closes and deletes the file. Use if there was an error while writing. diff --git a/src/BizHawk.Client.Common/movie/MovieSession.cs b/src/BizHawk.Client.Common/movie/MovieSession.cs index d63501bf4a9..f676826720b 100644 --- a/src/BizHawk.Client.Common/movie/MovieSession.cs +++ b/src/BizHawk.Client.Common/movie/MovieSession.cs @@ -290,7 +290,7 @@ public FileWriteResult StopMovie(bool saveChanges = true) Movie = null; - return result ?? new(); + return result ?? new(FileWriteEnum.Success, "", null); } public IMovie Get(string path, bool loadMovie) diff --git a/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs b/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs index 4b53ec7cd0a..2f0388424d6 100644 --- a/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs +++ b/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs @@ -23,7 +23,7 @@ public FileWriteResult SaveBackup() { if (string.IsNullOrWhiteSpace(Filename)) { - return new(); + return new(FileWriteEnum.Success, "", null); } string backupName = Filename.InsertBeforeLast('.', insert: $".{DateTime.Now:yyyy-MM-dd HH.mm.ss}", out _); @@ -54,7 +54,7 @@ protected virtual FileWriteResult Write(string fn, bool isBackup = false) catch (Exception ex) { saver.Abort(); - return new(FileWriteEnum.FailedDuringWrite, createResult.Paths, ex); + return new(FileWriteEnum.FailedDuringWrite, createResult.WritePath, ex); } FileWriteResult result = saver.CloseAndDispose(); diff --git a/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs b/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs index 68dcd337a69..84e36482fbb 100644 --- a/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs +++ b/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs @@ -48,10 +48,9 @@ public static FileWriteResult Create(string path, int compression /// This method must be called after writing has finished and must not be called twice. /// Dispose will be called regardless of the result. /// - /// If not null, renames the original file to this path. - public FileWriteResult CloseAndDispose(string? backupPath = null) + public FileWriteResult CloseAndDispose() { - FileWriteResult result = _zip.CloseAndDispose(backupPath); + FileWriteResult result = _zip.CloseAndDispose(); Dispose(); return result; } diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs index 85cdda2e7c2..5e9b2b9d6d6 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs @@ -989,7 +989,7 @@ private FileWriteResult SaveAsTas() MainForm.UpdateWindowTitle(); if (fileInfo != null) return saveResult; - else return new(); // user cancelled, so we were successful in not saving + else return new(FileWriteEnum.Success, "", null); // user cancelled, so we were successful in not saving } protected override string WindowTitle diff --git a/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs b/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs index c7497538b4c..954d9f4bfbe 100644 --- a/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs +++ b/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs @@ -73,7 +73,7 @@ public FakeMovieSession(IEmulator emulator) public FileWriteResult StopMovie(bool saveChanges = true) { Movie?.Stop(); - return new(); + return new(FileWriteEnum.Success, "", null); } } } From 6d4754e9f0c421307239f9790cca416c9de833a4 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:10:24 +1000 Subject: [PATCH 18/23] Revert "Handle exceptions while writing a movie file, and use a temporary file until writing has completed. Avoids crashing and prevents loss of data in the event of an exception mid-write." This reverts commit 4d8e8ba76e9007d8b85d7db3a02cf1a947e17032. --- .global.editorconfig.ini | 9 -- .../Api/Classes/MovieApi.cs | 4 +- src/BizHawk.Client.Common/FileWriteResult.cs | 96 ------------ src/BizHawk.Client.Common/FileWriter.cs | 131 ----------------- .../FrameworkZipWriter.cs | 96 ++---------- src/BizHawk.Client.Common/IZipWriter.cs | 12 -- .../movie/MovieConversionExtensions.cs | 12 +- .../movie/MovieSession.cs | 18 +-- .../movie/bk2/Bk2Movie.IO.cs | 38 ++--- .../movie/import/IMovieImport.cs | 6 +- .../movie/interfaces/IMovie.cs | 4 +- .../movie/interfaces/IMovieSession.cs | 2 +- .../movie/tasproj/TasMovie.cs | 4 +- .../savestates/SavestateFile.cs | 12 +- .../savestates/ZipStateSaver.cs | 32 +--- src/BizHawk.Client.EmuHawk/MainForm.Movie.cs | 9 +- src/BizHawk.Client.EmuHawk/MainForm.cs | 39 ++--- .../movie/EditCommentsForm.cs | 3 +- .../tools/TAStudio/TAStudio.IToolForm.cs | 17 +-- .../tools/TAStudio/TAStudio.MenuItems.cs | 48 ++---- .../tools/TAStudio/TAStudio.cs | 138 +++++------------- .../Movie/FakeMovieSession.cs | 6 +- 22 files changed, 117 insertions(+), 619 deletions(-) delete mode 100644 src/BizHawk.Client.Common/FileWriteResult.cs delete mode 100644 src/BizHawk.Client.Common/FileWriter.cs diff --git a/.global.editorconfig.ini b/.global.editorconfig.ini index d6f88a6f0e3..6d8d0f8bb38 100644 --- a/.global.editorconfig.ini +++ b/.global.editorconfig.ini @@ -156,15 +156,6 @@ dotnet_diagnostic.CA2229.severity = silent # Opt in to preview features before using them dotnet_diagnostic.CA2252.severity = silent # CSharpDetectPreviewFeatureAnalyzer very slow -## Nullable rules; generics are a bit wonky and I have no idea why we're allowed to configure these to be not errors or why they aren't errors by default. - -# Nullability of reference types in value doesn't match target type. -dotnet_diagnostic.CS8619.severity = error -# Make Foo an error for class Foo where T : class. Use `where T : class?` if Foo should be allowed. -dotnet_diagnostic.CS8634.severity = error -# Nullability of type argument doesn't match 'notnull' constraint. -dotnet_diagnostic.CS8714.severity = error - ## .NET DocumentationAnalyzers style rules # Place text in paragraphs diff --git a/src/BizHawk.Client.Common/Api/Classes/MovieApi.cs b/src/BizHawk.Client.Common/Api/Classes/MovieApi.cs index 47e059e96c5..50672fb7965 100644 --- a/src/BizHawk.Client.Common/Api/Classes/MovieApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/MovieApi.cs @@ -52,7 +52,6 @@ public string GetInputAsMnemonic(int frame) return Bk2LogEntryGenerator.GenerateLogEntry(_movieSession.Movie.GetInputState(frame)); } - // TODO: Change return type to FileWriteResult public void Save(string filename) { if (_movieSession.Movie.NotActive()) @@ -70,8 +69,7 @@ public void Save(string filename) } _movieSession.Movie.Filename = filename; } - FileWriteResult result = _movieSession.Movie.Save(); - if (result.Exception != null) throw result.Exception; + _movieSession.Movie.Save(); } public IReadOnlyDictionary GetHeader() diff --git a/src/BizHawk.Client.Common/FileWriteResult.cs b/src/BizHawk.Client.Common/FileWriteResult.cs deleted file mode 100644 index 1c5913326e8..00000000000 --- a/src/BizHawk.Client.Common/FileWriteResult.cs +++ /dev/null @@ -1,96 +0,0 @@ -#nullable enable - -using System.Diagnostics; -using System.IO; - -namespace BizHawk.Client.Common -{ - public enum FileWriteEnum - { - Success, - FailedToOpen, - FailedDuringWrite, - FailedToDeleteOldFile, - FailedToRename, - } - - /// - /// Provides information about the success or failure of an attempt to write to a file. - /// - public class FileWriteResult - { - public readonly FileWriteEnum Error = FileWriteEnum.Success; - public readonly Exception? Exception; - public readonly string WritePath; - - public bool IsError => Error != FileWriteEnum.Success; - - public FileWriteResult(FileWriteEnum error, string path, Exception? exception) - { - Error = error; - Exception = exception; - WritePath = path; - } - - /// - /// Converts this instance to a different generic type. - /// The new instance will take the value given only if this instance has no error. - /// - /// The value of the new instance. Ignored if this instance has an error. - public FileWriteResult Convert(T value) where T : class - { - if (Error == FileWriteEnum.Success) return new(value, WritePath); - else return new(this); - } - - public FileWriteResult(FileWriteResult other) : this(other.Error, other.WritePath, other.Exception) { } - - public string UserFriendlyErrorMessage() - { - switch (Error) - { - case FileWriteEnum.Success: - return $"The file {WritePath} was written successfully."; - case FileWriteEnum.FailedToOpen: - if (WritePath.Contains(".saving")) - { - return $"The temporary file {WritePath} already exists and could not be deleted."; - } - return $"The file {WritePath} could not be created."; - case FileWriteEnum.FailedDuringWrite: - return $"An error occurred while writing the file."; // No file name here; it should be deleted. - case FileWriteEnum.FailedToDeleteOldFile: - string fileWithoutPath = Path.GetFileName(WritePath); - return $"The file {WritePath} was created successfully, but the old file could not be deleted. You may manually rename the temporary file {fileWithoutPath}."; - case FileWriteEnum.FailedToRename: - fileWithoutPath = Path.GetFileName(WritePath); - return $"The file {WritePath} was created successfully, but could not be renamed. You may manually rename the temporary file {fileWithoutPath}."; - default: - return "unreachable"; - } - } - } - - /// - /// Provides information about the success or failure of an attempt to write to a file. - /// If successful, also provides a related object instance. - /// - public class FileWriteResult : FileWriteResult where T : class // Note: "class" also means "notnull". - { - /// - /// Value will be null if is true. - /// Otherwise, Value will not be null. - /// - public readonly T? Value = default; - - public FileWriteResult(FileWriteEnum error, string path, Exception? exception) : base(error, path, exception) { } - - public FileWriteResult(T value, string path) : base(FileWriteEnum.Success, path, null) - { - Debug.Assert(value != null, "Should not give a null value on success. Use the non-generic type if there is no value."); - Value = value; - } - - public FileWriteResult(FileWriteResult other) : base(other.Error, other.WritePath, other.Exception) { } - } -} diff --git a/src/BizHawk.Client.Common/FileWriter.cs b/src/BizHawk.Client.Common/FileWriter.cs deleted file mode 100644 index 4486d5bc098..00000000000 --- a/src/BizHawk.Client.Common/FileWriter.cs +++ /dev/null @@ -1,131 +0,0 @@ -#nullable enable - -using System.Diagnostics; -using System.IO; - -using BizHawk.Common.StringExtensions; - -namespace BizHawk.Client.Common -{ - public class FileWriter : IDisposable - { - private FileStream? _stream; // is never null until this.Dispose() - public FileStream Stream - { - get => _stream ?? throw new ObjectDisposedException("Cannot access a disposed FileStream."); - } - public string FinalPath; - public string TempPath; - - public bool UsingTempFile => TempPath != FinalPath; - - private bool _finished = false; - - private FileWriter(string final, string temp, FileStream stream) - { - FinalPath = final; - TempPath = temp; - _stream = stream; - } - - // There is no public constructor. This is the only way to create an instance. - public static FileWriteResult Create(string path) - { - string writePath = path; - try - { - // If the file already exists, we will write to a temporary location first and preserve the old one until we're done. - if (File.Exists(path)) - { - writePath = path.InsertBeforeLast('.', ".saving", out bool inserted); - if (!inserted) writePath = $"{path}.saving"; - - if (File.Exists(writePath)) - { - // The user should probably have dealt with this on the previously failed save. - // But maybe we should support plain old "try again", so let's delete it. - File.Delete(writePath); - } - } - FileStream fs = new(writePath, FileMode.Create, FileAccess.Write); - return new(new FileWriter(path, writePath, fs), writePath); - } - catch (Exception ex) // There are many exception types that file operations might raise. - { - return new(FileWriteEnum.FailedToOpen, writePath, ex); - } - } - - /// - /// This method must be called after writing has finished and must not be called twice. - /// Dispose will be called regardless of the result. - /// - /// If called twice. - public FileWriteResult CloseAndDispose() - { - // In theory it might make sense to allow the user to try again if we fail inside this method. - // If we implement that, it is probably best to make a static method that takes a FileWriteResult. - // So even then, this method should not ever be called twice. - if (_finished) throw new InvalidOperationException("Cannot close twice."); - - _finished = true; - Dispose(); - - if (!UsingTempFile) return new(FileWriteEnum.Success, FinalPath, null); - - if (File.Exists(FinalPath)) - { - try - { - File.Delete(FinalPath); - } - catch (Exception ex) - { - return new(FileWriteEnum.FailedToDeleteOldFile, TempPath, ex); - } - } - try - { - File.Move(TempPath, FinalPath); - } - catch (Exception ex) - { - return new(FileWriteEnum.FailedToRename, TempPath, ex); - } - - return new(FileWriteEnum.Success, FinalPath, null); - } - - /// - /// Closes and deletes the file. Use if there was an error while writing. - /// Do not call after this. - /// - public void Abort() - { - if (_dispoed) throw new ObjectDisposedException("Cannot use a disposed file stream."); - _finished = true; - Dispose(); - - try - { - // Delete because the file is almost certainly useless and just clutter. - File.Delete(TempPath); - } - catch { /* eat? this is probably not very important */ } - } - - private bool _dispoed; - public void Dispose() - { - if (_dispoed) return; - _dispoed = true; - - _stream!.Flush(flushToDisk: true); - _stream.Dispose(); - _stream = null; - - // The caller should call CloseAndDispose and handle potential failure. - Debug.Assert(_finished, $"{nameof(FileWriteResult)} should not be disposed before calling {nameof(CloseAndDispose)}"); - } - } -} diff --git a/src/BizHawk.Client.Common/FrameworkZipWriter.cs b/src/BizHawk.Client.Common/FrameworkZipWriter.cs index a0e18891b83..2f99e0d60f0 100644 --- a/src/BizHawk.Client.Common/FrameworkZipWriter.cs +++ b/src/BizHawk.Client.Common/FrameworkZipWriter.cs @@ -11,17 +11,16 @@ public class FrameworkZipWriter : IZipWriter { private ZipArchive? _archive; - private FileWriter? _fs; + private FileStream? _fs; private Zstd? _zstd; private readonly CompressionLevel _level; private readonly int _zstdCompressionLevel; - private Exception? _writeException = null; - private bool _disposed; - - private FrameworkZipWriter(int compressionLevel) + public FrameworkZipWriter(string path, int compressionLevel) { + _fs = new(path, FileMode.Create, FileAccess.Write); + _archive = new(_fs, ZipArchiveMode.Create, leaveOpen: true); if (compressionLevel == 0) _level = CompressionLevel.NoCompression; else if (compressionLevel < 5) @@ -35,95 +34,32 @@ private FrameworkZipWriter(int compressionLevel) _zstdCompressionLevel = compressionLevel * 2 + 1; } - public static FileWriteResult Create(string path, int compressionLevel) - { - FileWriteResult fs = FileWriter.Create(path); - if (fs.IsError) return new(fs); - - FrameworkZipWriter ret = new(compressionLevel); - ret._fs = fs.Value!; - ret._archive = new(ret._fs.Stream, ZipArchiveMode.Create, leaveOpen: true); - - return fs.Convert(ret); - } - - public FileWriteResult CloseAndDispose() - { - if (_archive == null || _fs == null) throw new ObjectDisposedException("Cannot use disposed ZipWriter."); - - // We actually have to do this here since it has to be done before the file stream is closed. - _archive.Dispose(); - _archive = null; - - FileWriteResult ret; - if (_writeException == null) - { - ret = _fs.CloseAndDispose(); - } - else - { - ret = new(FileWriteEnum.FailedDuringWrite, _fs.TempPath, _writeException); - _fs.Abort(); - } - - // And since we have to close stuff, there's really no point in not disposing here. - Dispose(); - return ret; - } - - public void Abort() - { - if (_archive == null || _fs == null) throw new ObjectDisposedException("Cannot use disposed ZipWriter."); - - _archive.Dispose(); - _archive = null; - - _fs.Abort(); - - Dispose(); - } - public void WriteItem(string name, Action callback, bool zstdCompress) { - if (_archive == null || _zstd == null) throw new ObjectDisposedException("Cannot use disposed ZipWriter."); - if (_writeException != null) return; + // don't compress with deflate if we're already compressing with zstd + // this won't produce meaningful compression, and would just be a timesink + using var stream = _archive!.CreateEntry(name, zstdCompress ? CompressionLevel.NoCompression : _level).Open(); - try + if (zstdCompress) { - // don't compress with deflate if we're already compressing with zstd - // this won't produce meaningful compression, and would just be a timesink - using var stream = _archive.CreateEntry(name, zstdCompress ? CompressionLevel.NoCompression : _level).Open(); - - if (zstdCompress) - { - using var z = _zstd.CreateZstdCompressionStream(stream, _zstdCompressionLevel); - callback(z); - } - else - { - callback(stream); - } + using var z = _zstd!.CreateZstdCompressionStream(stream, _zstdCompressionLevel); + callback(z); } - catch (Exception ex) + else { - _writeException = ex; - // We aren't returning the failure until closing. Should we? I don't want to refactor that much calling code without a good reason. + callback(stream); } } public void Dispose() { - if (_disposed) return; - _disposed = true; - - // _archive should already be disposed by CloseAndDispose, but just in case _archive?.Dispose(); _archive = null; - _zstd!.Dispose(); - _zstd = null; - - _fs!.Dispose(); + _fs?.Flush(flushToDisk: true); + _fs?.Dispose(); _fs = null; + _zstd?.Dispose(); + _zstd = null; } } } diff --git a/src/BizHawk.Client.Common/IZipWriter.cs b/src/BizHawk.Client.Common/IZipWriter.cs index e02768448d5..aba28cc0690 100644 --- a/src/BizHawk.Client.Common/IZipWriter.cs +++ b/src/BizHawk.Client.Common/IZipWriter.cs @@ -7,17 +7,5 @@ namespace BizHawk.Client.Common public interface IZipWriter : IDisposable { void WriteItem(string name, Action callback, bool zstdCompress); - - /// - /// This method must be called after writing has finished and must not be called twice. - /// Dispose will be called regardless of the result. - /// - FileWriteResult CloseAndDispose(); - - /// - /// Closes and deletes the file. Use if there was an error while writing. - /// Do not call after this. - /// - void Abort(); } } diff --git a/src/BizHawk.Client.Common/movie/MovieConversionExtensions.cs b/src/BizHawk.Client.Common/movie/MovieConversionExtensions.cs index aa4c913c243..945a216c7a8 100644 --- a/src/BizHawk.Client.Common/movie/MovieConversionExtensions.cs +++ b/src/BizHawk.Client.Common/movie/MovieConversionExtensions.cs @@ -71,7 +71,7 @@ public static IMovie ToBk2(this IMovie old) return bk2; } - public static FileWriteResult ConvertToSavestateAnchoredMovie(this ITasMovie old, int frame, byte[] savestate) + public static ITasMovie ConvertToSavestateAnchoredMovie(this ITasMovie old, int frame, byte[] savestate) { string newFilename = ConvertFileNameToTasMovie(old.Filename); @@ -115,11 +115,11 @@ public static FileWriteResult ConvertToSavestateAnchoredMovie(this IT } } - FileWriteResult saveResult = tas.Save(); - return saveResult.Convert(tas); + tas.Save(); + return tas; } - public static FileWriteResult ConvertToSaveRamAnchoredMovie(this ITasMovie old, byte[] saveRam) + public static ITasMovie ConvertToSaveRamAnchoredMovie(this ITasMovie old, byte[] saveRam) { string newFilename = ConvertFileNameToTasMovie(old.Filename); @@ -146,8 +146,8 @@ public static FileWriteResult ConvertToSaveRamAnchoredMovie(this ITas tas.Subtitles.Add(sub); } - FileWriteResult saveResult = tas.Save(); - return saveResult.Convert(tas); + tas.Save(); + return tas; } #pragma warning disable RCS1224 // private but for unit test diff --git a/src/BizHawk.Client.Common/movie/MovieSession.cs b/src/BizHawk.Client.Common/movie/MovieSession.cs index f676826720b..eaaeef1fdc5 100644 --- a/src/BizHawk.Client.Common/movie/MovieSession.cs +++ b/src/BizHawk.Client.Common/movie/MovieSession.cs @@ -244,9 +244,8 @@ public void RunQueuedMovie(bool recordMode, IEmulator emulator) public void AbortQueuedMovie() => _queuedMovie = null; - public FileWriteResult StopMovie(bool saveChanges = true) + public void StopMovie(bool saveChanges = true) { - FileWriteResult/*?*/ result = null; if (Movie.IsActive()) { var message = "Movie "; @@ -263,17 +262,8 @@ public FileWriteResult StopMovie(bool saveChanges = true) if (saveChanges && Movie.Changes) { - result = Movie.Save(); - if (result.IsError) - { - Output($"Failed to write {Path.GetFileName(Movie.Filename)} to disk."); - Output(result.UserFriendlyErrorMessage()); - return result; - } - else - { - Output($"{Path.GetFileName(Movie.Filename)} written to disk."); - } + Movie.Save(); + Output($"{Path.GetFileName(Movie.Filename)} written to disk."); } Movie.Stop(); @@ -289,8 +279,6 @@ public FileWriteResult StopMovie(bool saveChanges = true) } Movie = null; - - return result ?? new(FileWriteEnum.Success, "", null); } public IMovie Get(string path, bool loadMovie) diff --git a/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs b/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs index 2f0388424d6..6e8ca063afc 100644 --- a/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs +++ b/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs @@ -1,5 +1,3 @@ -#nullable enable - using System.Globalization; using System.IO; using System.Runtime.InteropServices; @@ -14,25 +12,25 @@ namespace BizHawk.Client.Common { public partial class Bk2Movie { - public FileWriteResult Save() + public void Save() { - return Write(Filename); + Write(Filename); } - public FileWriteResult SaveBackup() + public void SaveBackup() { if (string.IsNullOrWhiteSpace(Filename)) { - return new(FileWriteEnum.Success, "", null); + return; } - string backupName = Filename.InsertBeforeLast('.', insert: $".{DateTime.Now:yyyy-MM-dd HH.mm.ss}", out _); + var backupName = Filename.InsertBeforeLast('.', insert: $".{DateTime.Now:yyyy-MM-dd HH.mm.ss}", out _); backupName = Path.Combine(Session.BackupDirectory, Path.GetFileName(backupName)); - return Write(backupName, isBackup: true); + Write(backupName, isBackup: true); } - protected virtual FileWriteResult Write(string fn, bool isBackup = false) + protected virtual void Write(string fn, bool isBackup = false) { SetCycleValues(); // EmulatorVersion used to store the unchanging original emulator version. @@ -43,27 +41,13 @@ protected virtual FileWriteResult Write(string fn, bool isBackup = false) Header[HeaderKeys.EmulatorVersion] = VersionInfo.GetEmuVersion(); Directory.CreateDirectory(Path.GetDirectoryName(fn)!); - var createResult = ZipStateSaver.Create(fn, Session.Settings.MovieCompressionLevel); - if (createResult.IsError) return createResult; + using var bs = new ZipStateSaver(fn, Session.Settings.MovieCompressionLevel); + AddLumps(bs, isBackup); - ZipStateSaver saver = createResult.Value!; - try - { - AddLumps(saver, isBackup); - } - catch (Exception ex) - { - saver.Abort(); - return new(FileWriteEnum.FailedDuringWrite, createResult.WritePath, ex); - } - - FileWriteResult result = saver.CloseAndDispose(); - if (!isBackup && !result.IsError) + if (!isBackup) { Changes = false; } - - return result; } public void SetCycleValues() //TODO IEmulator should not be an instance prop of movies, it should be passed in to every call (i.e. from MovieService) --yoshi @@ -150,7 +134,7 @@ private void LoadBk2Fields(ZipStateLoader bl) bl.GetLump(BinaryStateLump.SyncSettings, abort: false, tr => { - string? line; + string line; while ((line = tr.ReadLine()) != null) { if (!string.IsNullOrWhiteSpace(line)) diff --git a/src/BizHawk.Client.Common/movie/import/IMovieImport.cs b/src/BizHawk.Client.Common/movie/import/IMovieImport.cs index c183d980ec9..91a30f734fa 100644 --- a/src/BizHawk.Client.Common/movie/import/IMovieImport.cs +++ b/src/BizHawk.Client.Common/movie/import/IMovieImport.cs @@ -67,11 +67,7 @@ public ImportResult Import( Result.Movie.Hash = hash; } - if (Result.Movie.Save().IsError) - { - Result.Errors.Add($"Could not write the file {newFileName}"); - return Result; - } + Result.Movie.Save(); } return Result; diff --git a/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs b/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs index 3455f2e0ced..0670e548a44 100644 --- a/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs +++ b/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs @@ -81,12 +81,12 @@ public interface IMovie : IBasicMovieInfo /// /// Forces the creation of a backup file of the current movie state /// - FileWriteResult SaveBackup(); + void SaveBackup(); /// /// Instructs the movie to save the current contents to Filename /// - FileWriteResult Save(); + void Save(); /// updates the and headers from the currently loaded core void SetCycleValues(); diff --git a/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs b/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs index 3ab9a23158e..908f3b75ed9 100644 --- a/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs +++ b/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs @@ -83,7 +83,7 @@ void QueueNewMovie( /// clears the queued movie void AbortQueuedMovie(); - FileWriteResult StopMovie(bool saveChanges = true); + void StopMovie(bool saveChanges = true); /// /// Create a new (Tas)Movie with the given path as filename. If is true, diff --git a/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs b/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs index fd72d9fed43..1d4bae7529b 100644 --- a/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs +++ b/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs @@ -216,9 +216,7 @@ public override bool ExtractInputLog(TextReader reader, out string errorMessage) // We are in record mode so replace the movie log with the one from the savestate if (Session.Settings.EnableBackupMovies && MakeBackup && Log.Count != 0) { - // TODO: This isn't ideal, but making it ideal would mean a big refactor. - FileWriteResult saveResult = SaveBackup(); - if (saveResult.Exception != null) throw saveResult.Exception; + SaveBackup(); MakeBackup = false; } diff --git a/src/BizHawk.Client.Common/savestates/SavestateFile.cs b/src/BizHawk.Client.Common/savestates/SavestateFile.cs index 393f8ac76ca..d2558a9b044 100644 --- a/src/BizHawk.Client.Common/savestates/SavestateFile.cs +++ b/src/BizHawk.Client.Common/savestates/SavestateFile.cs @@ -58,16 +58,14 @@ public SavestateFile( _userBag = userBag; } - public FileWriteResult Create(string filename, SaveStateConfig config) + public void Create(string filename, SaveStateConfig config) { - FileWriteResult createResult = ZipStateSaver.Create(filename, config.CompressionLevelNormal); - if (createResult.IsError) return createResult; - var bs = createResult.Value!; + // the old method of text savestate save is now gone. + // a text savestate is just like a binary savestate, but with a different core lump + using var bs = new ZipStateSaver(filename, config.CompressionLevelNormal); using (new SimpleTime("Save Core")) { - // the old method of text savestate save is now gone. - // a text savestate is just like a binary savestate, but with a different core lump if (config.Type == SaveStateType.Text) { bs.PutLump(BinaryStateLump.CorestateText, tw => _statable.SaveStateText(tw)); @@ -131,8 +129,6 @@ public FileWriteResult Create(string filename, SaveStateConfig config) { bs.PutLump(BinaryStateLump.LagLog, tw => tasMovie.LagLog.Save(tw), zstdCompress: true); } - - return bs.CloseAndDispose(); } public bool Load(string path, IDialogParent dialogParent) diff --git a/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs b/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs index 84e36482fbb..6a01f6c60b9 100644 --- a/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs +++ b/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs @@ -27,9 +27,9 @@ private static void WriteEmuVersion(Stream s) sw.WriteLine(VersionInfo.GetEmuVersion()); } - private ZipStateSaver(FrameworkZipWriter zip) + public ZipStateSaver(string path, int compressionLevel) { - _zip = zip; + _zip = new FrameworkZipWriter(path, compressionLevel); // we put these in every zip, so we know where they came from // a bit redundant for movie files given their headers, but w/e @@ -37,34 +37,6 @@ private ZipStateSaver(FrameworkZipWriter zip) PutLump(BinaryStateLump.BizVersion, WriteEmuVersion, false); } - public static FileWriteResult Create(string path, int compressionLevel) - { - FileWriteResult result = FrameworkZipWriter.Create(path, compressionLevel); - if (result.IsError) return new(result); - else return result.Convert(new ZipStateSaver(result.Value!)); - } - - /// - /// This method must be called after writing has finished and must not be called twice. - /// Dispose will be called regardless of the result. - /// - public FileWriteResult CloseAndDispose() - { - FileWriteResult result = _zip.CloseAndDispose(); - Dispose(); - return result; - } - - /// - /// Closes and deletes the file. Use if there was an error while writing. - /// Do not call after this. - /// - public void Abort() - { - _zip.Abort(); - Dispose(); - } - public void PutLump(BinaryStateLump lump, Action callback, bool zstdCompress = true) { var filePath = AsTarbomb ? lump.FileName : $"{TOP_LEVEL_DIR_NAME}/{lump.FileName}"; diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs b/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs index b9e7f04f946..f97519c3651 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs @@ -125,14 +125,7 @@ public void StopMovie(bool saveChanges = true) } else { - FileWriteResult saveResult = MovieSession.StopMovie(saveChanges); - if (saveResult.IsError) - { - this.ShowMessageBox( - $"Failed to save movie.\n{saveResult.UserFriendlyErrorMessage()}\n{saveResult.Exception.Message}", - "Error", - EMsgBoxIcon.Error); - } + MovieSession.StopMovie(saveChanges); SetMainformMovieInfo(); } } diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index de0dc6e9c3b..d2dd87aaeef 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -889,18 +889,7 @@ private void CheckMayCloseAndCleanup(object/*?*/ closingSender, CancelEventArgs } Tools.Close(); - FileWriteResult saveResult = MovieSession.StopMovie(); - if (saveResult.IsError) - { - if (!this.ModalMessageBox2( - caption: "Quit anyway?", - icon: EMsgBoxIcon.Question, - text: "The currently playing movie could not be saved. Continue and quit anyway? All unsaved changes will be lost.")) - { - closingArgs.Cancel = true; - return; - } - } + MovieSession.StopMovie(); // zero 03-nov-2015 - close game after other steps. tools might need to unhook themselves from a core. CloseGame(); SaveConfig(); @@ -2689,16 +2678,8 @@ private void SaveMovie() { if (MovieSession.Movie.IsActive()) { - FileWriteResult result = MovieSession.Movie.Save(); - if (result.IsError) - { - AddOnScreenMessage($"Failed to save {MovieSession.Movie.Filename}."); - AddOnScreenMessage(result.UserFriendlyErrorMessage()); - } - else - { - AddOnScreenMessage($"{MovieSession.Movie.Filename} saved."); - } + MovieSession.Movie.Save(); + AddOnScreenMessage($"{MovieSession.Movie.Filename} saved."); } } @@ -4185,14 +4166,10 @@ public void SaveState(string path, string userFriendlyStateName, bool fromLua = return; } - FileWriteResult result = new SavestateFile(Emulator, MovieSession, MovieSession.UserBag) - .Create(path, Config.Savestates); - if (result.IsError) - { - AddOnScreenMessage($"Unable to save state {path}"); - } - else + try { + new SavestateFile(Emulator, MovieSession, MovieSession.UserBag).Create(path, Config.Savestates); + if (SavestateSaved is not null) { StateSavedEventArgs args = new(userFriendlyStateName); @@ -4205,6 +4182,10 @@ public void SaveState(string path, string userFriendlyStateName, bool fromLua = AddOnScreenMessage($"Saved state: {userFriendlyStateName}"); } } + catch (IOException) + { + AddOnScreenMessage($"Unable to save state {path}"); + } if (!fromLua) { diff --git a/src/BizHawk.Client.EmuHawk/movie/EditCommentsForm.cs b/src/BizHawk.Client.EmuHawk/movie/EditCommentsForm.cs index c19de4fa71e..0ddb2e07fa3 100644 --- a/src/BizHawk.Client.EmuHawk/movie/EditCommentsForm.cs +++ b/src/BizHawk.Client.EmuHawk/movie/EditCommentsForm.cs @@ -55,8 +55,7 @@ private void Save() _movie.Comments.Add(c.Value.ToString()); } - FileWriteResult result = _movie.Save(); - if (result.IsError) throw result.Exception!; + _movie.Save(); } private void Cancel_Click(object sender, EventArgs e) diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs index ba5db1d5209..828ad450284 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs @@ -157,23 +157,12 @@ public override bool AskSaveChanges() } if (CurrentTasMovie?.Changes is not true) return true; - var shouldSaveResult = DialogController.DoWithTempMute(() => this.ModalMessageBox3( + var result = DialogController.DoWithTempMute(() => this.ModalMessageBox3( caption: "Closing with Unsaved Changes", icon: EMsgBoxIcon.Question, text: $"Save {WindowTitleStatic} project?")); - if (shouldSaveResult == true) - { - FileWriteResult saveResult = SaveTas(); - while (saveResult.IsError && shouldSaveResult != true) - { - shouldSaveResult = this.ModalMessageBox3( - $"Failed to save movie. {saveResult.UserFriendlyErrorMessage()}\n{saveResult.Exception.Message}\n\nTry again?", - "Error", - EMsgBoxIcon.Error); - if (shouldSaveResult == true) saveResult = SaveTas(); - } - } - if (shouldSaveResult is null) return false; + if (result is null) return false; + if (result.Value) SaveTas(); else CurrentTasMovie.ClearChanges(); return true; } diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs index 392eb0731c5..0777d2002a4 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs @@ -44,15 +44,11 @@ private void StartNewProjectFromNowMenuItem_Click(object sender, EventArgs e) { if (AskSaveChanges()) { - var result = CurrentTasMovie.ConvertToSavestateAnchoredMovie( + var newProject = CurrentTasMovie.ConvertToSavestateAnchoredMovie( Emulator.Frame, StatableEmulator.CloneSavestate()); - DisplayMessageIfFailed(() => result, "Failed to create movie."); - if (result.Value is ITasMovie newProject) - { - MainForm.PauseEmulator(); - LoadMovie(newProject, true); - } + MainForm.PauseEmulator(); + LoadMovie(newProject, true); } } @@ -62,14 +58,9 @@ private void StartANewProjectFromSaveRamMenuItem_Click(object sender, EventArgs { var saveRam = SaveRamEmulator?.CloneSaveRam(clearDirty: false) ?? throw new Exception("No SaveRam"); GoToFrame(AnyRowsSelected ? FirstSelectedRowIndex : 0); - var result = CurrentTasMovie.ConvertToSaveRamAnchoredMovie(saveRam); - DisplayMessageIfFailed(() => result, "Failed to create movie."); - - if (result.Value is ITasMovie newProject) - { - MainForm.PauseEmulator(); - LoadMovie(newProject, true); - } + var newProject = CurrentTasMovie.ConvertToSaveRamAnchoredMovie(saveRam); + MainForm.PauseEmulator(); + LoadMovie(newProject, true); } } @@ -125,30 +116,30 @@ public bool LoadMovieFile(string filename, bool askToSave = true) private void SaveTasMenuItem_Click(object sender, EventArgs e) { - DisplayMessageIfFailed(() => SaveTas(), "Failed to save movie."); + SaveTas(); if (Settings.BackupPerFileSave) { - DisplayMessageIfFailed(() => SaveTas(saveBackup: true), "Failed to save backup."); + SaveTas(saveBackup: true); } } private void SaveAsTasMenuItem_Click(object sender, EventArgs e) { - DisplayMessageIfFailed(() => SaveAsTas(), "Failed to save movie."); + SaveAsTas(); if (Settings.BackupPerFileSave) { - DisplayMessageIfFailed(() => SaveTas(saveBackup: true), "Failed to save backup."); + SaveTas(saveBackup: true); } } private void SaveBackupMenuItem_Click(object sender, EventArgs e) { - DisplayMessageIfFailed(() => SaveTas(saveBackup: true), "Failed to save backup."); + SaveTas(saveBackup: true); } private void SaveBk2BackupMenuItem_Click(object sender, EventArgs e) { - DisplayMessageIfFailed(() => SaveTas(saveAsBk2: true, saveBackup: true), "Failed to save backup."); + SaveTas(saveAsBk2: true, saveBackup: true); } private void SaveSelectionToMacroMenuItem_Click(object sender, EventArgs e) @@ -231,24 +222,13 @@ private void ToBk2MenuItem_Click(object sender, EventArgs e) { MessageStatusLabel.Text = "Exporting to .bk2..."; MessageStatusLabel.Owner.Update(); - Cursor = Cursors.WaitCursor; var bk2 = CurrentTasMovie.ToBk2(); bk2.Filename = fileInfo.FullName; bk2.Attach(Emulator); // required to be able to save the cycle count for ICycleTiming emulators - FileWriteResult saveResult = bk2.Save(); + bk2.Save(); + MessageStatusLabel.Text = $"{bk2.Name} exported."; Cursor = Cursors.Default; - - while (saveResult.IsError) - { - DialogResult d = MessageBox.Show( - $"Failed to save .bk2. {saveResult.UserFriendlyErrorMessage()}\nTry again?", - "Error", - MessageBoxButtons.YesNo); - if (d == DialogResult.Yes) saveResult = bk2.Save(); - else break; - } - if (!saveResult.IsError) MessageStatusLabel.Text = $"{bk2.Name} exported."; } else { diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs index 5e9b2b9d6d6..a066dffc8f8 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs @@ -35,7 +35,6 @@ public static Icon ToolIcon private const string FrameColumnName = "FrameColumn"; private UndoHistoryForm _undoForm; private Timer _autosaveTimer; - private bool _lastAutoSaveSuccess = true; private readonly int _defaultMainSplitDistance; private readonly int _defaultBranchMarkerSplitDistance; @@ -237,7 +236,11 @@ private void Tastudio_Load(object sender, EventArgs e) _autosaveTimer = new Timer(components); _autosaveTimer.Tick += AutosaveTimerEventProcessor; - ScheduleAutoSave(Settings.AutosaveInterval); + if (Settings.AutosaveInterval > 0) + { + _autosaveTimer.Interval = (int)Settings.AutosaveInterval; + _autosaveTimer.Start(); + } BranchesMarkersSplit.SetDistanceOrDefault( Settings.BranchMarkerSplitDistance, @@ -303,27 +306,20 @@ private bool Engage() { changesString = "The current movie has unsaved changes. Would you like to save before closing it?"; } - var shouldSaveResult = DialogController.ShowMessageBox3( + var result = DialogController.ShowMessageBox3( "TAStudio will create a new project file from the current movie.\n\n" + changesString, "Convert movie", EMsgBoxIcon.Question); - if (shouldSaveResult == true) + if (result == true) { - FileWriteResult saveResult = MovieSession.Movie.Save(); - if (saveResult.IsError) - { - DisplayMessageIfFailed(() => saveResult, "Failed to save movie."); - return false; - } + MovieSession.Movie.Save(); } - else if (shouldSaveResult == null) + else if (result == null) { return false; } - var tasMovie = MovieSession.Movie.ToTasMovie(); - // No need to save new movie, as there are no changes. - // User will save future changes if they want (potentially via auto-save). + var tasMovie = ConvertCurrentMovieToTasproj(); success = LoadMovie(tasMovie); } @@ -364,16 +360,6 @@ private bool Engage() return true; } - private void ScheduleAutoSave(uint secondsUntil) - { - _autosaveTimer.Stop(); - if (secondsUntil != 0) - { - _autosaveTimer.Interval = (int)secondsUntil; - _autosaveTimer.Start(); - } - } - private void AutosaveTimerEventProcessor(object sender, EventArgs e) { if (CurrentTasMovie == null) @@ -387,55 +373,28 @@ private void AutosaveTimerEventProcessor(object sender, EventArgs e) return; } - FileWriteResult saveResult; if (Settings.AutosaveAsBackupFile) { if (Settings.AutosaveAsBk2) { - saveResult = SaveTas(saveAsBk2: true, saveBackup: true); + SaveTas(saveAsBk2: true, saveBackup: true); } else { - saveResult = SaveTas(saveBackup: true); + SaveTas(saveBackup: true); } } else { if (Settings.AutosaveAsBk2) { - saveResult = SaveTas(saveAsBk2: true); + SaveTas(saveAsBk2: true); } else { - saveResult = SaveTas(); + SaveTas(); } } - - if (saveResult.IsError && _lastAutoSaveSuccess) - { - // Should we alert the user? - // Let's try again once after a bit, then alert if it fails again. - ScheduleAutoSave(60); - } - else if (saveResult.IsError) - { - _autosaveTimer.Stop(); - bool tryAgain = DialogController.ShowMessageBox2( - $"Failed to auto-save. {saveResult.UserFriendlyErrorMessage()}\n{saveResult.Exception.Message}\n\nTry again?", - "Error"); - if (tryAgain) - { - AutosaveTimerEventProcessor(null, EventArgs.Empty); - return; - } - ScheduleAutoSave(Settings.AutosaveInterval); - } - else - { - ScheduleAutoSave(Settings.AutosaveInterval); - } - - _lastAutoSaveSuccess = !saveResult.IsError; } private static readonly string[] N64CButtonSuffixes = { " C Up", " C Down", " C Left", " C Right" }; @@ -642,6 +601,13 @@ public IMovieController GetBranchInput(string branchId, int frame) return controller; } + private ITasMovie ConvertCurrentMovieToTasproj() + { + var tasMovie = MovieSession.Movie.ToTasMovie(); + tasMovie.Save(); // should this be done? + return tasMovie; + } + private bool LoadMovie(ITasMovie tasMovie, bool startsFromSavestate = false, int gotoFrame = 0) { _engaged = false; @@ -892,23 +858,12 @@ private string SuggestedTasProjName() $"{Game.FilesystemSafeName()}.{MovieService.TasMovieExtension}"); } - private void DisplayMessageIfFailed(Func action, string message) - { - FileWriteResult result = action(); - if (result.IsError) - { - DialogController.ShowMessageBox( - $"{message}\n{result.UserFriendlyErrorMessage()}\n{result.Exception.Message}", - "Error", - EMsgBoxIcon.Error); - } - } - - private FileWriteResult SaveTas(bool saveAsBk2 = false, bool saveBackup = false) + private void SaveTas(bool saveAsBk2 = false, bool saveBackup = false) { if (string.IsNullOrEmpty(CurrentTasMovie.Filename) || CurrentTasMovie.Filename == DefaultTasProjName()) { - return SaveAsTas(); + SaveAsTas(); + return; } _autosaveTimer.Stop(); @@ -925,29 +880,22 @@ private FileWriteResult SaveTas(bool saveAsBk2 = false, bool saveBackup = false) movieToSave.Attach(Emulator); } - FileWriteResult result; if (saveBackup) - result = movieToSave.SaveBackup(); + movieToSave.SaveBackup(); else - result = movieToSave.Save(); + movieToSave.Save(); - if (!result.IsError) - { - MessageStatusLabel.Text = saveBackup - ? $"Backup .{(saveAsBk2 ? MovieService.StandardMovieExtension : MovieService.TasMovieExtension)} saved to \"Movie backups\" path." - : "File saved."; - } - else + MessageStatusLabel.Text = saveBackup + ? $"Backup .{(saveAsBk2 ? MovieService.StandardMovieExtension : MovieService.TasMovieExtension)} saved to \"Movie backups\" path." + : "File saved."; + Cursor = Cursors.Default; + if (Settings.AutosaveInterval > 0) { - MessageStatusLabel.Text = "Failed to save movie."; + _autosaveTimer.Start(); } - - Cursor = Cursors.Default; - ScheduleAutoSave(Settings.AutosaveInterval); - return result; } - private FileWriteResult SaveAsTas() + private void SaveAsTas() { _autosaveTimer.Stop(); @@ -963,33 +911,25 @@ private FileWriteResult SaveAsTas() TAStudioProjectsFSFilterSet, this); - FileWriteResult saveResult = null; if (fileInfo != null) { MessageStatusLabel.Text = "Saving..."; MessageStatusLabel.Owner.Update(); Cursor = Cursors.WaitCursor; CurrentTasMovie.Filename = fileInfo.FullName; - saveResult = CurrentTasMovie.Save(); + CurrentTasMovie.Save(); Settings.RecentTas.Add(CurrentTasMovie.Filename); + MessageStatusLabel.Text = "File saved."; Cursor = Cursors.Default; + } - if (!saveResult.IsError) - { - MessageStatusLabel.Text = "File saved."; - ScheduleAutoSave(Settings.AutosaveInterval); - } - else - { - MessageStatusLabel.Text = "Failed to save."; - } + if (Settings.AutosaveInterval > 0) + { + _autosaveTimer.Start(); } UpdateWindowTitle(); // changing the movie's filename does not flag changes, so we need to ensure the window title is always updated MainForm.UpdateWindowTitle(); - - if (fileInfo != null) return saveResult; - else return new(FileWriteEnum.Success, "", null); // user cancelled, so we were successful in not saving } protected override string WindowTitle diff --git a/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs b/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs index 954d9f4bfbe..93c383efe95 100644 --- a/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs +++ b/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs @@ -70,10 +70,6 @@ public FakeMovieSession(IEmulator emulator) public void PopupMessage(string message) => throw new NotImplementedException(); public void QueueNewMovie(IMovie movie, string systemId, string loadedRomHash, PathEntryCollection pathEntries, IDictionary preferredCores) => throw new NotImplementedException(); public void RunQueuedMovie(bool recordMode, IEmulator emulator) => throw new NotImplementedException(); - public FileWriteResult StopMovie(bool saveChanges = true) - { - Movie?.Stop(); - return new(FileWriteEnum.Success, "", null); - } + public void StopMovie(bool saveChanges = true) => Movie?.Stop(); } } From 8ce051278123ba543c85f1a78b8a8267de769b80 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:21:44 +1000 Subject: [PATCH 19/23] reapply intervening changeset 1 to `IMainFormForTools.cs` --- .../IMainFormForTools.cs | 60 +++++++++---------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/src/BizHawk.Client.Common/IMainFormForTools.cs b/src/BizHawk.Client.Common/IMainFormForTools.cs index 417fabaccac..5212a46cfdd 100644 --- a/src/BizHawk.Client.Common/IMainFormForTools.cs +++ b/src/BizHawk.Client.Common/IMainFormForTools.cs @@ -1,8 +1,7 @@ using BizHawk.Bizware.Graphics; -using BizHawk.Client.Common; using BizHawk.Emulation.Common; -namespace BizHawk.Client.EmuHawk +namespace BizHawk.Client.Common { public interface IMainFormForTools : IDialogController { @@ -12,98 +11,95 @@ public interface IMainFormForTools : IDialogController /// referenced by 3 or more tools string CurrentlyOpenRom { get; } - /// referenced from and RetroAchievements + /// referenced from HexEditor and RetroAchievements LoadRomArgs CurrentlyOpenRomArgs { get; } - /// only referenced from + /// only referenced from TAStudio bool EmulatorPaused { get; } - /// only referenced from - bool GameIsClosing { get; } - - /// only referenced from + /// only referenced from PlaybackBox bool HoldFrameAdvance { get; set; } - /// only referenced from + /// only referenced from BasicBot bool InvisibleEmulation { get; set; } - /// only referenced from + /// only referenced from LuaConsole bool IsTurboing { get; } - /// only referenced from + /// only referenced from TAStudio bool IsFastForwarding { get; } - /// referenced from and + /// referenced from PlayMovie and TAStudio int? PauseOnFrame { get; set; } - /// only referenced from + /// only referenced from PlaybackBox bool PressRewind { get; set; } - /// referenced from and + /// referenced from BookmarksBranchesBox and VideoWriterChooserForm BitmapBuffer CaptureOSD(); - /// only referenced from + /// only referenced from TAStudio void DisableRewind(); - /// only referenced from + /// only referenced from TAStudio void EnableRewind(bool enabled); - /// only referenced from + /// only referenced from TAStudio bool EnsureCoreIsAccurate(); - /// only referenced from + /// only referenced from TAStudio void FrameAdvance(bool discardApiHawkSurfaces = true); - /// only referenced from + /// only referenced from LuaConsole /// Override void FrameBufferResized(bool forceWindowResize = false); - /// only referenced from + /// only referenced from BasicBot bool LoadQuickSave(int slot, bool suppressOSD = false); - /// referenced from and RetroAchievements + /// referenced from MultiDiskBundler and RetroAchievements bool LoadRom(string path, LoadRomArgs args); - /// only referenced from + /// only referenced from BookmarksBranchesBox BitmapBuffer MakeScreenshotImage(); - /// referenced from + /// referenced from ToolFormBase void MaybePauseFromMenuOpened(); - /// referenced from + /// referenced from ToolFormBase void MaybeUnpauseFromMenuClosed(); /// referenced by 3 or more tools void PauseEmulator(); - /// only referenced from + /// only referenced from TAStudio bool BlockFrameAdvance { get; set; } - /// referenced from and + /// referenced from PlaybackBox and TAStudio void SetMainformMovieInfo(); /// referenced by 3 or more tools bool StartNewMovie(IMovie movie, bool newMovie); - /// only referenced from + /// only referenced from BasicBot void Throttle(); - /// only referenced from + /// only referenced from TAStudio void TogglePause(); /// referenced by 3 or more tools void UnpauseEmulator(); - /// only referenced from + /// only referenced from BasicBot void Unthrottle(); - /// only referenced from + /// only referenced from LogWindow void UpdateDumpInfo(RomStatus? newStatus = null); - /// only referenced from + /// only referenced from BookmarksBranchesBox void UpdateStatusSlots(); - /// referenced from and RetroAchievements + /// only referenced from TAStudio void UpdateWindowTitle(); } } From a164223f3c6d288b5f25992aeb6a3fe616bc6fdf Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:23:16 +1000 Subject: [PATCH 20/23] reapply intervening changeset 2 to `IMainFormForTools.cs` --- src/BizHawk.Client.Common/IMainFormForTools.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/BizHawk.Client.Common/IMainFormForTools.cs b/src/BizHawk.Client.Common/IMainFormForTools.cs index 5212a46cfdd..798f6e181ea 100644 --- a/src/BizHawk.Client.Common/IMainFormForTools.cs +++ b/src/BizHawk.Client.Common/IMainFormForTools.cs @@ -20,9 +20,6 @@ public interface IMainFormForTools : IDialogController /// only referenced from PlaybackBox bool HoldFrameAdvance { get; set; } - /// only referenced from BasicBot - bool InvisibleEmulation { get; set; } - /// only referenced from LuaConsole bool IsTurboing { get; } From 974dbc77343f765e45f2d325b26139aa234b025d Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:26:35 +1000 Subject: [PATCH 21/23] reapply intervening changes to `LuaConsole.cs` --- .../tools/Lua/LuaConsole.cs | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs b/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs index 901a492c332..b36d53c530c 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs @@ -225,10 +225,10 @@ public override void Restart() return; } - runningScripts = LuaImp.ScriptList.Where(lf => lf.Enabled).ToList(); + runningScripts = _openedFiles.Where(lf => lf.Enabled).ToList(); // we don't use runningScripts here as the other scripts need to be stopped too - foreach (var file in LuaImp.ScriptList) + foreach (var file in _openedFiles) { file.Stop(); } @@ -263,7 +263,7 @@ public override void Restart() _nonFile = new LuaFile(Config.PathEntries.LuaAbsolutePath(), UpdateRegisteredFunctionsDialog); _nonFile.Start(LuaImp.SpawnBlankCoroutineAndSandbox(null)); - LuaImp.ScriptList.Insert(0, _nonFile); + _openedFiles.Insert(0, _nonFile); UpdateDialog(); } @@ -360,7 +360,7 @@ public void LoadLuaFile(string path) { var luaFile = new LuaFile(absolutePath, UpdateRegisteredFunctionsDialog); - LuaImp.ScriptList.Add(luaFile); + _openedFiles.Add(luaFile); _openedFiles.Add(luaFile); LuaListView.RowCount = _openedFiles.Count; Config.RecentLua.Add(absolutePath); @@ -403,7 +403,7 @@ private void RemoveLuaFile(LuaFile item) item.Stop(); RemoveFileWatcher(item); } - LuaImp.ScriptList.Remove(item); + _openedFiles.Remove(item); _openedFiles.Remove(item); } @@ -549,9 +549,9 @@ public void ClearOutputWindow() private void SyncScriptList() { - LuaImp.ScriptList.Clear(); - LuaImp.ScriptList.Add(_nonFile); - LuaImp.ScriptList.AddRange(_openedFiles.Where(static lf => !lf.IsSeparator)); + _openedFiles.Clear(); + _openedFiles.Add(_nonFile); + _openedFiles.AddRange(_openedFiles.Where(static lf => !lf.IsSeparator)); } public bool LoadLuaSession(string path) @@ -690,7 +690,7 @@ private void SaveSessionAs() var file = GetSaveFileFromUser(); if (file != null) { - LuaImp.ScriptList.Save(file.FullName); + _openedFiles.Save(file.FullName); Config.RecentLuaSession.Add(file.FullName); OutputMessages.Text = $"{file.Name} saved."; } @@ -715,14 +715,14 @@ private void LoadSessionFromRecent(string path) public override bool AskSaveChanges() { - if (!LuaImp.ScriptList.Changes || string.IsNullOrEmpty(LuaImp.ScriptList.Filename)) return true; + if (!_openedFiles.Changes || string.IsNullOrEmpty(_openedFiles.Filename)) return true; var result = DialogController.DoWithTempMute(() => this.ModalMessageBox3( caption: "Closing with Unsaved Changes", icon: EMsgBoxIcon.Question, text: $"Save {WindowTitleStatic} session?")); if (result is null) return false; if (result.Value) SaveOrSaveAs(); - else LuaImp.ScriptList.Changes = false; + else _openedFiles.Changes = false; return true; } @@ -732,16 +732,16 @@ private void UpdateRegisteredFunctionsDialog() foreach (var form in Application.OpenForms.OfType().ToList()) { - form.UpdateValues(LuaImp.ScriptList); + form.UpdateValues(_openedFiles); } } private void SaveOrSaveAs() { - if (!string.IsNullOrWhiteSpace(LuaImp.ScriptList.Filename)) + if (!string.IsNullOrWhiteSpace(_openedFiles.Filename)) { - LuaImp.ScriptList.Save(LuaImp.ScriptList.Filename); - Config.RecentLuaSession.Add(LuaImp.ScriptList.Filename); + _openedFiles.Save(_openedFiles.Filename); + Config.RecentLuaSession.Add(_openedFiles.Filename); } else { @@ -786,10 +786,10 @@ private void OpenSessionMenuItem_Click(object sender, EventArgs e) private void SaveSessionMenuItem_Click(object sender, EventArgs e) { - if (LuaImp.ScriptList.Changes) + if (_openedFiles.Changes) { SaveOrSaveAs(); - OutputMessages.Text = $"{Path.GetFileName(LuaImp.ScriptList.Filename)} saved."; + OutputMessages.Text = $"{Path.GetFileName(_openedFiles.Filename)} saved."; } } @@ -1086,7 +1086,7 @@ private void RegisteredFunctionsMenuItem_Click(object sender, EventArgs e) if (!alreadyOpen) { - new LuaRegisteredFunctionsList(LuaImp.ScriptList) + new LuaRegisteredFunctionsList(_openedFiles) { StartLocation = this.ChildPointToScreen(LuaListView), }.Show(); @@ -1199,15 +1199,15 @@ private void ScriptListContextMenu_Opening(object sender, CancelEventArgs e) StopAllScriptsContextItem.Visible = ScriptContextSeparator.Visible = - LuaImp.ScriptList.Exists(file => file.Enabled); + _openedFiles.Exists(file => file.Enabled); - ClearRegisteredFunctionsContextItem.Enabled = LuaImp.ScriptList.Exists(lf => lf.Functions.Count != 0); + ClearRegisteredFunctionsContextItem.Enabled = _openedFiles.Exists(lf => lf.Functions.Count != 0); } private void ConsoleContextMenu_Opening(object sender, CancelEventArgs e) { RegisteredFunctionsContextItem.Enabled = ClearRegisteredFunctionsLogContextItem.Enabled - = LuaImp.ScriptList.Exists(lf => lf.Functions.Count != 0); + = _openedFiles.Exists(lf => lf.Functions.Count != 0); CopyContextItem.Enabled = OutputBox.SelectedText.Length is not 0; ClearConsoleContextItem.Enabled = SelectAllContextItem.Enabled = OutputBox.Text.Length is not 0; } @@ -1241,7 +1241,7 @@ private void CopyContextItem_Click(object sender, EventArgs e) private void ClearRegisteredFunctionsContextMenuItem_Click(object sender, EventArgs e) { - foreach (LuaFile lf in LuaImp.ScriptList) + foreach (LuaFile lf in _openedFiles) lf.Functions.Clear(); } From d98c3c218b0fd2042205fa63f10d2f78145f20a0 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 03:19:08 +1000 Subject: [PATCH 22/23] reapply intervening changes to `MovieZone.cs` --- src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs b/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs index ee3b951c17b..adbd1b42d29 100644 --- a/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs +++ b/src/BizHawk.Client.Common/tools/TAStudio/MovieZone.cs @@ -159,12 +159,12 @@ private void PlaceMacroInternal(IMovie movie, int start) public void Save(string fileName) { // Save the controller definition/LogKey - // Save the controller name and player count. (Only for the user.) + // Save the controller name and controller groups count. (Only for the user) // Save whether or not the macro should use overlay input, and/or replace string[] header = new string[4]; header[0] = InputKey; - header[1] = _emulator.ControllerDefinition.Name; - header[2] = _emulator.ControllerDefinition.PlayerCount.ToString(); + header[1] = _movieDefinition.Name; + header[2] = _movieDefinition.ControlsOrdered.Count.ToString(); header[3] = $"{Overlay},{Replace}"; File.WriteAllLines(fileName, header); From 1c84b2b8c516ecc3eb76d1eef8dae35708b4d1fa Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 28 Apr 2026 02:28:19 +1000 Subject: [PATCH 23/23] reapply intervening changeset 2 to `TAStudio.MenuItems.cs` --- .../tools/TAStudio/TAStudio.MenuItems.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs index 0777d2002a4..354616ce0b6 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs @@ -164,13 +164,11 @@ private void SaveSelectionToMacroMenuItem_Click(object sender, EventArgs e) if (file != null) { - var selectionStart = TasView.SelectionStartIndex!.Value; + var selectionStart = FirstSelectedRowIndex; new MovieZone( - Emulator, - Tools, - MovieSession, + CurrentTasMovie, start: selectionStart, - length: TasView.SelectionEndIndex!.Value - selectionStart + 1) + length: endIndex - selectionStart + 1) .Save(file.FullName); Config.RecentMacros.Add(file.FullName);