Handle file writing errors for movies and all tools.

Author: SuuperW

src/BizHawk.Client.Common/DialogControllerExtensions.cs

@@ -6,6 +6,13 @@
 
 namespace BizHawk.Client.Common
 {
+	public enum TryAgainResult
+	{
+		Saved,
+		IgnoredFailure,
+		Canceled,
+	}
+
 	public static class DialogControllerExtensions
 	{
 		public static void AddOnScreenMessage(
@@ -84,7 +91,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.
 		/// </summary>
 		/// <returns>Returns true on success or if the user said no. Returns false if the user said cancel.</returns>
-		public static bool DoWithTryAgainBox(
+		public static TryAgainResult DoWithTryAgainBox(
 			this IDialogParent dialogParent,
 			Func<FileWriteResult> action,
 			string message)
@@ -98,12 +105,12 @@ public static bool DoWithTryAgainBox(
 							$"{fileResult.UserFriendlyErrorMessage()}\n{fileResult.Exception!.Message}",
 						caption: "Error",
 						icon: EMsgBoxIcon.Error);
-				if (askResult == null) return false;
-				if (askResult == false) return true;
+				if (askResult == null) return TryAgainResult.Canceled;
+				if (askResult == false) return TryAgainResult.IgnoredFailure;
 				if (askResult == true) fileResult = action();
 			}
 
-			return true;
+			return TryAgainResult.Saved;
 		}
 
 		/// <summary>Creates and shows a <c>System.Windows.Forms.OpenFileDialog</c> or equivalent with the receiver (<paramref name="dialogParent"/>) as its parent</summary>

src/BizHawk.Client.Common/FileWriter.cs

@@ -56,6 +56,22 @@ public static FileWriteResult Write(string path, byte[] bytes, string? backupPat
 			return createResult.Value.CloseAndDispose(backupPath);
 		}
 
+		public static FileWriteResult Write(string path, Action<Stream> writeCallback, string? backupPath = null)
+		{
+			FileWriteResult<FileWriter> 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);
+		}
 
 		/// <summary>
 		/// Create a FileWriter instance, or return an error if unable to access the file.
@@ -76,6 +92,7 @@ public static FileWriteResult<FileWriter> 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);
 			}

src/BizHawk.Client.Common/lua/LuaFileList.cs

@@ -102,9 +102,8 @@ public bool Load(string path, bool disableOnLoad)
 			return true;
 		}
 
-		public void Save(string path)
+		public FileWriteResult 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)
@@ -123,10 +122,19 @@ public void Save(string path)
 				}
 			}
 
-			sw.Write(sb.ToString());
+			FileWriteResult result = FileWriter.Write(path, (fs) =>
+			{
+				using var sw = new StreamWriter(fs);
+				sw.Write(sb.ToString());
+			});
 
-			Filename = path;
-			Changes = false;
+			if (!result.IsError)
+			{
+				Filename = path;
+				Changes = false;
+			}
+
+			return result;
 		}
 	}
 }

src/BizHawk.Client.Common/movie/MovieSession.cs

@@ -385,6 +385,8 @@ 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:

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 void SaveOnClose()
+		public FileWriteResult SaveOnClose()
 		{
 			if (_config.AutoSaveOnClose)
 			{
@@ -221,17 +221,27 @@ public void SaveOnClose()
 						CurrentFileName = _defaultFileName;
 					}
 
-					SaveFile(CurrentFileName);
+					return SaveFile(CurrentFileName);
 				}
 				else if (_cheatList.Count is 0 && !string.IsNullOrWhiteSpace(CurrentFileName))
 				{
-					File.Delete(CurrentFileName);
+					try
+					{
+						File.Delete(CurrentFileName);
+					}
+					catch (Exception ex)
+					{
+						return new(FileWriteEnum.FailedToDeleteGeneric, new(CurrentFileName, ""), ex);
+					}
 					_config.Recent.Remove(CurrentFileName);
+					return new();
 				}
 			}
+
+			return new();
 		}
 
-		public bool Save()
+		public FileWriteResult Save()
 		{
 			if (string.IsNullOrWhiteSpace(CurrentFileName))
 			{
@@ -241,54 +251,51 @@ public bool Save()
 			return SaveFile(CurrentFileName);
 		}
 
-		public bool SaveFile(string path)
+		public FileWriteResult SaveFile(string path)
 		{
-			try
-			{
-				new FileInfo(path).Directory?.Create();
-				var sb = new StringBuilder();
+			var sb = new StringBuilder();
 
-				foreach (var cheat in _cheatList)
+			foreach (var cheat in _cheatList)
+			{
+				if (cheat.IsSeparator)
 				{
-					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);
-					}
+					sb.AppendLine("----");
 				}
-
-				File.WriteAllText(path, sb.ToString());
-
+				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)
+			{
 				CurrentFileName = path;
 				_config.Recent.Add(CurrentFileName);
 				Changes = false;
-				return true;
-			}
-			catch
-			{
-				return false;
 			}
+			return result;
 		}
 
 		public bool Load(IMemoryDomains domains, string path, bool append)

src/BizHawk.Client.Common/tools/Watch/WatchList/WatchList.cs

@@ -1,5 +1,6 @@
 using System.Collections;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Globalization;
 using System.IO;
 using System.Linq;
@@ -338,39 +339,37 @@ public void Reload()
 			}
 		}
 
-		public bool Save()
+		public FileWriteResult Save()
 		{
 			if (string.IsNullOrWhiteSpace(CurrentFileName))
 			{
-				return false;
+				return new();
 			}
 
-			using (var sw = new StreamWriter(CurrentFileName))
-			{
-				var sb = new StringBuilder();
-				sb.Append("SystemID ").AppendLine(_systemId);
+			var sb = new StringBuilder();
+			sb.Append("SystemID ").AppendLine(_systemId);
 
-				foreach (var watch in _watchList)
-				{
-					sb.AppendLine(watch.ToString());
-				}
+			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());
-			}
+			});
 
-			Changes = false;
-			return true;
+			if (!result.IsError) Changes = false;
+			return result;
 		}
 
-		public bool SaveAs(FileInfo file)
+		public FileWriteResult SaveAs(FileInfo file)
 		{
-			if (file != null)
-			{
-				CurrentFileName = file.FullName;
-				return Save();
-			}
+			Debug.Assert(file != null, "Cannot save as without a file name.");
 
-			return false;
+			CurrentFileName = file.FullName;
+			return Save();
 		}
 
 		private bool LoadFile(string path, bool append)

src/BizHawk.Client.EmuHawk/MainForm.cs

@@ -3886,26 +3886,25 @@ 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.
-			CheatList.SaveOnClose();
+			TryAgainResult cheatSaveResult = this.DoWithTryAgainBox(CheatList.SaveOnClose, "Failed to save cheats.");
+			if (cheatSaveResult == TryAgainResult.Canceled)
+			{
+				return false;
+			}
 
-			FileWriteResult saveResult = MovieSession.StopMovie();
-			if (saveResult.IsError)
+			// If TAStudio is open, we already asked about saving the movie.
+			if (!Tools.IsLoaded<TAStudio>())
 			{
-				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;
-				}
+				TryAgainResult saveMovieResult = this.DoWithTryAgainBox(() => MovieSession.StopMovie(), "Failed to save movie.");
+				if (saveMovieResult == TryAgainResult.Canceled) return false;
 			}
 
 			if (clearSram)
 			{
 				var path = Config.PathEntries.SaveRamAbsolutePath(Game, MovieSession.Movie);
 				if (File.Exists(path))
 				{
-					bool clearResult = this.DoWithTryAgainBox(() => {
+					TryAgainResult clearResult = this.DoWithTryAgainBox(() => {
 						try
 						{
 							File.Delete(path);
@@ -3917,22 +3916,22 @@ private bool CloseGame(bool clearSram = false)
 							return new(FileWriteEnum.FailedToDeleteGeneric, new(path, ""), ex);
 						}
 					}, "Failed to clear SRAM.");
-					if (!clearResult)
+					if (clearResult == TryAgainResult.Canceled)
 					{
 						return false;
 					}
 				}
 			}
 			else if (Emulator.HasSaveRam())
 			{
-				bool flushResult = this.DoWithTryAgainBox(
+				TryAgainResult flushResult = this.DoWithTryAgainBox(
 					() => FlushSaveRAM(),
 					"Failed flushing the game's Save RAM to your disk.");
-				if (!flushResult) return false;
+				if (flushResult == TryAgainResult.Canceled) return false;
 			}
 
-			bool stateSaveResult = this.DoWithTryAgainBox(AutoSaveStateIfConfigured, "Failed to auto-save state.");
-			if (!stateSaveResult) return false;
+			TryAgainResult stateSaveResult = this.DoWithTryAgainBox(AutoSaveStateIfConfigured, "Failed to auto-save state.");
+			if (stateSaveResult == TryAgainResult.Canceled) return false;
 
 			StopAv();