Morilli
commented
3 months ago Can you explain when this happens? Just playing a movie using the play movie dialog does respect the "On Movie End" settings for me.
Open RetroEdit opened 3 months ago
Morilli
commented
3 months ago Can you explain when this happens? Just playing a movie using the play movie dialog does respect the "On Movie End" settings for me.
RetroEdit
commented
3 months ago Okay, I can reproduce this, but my steps are a little weird and artificial. Starting with no config:
Config > Save Configwhile true do end into it, when it says the EmuHawk is not responding, close it.There seem to only be two places in the entire codebase where the OnMovieEnd setting is changed implicitly: TAStudio and Macros.
To me, implicit config changes to OnMovieEnd seem kind of hacky, especially when it's changing it to "Record", which can append junk to the end of your movie. As demonstrated above, if you open TAStudio and it doesn't clean up properly (e.g., maybe it crashed for some reason), it will indeed permanently and silently save what is intended as a temporary setting change to your config. This TAStudio detail seems the most likely culprit for the issue I encountered 11 months ago -- although it was probably more along the lines of TAStudio exiting unusually and then me closing the emulator and having the config save upon BizHawk closing.
TAStudio should keep that info within the TAStudio session if it needs it, not touch the config at all. No need to preserve the original 10-year old code of b53cc908b8e2543e8a8ca4869121c96e73e27ed9 .
I can't speak to macros because I haven't used them enough, but silent implicit modification also seems questionable there.
Morilli
commented
3 months ago This doesn't seem trivial to do. Well, it is kinda, but it's a bit hacky. The main issue here is that the following code needs to know whether tastudio is active: https://github.com/TASEmulators/BizHawk/blob/ccb69b18a9ef715f7f32bd7cba9ef546d0ea2d71/src/BizHawk.Client.Common/movie/MovieSession.cs#L97-L107
Just knowing that the current movie is a TasMovie is not enough when we want to keep support for the movie end action when playing tasproj movies outside of TAStudio. But checking TAStudio's load state in MovieSession is... not possible without either passing it in or doing other questionable things. Here's a conceptual diff that would work if ^ was solved:
diff --git a/src/BizHawk.Client.Common/movie/MovieSession.cs b/src/BizHawk.Client.Common/movie/MovieSession.cs
index d1a748fe1..11ef21269 100644
--- a/src/BizHawk.Client.Common/movie/MovieSession.cs
+++ b/src/BizHawk.Client.Common/movie/MovieSession.cs
@@ -99,7 +99,8 @@ namespace BizHawk.Client.Common
if (Movie is ITasMovie tasMovie)
{
tasMovie.GreenzoneCurrentFrame();
- if (tasMovie.IsPlayingOrFinished() && Settings.MovieEndAction == MovieEndAction.Record && Movie.Emulator.Frame >= tasMovie.InputLogLength)
+ bool tastudioLoaded = false;
+ if (tasMovie.IsPlayingOrFinished() && (tastudioLoaded || Settings.MovieEndAction == MovieEndAction.Record) && Movie.Emulator.Frame >= tasMovie.InputLogLength)
{
HandleFrameLoopForRecordMode();
return;
diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs
index 5fb9121a1..f7ebd3ecb 100644
--- a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs
+++ b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs
@@ -189,13 +189,6 @@ namespace BizHawk.Client.EmuHawk
MovieEndRecordMenuItem.Checked = Config.Movies.MovieEndAction == MovieEndAction.Record;
MovieEndStopMenuItem.Checked = Config.Movies.MovieEndAction == MovieEndAction.Stop;
MovieEndPauseMenuItem.Checked = Config.Movies.MovieEndAction == MovieEndAction.Pause;
-
- // Arguably an IControlMainForm property should be set here, but in reality only Tastudio is ever going to interfere with this logic
- MovieEndFinishMenuItem.Enabled =
- MovieEndRecordMenuItem.Enabled =
- MovieEndStopMenuItem.Enabled =
- MovieEndPauseMenuItem.Enabled =
- !Tools.Has<TAStudio>();
}
private void AVSubMenu_DropDownOpened(object sender, EventArgs e)
diff --git a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs
index 04c2917fd..2e353177d 100644
--- a/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs
+++ b/src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs
@@ -34,7 +34,6 @@ namespace BizHawk.Client.EmuHawk
private readonly List<TasClipboardEntry> _tasClipboard = new List<TasClipboardEntry>();
private const string CursorColumnName = "CursorColumn";
private const string FrameColumnName = "FrameColumn";
- private MovieEndAction _originalEndAction; // The movie end behavior selected by the user (that is overridden by TAStudio)
private UndoHistoryForm _undoForm;
private Timer _autosaveTimer;
@@ -287,9 +286,7 @@ namespace BizHawk.Client.EmuHawk
MainForm.AddOnScreenMessage("TAStudio engaged");
SetTasMovieCallbacks(CurrentTasMovie);
UpdateWindowTitle();
- _originalEndAction = Config.Movies.MovieEndAction;
MainForm.DisableRewind();
- Config.Movies.MovieEndAction = MovieEndAction.Record;
MainForm.SetMainformMovieInfo();
MovieSession.ReadOnly = true;
SetSplicer();
@@ -716,7 +713,6 @@ namespace BizHawk.Client.EmuHawk
_engaged = false;
MainForm.PauseOnFrame = null;
MainForm.AddOnScreenMessage("TAStudio disengaged");
- Config.Movies.MovieEndAction = _originalEndAction;
WantsToControlRewind = false;
MainForm.EnableRewind(true);
MainForm.SetMainformMovieInfo();
(Split from #3735 ; originally written 2023-09-22) I recently realized Play Movie will automatically override your
On Movie Endsetting to beRecord. EDIT 2024-08-18: See below: it's probably not directly Play Movie related.This is extremely undesirable behavior for me... it's definitely possible to change the toggle insidePlay Movieto pause on a specific frame, but the current behavior of silently altering user-selected settings seems extremely bad practice to me. It's sort of a UI choice more than a bug per se, though.At the very least, it should remember if you wanted to end on a specific frame (unfortunately, this alone may have downsides because pausing on a non-movie end frame via play movie is sort of specific to the movie). Personally, I would like the a direct equivalent to theOn Movie Endoptions selection visible within Play Movie if it's going to permanently alter those settings.