diff --git a/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs index fbc5c2636f..1e00e4ba45 100644 --- a/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs @@ -160,7 +160,16 @@ namespace BizHawk.Client.Common public void SaveRam() => _mainForm.FlushSaveRAM(); - public void SaveState(string name) => _mainForm.SaveState(Path.Combine(_config.PathEntries.SaveStateAbsolutePath(Game.System), $"{name}.State"), name, fromLua: false); + // 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 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 9bbe860e9c..271f036ed2 100644 --- a/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs @@ -39,8 +39,17 @@ namespace BizHawk.Client.Common return _mainForm.LoadQuickSave(slotNum, suppressOSD: suppressOSD); } - public void Save(string path, bool suppressOSD) => _mainForm.SaveState(path, path, true, 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; + } + } + // 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); @@ -49,7 +58,11 @@ namespace BizHawk.Client.Common LogCallback(ERR_MSG_USE_SLOT_10); slotNum = 10; } - _mainForm.SaveQuickSave(slotNum, suppressOSD: suppressOSD, fromLua: true); + FileWriteResult result = _mainForm.SaveQuickSave(slotNum, suppressOSD: suppressOSD); + if (result.Exception != null && result.Exception is not UnlessUsingApiException) + { + throw result.Exception; + } } } } diff --git a/src/BizHawk.Client.Common/DialogControllerExtensions.cs b/src/BizHawk.Client.Common/DialogControllerExtensions.cs index 768dd0a6e2..8e861b1358 100644 --- a/src/BizHawk.Client.Common/DialogControllerExtensions.cs +++ b/src/BizHawk.Client.Common/DialogControllerExtensions.cs @@ -1,6 +1,7 @@ #nullable enable using System.Collections.Generic; +using System.Diagnostics; namespace BizHawk.Client.Common { @@ -60,6 +61,20 @@ namespace BizHawk.Client.Common 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 633efbd7a3..c11193e347 100644 --- a/src/BizHawk.Client.Common/FileWriteResult.cs +++ b/src/BizHawk.Client.Common/FileWriteResult.cs @@ -7,12 +7,17 @@ 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, } /// @@ -26,7 +31,7 @@ namespace BizHawk.Client.Common public bool IsError => Error != FileWriteEnum.Success; - internal FileWriteResult(FileWriteEnum error, FileWritePaths writer, Exception? exception) + public FileWriteResult(FileWriteEnum error, FileWritePaths writer, Exception? exception) { Error = error; Exception = exception; @@ -65,6 +70,15 @@ namespace BizHawk.Client.Common 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"; @@ -106,4 +120,12 @@ namespace BizHawk.Client.Common 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 790179a330..a9580ec178 100644 --- a/src/BizHawk.Client.Common/IMainFormForApi.cs +++ b/src/BizHawk.Client.Common/IMainFormForApi.cs @@ -89,10 +89,16 @@ namespace BizHawk.Client.Common /// only referenced from bool RestartMovie(); - /// only referenced from - void SaveQuickSave(int slot, bool suppressOSD = false, bool fromLua = false); + FileWriteResult SaveQuickSave(int slot, bool suppressOSD = false); - void SaveState(string path, string userFriendlyStateName, bool fromLua = false, bool suppressOSD = 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 HUD. + /// If true, the client HUD will not show a success message. + /// Returns a value indicating if there was an error and (if there was) why. + FileWriteResult SaveState(string path, string userFriendlyStateName, bool suppressOSD = false); void SeekFrameAdvance(); diff --git a/src/BizHawk.Client.Common/SaveSlotManager.cs b/src/BizHawk.Client.Common/SaveSlotManager.cs index ea95e684f8..774e982103 100644 --- a/src/BizHawk.Client.Common/SaveSlotManager.cs +++ b/src/BizHawk.Client.Common/SaveSlotManager.cs @@ -69,31 +69,65 @@ namespace BizHawk.Client.Common public bool IsRedo(IMovie movie, int slot) => slot is >= 1 and <= 10 && movie is not ITasMovie && _redo[slot - 1]; - public void SwapBackupSavestate(IMovie movie, string path, int currentSlot) + /// + /// Takes the .state and .bak files and swaps them + /// + public FileWriteResult SwapBackupSavestate(IMovie movie, string path, int currentSlot) { - // Takes the .state and .bak files and swaps them + string backupPath = $"{path}.bak"; + string tempPath = $"{path}.bak.tmp"; + var state = new FileInfo(path); - var backup = new FileInfo($"{path}.bak"); - var temp = new FileInfo($"{path}.bak.tmp"); + var backup = new FileInfo(backupPath); if (!state.Exists || !backup.Exists) { - return; + return new(); } - if (temp.Exists) + // Delete old temp file if it exists. + try { - temp.Delete(); + if (File.Exists(tempPath)) File.Delete(tempPath); + } + catch (Exception ex) + { + return new(FileWriteEnum.FailedToDeleteGeneric, new(tempPath, ""), ex); } - backup.CopyTo($"{path}.bak.tmp"); - backup.Delete(); - state.CopyTo($"{path}.bak"); - state.Delete(); - temp.CopyTo(path); - 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); + } ToggleRedo(movie, currentSlot); + return new(); } } } diff --git a/src/BizHawk.Client.Common/savestates/SavestateFile.cs b/src/BizHawk.Client.Common/savestates/SavestateFile.cs index bb508a89a2..57d908bc65 100644 --- a/src/BizHawk.Client.Common/savestates/SavestateFile.cs +++ b/src/BizHawk.Client.Common/savestates/SavestateFile.cs @@ -50,7 +50,7 @@ namespace BizHawk.Client.Common _userBag = userBag; } - public FileWriteResult Create(string filename, SaveStateConfig config) + public FileWriteResult Create(string filename, SaveStateConfig config, bool makeBackup) { FileWriteResult createResult = ZipStateSaver.Create(filename, config.CompressionLevelNormal); if (createResult.IsError) return createResult; @@ -115,7 +115,8 @@ namespace BizHawk.Client.Common bs.PutLump(BinaryStateLump.LagLog, tw => ((ITasMovie) _movieSession.Movie).LagLog.Save(tw)); } - return bs.CloseAndDispose(); + makeBackup = makeBackup && config.MakeBackups; + return bs.CloseAndDispose(makeBackup ? $"{filename}.bak" : null); } 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 1d4d25ce0d..5c584f8118 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs @@ -270,7 +270,7 @@ namespace BizHawk.Client.EmuHawk } private void QuickSavestateMenuItem_Click(object sender, EventArgs e) - => SaveQuickSave(int.Parse(((ToolStripMenuItem) sender).Text)); + => SaveQuickSaveAndShowError(int.Parse(((ToolStripMenuItem) sender).Text)); private void SaveNamedStateMenuItem_Click(object sender, EventArgs e) => SaveStateAs(); @@ -306,7 +306,7 @@ namespace BizHawk.Client.EmuHawk => SavestateCurrentSlot(); private void SavestateCurrentSlot() - => SaveQuickSave(Config.SaveSlot); + => SaveQuickSaveAndShowError(Config.SaveSlot); private void LoadCurrentSlotMenuItem_Click(object sender, EventArgs e) => LoadstateCurrentSlot(); @@ -1375,8 +1375,15 @@ namespace BizHawk.Client.EmuHawk private void UndoSavestateContextMenuItem_Click(object sender, EventArgs e) { var slot = Config.SaveSlot; - _stateSlots.SwapBackupSavestate(MovieSession.Movie, $"{SaveStatePrefix()}.QuickSave{slot % 10}.State", slot); - AddOnScreenMessage($"Save slot {slot} restored."); + 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."); + } } private void ClearSramContextMenuItem_Click(object sender, EventArgs e) @@ -1446,7 +1453,7 @@ namespace BizHawk.Client.EmuHawk if (sender == Slot9StatusButton) slot = 9; if (sender == Slot0StatusButton) slot = 10; - if (e.Button is MouseButtons.Right) SaveQuickSave(slot); + if (e.Button is MouseButtons.Right) SaveQuickSaveAndShowError(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 86c5da113c..ba4008e60c 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs @@ -10,7 +10,7 @@ namespace BizHawk.Client.EmuHawk { void SelectAndSaveToSlot(int slot) { - SaveQuickSave(slot); + SaveQuickSaveAndShowError(slot); Config.SaveSlot = slot; UpdateStatusSlots(); } diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index 6637292c96..091670c339 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -4137,8 +4137,21 @@ namespace BizHawk.Client.EmuHawk } } + 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(); @@ -4160,9 +4173,14 @@ namespace BizHawk.Client.EmuHawk GameIsClosing = false; } - private void AutoSaveStateIfConfigured() + private FileWriteResult AutoSaveStateIfConfigured() { - if (Config.AutoSaveLastSaveSlot && Emulator.HasSavestates()) SavestateCurrentSlot(); + if (Config.AutoSaveLastSaveSlot && Emulator.HasSavestates()) + { + return SaveQuickSave(Config.SaveSlot); + } + + return new(); } public bool GameIsClosing { get; private set; } // Lets tools make better decisions when being called by CloseGame @@ -4327,27 +4345,47 @@ namespace BizHawk.Client.EmuHawk return LoadState(path: path, userFriendlyStateName: quickSlotName, suppressOSD: suppressOSD); } - public void SaveState(string path, string userFriendlyStateName, bool fromLua = false, bool suppressOSD = false) + private FileWriteResult SaveStateInternal(string path, string userFriendlyStateName, bool suppressOSD, bool isQuickSave) { if (!Emulator.HasSavestates()) { - return; + return new(FileWriteEnum.Aborted, new("", ""), new UnlessUsingApiException("The current emulator does not support savestates.")); + } + + if (isQuickSave) + { + var handled = false; + if (QuicksaveSave is not null) + { + BeforeQuickSaveEventArgs args = new(userFriendlyStateName); + QuicksaveSave(this, args); + handled = args.Handled; + } + if (handled) + { + // I suppose this is a success? But we have no path. + return new(); + } } if (ToolControllingSavestates is { } tool) { - tool.SaveState(); - return; + if (isQuickSave) tool.SaveQuickSave(SlotToInt(userFriendlyStateName)); + else tool.SaveState(); + // assume success by the tool: state was created, but not as a file. So no path. + return new(); } if (MovieSession.Movie.IsActive() && Emulator.Frame > MovieSession.Movie.FrameCount) { - AddOnScreenMessage("Cannot savestate after movie end!"); - return; + 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)); } FileWriteResult result = new SavestateFile(Emulator, MovieSession, MovieSession.UserBag) - .Create(path, Config.Savestates); + .Create(path, Config.Savestates, isQuickSave); if (result.IsError) { AddOnScreenMessage($"Unable to save state {path}"); @@ -4361,58 +4399,45 @@ namespace BizHawk.Client.EmuHawk } RA?.OnSaveState(path); + if (Tools.Has()) + { + Tools.LuaConsole.LuaImp.CallSaveStateEvent(userFriendlyStateName); + } + if (!suppressOSD) { AddOnScreenMessage($"Saved state: {userFriendlyStateName}"); } } - if (!fromLua) - { - UpdateStatusSlots(); - } + UpdateStatusSlots(); + + return result; + } + + public FileWriteResult SaveState(string path, string userFriendlyStateName, bool suppressOSD = false) + { + return SaveStateInternal(path, userFriendlyStateName, suppressOSD, false); } // TODO: should backup logic be stuffed in into Client.Common.SaveStateManager? - public void SaveQuickSave(int slot, bool suppressOSD = false, bool fromLua = false) + public FileWriteResult SaveQuickSave(int slot, bool suppressOSD = false) { - if (!Emulator.HasSavestates()) - { - return; - } var quickSlotName = $"QuickSave{slot % 10}"; - var handled = false; - if (QuicksaveSave is not null) - { - BeforeQuickSaveEventArgs args = new(quickSlotName); - QuicksaveSave(this, args); - handled = args.Handled; - } - if (handled) - { - return; - } - - if (ToolControllingSavestates is { } tool) - { - tool.SaveQuickSave(SlotToInt(quickSlotName)); - return; - } - var path = $"{SaveStatePrefix()}.{quickSlotName}.State"; - new FileInfo(path).Directory?.Create(); - // Make backup first - if (Config.Savestates.MakeBackups) + return SaveStateInternal(path, quickSlotName, suppressOSD, true); + } + + /// + /// Runs and displays a pop up message if there was an error. + /// + private void SaveQuickSaveAndShowError(int slot) + { + FileWriteResult result = SaveQuickSave(slot); + if (result.IsError) { - Util.TryMoveBackupFile(path, $"{path}.bak"); - } - - SaveState(path, quickSlotName, fromLua, suppressOSD); - - if (Tools.Has()) - { - Tools.LuaConsole.LuaImp.CallSaveStateEvent(quickSlotName); + this.ErrorMessageBox(result, "Quick save failed."); } } @@ -4476,12 +4501,19 @@ namespace BizHawk.Client.EmuHawk var path = Config.PathEntries.SaveStateAbsolutePath(Game.System); new FileInfo(path).Directory?.Create(); - var result = this.ShowFileSaveDialog( + var shouldSaveResult = this.ShowFileSaveDialog( fileExt: "State", filter: EmuHawkSaveStatesFSFilterSet, initDir: path, initFileName: $"{SaveStatePrefix()}.QuickSave0.State"); - if (result is not null) SaveState(path: result, userFriendlyStateName: result); + if (shouldSaveResult is not null) + { + FileWriteResult saveResult = SaveState(path: shouldSaveResult, userFriendlyStateName: shouldSaveResult); + if (saveResult.IsError) + { + this.ErrorMessageBox(saveResult, "Unable to save."); + } + } if (Tools.IsLoaded()) {