diff --git a/.global.editorconfig.ini b/.global.editorconfig.ini index 0849584d616..df2bbc32975 100644 --- a/.global.editorconfig.ini +++ b/.global.editorconfig.ini @@ -149,6 +149,15 @@ 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/EmuClientApi.cs b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs index b413e9dc366..62e531f48f5 100644 --- a/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/EmuClientApi.cs @@ -86,7 +86,7 @@ private void CallStateSaved(object sender, StateSavedEventArgs args) public void CloseEmulator(int? exitCode = null) => _mainForm.CloseEmulator(exitCode); - public void CloseRom() => _mainForm.CloseRom(); + public void CloseRom() => _mainForm.LoadNullRom(); public void DisplayMessages(bool value) => _config.DisplayMessages = value; @@ -158,9 +158,19 @@ public bool OpenRom(string path) public void RebootCore() => _mainForm.RebootCore(); + // TODO: Change return type to FileWriteResult. 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/MovieApi.cs b/src/BizHawk.Client.Common/Api/Classes/MovieApi.cs index 50672fb7965..47e059e96c5 100644 --- a/src/BizHawk.Client.Common/Api/Classes/MovieApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/MovieApi.cs @@ -52,6 +52,7 @@ 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()) @@ -69,7 +70,8 @@ public void Save(string filename) } _movieSession.Movie.Filename = filename; } - _movieSession.Movie.Save(); + FileWriteResult result = _movieSession.Movie.Save(); + if (result.Exception != null) throw result.Exception; } public IReadOnlyDictionary GetHeader() diff --git a/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs b/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs index 9bbe860e9c5..271f036ed20 100644 --- a/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs +++ b/src/BizHawk.Client.Common/Api/Classes/SaveStateApi.cs @@ -39,8 +39,17 @@ public bool LoadSlot(int slotNum, bool suppressOSD) 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 @@ public void SaveSlot(int slotNum, bool suppressOSD) 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 cd580498990..07a1fbfe329 100644 --- a/src/BizHawk.Client.Common/DialogControllerExtensions.cs +++ b/src/BizHawk.Client.Common/DialogControllerExtensions.cs @@ -1,10 +1,18 @@ #nullable enable using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; namespace BizHawk.Client.Common { + public enum TryAgainResult + { + Saved, + IgnoredFailure, + Canceled, + } + public static class DialogControllerExtensions { public static void AddOnScreenMessage( @@ -64,6 +72,47 @@ 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 ?? ""; + dialogParent.ModalMessageBox( + text: $"{prefix}\n{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 TryAgainResult 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 TryAgainResult.Canceled; + if (askResult == false) return TryAgainResult.IgnoredFailure; + if (askResult == true) fileResult = action(); + } + + return TryAgainResult.Saved; + } + /// 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 new file mode 100644 index 00000000000..c11193e347c --- /dev/null +++ b/src/BizHawk.Client.Common/FileWriteResult.cs @@ -0,0 +1,131 @@ +#nullable enable + +using System.Diagnostics; + +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, + } + + /// + /// 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; + internal readonly FileWritePaths Paths; + + public bool IsError => Error != FileWriteEnum.Success; + + public FileWriteResult(FileWriteEnum error, FileWritePaths writer, Exception? exception) + { + Error = error; + Exception = exception; + Paths = writer; + } + + 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. + /// + /// 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); + else return new(this); + } + + public FileWriteResult(FileWriteResult other) : this(other.Error, other.Paths, 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."; + case FileWriteEnum.FailedToOpen: + if (Paths.Final != Paths.Temp) + { + return $"The temporary file \"{Paths.Temp}\" could not be opened."; + } + 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"; + 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}\"."; + case FileWriteEnum.FailedToRename: + return $"{success} to \"{Paths.Final}\"."; + 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; + + internal FileWriteResult(FileWriteEnum error, FileWritePaths paths, Exception? exception) : base(error, paths, exception) { } + + internal FileWriteResult(T value, FileWritePaths paths) : base(FileWriteEnum.Success, paths, 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) { } + } + + /// + /// 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/FileWriter.cs b/src/BizHawk.Client.Common/FileWriter.cs new file mode 100644 index 00000000000..227ebb01681 --- /dev/null +++ b/src/BizHawk.Client.Common/FileWriter.cs @@ -0,0 +1,241 @@ +#nullable enable + +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 bool UsingTempFile => Paths.Temp != Paths.Final; + + private bool _finished = false; + + private FileWriter(FileWritePaths paths, FileStream stream) + { + Paths = paths; + _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); + } + + 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. + /// + 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 + { + Directory.CreateDirectory(Path.GetDirectoryName(path)); + FileStream fs = new(writePath, FileMode.Create, FileAccess.Write); + return new(new FileWriter(paths, fs), paths); + } + catch (Exception ex) // There are many exception types that file operations might raise. + { + return new(FileWriteEnum.FailedToOpen, paths, 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 not null, renames the original file to this path. + /// If called twice. + public FileWriteResult CloseAndDispose(string? backupPath = null) + { + // 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(); + + Paths.Backup = backupPath; + if (!UsingTempFile) + { + // 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 + { + // When everything goes right, this is all we need. + File.Replace(Paths.Temp, Paths.Final, backupPath); + return new(FileWriteEnum.Success, Paths, null); + } + catch + { + // 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.")); + } + + 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); + } + + /// + /// 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(Paths.Temp); + } + catch { /* eat? this is probably not very important */ } + } + + private bool _dispoed; + public void Dispose() + { + if (_dispoed) return; + _dispoed = 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)}"); + } + + + 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 e57bcfef63b..a8c9fcd6521 100644 --- a/src/BizHawk.Client.Common/FrameworkZipWriter.cs +++ b/src/BizHawk.Client.Common/FrameworkZipWriter.cs @@ -1,3 +1,5 @@ +#nullable enable + using System.IO; using System.IO.Compression; @@ -7,18 +9,19 @@ namespace BizHawk.Client.Common { public class FrameworkZipWriter : IZipWriter { - private ZipArchive _archive; + private ZipArchive? _archive; - private FileStream _fs; + private FileWriter? _fs; - private Zstd _zstd; + private Zstd? _zstd; private readonly CompressionLevel _level; private readonly int _zstdCompressionLevel; - public FrameworkZipWriter(string path, int compressionLevel) + private Exception? _writeException = null; + private bool _disposed; + + private FrameworkZipWriter(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) @@ -32,38 +35,95 @@ public FrameworkZipWriter(string path, int compressionLevel) _zstdCompressionLevel = compressionLevel * 2 + 1; } - public void WriteItem(string name, Action callback, bool zstdCompress) + 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(string? backupPath = null) { - // 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 (_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; - if (zstdCompress) + FileWriteResult ret; + if (_writeException == null) { - using var z = _zstd.CreateZstdCompressionStream(stream, _zstdCompressionLevel); - callback(z); + ret = _fs.CloseAndDispose(backupPath); } else { - callback(stream); + ret = new(FileWriteEnum.FailedDuringWrite, _fs.Paths, _writeException); + _fs.Abort(); } + + // And since we have to close stuff, there's really no point in not disposing here. + Dispose(); + return ret; } - public void Dispose() + 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) + if (_archive == null || _zstd == null) throw new ObjectDisposedException("Cannot use disposed ZipWriter."); + if (_writeException != null) return; + + try { - _archive.Dispose(); - _archive = null; + // 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); + } } - _fs?.Flush(flushToDisk: true); - _fs?.Dispose(); - _fs = null; - if (_zstd != null) + catch (Exception ex) { - _zstd.Dispose(); - _zstd = null; + _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. } } + + 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 = null; + } } } diff --git a/src/BizHawk.Client.Common/IMainFormForApi.cs b/src/BizHawk.Client.Common/IMainFormForApi.cs index 5577fe3a014..3b409f0f9a8 100644 --- a/src/BizHawk.Client.Common/IMainFormForApi.cs +++ b/src/BizHawk.Client.Common/IMainFormForApi.cs @@ -43,7 +43,7 @@ public interface IMainFormForApi void CloseEmulator(int? exitCode = null); /// only referenced from EmuClientApi - void CloseRom(bool clearSram = false); + void LoadNullRom(bool clearSram = false); /// only referenced from ClientLuaLibrary IDecodeResult DecodeCheatForAPI(string code, out MemoryDomain/*?*/ domain); @@ -52,7 +52,7 @@ public interface IMainFormForApi void EnableRewind(bool enabled); /// only referenced from EmuClientApi - bool FlushSaveRAM(bool autosave = false); + FileWriteResult FlushSaveRAM(bool autosave = false); /// only referenced from EmuClientApi void FrameAdvance(bool discardApiHawkSurfaces = true); @@ -93,10 +93,16 @@ public interface IMainFormForApi /// 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/IZipWriter.cs b/src/BizHawk.Client.Common/IZipWriter.cs index fa1de4e6513..dc67fde2f5a 100644 --- a/src/BizHawk.Client.Common/IZipWriter.cs +++ b/src/BizHawk.Client.Common/IZipWriter.cs @@ -5,5 +5,18 @@ 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. + /// + /// If not null, renames the original file to this path. + FileWriteResult CloseAndDispose(string backupPath = null); + + /// + /// 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/SaveSlotManager.cs b/src/BizHawk.Client.Common/SaveSlotManager.cs index ea95e684f86..774e9821034 100644 --- a/src/BizHawk.Client.Common/SaveSlotManager.cs +++ b/src/BizHawk.Client.Common/SaveSlotManager.cs @@ -69,31 +69,65 @@ 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]; - 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 + { + if (File.Exists(tempPath)) File.Delete(tempPath); + } + catch (Exception ex) { - temp.Delete(); + 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/config/ConfigService.cs b/src/BizHawk.Client.Common/config/ConfigService.cs index 70286ddedb4..357d02ecc49 100644 --- a/src/BizHawk.Client.Common/config/ConfigService.cs +++ b/src/BizHawk.Client.Common/config/ConfigService.cs @@ -104,19 +104,14 @@ public static bool IsFromSameVersion(string filepath, out string msg) return config ?? new T(); } - public static void Save(string filepath, object config) + public static FileWriteResult Save(string filepath, object config) { - var file = new FileInfo(filepath); - try + return FileWriter.Write(filepath, (fs) => { - using var writer = file.CreateText(); + using var writer = new StreamWriter(fs); 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.Common/lua/LuaFileList.cs b/src/BizHawk.Client.Common/lua/LuaFileList.cs index 13784bf6f8e..15b2493c0a0 100644 --- a/src/BizHawk.Client.Common/lua/LuaFileList.cs +++ b/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; } } } diff --git a/src/BizHawk.Client.Common/movie/MovieConversionExtensions.cs b/src/BizHawk.Client.Common/movie/MovieConversionExtensions.cs index 945a216c7a8..aa4c913c243 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 ITasMovie ConvertToSavestateAnchoredMovie(this ITasMovie old, int frame, byte[] savestate) + public static FileWriteResult ConvertToSavestateAnchoredMovie(this ITasMovie old, int frame, byte[] savestate) { string newFilename = ConvertFileNameToTasMovie(old.Filename); @@ -115,11 +115,11 @@ public static ITasMovie ConvertToSavestateAnchoredMovie(this ITasMovie old, int } } - tas.Save(); - return tas; + FileWriteResult saveResult = tas.Save(); + return saveResult.Convert(tas); } - public static ITasMovie ConvertToSaveRamAnchoredMovie(this ITasMovie old, byte[] saveRam) + public static FileWriteResult ConvertToSaveRamAnchoredMovie(this ITasMovie old, byte[] saveRam) { string newFilename = ConvertFileNameToTasMovie(old.Filename); @@ -146,8 +146,8 @@ public static ITasMovie ConvertToSaveRamAnchoredMovie(this ITasMovie old, byte[] tas.Subtitles.Add(sub); } - tas.Save(); - return tas; + FileWriteResult saveResult = tas.Save(); + return saveResult.Convert(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 eaaeef1fdc5..52055e4ce9b 100644 --- a/src/BizHawk.Client.Common/movie/MovieSession.cs +++ b/src/BizHawk.Client.Common/movie/MovieSession.cs @@ -244,8 +244,9 @@ public void RunQueuedMovie(bool recordMode, IEmulator emulator) public void AbortQueuedMovie() => _queuedMovie = null; - public void StopMovie(bool saveChanges = true) + public FileWriteResult StopMovie(bool saveChanges = true) { + FileWriteResult/*?*/ result = null; if (Movie.IsActive()) { var message = "Movie "; @@ -262,8 +263,17 @@ public void StopMovie(bool saveChanges = true) if (saveChanges && Movie.Changes) { - Movie.Save(); - Output($"{Path.GetFileName(Movie.Filename)} written to disk."); + 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.Stop(); @@ -279,6 +289,8 @@ public void StopMovie(bool saveChanges = true) } Movie = null; + + return result ?? new(); } public IMovie Get(string path, bool loadMovie) @@ -373,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: diff --git a/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs b/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs index 6e8ca063afc..4b53ec7cd0a 100644 --- a/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs +++ b/src/BizHawk.Client.Common/movie/bk2/Bk2Movie.IO.cs @@ -1,3 +1,5 @@ +#nullable enable + using System.Globalization; using System.IO; using System.Runtime.InteropServices; @@ -12,25 +14,25 @@ namespace BizHawk.Client.Common { public partial class Bk2Movie { - public void Save() + public FileWriteResult Save() { - Write(Filename); + return Write(Filename); } - public void SaveBackup() + public FileWriteResult SaveBackup() { if (string.IsNullOrWhiteSpace(Filename)) { - return; + return new(); } - var backupName = Filename.InsertBeforeLast('.', insert: $".{DateTime.Now:yyyy-MM-dd HH.mm.ss}", out _); + string backupName = Filename.InsertBeforeLast('.', insert: $".{DateTime.Now:yyyy-MM-dd HH.mm.ss}", out _); backupName = Path.Combine(Session.BackupDirectory, Path.GetFileName(backupName)); - Write(backupName, isBackup: true); + return Write(backupName, isBackup: true); } - protected virtual void Write(string fn, bool isBackup = false) + protected virtual FileWriteResult Write(string fn, bool isBackup = false) { SetCycleValues(); // EmulatorVersion used to store the unchanging original emulator version. @@ -41,13 +43,27 @@ protected virtual void Write(string fn, bool isBackup = false) Header[HeaderKeys.EmulatorVersion] = VersionInfo.GetEmuVersion(); Directory.CreateDirectory(Path.GetDirectoryName(fn)!); - using var bs = new ZipStateSaver(fn, Session.Settings.MovieCompressionLevel); - AddLumps(bs, isBackup); + var createResult = ZipStateSaver.Create(fn, Session.Settings.MovieCompressionLevel); + if (createResult.IsError) return createResult; - if (!isBackup) + ZipStateSaver saver = createResult.Value!; + try + { + AddLumps(saver, isBackup); + } + catch (Exception ex) + { + saver.Abort(); + return new(FileWriteEnum.FailedDuringWrite, createResult.Paths, ex); + } + + FileWriteResult result = saver.CloseAndDispose(); + if (!isBackup && !result.IsError) { 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 @@ -134,7 +150,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 91a30f734fa..c183d980ec9 100644 --- a/src/BizHawk.Client.Common/movie/import/IMovieImport.cs +++ b/src/BizHawk.Client.Common/movie/import/IMovieImport.cs @@ -67,7 +67,11 @@ public ImportResult Import( Result.Movie.Hash = hash; } - Result.Movie.Save(); + if (Result.Movie.Save().IsError) + { + Result.Errors.Add($"Could not write the file {newFileName}"); + return Result; + } } return Result; diff --git a/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs b/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs index c6b9d68c80a..ac3d276eea5 100644 --- a/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs +++ b/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs @@ -74,12 +74,12 @@ public interface IMovie : IBasicMovieInfo /// /// Forces the creation of a backup file of the current movie state /// - void SaveBackup(); + FileWriteResult SaveBackup(); /// /// Instructs the movie to save the current contents to Filename /// - void Save(); + FileWriteResult 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 908f3b75ed9..3ab9a23158e 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(); - void StopMovie(bool saveChanges = true); + FileWriteResult 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 ef9fa11626e..0119e597412 100644 --- a/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs +++ b/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs @@ -210,7 +210,9 @@ 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) { - SaveBackup(); + // TODO: This isn't ideal, but making it ideal would mean a big refactor. + FileWriteResult saveResult = SaveBackup(); + if (saveResult.Exception != null) throw saveResult.Exception; MakeBackup = false; } diff --git a/src/BizHawk.Client.Common/savestates/SavestateFile.cs b/src/BizHawk.Client.Common/savestates/SavestateFile.cs index d2558a9b044..966ca04c234 100644 --- a/src/BizHawk.Client.Common/savestates/SavestateFile.cs +++ b/src/BizHawk.Client.Common/savestates/SavestateFile.cs @@ -58,14 +58,16 @@ public SavestateFile( _userBag = userBag; } - public void Create(string filename, SaveStateConfig config) + public FileWriteResult Create(string filename, SaveStateConfig config, bool makeBackup) { - // 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); + FileWriteResult createResult = ZipStateSaver.Create(filename, config.CompressionLevelNormal); + if (createResult.IsError) return createResult; + var bs = createResult.Value!; 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)); @@ -129,6 +131,9 @@ public void Create(string filename, SaveStateConfig config) { bs.PutLump(BinaryStateLump.LagLog, tw => tasMovie.LagLog.Save(tw), zstdCompress: true); } + + makeBackup = makeBackup && config.MakeBackups; + return bs.CloseAndDispose(makeBackup ? $"{filename}.bak" : null); } 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 b2a9bbf42ec..68dcd337a69 100644 --- a/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs +++ b/src/BizHawk.Client.Common/savestates/ZipStateSaver.cs @@ -1,3 +1,5 @@ +#nullable enable + using System.IO; using BizHawk.Common; @@ -25,9 +27,9 @@ private static void WriteEmuVersion(Stream s) sw.WriteLine(VersionInfo.GetEmuVersion()); } - public ZipStateSaver(string path, int compressionLevel) + private ZipStateSaver(FrameworkZipWriter zip) { - _zip = new FrameworkZipWriter(path, compressionLevel); + _zip = zip; // 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 @@ -35,6 +37,35 @@ public ZipStateSaver(string path, int compressionLevel) 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. + /// + /// If not null, renames the original file to this path. + public FileWriteResult CloseAndDispose(string? backupPath = null) + { + FileWriteResult result = _zip.CloseAndDispose(backupPath); + 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.Common/tools/CheatList.cs b/src/BizHawk.Client.Common/tools/CheatList.cs index a9cca688a9a..5de61df7a35 100644 --- a/src/BizHawk.Client.Common/tools/CheatList.cs +++ b/src/BizHawk.Client.Common/tools/CheatList.cs @@ -88,20 +88,10 @@ public bool AttemptToLoadCheatFile(IMemoryDomains domains) return file.Exists && Load(domains, file.FullName, false); } - public void NewList(string defaultFileName, bool autosave = false) + public void NewList(string defaultFileName) { _defaultFileName = defaultFileName; - if (autosave && _changes && _cheatList.Count is not 0) - { - if (string.IsNullOrEmpty(CurrentFileName)) - { - CurrentFileName = _defaultFileName; - } - - Save(); - } - _cheatList.Clear(); CurrentFileName = ""; Changes = false; @@ -220,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) { @@ -231,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)) { @@ -251,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) diff --git a/src/BizHawk.Client.Common/tools/Watch/WatchList/WatchList.cs b/src/BizHawk.Client.Common/tools/Watch/WatchList/WatchList.cs index 0a3e12d913c..eb5afeb1ded 100644 --- a/src/BizHawk.Client.Common/tools/Watch/WatchList/WatchList.cs +++ b/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) diff --git a/src/BizHawk.Client.EmuHawk/IMainFormForTools.cs b/src/BizHawk.Client.EmuHawk/IMainFormForTools.cs index 3a2649872b9..1e5268bf1dd 100644 --- a/src/BizHawk.Client.EmuHawk/IMainFormForTools.cs +++ b/src/BizHawk.Client.EmuHawk/IMainFormForTools.cs @@ -20,9 +20,6 @@ public interface IMainFormForTools : IDialogController // TODO: remove? or does anything ever need access to the FirmwareManager FirmwareManager FirmwareManager { 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 6a2a7c35e4d..1e636a767de 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs @@ -264,12 +264,12 @@ 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}"); - CloseRom(); + LoadNullRom(); Console.WriteLine($"Closing rom clicked DONE Frame: {Emulator.Frame} Emulator: {Emulator.GetType().Name}"); } 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(); @@ -305,7 +305,7 @@ private void SaveToCurrentSlotMenuItem_Click(object sender, EventArgs e) => SavestateCurrentSlot(); private void SavestateCurrentSlot() - => SaveQuickSave(Config.SaveSlot); + => SaveQuickSaveAndShowError(Config.SaveSlot); private void LoadCurrentSlotMenuItem_Click(object sender, EventArgs e) => LoadstateCurrentSlot(); @@ -315,7 +315,7 @@ private bool LoadstateCurrentSlot() private void FlushSaveRAMMenuItem_Click(object sender, EventArgs e) { - FlushSaveRAM(); + ShowMessageIfError(() => FlushSaveRAM(), "Failed to flush saveram!"); } private void ReadonlyMenuItem_Click(object sender, EventArgs e) @@ -983,8 +983,15 @@ private void HkOverInputMenuItem_Click(object sender, EventArgs e) private void SaveConfigMenuItem_Click(object sender, EventArgs e) { - SaveConfig(); - AddOnScreenMessage("Saved settings"); + FileWriteResult result = SaveConfig(); + if (result.IsError) + { + this.ErrorMessageBox(result); + } + else + { + AddOnScreenMessage("Saved settings"); + } } private void SaveConfigAsMenuItem_Click(object sender, EventArgs e) @@ -996,8 +1003,15 @@ private void SaveConfigAsMenuItem_Click(object sender, EventArgs e) initFileName: file); if (result is not null) { - SaveConfig(result); - AddOnScreenMessage("Copied settings"); + FileWriteResult saveResult = SaveConfig(result); + if (saveResult.IsError) + { + this.ErrorMessageBox(saveResult); + } + else + { + AddOnScreenMessage("Copied settings"); + } } } @@ -1382,13 +1396,20 @@ private void ViewCommentsContextMenuItem_Click(object sender, EventArgs e) 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) { - CloseRom(clearSram: true); + LoadNullRom(clearSram: true); } private void ShowMenuContextMenuItem_Click(object sender, EventArgs e) @@ -1453,7 +1474,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) 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 e11775ce383..1fb1bf0c6a3 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) { - SaveQuickSave(slot); + SaveQuickSaveAndShowError(slot); Config.SaveSlot = slot; UpdateStatusSlots(); } @@ -89,13 +89,13 @@ void SelectAndLoadFromSlot(int slot) OpenRom(); break; case "Close ROM": - CloseRom(); + LoadNullRom(); break; case "Load Last ROM": LoadMostRecentROM(); break; case "Flush SaveRAM": - FlushSaveRAM(); + FlushSaveRAMMenuItem_Click(null, EventArgs.Empty); break; case "Display FPS": ToggleFps(); diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs b/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs index f97519c3651..b9e7f04f946 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs @@ -125,7 +125,14 @@ public void StopMovie(bool saveChanges = true) } else { - MovieSession.StopMovie(saveChanges); + FileWriteResult saveResult = MovieSession.StopMovie(saveChanges); + if (saveResult.IsError) + { + this.ShowMessageBox( + $"Failed to save movie.\n{saveResult.UserFriendlyErrorMessage()}\n{saveResult.Exception.Message}", + "Error", + EMsgBoxIcon.Error); + } SetMainformMovieInfo(); } } diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index 55fb6f208c9..f68f2038e91 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -879,20 +879,23 @@ 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(); } - if (!Tools.AskSave()) + TryAgainResult configSaveResult = this.DoWithTryAgainBox(() => SaveConfig(), "Failed to save config file."); + if (configSaveResult == TryAgainResult.Canceled) { closingArgs.Cancel = true; return; } + if (!CloseGame()) + { + closingArgs.Cancel = true; + return; + } Tools.Close(); - MovieSession.StopMovie(); - // zero 03-nov-2015 - close game after other steps. tools might need to unhook themselves from a core. - CloseGame(); - SaveConfig(); } private readonly bool _suppressSyncSettingsWarning; @@ -1731,6 +1734,7 @@ public bool RunLibretroCoreChooser() // countdown for saveram autoflushing public int AutoFlushSaveRamIn { get; set; } + private bool AutoFlushSaveRamFailed; private void SetStatusBar() { @@ -1933,7 +1937,7 @@ private void LoadSaveRam() } } - public bool FlushSaveRAM(bool autosave = false) + public FileWriteResult FlushSaveRAM(bool autosave = false) { if (Emulator.HasSaveRam()) { @@ -1947,53 +1951,13 @@ public bool 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 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 FileWriter.Write(path, saveram, $"{path}.bak"); } - return true; + return new(); } private void RewireSound() @@ -2107,7 +2071,7 @@ private void LoadRomFromRecent(string rom) if (!LoadRom(romPath, new LoadRomArgs(ioa), out var failureIsFromAskSave)) { - if (failureIsFromAskSave) AddOnScreenMessage("ROM loading cancelled; a tool had unsaved changes"); + if (failureIsFromAskSave) AddOnScreenMessage("ROM loading cancelled due to unsaved changes"); else if (ioa is OpenAdvanced_LibretroNoGame || File.Exists(romPath)) AddOnScreenMessage("ROM loading failed"); else Config.RecentRoms.HandleLoadError(this, romPath, rom); } @@ -2418,7 +2382,7 @@ public ISettingsAdapter GetSettingsAdapterForLoadedCore() public SettingsAdapter GetSettingsAdapterForLoadedCoreUntyped() => new(Emulator, static () => true, HandlePutCoreSettings, MayPutCoreSyncSettings, HandlePutCoreSyncSettings); - private void SaveConfig(string path = "") + private FileWriteResult SaveConfig(string path = "") { if (Config.SaveWindowPosition) { @@ -2444,7 +2408,7 @@ private void SaveConfig(string path = "") } CommitCoreSettingsToConfig(); - ConfigService.Save(path, Config); + return ConfigService.Save(path, Config); } private void ToggleFps() @@ -2665,8 +2629,16 @@ private void SaveMovie() { if (MovieSession.Movie.IsActive()) { - MovieSession.Movie.Save(); - AddOnScreenMessage($"{MovieSession.Movie.Filename} saved."); + FileWriteResult result = MovieSession.Movie.Save(); + if (result.IsError) + { + AddOnScreenMessage($"Failed to save {MovieSession.Movie.Filename}."); + AddOnScreenMessage(result.UserFriendlyErrorMessage()); + } + else + { + AddOnScreenMessage($"{MovieSession.Movie.Filename} saved."); + } } } @@ -3007,8 +2979,22 @@ private void StepRunLoop_Core(bool force = false) AutoFlushSaveRamIn--; if (AutoFlushSaveRamIn <= 0) { - FlushSaveRAM(true); - AutoFlushSaveRamIn = Config.FlushSaveRamFrames; + 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; + } } } // why not skip audio if the user doesn't want sound @@ -3563,6 +3549,12 @@ 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) @@ -3590,12 +3582,6 @@ 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, @@ -3609,12 +3595,6 @@ 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(); IOpenAdvanced ioa = args.OpenAdvanced; @@ -3784,7 +3764,7 @@ private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFr if (previousRom != CurrentlyOpenRom) { - CheatList.NewList(Tools.GenerateDefaultCheatFilename(), autosave: true); + CheatList.NewList(Tools.GenerateDefaultCheatFilename()); if (Config.Cheats.LoadFileByGame && Emulator.HasMemoryDomains()) { if (CheatList.AttemptToLoadCheatFile(Emulator.AsMemoryDomains())) @@ -3801,7 +3781,7 @@ private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFr } else { - CheatList.NewList(Tools.GenerateDefaultCheatFilename(), autosave: true); + CheatList.NewList(Tools.GenerateDefaultCheatFilename()); } } @@ -3844,7 +3824,7 @@ private bool LoadRomInternal(string path, LoadRomArgs args, out bool failureIsFr DisplayManager.UpdateGlobals(Config, Emulator); DisplayManager.Blank(); ExtToolManager.BuildToolStrip(); - CheatList.NewList("", autosave: true); + CheatList.NewList(""); OnRomChanged(); return false; } @@ -3897,74 +3877,100 @@ private void CommitCoreSettingsToConfig() } } - // 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) + /// + /// 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) { - GameIsClosing = true; + 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. + TryAgainResult cheatSaveResult = this.DoWithTryAgainBox(CheatList.SaveOnClose, "Failed to save cheats."); + if (cheatSaveResult == TryAgainResult.Canceled) return false; + + // If TAStudio is open, we already asked about saving the movie. + if (!Tools.IsLoaded()) + { + 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)) { - File.Delete(path); - AddOnScreenMessage("SRAM cleared."); + TryAgainResult 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 == TryAgainResult.Canceled) + { + return false; + } } } else if (Emulator.HasSaveRam()) { - 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; - } + TryAgainResult flushResult = this.DoWithTryAgainBox( + () => FlushSaveRAM(), + "Failed flushing the game's Save RAM to your disk."); + if (flushResult == TryAgainResult.Canceled) return false; } + TryAgainResult stateSaveResult = this.DoWithTryAgainBox(AutoSaveStateIfConfigured, "Failed to auto-save state."); + if (stateSaveResult == TryAgainResult.Canceled) return false; + StopAv(); - AutoSaveStateIfConfigured(); - CommitCoreSettingsToConfig(); DisableRewind(); - if (MovieSession.Movie.IsActive()) // Note: this must be called after CommitCoreSettingsToConfig() - { - StopMovie(); - } - RA?.Stop(); - 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; + + return true; } - private void AutoSaveStateIfConfigured() + private FileWriteResult AutoSaveStateIfConfigured() { - if (Config.AutoSaveLastSaveSlot && Emulator.HasSavestates()) SavestateCurrentSlot(); - } + if (Config.AutoSaveLastSaveSlot && Emulator.HasSavestates()) + { + return SaveQuickSave(Config.SaveSlot); + } - public bool GameIsClosing { get; private set; } // Lets tools make better decisions when being called by CloseGame + return new(); + } - public void CloseRom(bool clearSram = false) + /// + /// 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) { - // This gets called after Close Game gets called. - // Tested with NESHawk and SMB3 (U) if (Tools.AskSave()) { CloseGame(clearSram); @@ -3974,7 +3980,7 @@ public void CloseRom(bool clearSram = false) PauseOnFrame = null; CurrentlyOpenRom = null; CurrentlyOpenRomArgs = null; - CheatList.NewList("", autosave: true); + CheatList.NewList(""); OnRomChanged(); } } @@ -4113,29 +4119,36 @@ public bool LoadQuickSave(int slot, bool suppressOSD = false) 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 makeBackup) { if (!Emulator.HasSavestates()) { - return; + return new(FileWriteEnum.Aborted, new("", ""), new UnlessUsingApiException("The current emulator does not support savestates.")); } if (ToolControllingSavestates is { } tool) { tool.SaveState(); - return; + // 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)); } - try + FileWriteResult result = new SavestateFile(Emulator, MovieSession, MovieSession.UserBag) + .Create(path, Config.Savestates, makeBackup); + if (result.IsError) + { + AddOnScreenMessage($"Unable to save state {path}"); + } + else { - new SavestateFile(Emulator, MovieSession, MovieSession.UserBag).Create(path, Config.Savestates); - if (SavestateSaved is not null) { StateSavedEventArgs args = new(userFriendlyStateName); @@ -4143,29 +4156,32 @@ public void SaveState(string path, string userFriendlyStateName, bool fromLua = } RA?.OnSaveState(path); + if (Tools.Has()) + { + Tools.LuaConsole.CallStateSaveCallbacks(userFriendlyStateName); + } + if (!suppressOSD) { AddOnScreenMessage($"Saved state: {userFriendlyStateName}"); } } - catch (IOException) - { - AddOnScreenMessage($"Unable to save state {path}"); - } - if (!fromLua) - { - UpdateStatusSlots(); - } + return result; } - // TODO: should backup logic be stuffed in into Client.Common.SaveStateManager? - public void SaveQuickSave(int slot, bool suppressOSD = false, bool fromLua = false) + public FileWriteResult SaveState(string path, string userFriendlyStateName, bool suppressOSD = false) + { + return SaveStateInternal(path, userFriendlyStateName, suppressOSD, false); + } + + public FileWriteResult SaveQuickSave(int slot, bool suppressOSD = false) { if (!Emulator.HasSavestates()) { - return; + return new(FileWriteEnum.Aborted, new("", ""), new UnlessUsingApiException("The current emulator does not support savestates.")); } + var quickSlotName = $"QuickSave{slot % 10}"; var handled = false; if (QuicksaveSave is not null) @@ -4176,27 +4192,29 @@ public void SaveQuickSave(int slot, bool suppressOSD = false, bool fromLua = fal } if (handled) { - return; + // I suppose this is a success? But we have no path. + return new(); } if (ToolControllingSavestates is { } tool) { tool.SaveQuickSave(slot); - return; + // assume success by the tool: state was created, but not as a file. So no path. + return new(); } var path = $"{SaveStatePrefix()}.{quickSlotName}.State"; - new FileInfo(path).Directory?.Create(); - - // Make backup first - if (Config.Savestates.MakeBackups) - { - Util.TryMoveBackupFile(path, $"{path}.bak"); - } - - SaveState(path, quickSlotName, fromLua, suppressOSD); + var ret = SaveStateInternal(path, quickSlotName, suppressOSD, true); + UpdateStatusSlots(); + return ret; + } - if (Tools.Has()) Tools.LuaConsole.CallStateSaveCallbacks(quickSlotName); + /// + /// Runs and displays a pop up message if there was an error. + /// + private void SaveQuickSaveAndShowError(int slot) + { + ShowMessageIfError(() => SaveQuickSave(slot), "Quick save failed."); } public bool EnsureCoreIsAccurate() @@ -4259,12 +4277,17 @@ private void SaveStateAs() 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) + { + ShowMessageIfError( + () => SaveState(path: shouldSaveResult, userFriendlyStateName: shouldSaveResult), + "Unable to save state."); + } if (Tools.IsLoaded()) { @@ -4553,6 +4576,15 @@ 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(); diff --git a/src/BizHawk.Client.EmuHawk/config/ControllerConfig.cs b/src/BizHawk.Client.EmuHawk/config/ControllerConfig.cs index 752973ff38a..269a9c38c5f 100644 --- a/src/BizHawk.Client.EmuHawk/config/ControllerConfig.cs +++ b/src/BizHawk.Client.EmuHawk/config/ControllerConfig.cs @@ -470,7 +470,11 @@ private void ButtonSaveDefaults_Click(object sender, EventArgs e) SaveToDefaults(cd); - ConfigService.Save(Config.ControlDefaultPath, cd); + FileWriteResult saveResult = ConfigService.Save(Config.ControlDefaultPath, cd); + if (saveResult.IsError) + { + this.ErrorMessageBox(saveResult); + } } } diff --git a/src/BizHawk.Client.EmuHawk/movie/EditCommentsForm.cs b/src/BizHawk.Client.EmuHawk/movie/EditCommentsForm.cs index 0ddb2e07fa3..c19de4fa71e 100644 --- a/src/BizHawk.Client.EmuHawk/movie/EditCommentsForm.cs +++ b/src/BizHawk.Client.EmuHawk/movie/EditCommentsForm.cs @@ -55,7 +55,8 @@ private void Save() _movie.Comments.Add(c.Value.ToString()); } - _movie.Save(); + FileWriteResult result = _movie.Save(); + if (result.IsError) throw result.Exception!; } private void Cancel_Click(object sender, EventArgs e) diff --git a/src/BizHawk.Client.EmuHawk/tools/CDL.cs b/src/BizHawk.Client.EmuHawk/tools/CDL.cs index 5c12ce5572e..317a1041285 100644 --- a/src/BizHawk.Client.EmuHawk/tools/CDL.cs +++ b/src/BizHawk.Client.EmuHawk/tools/CDL.cs @@ -214,19 +214,23 @@ public override bool AskSaveChanges() { if (_currentFilename != null) { - RunSave(); + bool saveResult2 = RunSave(); ShutdownCDL(); - return true; + return saveResult2; } } - // 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) + var result = DialogController.ShowMessageBox3("Save changes to CDL session?", "CDL Save", EMsgBoxIcon.Question); + if (result == false) { ShutdownCDL(); return true; } + else if (result == null) + { + ShutdownCDL(); + return false; + } if (string.IsNullOrWhiteSpace(_currentFilename)) { @@ -240,9 +244,9 @@ public override bool AskSaveChanges() return false; } - RunSave(); + bool saveResult = RunSave(); ShutdownCDL(); - return true; + return saveResult; } private bool _autoloading; @@ -341,11 +345,20 @@ private void OpenMenuItem_Click(object sender, EventArgs e) LoadFile(file.FullName); } - private void RunSave() + /// + /// returns false if the operation was canceled + /// + private bool RunSave() { - _recent.Add(_currentFilename); - using var fs = new FileStream(_currentFilename, FileMode.Create, FileAccess.Write); - _cdl.Save(fs); + 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; } private void SaveMenuItem_Click(object sender, EventArgs e) @@ -386,8 +399,7 @@ private bool RunSaveAs() return false; SetCurrentFilename(file.FullName); - RunSave(); - return true; + return RunSave(); } 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 c32ede434bd..69c8d4a4fd3 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 bool SaveAs() + private FileWriteResult SaveAs() { var fileName = MainForm.CheatList.CurrentFileName; if (string.IsNullOrWhiteSpace(fileName)) @@ -156,7 +156,8 @@ private bool SaveAs() CheatsFSFilterSet, this); - return file != null && MainForm.CheatList.SaveFile(file.FullName); + if (file == null) return new(); + else return MainForm.CheatList.SaveFile(file.FullName); } private void Cheats_Load(object sender, EventArgs e) @@ -361,7 +362,12 @@ private void SaveMenuItem_Click(object sender, EventArgs e) { if (MainForm.CheatList.Changes) { - if (MainForm.CheatList.Save()) + FileWriteResult result = MainForm.CheatList.Save(); + if (result.IsError) + { + this.ErrorMessageBox(result); + } + else { UpdateMessageLabel(saved: true); } @@ -374,7 +380,12 @@ private void SaveMenuItem_Click(object sender, EventArgs e) private void SaveAsMenuItem_Click(object sender, EventArgs e) { - if (SaveAs()) + FileWriteResult result = SaveAs(); + if (result.IsError) + { + this.ErrorMessageBox(result); + } + else { UpdateMessageLabel(saved: true); } diff --git a/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs b/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs index 6e9a4ca6f45..705839f277f 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs @@ -686,15 +686,25 @@ private FileInfo GetSaveFileFromUser() return result is not null ? new FileInfo(result) : null; } - private void SaveSessionAs() + private FileWriteResult SaveSessionAs() { var file = GetSaveFileFromUser(); if (file != null) { - LuaImp.ScriptList.Save(file.FullName); - Config.RecentLuaSession.Add(file.FullName); - OutputMessages.Text = $"{file.Name} saved."; + 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; } + return new(); } private void LoadSessionFromRecent(string path) @@ -722,7 +732,11 @@ public override bool AskSaveChanges() icon: EMsgBoxIcon.Question, text: $"Save {WindowTitleStatic} session?")); if (result is null) return false; - if (result.Value) SaveOrSaveAs(); + if (result.Value) + { + TryAgainResult saveResult = this.DoWithTryAgainBox(SaveOrSaveAs, "Failed to save Lua session."); + return saveResult != TryAgainResult.Canceled; + } else LuaImp.ScriptList.Changes = false; return true; } @@ -737,16 +751,20 @@ private void UpdateRegisteredFunctionsDialog() } } - private void SaveOrSaveAs() + private FileWriteResult SaveOrSaveAs() { if (!string.IsNullOrWhiteSpace(LuaImp.ScriptList.Filename)) { - LuaImp.ScriptList.Save(LuaImp.ScriptList.Filename); - Config.RecentLuaSession.Add(LuaImp.ScriptList.Filename); + FileWriteResult result = LuaImp.ScriptList.Save(LuaImp.ScriptList.Filename); + if (!result.IsError) + { + Config.RecentLuaSession.Add(LuaImp.ScriptList.Filename); + } + return result; } else { - SaveSessionAs(); + return SaveSessionAs(); } } @@ -789,8 +807,16 @@ private void SaveSessionMenuItem_Click(object sender, EventArgs e) { if (LuaImp.ScriptList.Changes) { - SaveOrSaveAs(); - OutputMessages.Text = $"{Path.GetFileName(LuaImp.ScriptList.Filename)} saved."; + 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."; + } } } diff --git a/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs b/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs index 7d2e46f7e10..4576c5db428 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Macros/MacroInput.cs @@ -112,6 +112,7 @@ 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]); @@ -288,7 +289,7 @@ private bool SaveMacroAs(MovieZone macro) return false; } - macro.Save(result); + macro.Save(result); // ignore errors: This tool is going to be removed. Config!.RecentMacros.Add(result); return true; diff --git a/src/BizHawk.Client.EmuHawk/tools/Macros/MovieZone.cs b/src/BizHawk.Client.EmuHawk/tools/Macros/MovieZone.cs index 208899b3c49..e6f32f24416 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Macros/MovieZone.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Macros/MovieZone.cs @@ -199,19 +199,25 @@ public void PlaceZone(IMovie movie, Config config) } } - public void Save(string fileName) + public FileWriteResult 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 - 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); + + 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); + } + }); } public MovieZone(string fileName, IDialogController dialogController, IEmulator emulator, IMovieSession movieSession, ToolManager tools) diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IControlMainForm.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IControlMainForm.cs index fc5192a0f72..6455b24610f 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IControlMainForm.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IControlMainForm.cs @@ -59,13 +59,10 @@ public void ToggleReadOnly() public void StopMovie(bool suppressSave) { - if (!MainForm.GameIsClosing) - { - Activate(); - _suppressAskSave = suppressSave; - StartNewTasMovie(); - _suppressAskSave = false; - } + Activate(); + _suppressAskSave = suppressSave; + StartNewTasMovie(); + _suppressAskSave = false; } public bool WantsToControlRewind { get; private set; } = true; diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs index 062edf5707e..fccc77fad62 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs @@ -157,12 +157,16 @@ public override bool AskSaveChanges() } if (CurrentTasMovie?.Changes is not true) return true; - var result = DialogController.DoWithTempMute(() => this.ModalMessageBox3( + var shouldSaveResult = DialogController.DoWithTempMute(() => this.ModalMessageBox3( caption: "Closing with Unsaved Changes", icon: EMsgBoxIcon.Question, text: $"Save {WindowTitleStatic} project?")); - if (result is null) return false; - if (result.Value) SaveTas(); + if (shouldSaveResult == true) + { + TryAgainResult saveResult = this.DoWithTryAgainBox(() => SaveTas(), "Failed to save movie."); + return saveResult != TryAgainResult.Canceled; + } + if (shouldSaveResult is null) return false; 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 41768421212..3ed1d273f49 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs @@ -43,11 +43,15 @@ private void StartNewProjectFromNowMenuItem_Click(object sender, EventArgs e) { if (AskSaveChanges()) { - var newProject = CurrentTasMovie.ConvertToSavestateAnchoredMovie( + var result = CurrentTasMovie.ConvertToSavestateAnchoredMovie( Emulator.Frame, StatableEmulator.CloneSavestate()); + DisplayMessageIfFailed(() => result, "Failed to create movie."); - MainForm.PauseEmulator(); - LoadMovie(newProject, true); + if (result.Value is ITasMovie newProject) + { + MainForm.PauseEmulator(); + LoadMovie(newProject, true); + } } } @@ -57,9 +61,14 @@ private void StartANewProjectFromSaveRamMenuItem_Click(object sender, EventArgs { var saveRam = SaveRamEmulator?.CloneSaveRam(clearDirty: false) ?? throw new Exception("No SaveRam"); GoToFrame(TasView.AnyRowsSelected ? TasView.FirstSelectedRowIndex : 0); - var newProject = CurrentTasMovie.ConvertToSaveRamAnchoredMovie(saveRam); - MainForm.PauseEmulator(); - LoadMovie(newProject, true); + var result = CurrentTasMovie.ConvertToSaveRamAnchoredMovie(saveRam); + DisplayMessageIfFailed(() => result, "Failed to create movie."); + + if (result.Value is ITasMovie newProject) + { + MainForm.PauseEmulator(); + LoadMovie(newProject, true); + } } } @@ -115,30 +124,30 @@ public bool LoadMovieFile(string filename, bool askToSave = true) private void SaveTasMenuItem_Click(object sender, EventArgs e) { - SaveTas(); + DisplayMessageIfFailed(() => SaveTas(), "Failed to save movie."); if (Settings.BackupPerFileSave) { - SaveTas(saveBackup: true); + DisplayMessageIfFailed(() => SaveTas(saveBackup: true), "Failed to save backup."); } } private void SaveAsTasMenuItem_Click(object sender, EventArgs e) { - SaveAsTas(); + DisplayMessageIfFailed(() => SaveAsTas(), "Failed to save movie."); if (Settings.BackupPerFileSave) { - SaveTas(saveBackup: true); + DisplayMessageIfFailed(() => SaveTas(saveBackup: true), "Failed to save backup."); } } private void SaveBackupMenuItem_Click(object sender, EventArgs e) { - SaveTas(saveBackup: true); + DisplayMessageIfFailed(() => SaveTas(saveBackup: true), "Failed to save backup."); } private void SaveBk2BackupMenuItem_Click(object sender, EventArgs e) { - SaveTas(saveAsBk2: true, saveBackup: true); + DisplayMessageIfFailed(() => SaveTas(saveAsBk2: true, saveBackup: true), "Failed to save backup."); } private void SaveSelectionToMacroMenuItem_Click(object sender, EventArgs e) @@ -163,15 +172,25 @@ private void SaveSelectionToMacroMenuItem_Click(object sender, EventArgs e) if (file != null) { var selectionStart = TasView.SelectionStartIndex!.Value; - new MovieZone( + MovieZone macro = new( Emulator, Tools, MovieSession, start: selectionStart, - length: TasView.SelectionEndIndex!.Value - selectionStart + 1) - .Save(file.FullName); + length: TasView.SelectionEndIndex!.Value - selectionStart + 1); + FileWriteResult saveResult = macro.Save(file.FullName); - Config.RecentMacros.Add(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); + } } } @@ -220,13 +239,24 @@ 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 - bk2.Save(); - MessageStatusLabel.Text = $"{bk2.Name} exported."; + FileWriteResult saveResult = bk2.Save(); 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 0e7c7fc0987..d6a04741e1e 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs @@ -36,6 +36,7 @@ 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; @@ -198,11 +199,7 @@ private void Tastudio_Load(object sender, EventArgs e) _autosaveTimer = new Timer(components); _autosaveTimer.Tick += AutosaveTimerEventProcessor; - if (Settings.AutosaveInterval > 0) - { - _autosaveTimer.Interval = (int)Settings.AutosaveInterval; - _autosaveTimer.Start(); - } + ScheduleAutoSave(Settings.AutosaveInterval); MainVertialSplit.SetDistanceOrDefault( Settings.MainVerticalSplitDistance, @@ -272,20 +269,27 @@ private bool Engage() { changesString = "The current movie has unsaved changes. Would you like to save before closing it?"; } - var result = DialogController.ShowMessageBox3( + var shouldSaveResult = DialogController.ShowMessageBox3( "TAStudio will create a new project file from the current movie.\n\n" + changesString, "Convert movie", EMsgBoxIcon.Question); - if (result == true) + if (shouldSaveResult == true) { - MovieSession.Movie.Save(); + FileWriteResult saveResult = MovieSession.Movie.Save(); + if (saveResult.IsError) + { + DisplayMessageIfFailed(() => saveResult, "Failed to save movie."); + return false; + } } - else if (result == null) + else if (shouldSaveResult == null) { return false; } - var tasMovie = ConvertCurrentMovieToTasproj(); + 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). success = LoadMovie(tasMovie); } @@ -326,6 +330,16 @@ 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) @@ -339,28 +353,55 @@ private void AutosaveTimerEventProcessor(object sender, EventArgs e) return; } + FileWriteResult saveResult; if (Settings.AutosaveAsBackupFile) { if (Settings.AutosaveAsBk2) { - SaveTas(saveAsBk2: true, saveBackup: true); + saveResult = SaveTas(saveAsBk2: true, saveBackup: true); } else { - SaveTas(saveBackup: true); + saveResult = SaveTas(saveBackup: true); } } else { if (Settings.AutosaveAsBk2) { - SaveTas(saveAsBk2: true); + saveResult = SaveTas(saveAsBk2: true); } else { - SaveTas(); + saveResult = 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" }; @@ -550,13 +591,6 @@ private int? FirstNonEmptySelectedFrame } } - 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; @@ -778,12 +812,23 @@ private string SuggestedTasProjName() $"{Game.FilesystemSafeName()}.{MovieService.TasMovieExtension}"); } - private void SaveTas(bool saveAsBk2 = false, bool saveBackup = false) + 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) { if (string.IsNullOrEmpty(CurrentTasMovie.Filename) || CurrentTasMovie.Filename == DefaultTasProjName()) { - SaveAsTas(); - return; + return SaveAsTas(); } _autosaveTimer.Stop(); @@ -800,22 +845,29 @@ private void SaveTas(bool saveAsBk2 = false, bool saveBackup = false) movieToSave.Attach(Emulator); } + FileWriteResult result; if (saveBackup) - movieToSave.SaveBackup(); + result = movieToSave.SaveBackup(); else - movieToSave.Save(); + result = movieToSave.Save(); - MessageStatusLabel.Text = saveBackup - ? $"Backup .{(saveAsBk2 ? MovieService.StandardMovieExtension : MovieService.TasMovieExtension)} saved to \"Movie backups\" path." - : "File saved."; - Cursor = Cursors.Default; - if (Settings.AutosaveInterval > 0) + if (!result.IsError) { - _autosaveTimer.Start(); + MessageStatusLabel.Text = saveBackup + ? $"Backup .{(saveAsBk2 ? MovieService.StandardMovieExtension : MovieService.TasMovieExtension)} saved to \"Movie backups\" path." + : "File saved."; + } + else + { + MessageStatusLabel.Text = "Failed to save movie."; } + + Cursor = Cursors.Default; + ScheduleAutoSave(Settings.AutosaveInterval); + return result; } - private void SaveAsTas() + private FileWriteResult SaveAsTas() { _autosaveTimer.Stop(); @@ -831,25 +883,33 @@ private void SaveAsTas() TAStudioProjectsFSFilterSet, this); + FileWriteResult saveResult = null; if (fileInfo != null) { MessageStatusLabel.Text = "Saving..."; MessageStatusLabel.Owner.Update(); Cursor = Cursors.WaitCursor; CurrentTasMovie.Filename = fileInfo.FullName; - CurrentTasMovie.Save(); + saveResult = CurrentTasMovie.Save(); Settings.RecentTas.Add(CurrentTasMovie.Filename); - MessageStatusLabel.Text = "File saved."; Cursor = Cursors.Default; - } - if (Settings.AutosaveInterval > 0) - { - _autosaveTimer.Start(); + if (!saveResult.IsError) + { + MessageStatusLabel.Text = "File saved."; + ScheduleAutoSave(Settings.AutosaveInterval); + } + else + { + MessageStatusLabel.Text = "Failed to save."; + } } 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(); // user cancelled, so we were successful in not saving } protected override string WindowTitle diff --git a/src/BizHawk.Client.EmuHawk/tools/Watch/RamSearch.cs b/src/BizHawk.Client.EmuHawk/tools/Watch/RamSearch.cs index a7371f5716f..020ff8916ef 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Watch/RamSearch.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Watch/RamSearch.cs @@ -1007,7 +1007,13 @@ private void SaveMenuItem_Click(object sender, EventArgs e) if (!string.IsNullOrWhiteSpace(watches.CurrentFileName)) { - if (watches.Save()) + FileWriteResult saveResult = watches.Save(); + if (saveResult.IsError) + { + this.ErrorMessageBox(saveResult); + MessageLabel.Text = $"Failed to save {Path.GetFileName(_currentFileName)}"; + } + else { _currentFileName = watches.CurrentFileName; MessageLabel.Text = $"{Path.GetFileName(_currentFileName)} saved"; @@ -1016,11 +1022,20 @@ private void SaveMenuItem_Click(object sender, EventArgs e) } else { - var result = watches.SaveAs(GetWatchSaveFileFromUser(CurrentFileName())); - if (result) + FileInfo/*?*/ file = GetWatchSaveFileFromUser(CurrentFileName()); + if (file != null) { - MessageLabel.Text = $"{Path.GetFileName(_currentFileName)} saved"; - Settings.RecentSearches.Add(watches.CurrentFileName); + 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); + } } } } @@ -1034,7 +1049,15 @@ private void SaveAsMenuItem_Click(object sender, EventArgs e) watches.Add(_searches[i]); } - if (watches.SaveAs(GetWatchSaveFileFromUser(CurrentFileName()))) + 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 { _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 e54bf3a62ff..f74609e0ae6 100644 --- a/src/BizHawk.Client.EmuHawk/tools/Watch/RamWatch.cs +++ b/src/BizHawk.Client.EmuHawk/tools/Watch/RamWatch.cs @@ -213,15 +213,8 @@ public override bool AskSaveChanges() if (result is null) return false; if (result.Value) { - if (string.IsNullOrWhiteSpace(_watches.CurrentFileName)) - { - SaveAs(); - } - else - { - _watches.Save(); - Config.RecentWatches.Add(_watches.CurrentFileName); - } + TryAgainResult saveResult = this.DoWithTryAgainBox(Save, "Failed to save watch list."); + return saveResult != TryAgainResult.Canceled; } else { @@ -591,13 +584,45 @@ private string CurrentFileName() : Game.FilesystemSafeName(); } - private void SaveAs() + private FileWriteResult SaveAs() { - var result = _watches.SaveAs(GetWatchSaveFileFromUser(CurrentFileName())); - if (result) + FileInfo/*?*/ file = GetWatchSaveFileFromUser(CurrentFileName()); + if (file == null) return new(); + + FileWriteResult result = _watches.SaveAs(file); + if (result.IsError) { - UpdateStatusBar(saved: true); + 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 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; } } @@ -727,23 +752,20 @@ private void OpenMenuItem_Click(object sender, EventArgs e) private void SaveMenuItem_Click(object sender, EventArgs e) { - if (!string.IsNullOrWhiteSpace(_watches.CurrentFileName)) + FileWriteResult saveResult = Save(); + if (saveResult.IsError) { - if (_watches.Save()) - { - Config.RecentWatches.Add(_watches.CurrentFileName); - UpdateStatusBar(saved: true); - } - } - else - { - SaveAs(); + this.ErrorMessageBox(saveResult, "Failed to save watch list."); } } private void SaveAsMenuItem_Click(object sender, EventArgs e) { - SaveAs(); + FileWriteResult saveResult = SaveAs(); + if (saveResult.IsError) + { + this.ErrorMessageBox(saveResult, "Failed to save watch list."); + } } private void RecentSubMenu_DropDownOpened(object sender, EventArgs e) diff --git a/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs b/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs index 1eba1af9d96..af65722f755 100644 --- a/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs +++ b/src/BizHawk.Tests.Client.Common/Movie/FakeMovieSession.cs @@ -67,6 +67,10 @@ 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 void StopMovie(bool saveChanges = true) => Movie?.Stop(); + public FileWriteResult StopMovie(bool saveChanges = true) + { + Movie?.Stop(); + return new(); + } } }