From 42aa9d991dc3dde03234a15ddfdc2efdebf1c097 Mon Sep 17 00:00:00 2001 From: Morilli <35152647+Morilli@users.noreply.github.com> Date: Mon, 10 Jun 2024 04:02:38 +0200 Subject: [PATCH] Reduce hacks and duplication in TAStudio movie loading Previously, there was no way to load a movie without saving it to a file first. This has been changed, so lots of saves and loads are no longer necessary. This also partially gets rid of the default.tasproj, although the name is still used as a placeholder for now. --- .../movie/BasicMovieInfo.cs | 3 +- .../movie/MovieSession.cs | 90 +++++++------------ .../movie/interfaces/IMovieSession.cs | 18 ++-- .../MainForm.FileLoader.cs | 2 +- src/BizHawk.Client.EmuHawk/MainForm.Movie.cs | 1 - src/BizHawk.Client.EmuHawk/MainForm.cs | 6 +- src/BizHawk.Client.EmuHawk/movie/PlayMovie.cs | 8 +- .../movie/RecordMovie.cs | 1 - .../tools/TAStudio/TAStudio.MenuItems.cs | 21 ++--- .../tools/TAStudio/TAStudio.cs | 62 ++++++------- 10 files changed, 79 insertions(+), 133 deletions(-) diff --git a/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs b/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs index ec0bc882a8..fe1148aa0a 100644 --- a/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs +++ b/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs @@ -146,8 +146,7 @@ namespace BizHawk.Client.Common public bool Load() { - var file = new FileInfo(Filename); - if (!file.Exists) + if (!File.Exists(Filename)) { return false; } diff --git a/src/BizHawk.Client.Common/movie/MovieSession.cs b/src/BizHawk.Client.Common/movie/MovieSession.cs index 83eb935ca7..77d2b4c5de 100644 --- a/src/BizHawk.Client.Common/movie/MovieSession.cs +++ b/src/BizHawk.Client.Common/movie/MovieSession.cs @@ -61,11 +61,6 @@ namespace BizHawk.Client.Common return new Bk2Controller("", definition ?? MovieController.Definition); } - public void SetMovieController(ControllerDefinition definition) - { - MovieController = GenerateMovieController(definition); - } - public void HandleFrameBefore() { if (Movie.NotActive()) @@ -189,72 +184,49 @@ namespace BizHawk.Client.Common return true; } - /// is and . does not match . + /// . does not match . public void QueueNewMovie( IMovie movie, - bool record, string systemId, string loadedRomHash, PathEntryCollection pathEntries, IDictionary preferredCores) { - if (movie.IsActive() && movie.Changes) + if (movie.SystemID != systemId) { - movie.Save(); + throw new MoviePlatformMismatchException( + $"Movie system Id ({movie.SystemID}) does not match the currently loaded platform ({systemId}), unable to load"); } - if (!record) // The semantics of record is that we are starting a new movie, and even wiping a pre-existing movie with the same path, but non-record means we are loading an existing movie into playback mode + if (!(string.IsNullOrEmpty(movie.Hash) || loadedRomHash.Equals(movie.Hash, StringComparison.Ordinal)) + && movie is TasMovie tasproj) { - movie.Load(); - - if (movie.SystemID != systemId) + var result = _dialogParent.ModalMessageBox2( + caption: "Discard GreenZone?", + text: $"The TAStudio project {movie.Filename.MakeRelativeTo(pathEntries.MovieAbsolutePath())} appears to be for a different game than the one that's loaded.\n" + + "Choose \"No\" to continue anyway, which may lead to an invalid savestate being loaded.\n" + + "Choose \"Yes\" to discard the GreenZone (savestate history). This is safer, and at worst you'll only need to watch through the whole movie."); + //TODO add abort option + if (result) { - throw new MoviePlatformMismatchException( - $"Movie system Id ({movie.SystemID}) does not match the currently loaded platform ({systemId}), unable to load"); - } - - if (!(string.IsNullOrEmpty(movie.Hash) || loadedRomHash.Equals(movie.Hash, StringComparison.Ordinal)) - && movie is TasMovie tasproj) - { - var result = _dialogParent.ModalMessageBox2( - caption: "Discard GreenZone?", - text: $"The TAStudio project {movie.Filename.MakeRelativeTo(pathEntries.MovieAbsolutePath())} appears to be for a different game than the one that's loaded.\n" - + "Choose \"No\" to continue anyway, which may lead to an invalid savestate being loaded.\n" - + "Choose \"Yes\" to discard the GreenZone (savestate history). This is safer, and at worst you'll only need to watch through the whole movie."); - //TODO add abort option - if (result) - { - tasproj.TasSession.UpdateValues(frame: 0, currentBranch: tasproj.TasSession.CurrentBranch); // wtf is this API --yoshi - tasproj.InvalidateEntireGreenzone(); - } + tasproj.TasSession.UpdateValues(frame: 0, currentBranch: tasproj.TasSession.CurrentBranch); // wtf is this API --yoshi + tasproj.InvalidateEntireGreenzone(); } } - if (!record) + if (string.IsNullOrWhiteSpace(movie.Core)) { - if (string.IsNullOrWhiteSpace(movie.Core)) - { - PopupMessage(preferredCores.TryGetValue(systemId, out var coreName) - ? $"No core specified in the movie file, using the preferred core {coreName} instead." - : "No core specified in the movie file, using the default core instead."); - } - else - { - var keys = preferredCores.Keys.ToList(); - foreach (var k in keys) - { - preferredCores[k] = movie.Core; - } - } - } - - if (record) // This is a hack really, we need to set the movie to its proper state so that it will be considered active later - { - movie.SwitchToRecord(); + PopupMessage(preferredCores.TryGetValue(systemId, out var coreName) + ? $"No core specified in the movie file, using the preferred core {coreName} instead." + : "No core specified in the movie file, using the default core instead."); } else { - movie.SwitchToPlay(); + var keys = preferredCores.Keys.ToList(); + foreach (var k in keys) + { + preferredCores[k] = movie.Core; + } } _queuedMovie = movie; @@ -331,15 +303,17 @@ namespace BizHawk.Client.Common Movie.SwitchToPlay(); } - public IMovie Get(string path) + public IMovie Get(string path, bool loadMovie) { // TODO: change IMovies to take HawkFiles only and not path - if (Path.GetExtension(path)?.EndsWithOrdinal("tasproj") ?? false) - { - return new TasMovie(this, path); - } + IMovie movie = Path.GetExtension(path)?.EndsWithOrdinal("tasproj") is true + ? new TasMovie(this, path) + : new Bk2Movie(this, path); - return new Bk2Movie(this, path); + if (loadMovie) + movie.Load(); + + return movie; } public void PopupMessage(string message) => _dialogParent.ModalMessageBox(message, "Warning", EMsgBoxIcon.Warning); diff --git a/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs b/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs index 50880865a1..8ba18108a5 100644 --- a/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs +++ b/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs @@ -54,17 +54,6 @@ namespace BizHawk.Client.Common /// IMovieController GenerateMovieController(ControllerDefinition definition = null); - /// - /// Hack only used for TAStudio when starting a new movie - /// This is due to needing to save a "dummy" default.tasproj - /// This dummy file's initial save bypasses the normal queue/run - /// new movie code (which normally sets the controller), although - /// once it saves it goes through the normal queue/run code anyway. - /// TODO: Stop relying on this dummy file so we do not need this ugly hack - /// - /// current IEmulator ControllerDefinition - void SetMovieController(ControllerDefinition definition); - void HandleFrameBefore(); void HandleFrameAfter(); void HandleSaveState(TextWriter writer); @@ -79,7 +68,6 @@ namespace BizHawk.Client.Common /// void QueueNewMovie( IMovie movie, - bool record, string systemId, string loadedRomHash, PathEntryCollection pathEntries, @@ -100,7 +88,11 @@ namespace BizHawk.Client.Common /// void ConvertToTasProj(); - IMovie Get(string path); + /// + /// Create a new (Tas)Movie with the given path as filename. If is true, + /// will also attempt to load an existing movie from . + /// + IMovie Get(string path, bool loadMovie = false); string BackupDirectory { get; set; } diff --git a/src/BizHawk.Client.EmuHawk/MainForm.FileLoader.cs b/src/BizHawk.Client.EmuHawk/MainForm.FileLoader.cs index 3e094cdbb1..382c7cbeb8 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.FileLoader.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.FileLoader.cs @@ -95,7 +95,7 @@ namespace BizHawk.Client.EmuHawk } return Tools.IsLoaded() ? Tools.TAStudio.LoadMovieFile(filename) - : StartNewMovie(MovieSession.Get(filename), false); + : StartNewMovie(MovieSession.Get(filename, true), false); } private bool LoadRom(string filename, string archive = null) diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs b/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs index fe87bf433c..a553374066 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs @@ -29,7 +29,6 @@ namespace BizHawk.Client.EmuHawk { MovieSession.QueueNewMovie( movie, - record: record, systemId: Emulator.SystemId, loadedRomHash: Game.Hash, Config.PathEntries, diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index 598c06cc30..f5ca6d9936 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -668,7 +668,7 @@ namespace BizHawk.Client.EmuHawk // If user picked a game, then do the commandline logic if (!Game.IsNullInstance()) { - var movie = MovieSession.Get(_argParser.cmdMovie); + var movie = MovieSession.Get(_argParser.cmdMovie, true); MovieSession.ReadOnly = true; // if user is dumping and didn't supply dump length, make it as long as the loaded movie @@ -703,7 +703,7 @@ namespace BizHawk.Client.EmuHawk { if (File.Exists(Config.RecentMovies.MostRecent)) { - StartNewMovie(MovieSession.Get(Config.RecentMovies.MostRecent), false); + StartNewMovie(MovieSession.Get(Config.RecentMovies.MostRecent, true), false); } else { @@ -2284,7 +2284,7 @@ namespace BizHawk.Client.EmuHawk { if (File.Exists(path)) { - var movie = MovieSession.Get(path); + var movie = MovieSession.Get(path, true); MovieSession.ReadOnly = true; StartNewMovie(movie, false); } diff --git a/src/BizHawk.Client.EmuHawk/movie/PlayMovie.cs b/src/BizHawk.Client.EmuHawk/movie/PlayMovie.cs index f5a55cbd6a..93440700ea 100644 --- a/src/BizHawk.Client.EmuHawk/movie/PlayMovie.cs +++ b/src/BizHawk.Client.EmuHawk/movie/PlayMovie.cs @@ -102,7 +102,7 @@ namespace BizHawk.Client.EmuHawk var indices = MovieView.SelectedIndices; if (indices.Count > 0) // Import file if necessary { - var movie = _movieSession.Get(_movieList[MovieView.SelectedIndices[0]].Filename); + var movie = _movieSession.Get(_movieList[MovieView.SelectedIndices[0]].Filename, true); _mainForm.StartNewMovie(movie, false); } } @@ -523,8 +523,7 @@ namespace BizHawk.Client.EmuHawk if (indices.Count > 0) { // TODO this will allocate unnecessary memory when this movie is a TasMovie due to TasStateManager - var movie = _movieSession.Get(_movieList[MovieView.SelectedIndices[0]].Filename); - movie.Load(); + var movie = _movieSession.Get(_movieList[MovieView.SelectedIndices[0]].Filename, true); var form = new EditCommentsForm(movie, readOnly: false, disposeOnClose: true); form.Show(); } @@ -536,8 +535,7 @@ namespace BizHawk.Client.EmuHawk if (indices.Count > 0) { // TODO this will allocate unnecessary memory when this movie is a TasMovie due to TasStateManager - var movie = _movieSession.Get(_movieList[MovieView.SelectedIndices[0]].Filename); - movie.Load(); + var movie = _movieSession.Get(_movieList[MovieView.SelectedIndices[0]].Filename, true); var form = new EditSubtitlesForm(DialogController, movie, _config.PathEntries, readOnly: false, disposeOnClose: true); form.Show(); } diff --git a/src/BizHawk.Client.EmuHawk/movie/RecordMovie.cs b/src/BizHawk.Client.EmuHawk/movie/RecordMovie.cs index 467a048a43..c61764c407 100644 --- a/src/BizHawk.Client.EmuHawk/movie/RecordMovie.cs +++ b/src/BizHawk.Client.EmuHawk/movie/RecordMovie.cs @@ -267,7 +267,6 @@ namespace BizHawk.Client.EmuHawk _game, _firmwareManager, AuthorBox.Text ?? _config.DefaultAuthor); - movieToRecord.Save(); _mainForm.StartNewMovie(movieToRecord, true); _config.UseDefaultAuthor = DefaultAuthorCheckBox.Checked; diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs index 9e2c5846b8..0fbc3b612d 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs @@ -50,7 +50,7 @@ namespace BizHawk.Client.EmuHawk Emulator.Frame, StatableEmulator.CloneSavestate()); MainForm.PauseEmulator(); - LoadFile(new FileInfo(newProject.Filename), true); + LoadMovie(newProject, true); } } @@ -62,7 +62,7 @@ namespace BizHawk.Client.EmuHawk GoToFrame(TasView.AnyRowsSelected ? TasView.FirstSelectedRowIndex : 0); var newProject = CurrentTasMovie.ConvertToSaveRamAnchoredMovie(saveRam); MainForm.PauseEmulator(); - LoadFile(new FileInfo(newProject.Filename), true); + LoadMovie(newProject, true); } } @@ -100,8 +100,7 @@ namespace BizHawk.Client.EmuHawk if (askToSave && !AskSaveChanges()) return false; if (filename.EndsWithOrdinal(MovieService.TasMovieExtension)) { - LoadFileWithFallback(filename); - return true; //TODO should this return false if it fell back to a new project? + return LoadFileWithFallback(filename); } if (filename.EndsWithOrdinal(MovieService.StandardMovieExtension)) { @@ -113,18 +112,8 @@ namespace BizHawk.Client.EmuHawk { return false; } - _initializing = true; // Starting a new movie causes a core reboot - WantsToControlReboot = false; - _engaged = false; - if (!MainForm.StartNewMovie(MovieSession.Get(filename), record: false)) return false; - ConvertCurrentMovieToTasproj(); - _initializing = false; - var success = StartNewMovieWrapper(CurrentTasMovie); - _engaged = true; - WantsToControlReboot = true; - SetUpColumns(); - UpdateWindowTitle(); - return success; //TODO is this correct? + + return LoadFileWithFallback(filename); } DialogController.ShowMessageBox( caption: "Movie load error", diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs index d5037e8589..bca6ff8431 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs @@ -204,11 +204,7 @@ namespace BizHawk.Client.EmuHawk private void LoadMostRecentOrStartNew() { - if (!LoadFile(new(Settings.RecentTas.MostRecent))) - { - TasView.AllColumns.Clear(); - StartNewTasMovie(); - } + LoadFileWithFallback(Settings.RecentTas.MostRecent); } private bool Engage() @@ -255,7 +251,7 @@ namespace BizHawk.Client.EmuHawk // Start Scenario 2: A tasproj is already active else if (MovieSession.Movie.IsActive() && MovieSession.Movie is ITasMovie) { - bool result = LoadFile(new FileInfo(CurrentTasMovie.Filename), gotoFrame: Emulator.Frame); + bool result = LoadMovie(CurrentTasMovie, gotoFrame: Emulator.Frame); if (!result) { TasView.AllColumns.Clear(); @@ -520,26 +516,18 @@ namespace BizHawk.Client.EmuHawk CurrentTasMovie.PropertyChanged += TasMovie_OnPropertyChanged; } - private bool LoadFile(FileInfo file, bool startsFromSavestate = false, int gotoFrame = 0) + private bool LoadMovie(ITasMovie tasMovie, bool startsFromSavestate = false, int gotoFrame = 0) { - if (!file.Exists) - { - Settings.RecentTas.HandleLoadError(MainForm, file.FullName); - return false; - } - _engaged = false; - var newMovie = (ITasMovie)MovieSession.Get(file.FullName); - newMovie.BindMarkersToInput = Settings.BindMarkersToInput; - newMovie.GreenzoneInvalidated = GreenzoneInvalidated; + tasMovie.BindMarkersToInput = Settings.BindMarkersToInput; + tasMovie.GreenzoneInvalidated = GreenzoneInvalidated; - if (!HandleMovieLoadStuff(newMovie)) + if (!HandleMovieLoadStuff(tasMovie)) { return false; } _engaged = true; - Settings.RecentTas.Add(newMovie.Filename); // only add if it did load if (startsFromSavestate) { @@ -602,14 +590,8 @@ namespace BizHawk.Client.EmuHawk Config.DefaultAuthor); SetTasMovieCallbacks(tasMovie); - MovieSession.SetMovieController(Emulator.ControllerDefinition); // hack, see interface comment - tasMovie.ClearChanges(); // Don't ask to save changes here. - tasMovie.Save(); + tasMovie.ClearChanges(); _ = HandleMovieLoadStuff(tasMovie); - // let's not keep this longer than we actually need - // the user will be prompted to enter a proper name - // when they want to save - File.Delete(tasMovie.Filename); // clear all selections TasView.DeselectAll(); @@ -623,7 +605,6 @@ namespace BizHawk.Client.EmuHawk private bool HandleMovieLoadStuff(ITasMovie movie) { WantsToControlStopMovie = false; - WantsToControlReboot = false; var result = StartNewMovieWrapper(movie); if (!result) @@ -632,10 +613,8 @@ namespace BizHawk.Client.EmuHawk } WantsToControlStopMovie = true; - WantsToControlReboot = true; CurrentTasMovie.ChangeLog.Clear(); - CurrentTasMovie.ClearChanges(); UpdateWindowTitle(); MessageStatusLabel.Text = $"{Path.GetFileName(CurrentTasMovie.Filename)} loaded."; @@ -650,7 +629,9 @@ namespace BizHawk.Client.EmuHawk SetTasMovieCallbacks(movie); SuspendLayout(); + WantsToControlReboot = false; bool result = MainForm.StartNewMovie(movie, false); + WantsToControlReboot = true; ResumeLayout(); if (result) { @@ -672,16 +653,31 @@ namespace BizHawk.Client.EmuHawk } } - private void LoadFileWithFallback(string path) + private bool LoadFileWithFallback(string path) { - var result = LoadFile(new FileInfo(path)); - if (!result) + bool movieLoadSucceeded = false; + + if (!File.Exists(path)) + { + Settings.RecentTas.HandleLoadError(MainForm, path); + } + else + { + var movie = MovieSession.Get(path, true); + + var tasMovie = movie as ITasMovie ?? movie.ToTasMovie(); + movieLoadSucceeded = LoadMovie(tasMovie); + } + + if (!movieLoadSucceeded) { TasView.AllColumns.Clear(); WantsToControlReboot = false; StartNewTasMovie(); _engaged = true; } + + return movieLoadSucceeded; } private void DummyLoadMacro(string path) @@ -939,7 +935,7 @@ namespace BizHawk.Client.EmuHawk if (fromLua) { bool wasPaused = MainForm.EmulatorPaused; - + // why not use this? because I'm not letting the form freely run. it all has to be under this loop. // i could use this and then poll StepRunLoop_Core() repeatedly, but.. that's basically what I'm doing // PauseOnFrame = frame; @@ -1095,7 +1091,7 @@ namespace BizHawk.Client.EmuHawk { CurrentTasMovie.ClearFrame(i); } - + if (needsToRollback) { GoToLastEmulatedFrameIfNecessary(beginningFrame);