Fix movie header values being potentially outdated upon starting a movie record (#4010)

* Rename StartNewMovie parameter

* Move PopulateWithDefaultHeaderValues to MainForm.Movie, make private

Author still needs to be set locally

* Make PopulateWithDefaultHeaderValues non-static

* Remove now-unnecessary firmware parameter in RecordMovie

* add TODO comment

* add Prepopulate function before movie load

Now, the main header population happens after the core is reloaded, ensuring all values are in the correct state.

A few movie values still need to be set beforehand though to ensure the core reboot works correctly.

* Allow recording movies even with outstanding reboot

Should be safe now with the previous commit.
This commit is contained in:
Moritz Bender 2024-09-07 15:27:20 +02:00 committed by GitHub
parent 650c7cae17
commit f47c58f550
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 153 additions and 157 deletions

View File

@ -4,17 +4,6 @@ using System.Linq;
using BizHawk.Common;
using BizHawk.Common.PathExtensions;
using BizHawk.Emulation.Common;
using BizHawk.Emulation.Cores.Arcades.MAME;
using BizHawk.Emulation.Cores.Atari.Jaguar;
using BizHawk.Emulation.Cores.Consoles.Nintendo.Ares64;
using BizHawk.Emulation.Cores.Consoles.Nintendo.Gameboy;
using BizHawk.Emulation.Cores.Consoles.Nintendo.NDS;
using BizHawk.Emulation.Cores.Consoles.Sega.gpgx;
using BizHawk.Emulation.Cores.Consoles.Sega.PicoDrive;
using BizHawk.Emulation.Cores.Nintendo.NES;
using BizHawk.Emulation.Cores.Nintendo.SubNESHawk;
using BizHawk.Emulation.Cores.Sega.MasterSystem;
namespace BizHawk.Client.Common
{
@ -183,127 +172,6 @@ namespace BizHawk.Client.Common
return tas;
}
// TODO: This doesn't really belong here, but not sure where to put it
public static void PopulateWithDefaultHeaderValues(
this IMovie movie,
IEmulator emulator,
ISettingsAdapter settable,
IGameInfo game,
FirmwareManager firmwareManager,
string author)
{
movie.Author = author;
movie.EmulatorVersion = VersionInfo.GetEmuVersion();
movie.OriginalEmulatorVersion = VersionInfo.GetEmuVersion();
movie.SystemID = emulator.SystemId;
if (settable.HasSyncSettings)
{
movie.SyncSettingsJson = ConfigService.SaveWithType(settable.GetSyncSettings());
}
movie.GameName = game.FilesystemSafeName();
movie.Hash = game.Hash;
if (game.FirmwareHash != null)
{
movie.FirmwareHash = game.FirmwareHash;
}
if (emulator.HasBoardInfo())
{
movie.BoardName = emulator.AsBoardInfo().BoardName;
}
if (emulator.HasRegions())
{
var region = emulator.AsRegionable().Region;
if (region == DisplayType.PAL)
{
movie.HeaderEntries.Add(HeaderKeys.Pal, "1");
}
}
if (firmwareManager.RecentlyServed.Count != 0)
{
foreach (var firmware in firmwareManager.RecentlyServed)
{
var key = firmware.ID.MovieHeaderKey;
if (!movie.HeaderEntries.ContainsKey(key))
{
movie.HeaderEntries.Add(key, firmware.Hash);
}
}
}
if (emulator is NDS nds && nds.IsDSi)
{
movie.HeaderEntries.Add("IsDSi", "1");
if (nds.IsDSiWare)
{
movie.HeaderEntries.Add("IsDSiWare", "1");
}
}
if ((emulator is NES nes && nes.IsVS)
|| (emulator is SubNESHawk subnes && subnes.IsVs))
{
movie.HeaderEntries.Add("IsVS", "1");
}
if (emulator is IGameboyCommon gb)
{
//TODO doesn't IsCGBDMGMode imply IsCGBMode?
if (gb.IsCGBMode) movie.HeaderEntries.Add(gb.IsCGBDMGMode ? "IsCGBDMGMode" : "IsCGBMode", "1");
}
if (emulator is SMS sms)
{
if (sms.IsSG1000)
{
movie.HeaderEntries.Add("IsSGMode", "1");
}
if (sms.IsGameGear)
{
movie.HeaderEntries.Add("IsGGMode", "1");
}
}
if (emulator is GPGX gpgx && gpgx.IsMegaCD)
{
movie.HeaderEntries.Add("IsSegaCDMode", "1");
}
if (emulator is PicoDrive pico && pico.Is32XActive)
{
movie.HeaderEntries.Add("Is32X", "1");
}
if (emulator is VirtualJaguar jag && jag.IsJaguarCD)
{
movie.HeaderEntries.Add("IsJaguarCD", "1");
}
if (emulator is Ares64 ares && ares.IsDD)
{
movie.HeaderEntries.Add("IsDD", "1");
}
if (emulator is MAME mame)
{
movie.HeaderEntries.Add(HeaderKeys.VsyncAttoseconds, mame.VsyncAttoseconds.ToString());
}
if (emulator.HasCycleTiming())
{
movie.HeaderEntries.Add(HeaderKeys.CycleCount, "0");
movie.HeaderEntries.Add(HeaderKeys.ClockRate, "0");
}
movie.Core = emulator.Attributes().CoreName;
}
internal static string ConvertFileNameToTasMovie(string oldFileName)
{
if (oldFileName is null) return null;

View File

@ -17,6 +17,7 @@ namespace BizHawk.Client.EmuHawk
bool EmulatorPaused { get; }
/// <remarks>only referenced from <see cref="TAStudio"/></remarks>
// TODO: remove? or does anything ever need access to the FirmwareManager
FirmwareManager FirmwareManager { get; }
/// <remarks>only referenced from <see cref="TAStudio"/></remarks>
@ -81,7 +82,7 @@ namespace BizHawk.Client.EmuHawk
void SetMainformMovieInfo();
bool StartNewMovie(IMovie movie, bool record);
bool StartNewMovie(IMovie movie, bool newMovie);
/// <remarks>only referenced from <see cref="BasicBot"/></remarks>
void Throttle();

View File

@ -173,10 +173,6 @@ namespace BizHawk.Client.EmuHawk
= RecentMovieSubMenu.Enabled
= !Emulator.IsNull() && !Tools.IsLoaded<TAStudio>();
// Record movie dialog should not be opened while in need of a reboot,
// Otherwise the wrong sync settings could be set for the recording movie and cause crashes
RecordMovieMenuItem.Enabled &= !RebootStatusBarIcon.Visible;
PlayFromBeginningMenuItem.Enabled = MovieSession.Movie.IsActive() && !Tools.IsLoaded<TAStudio>();
}
@ -368,7 +364,7 @@ namespace BizHawk.Client.EmuHawk
// Nag user to user a more accurate core, but let them continue anyway
EnsureCoreIsAccurate();
using var form = new RecordMovie(this, Config, Game, Emulator, MovieSession, FirmwareManager);
using var form = new RecordMovie(this, Config, Game, Emulator, MovieSession);
this.ShowDialogWithTempMute(form);
}

View File

@ -2,13 +2,24 @@ using System.Collections.Generic;
using System.Windows.Forms;
using BizHawk.Client.Common;
using BizHawk.Common;
using BizHawk.Emulation.Common;
using BizHawk.Emulation.Cores.Arcades.MAME;
using BizHawk.Emulation.Cores.Atari.Jaguar;
using BizHawk.Emulation.Cores.Consoles.Nintendo.Ares64;
using BizHawk.Emulation.Cores.Consoles.Nintendo.Gameboy;
using BizHawk.Emulation.Cores.Consoles.Nintendo.NDS;
using BizHawk.Emulation.Cores.Consoles.Sega.gpgx;
using BizHawk.Emulation.Cores.Consoles.Sega.PicoDrive;
using BizHawk.Emulation.Cores.Nintendo.NES;
using BizHawk.Emulation.Cores.Nintendo.SubNESHawk;
using BizHawk.Emulation.Cores.Sega.MasterSystem;
namespace BizHawk.Client.EmuHawk
{
public partial class MainForm
{
public bool StartNewMovie(IMovie movie, bool record)
public bool StartNewMovie(IMovie movie, bool newMovie)
{
if (movie is null) throw new ArgumentNullException(paramName: nameof(movie));
@ -24,6 +35,7 @@ namespace BizHawk.Client.EmuHawk
var oldPreferredCores = new Dictionary<string, string>(Config.PreferredCores);
try
{
if (newMovie) PrepopulateMovieHeaderValues(movie);
try
{
MovieSession.QueueNewMovie(
@ -48,7 +60,13 @@ namespace BizHawk.Client.EmuHawk
Config.RecentMovies.Add(movie.Filename);
MovieSession.RunQueuedMovie(record, Emulator);
MovieSession.RunQueuedMovie(newMovie, Emulator);
if (newMovie)
{
PopulateWithDefaultHeaderValues(movie);
if (movie is ITasMovie tasMovie)
tasMovie.ClearChanges();
}
}
finally
{
@ -138,5 +156,132 @@ namespace BizHawk.Client.EmuHawk
}
}
}
/// <summary>
/// Sets necessary movie values in order to be able to start this <paramref name="movie"/>
/// Only required when creating a new movie from scratch.
/// </summary>
/// <param name="movie">The movie to fill with values</param>
private void PrepopulateMovieHeaderValues(IMovie movie)
{
movie.Core = Emulator.Attributes().CoreName;
movie.SystemID = Emulator.SystemId; // TODO: I feel like setting this shouldn't be necessary, but it is currently
var settable = GetSettingsAdapterForLoadedCoreUntyped();
if (settable.HasSyncSettings)
{
movie.SyncSettingsJson = ConfigService.SaveWithType(settable.GetSyncSettings());
}
}
/// <summary>
/// Sets default header values for the given <paramref name="movie"/>. Notably needs to be done after loading the core
/// to make sure all values are in the correct state, see https://github.com/TASEmulators/BizHawk/issues/3980
/// </summary>
/// <param name="movie">The movie to fill with values</param>
private void PopulateWithDefaultHeaderValues(IMovie movie)
{
movie.EmulatorVersion = VersionInfo.GetEmuVersion();
movie.OriginalEmulatorVersion = VersionInfo.GetEmuVersion();
movie.GameName = Game.FilesystemSafeName();
movie.Hash = Game.Hash;
if (Game.FirmwareHash != null)
{
movie.FirmwareHash = Game.FirmwareHash;
}
if (Emulator.HasBoardInfo())
{
movie.BoardName = Emulator.AsBoardInfo().BoardName;
}
if (Emulator.HasRegions())
{
var region = Emulator.AsRegionable().Region;
if (region == DisplayType.PAL)
{
movie.HeaderEntries.Add(HeaderKeys.Pal, "1");
}
}
if (FirmwareManager.RecentlyServed.Count != 0)
{
foreach (var firmware in FirmwareManager.RecentlyServed)
{
var key = firmware.ID.MovieHeaderKey;
if (!movie.HeaderEntries.ContainsKey(key))
{
movie.HeaderEntries.Add(key, firmware.Hash);
}
}
}
if (Emulator is NDS nds && nds.IsDSi)
{
movie.HeaderEntries.Add("IsDSi", "1");
if (nds.IsDSiWare)
{
movie.HeaderEntries.Add("IsDSiWare", "1");
}
}
if ((Emulator is NES nes && nes.IsVS)
|| (Emulator is SubNESHawk subnes && subnes.IsVs))
{
movie.HeaderEntries.Add("IsVS", "1");
}
if (Emulator is IGameboyCommon gb)
{
//TODO doesn't IsCGBDMGMode imply IsCGBMode?
if (gb.IsCGBMode) movie.HeaderEntries.Add(gb.IsCGBDMGMode ? "IsCGBDMGMode" : "IsCGBMode", "1");
}
if (Emulator is SMS sms)
{
if (sms.IsSG1000)
{
movie.HeaderEntries.Add("IsSGMode", "1");
}
if (sms.IsGameGear)
{
movie.HeaderEntries.Add("IsGGMode", "1");
}
}
if (Emulator is GPGX gpgx && gpgx.IsMegaCD)
{
movie.HeaderEntries.Add("IsSegaCDMode", "1");
}
if (Emulator is PicoDrive pico && pico.Is32XActive)
{
movie.HeaderEntries.Add("Is32X", "1");
}
if (Emulator is VirtualJaguar jag && jag.IsJaguarCD)
{
movie.HeaderEntries.Add("IsJaguarCD", "1");
}
if (Emulator is Ares64 ares && ares.IsDD)
{
movie.HeaderEntries.Add("IsDD", "1");
}
if (Emulator is MAME mame)
{
movie.HeaderEntries.Add(HeaderKeys.VsyncAttoseconds, mame.VsyncAttoseconds.ToString());
}
if (Emulator.HasCycleTiming())
{
movie.HeaderEntries.Add(HeaderKeys.CycleCount, "0");
movie.HeaderEntries.Add(HeaderKeys.ClockRate, "0");
}
}
}
}

View File

@ -24,7 +24,6 @@ namespace BizHawk.Client.EmuHawk
private readonly GameInfo _game;
private readonly IEmulator _emulator;
private readonly IMovieSession _movieSession;
private readonly FirmwareManager _firmwareManager;
private readonly TextBox AuthorBox;
@ -41,8 +40,7 @@ namespace BizHawk.Client.EmuHawk
Config config,
GameInfo game,
IEmulator core,
IMovieSession movieSession,
FirmwareManager firmwareManager)
IMovieSession movieSession)
{
if (game.IsNullInstance()) throw new InvalidOperationException("how is the traditional Record dialog open with no game loaded? please report this including as much detail as possible");
@ -51,7 +49,6 @@ namespace BizHawk.Client.EmuHawk
_game = game;
_emulator = core;
_movieSession = movieSession;
_firmwareManager = firmwareManager;
SuspendLayout();
@ -222,6 +219,7 @@ namespace BizHawk.Client.EmuHawk
}
var movieToRecord = _movieSession.Get(path);
movieToRecord.Author = AuthorBox.Text ?? _config.DefaultAuthor;
var fileInfo = new FileInfo(path);
if (!fileInfo.Exists)
@ -260,12 +258,6 @@ namespace BizHawk.Client.EmuHawk
movieToRecord.SaveRam = core.CloneSaveRam();
}
movieToRecord.PopulateWithDefaultHeaderValues(
_emulator,
((MainForm) _mainForm).GetSettingsAdapterForLoadedCoreUntyped(), //HACK
_game,
_firmwareManager,
AuthorBox.Text ?? _config.DefaultAuthor);
_mainForm.StartNewMovie(movieToRecord, true);
_config.UseDefaultAuthor = DefaultAuthorCheckBox.Checked;

View File

@ -580,18 +580,12 @@ namespace BizHawk.Client.EmuHawk
var filename = DefaultTasProjName(); // TODO don't do this, take over any mainform actions that can crash without a filename
var tasMovie = (ITasMovie)MovieSession.Get(filename);
tasMovie.Author = Config.DefaultAuthor;
tasMovie.BindMarkersToInput = Settings.BindMarkersToInput;
tasMovie.GreenzoneInvalidated = GreenzoneInvalidated;
tasMovie.PropertyChanged += TasMovie_OnPropertyChanged;
tasMovie.PopulateWithDefaultHeaderValues(
Emulator,
((MainForm) MainForm).GetSettingsAdapterForLoadedCoreUntyped(), //HACK
Game,
MainForm.FirmwareManager,
Config.DefaultAuthor);
_ = StartNewMovieWrapper(tasMovie, isNew: true);
// clear all selections