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 ec0bc882a89..fe1148aa0a0 100644 --- a/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs +++ b/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs @@ -146,8 +146,7 @@ public virtual string FirmwareHash 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 83eb935ca74..77d2b4c5dea 100644 --- a/src/BizHawk.Client.Common/movie/MovieSession.cs +++ b/src/BizHawk.Client.Common/movie/MovieSession.cs @@ -61,11 +61,6 @@ public IMovieController GenerateMovieController(ControllerDefinition definition return new Bk2Controller("", definition ?? MovieController.Definition); } - public void SetMovieController(ControllerDefinition definition) - { - MovieController = GenerateMovieController(definition); - } - public void HandleFrameBefore() { if (Movie.NotActive()) @@ -189,72 +184,49 @@ public bool HandleLoadState(TextReader reader) 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) - { - movie.Save(); - } - - 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 (movie.SystemID != systemId) { - movie.Load(); - - if (movie.SystemID != systemId) - { - 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(); - } - } + throw new MoviePlatformMismatchException( + $"Movie system Id ({movie.SystemID}) does not match the currently loaded platform ({systemId}), unable to load"); } - if (!record) + if (!(string.IsNullOrEmpty(movie.Hash) || loadedRomHash.Equals(movie.Hash, StringComparison.Ordinal)) + && movie is TasMovie tasproj) { - 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 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) { - var keys = preferredCores.Keys.ToList(); - foreach (var k in keys) - { - preferredCores[k] = movie.Core; - } + tasproj.TasSession.UpdateValues(frame: 0, currentBranch: tasproj.TasSession.CurrentBranch); // wtf is this API --yoshi + tasproj.InvalidateEntireGreenzone(); } } - 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 + if (string.IsNullOrWhiteSpace(movie.Core)) { - 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 @@ public void ConvertToTasProj() 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); + + if (loadMovie) + movie.Load(); - return new Bk2Movie(this, path); + 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 50880865a1f..8ba18108a54 100644 --- a/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs +++ b/src/BizHawk.Client.Common/movie/interfaces/IMovieSession.cs @@ -54,17 +54,6 @@ public interface IMovieSession /// 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 @@ public interface IMovieSession /// void QueueNewMovie( IMovie movie, - bool record, string systemId, string loadedRomHash, PathEntryCollection pathEntries, @@ -100,7 +88,11 @@ void QueueNewMovie( /// 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 3e094cdbb1b..382c7cbeb87 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.FileLoader.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.FileLoader.cs @@ -95,7 +95,7 @@ public bool LoadMovie(string filename, string archive = null) } 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 fe87bf433ce..a553374066e 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Movie.cs @@ -29,7 +29,6 @@ public bool StartNewMovie(IMovie movie, bool record) { 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 598c06cc30a..f5ca6d99362 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -668,7 +668,7 @@ _argParser.SocketAddress is var (socketIP, socketPort) // 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 @@ _argParser.SocketAddress is var (socketIP, socketPort) { 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 @@ private void LoadMoviesFromRecent(string path) { 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 f5a55cbd6aa..93440700ea7 100644 --- a/src/BizHawk.Client.EmuHawk/movie/PlayMovie.cs +++ b/src/BizHawk.Client.EmuHawk/movie/PlayMovie.cs @@ -102,7 +102,7 @@ private void Run() 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 @@ private void CommentsBtn_Click(object sender, EventArgs e) 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 @@ private void SubtitlesBtn_Click(object sender, EventArgs e) 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 467a048a430..c61764c4073 100644 --- a/src/BizHawk.Client.EmuHawk/movie/RecordMovie.cs +++ b/src/BizHawk.Client.EmuHawk/movie/RecordMovie.cs @@ -267,7 +267,6 @@ private void Ok_Click(object sender, EventArgs e) _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 9e2c5846b86..0fbc3b612dd 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 @@ private void StartNewProjectFromNowMenuItem_Click(object sender, EventArgs e) Emulator.Frame, StatableEmulator.CloneSavestate()); MainForm.PauseEmulator(); - LoadFile(new FileInfo(newProject.Filename), true); + LoadMovie(newProject, true); } } @@ -62,7 +62,7 @@ private void StartANewProjectFromSaveRamMenuItem_Click(object sender, EventArgs 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 @@ public bool LoadMovieFile(string filename, bool askToSave = true) 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 @@ public bool LoadMovieFile(string filename, bool askToSave = true) { 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 d5037e8589f..bca6ff8431b 100644 --- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs +++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs @@ -204,11 +204,7 @@ private void Tastudio_Load(object sender, EventArgs e) private void LoadMostRecentOrStartNew() { - if (!LoadFile(new(Settings.RecentTas.MostRecent))) - { - TasView.AllColumns.Clear(); - StartNewTasMovie(); - } + LoadFileWithFallback(Settings.RecentTas.MostRecent); } private bool Engage() @@ -255,7 +251,7 @@ private bool Engage() // 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 @@ private void ConvertCurrentMovieToTasproj() 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 @@ private void StartNewTasMovie() 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 @@ private void StartNewTasMovie() private bool HandleMovieLoadStuff(ITasMovie movie) { WantsToControlStopMovie = false; - WantsToControlReboot = false; var result = StartNewMovieWrapper(movie); if (!result) @@ -632,10 +613,8 @@ private bool HandleMovieLoadStuff(ITasMovie movie) } WantsToControlStopMovie = true; - WantsToControlReboot = true; CurrentTasMovie.ChangeLog.Clear(); - CurrentTasMovie.ClearChanges(); UpdateWindowTitle(); MessageStatusLabel.Text = $"{Path.GetFileName(CurrentTasMovie.Filename)} loaded."; @@ -650,7 +629,9 @@ private bool StartNewMovieWrapper(ITasMovie movie) SetTasMovieCallbacks(movie); SuspendLayout(); + WantsToControlReboot = false; bool result = MainForm.StartNewMovie(movie, false); + WantsToControlReboot = true; ResumeLayout(); if (result) { @@ -672,16 +653,31 @@ private void DummyLoadProject(string path) } } - 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 @@ private void StartAtNearestFrameAndEmulate(int frame, bool fromLua, bool fromRew 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 @@ public void ClearFrames(int beginningFrame, int numberOfFrames) { CurrentTasMovie.ClearFrame(i); } - + if (needsToRollback) { GoToLastEmulatedFrameIfNecessary(beginningFrame);