Handle most errors related to savestate files. Autosave last slot is still broken on close and on TAStudio open.

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;
 

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;
+			}
 		}
 	}
 }

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);
+		}
+
 		/// <summary>Creates and shows a <c>System.Windows.Forms.OpenFileDialog</c> or equivalent with the receiver (<paramref name="dialogParent"/>) as its parent</summary>
 		/// <param name="discardCWDChange"><c>OpenFileDialog.RestoreDirectory</c> (isn't this useless when specifying <paramref name="initDir"/>? keeping it for backcompat)</param>
 		/// <param name="filter"><c>OpenFileDialog.Filter</c></param>

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,
 	}
 
 	/// <summary>
@@ -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) { }
 	}
+
+	/// <summary>
+	/// This only exists as a way to avoid changing the API behavior.
+	/// </summary>
+	public class UnlessUsingApiException : Exception
+	{
+		public UnlessUsingApiException(string message) : base(message) { }
+	}
 }

src/BizHawk.Client.Common/IMainFormForApi.cs

@@ -89,10 +89,16 @@ namespace BizHawk.Client.Common
 		/// <remarks>only referenced from <see cref="MovieApi"/></remarks>
 		bool RestartMovie();
 
-		/// <remarks>only referenced from <see cref="SaveStateApi"/></remarks>
+		FileWriteResult SaveQuickSave(int slot, bool suppressOSD = false);
-		void SaveQuickSave(int slot, bool suppressOSD = false, bool fromLua = false);
 
-		void SaveState(string path, string userFriendlyStateName, bool fromLua = false, bool suppressOSD = false);
+		/// <summary>
+		/// Creates a savestate and writes it to a file.
+		/// </summary>
+		/// <param name="path">The path of the file to write.</param>
+		/// <param name="userFriendlyStateName">The name to use for the state on the client's HUD.</param>
+		/// <param name="suppressOSD">If true, the client HUD will not show a success message.</param>
+		/// <returns>Returns a value indicating if there was an error and (if there was) why.</returns>
+		FileWriteResult SaveState(string path, string userFriendlyStateName, bool suppressOSD = false);
 
 		void SeekFrameAdvance();
 

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)
+		/// <summary>
+		/// Takes the .state and .bak files and swaps them
+		/// </summary>
+		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 backup = new FileInfo(backupPath);
-			var temp = new FileInfo($"{path}.bak.tmp");
 
 			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");
+			// Move backup to temp.
-			backup.Delete();
+			try
-			state.CopyTo($"{path}.bak");
+			{
-			state.Delete();
+				backup.MoveTo(tempPath);
-			temp.CopyTo(path);
+			}
-			temp.Delete();
+			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();
 		}
 	}
 }

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<ZipStateSaver> 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)

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,9 +1375,16 @@ 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);
+			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);
 		}
 

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();
 			}

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();
+				if (isQuickSave) tool.SaveQuickSave(SlotToInt(userFriendlyStateName));
-				return;
+				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!");
+				const string errmsg = "Cannot savestate after movie end!";
-				return;
+				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<LuaConsole>())
+				{
+					Tools.LuaConsole.LuaImp.CallSaveStateEvent(userFriendlyStateName);
+				}
+
 				if (!suppressOSD)
 				{
 					AddOnScreenMessage($"Saved state: {userFriendlyStateName}");
 				}
 			}
 
-			if (!fromLua)
-			{
 			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
+			return SaveStateInternal(path, quickSlotName, suppressOSD, true);
-			if (Config.Savestates.MakeBackups)
-			{
-				Util.TryMoveBackupFile(path, $"{path}.bak");
 		}
 
-			SaveState(path, quickSlotName, fromLua, suppressOSD);
+		/// <summary>
-
+		/// Runs <see cref="SaveQuickSave(int, bool)"/> and displays a pop up message if there was an error.
-			if (Tools.Has<LuaConsole>())
+		/// </summary>
+		private void SaveQuickSaveAndShowError(int slot)
 		{
-				Tools.LuaConsole.LuaImp.CallSaveStateEvent(quickSlotName);
+			FileWriteResult result = SaveQuickSave(slot);
+			if (result.IsError)
+			{
+				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<TAStudio>())
 			{