Round 1 of Movie loading system overhaul, this fixes the problem of loading sram when a movie is loaded. Still todo: some cleanup of some hacks that are no longer needed, and restoring the ability to open the play movie dialog and other things, while a movie is active

BizHawk.Client.Common/movie/MovieSession.cs

@@ -2,6 +2,12 @@
 using System.IO;
 
 using BizHawk.Emulation.Common;
+using BizHawk.Emulation.Cores.Consoles.Nintendo.QuickNES;
+using BizHawk.Emulation.Cores.Nintendo.NES;
+using BizHawk.Emulation.Cores.Nintendo.SNES9X;
+using BizHawk.Emulation.Cores.Nintendo.SNES;
+
+using BizHawk.Client.Common;
 
 namespace BizHawk.Client.Common
 {
@@ -17,6 +23,21 @@ namespace BizHawk.Client.Common
 			MovieControllerAdapter = MovieService.DefaultInstance.LogGeneratorInstance().MovieControllerAdapter;
 		}
 
+		/// <summary>
+		/// When initializing a movie, it will be stored here until Rom processes have been completed, then it will be moved to the Movie property
+		/// If an existing movie is still active, it will remain in the Movie property while the new movie is queued
+		/// </summary>
+		public IMovie QueuedMovie { get; set; }
+
+		// This wrapper but the logic could change, don't make the client code understand these details
+		public bool MovieIsQueued
+		{
+			get { return QueuedMovie != null; }
+		}
+
+		//TODO: this shouldn't be a hack anymore, the contents of this should be reliably in QueuedMovie
+		public object SyncSettingsHack { get; set; }
+
 		public MultitrackRecording MultiTrack { get { return _multiTrack; } }
 		public IMovieController MovieControllerAdapter{ get; set; }
 
@@ -149,6 +170,7 @@ namespace BizHawk.Client.Common
 			ModeChangedCallback();
 		}
 
+		// Movie Refactor TODO: delete me, any code calling this is poorly designed
 		public bool MovieLoad()
 		{
 			MovieControllerAdapter = Movie.LogGeneratorInstance().MovieControllerAdapter;
@@ -387,5 +409,95 @@ namespace BizHawk.Client.Common
 				MessageCallback("MultiTrack cannot be enabled while not recording.");
 			}
 		}
+
+		// Movie Load Refactor TODO: a better name
+		/// <summary>
+		/// Sets the Movie property with the QueuedMovie, clears the queued movie, and starts the new movie
+		/// </summary>
+		public void RunQueuedMovie(bool recordMode)
+		{
+			Movie = QueuedMovie;
+			QueuedMovie = null;
+
+			if (Movie.IsRecording)
+			{
+				Movie.StartNewRecording();
+				ReadOnly = false;
+			}
+			else
+			{
+				Movie.StartNewPlayback();
+			}
+		}
+
+		public void QueueNewMovie(IMovie movie, bool record)
+		{
+			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
+			{
+				movie.Load();
+				if (movie.SystemID != Global.Emulator.SystemId)
+				{
+					MessageCallback("Movie does not match the currently loaded system, unable to load");
+					return;
+				}
+			}
+
+			//If a movie is already loaded, save it before starting a new movie
+			if (Global.MovieSession.Movie.IsActive && !string.IsNullOrEmpty(Global.MovieSession.Movie.Filename))
+			{
+				Global.MovieSession.Movie.Save();
+			}
+
+			// Note: this populates MovieControllerAdapter's Type with the approparite controller
+			// Don't set it to a movie instance of the adapter or you will lose the definition!
+			InputManager.RewireInputChain();
+
+			if (!record && Global.Emulator.SystemId == "NES") // For NES we need special logic since the movie will drive which core to load
+			{
+				var quicknesName = ((CoreAttributes)Attribute.GetCustomAttribute(typeof(QuickNES), typeof(CoreAttributes))).CoreName;
+				var neshawkName = ((CoreAttributes)Attribute.GetCustomAttribute(typeof(NES), typeof(CoreAttributes))).CoreName;
+
+				// If either is specified use that, else use whatever is currently set
+				if (Global.MovieSession.Movie.Core == quicknesName)
+				{
+					Global.Config.NES_InQuickNES = true;
+				}
+				else if (Global.MovieSession.Movie.Core == neshawkName)
+				{
+					Global.Config.NES_InQuickNES = false;
+				}
+			}
+			else if (!record && Global.Emulator.SystemId == "SNES") // ditto with snes9x vs bsnes
+			{
+				var snes9xName = ((CoreAttributes)Attribute.GetCustomAttribute(typeof(Snes9x), typeof(CoreAttributes))).CoreName;
+				var bsnesName = ((CoreAttributes)Attribute.GetCustomAttribute(typeof(LibsnesCore), typeof(CoreAttributes))).CoreName;
+
+				if (Global.MovieSession.Movie.Core == snes9xName)
+				{
+					Global.Config.SNES_InSnes9x = true;
+				}
+				else
+				{
+					Global.Config.SNES_InSnes9x = false;
+				}
+			}
+
+			var s = Global.MovieSession.Movie.SyncSettingsJson;
+			if (!string.IsNullOrWhiteSpace(s))
+			{
+				SyncSettingsHack = ConfigService.LoadWithType(s);
+			}
+
+			if (record) // This is a hack really, we need to set the movie to its propert state so that it will be considered active later
+			{
+				movie.SwitchToRecord();
+			}
+			else
+			{
+				movie.SwitchToPlay();
+			}
+
+			QueuedMovie = movie;
+		}
 	}
 }

BizHawk.Client.EmuHawk/MainForm.Events.cs

@@ -300,11 +300,6 @@ namespace BizHawk.Client.EmuHawk
 
 		private void CloseRomMenuItem_Click(object sender, EventArgs e)
 		{
-			if (Global.MovieSession.Movie.IsActive)
-			{
-				Global.MovieSession.Movie.Stop();
-			}
-
 			CloseRom();
 		}
 
@@ -2377,7 +2372,7 @@ namespace BizHawk.Client.EmuHawk
 				}
 				else
 				{
-					StopMovieThenLoadRom(CurrentlyOpenRom);
+					LoadRom(CurrentlyOpenRom);
 				}
 
 				string errorMsg;
@@ -2399,7 +2394,7 @@ namespace BizHawk.Client.EmuHawk
 			}
 			else
 			{
-				StopMovieThenLoadRom(filePaths[0]);
+				LoadRom(filePaths[0]);
 			}
 		}
 

BizHawk.Client.EmuHawk/MainForm.Movie.cs

@@ -14,116 +14,27 @@ namespace BizHawk.Client.EmuHawk
 	{
 		public void StartNewMovie(IMovie movie, bool record)
 		{
-			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
+			Global.MovieSession.QueueNewMovie(movie, record);
-			{
-				movie.Load();
-			}
 
-			if (movie.SystemID != Global.Emulator.SystemId)
+			LoadRom(GlobalWin.MainForm.CurrentlyOpenRom);
-			{
-				GlobalWin.OSD.AddMessage("Movie does not match the currently loaded system, unable to load");
-				return;
-			}
-
-			//If a movie is already loaded, save it before starting a new movie
-			if (Global.MovieSession.Movie.IsActive && !string.IsNullOrEmpty(Global.MovieSession.Movie.Filename))
-			{
-				Global.MovieSession.Movie.Save();
-			}
-
-			Global.MovieSession = new MovieSession
-			{
-				Movie = movie,
-				MovieControllerAdapter = movie.LogGeneratorInstance().MovieControllerAdapter,
-				MessageCallback = GlobalWin.OSD.AddMessage,
-				AskYesNoCallback = StateErrorAskUser,
-				PauseCallback = PauseEmulator,
-				ModeChangedCallback = SetMainformMovieInfo
-			};
-
-			InputManager.RewireInputChain();
-
-			if (!record)
-			{
-				Global.MovieSession.MovieLoad(); // TODO this loads it a 2nd time, ugh
-			}
-
-			try
-			{
-				if (!record && Global.Emulator.SystemId == "NES") // For NES we need special logic since the movie will drive which core to load
-				{
-					var quicknesName = ((CoreAttributes)Attribute.GetCustomAttribute(typeof(QuickNES), typeof(CoreAttributes))).CoreName;
-					var neshawkName = ((CoreAttributes)Attribute.GetCustomAttribute(typeof(NES), typeof(CoreAttributes))).CoreName;
-
-					// If either is specified use that, else use whatever is currently set
-					if (Global.MovieSession.Movie.Core == quicknesName)
-					{
-						Global.Config.NES_InQuickNES = true;
-					}
-					else if (Global.MovieSession.Movie.Core == neshawkName)
-					{
-						Global.Config.NES_InQuickNES = false;
-					}
-				}
-				else if (!record && Global.Emulator.SystemId == "SNES") // ditto with snes9x vs bsnes
-				{
-					var snes9xName = ((CoreAttributes)Attribute.GetCustomAttribute(typeof(Snes9x), typeof(CoreAttributes))).CoreName;
-					var bsnesName = ((CoreAttributes)Attribute.GetCustomAttribute(typeof(LibsnesCore), typeof(CoreAttributes))).CoreName;
-
-					if (Global.MovieSession.Movie.Core == snes9xName)
-					{
-						Global.Config.SNES_InSnes9x = true;
-					}
-					else
-					{
-						Global.Config.SNES_InSnes9x = false;
-					}
-				}
-
-				var s = Global.MovieSession.Movie.SyncSettingsJson;
-				if (!string.IsNullOrWhiteSpace(s))
-				{
-					_syncSettingsHack = ConfigService.LoadWithType(s);
-				}
-
-				if (record) // This is a hack really, we need to set the movie to its propert state so that it will be considered active later
-				{
-					Global.MovieSession.Movie.SwitchToRecord();
-				}
-				else
-				{
-					Global.MovieSession.Movie.SwitchToRecord();
-				}
-
-				LoadRom(GlobalWin.MainForm.CurrentlyOpenRom);
-			}
-			finally
-			{
-				// ensure subsequent calls to LoadRom won't get the settings object created here
-				this._syncSettingsHack = null;
-			}
 
 			Global.Config.RecentMovies.Add(movie.Filename);
 
 			if (Global.MovieSession.Movie.StartsFromSavestate)
 			{
 				if (Global.MovieSession.Movie.TextSavestate != null)
+				{
 					Global.Emulator.LoadStateText(new StringReader(Global.MovieSession.Movie.TextSavestate));
+				}
 				else
+				{
 					Global.Emulator.LoadStateBinary(new BinaryReader(new MemoryStream(Global.MovieSession.Movie.BinarySavestate, false)));
+				}
 
 				Global.Emulator.ResetCounters();
 			}
 
-			if (record)
+			Global.MovieSession.RunQueuedMovie(record);
-			{
-				Global.MovieSession.Movie.StartNewRecording();
-				Global.MovieSession.ReadOnly = false;
-			}
-			else
-			{
-				Global.MovieSession.Movie.StartNewPlayback();
-			}
 
 			SetMainformMovieInfo();
 			UpdateStatusSlots();
@@ -156,11 +67,12 @@ namespace BizHawk.Client.EmuHawk
 			SetWindowText();
 		}
 
+		// Movie Refactor TODO: this needs to be considered, and adapated to the queue system
 		public void RestartMovie()
 		{
 			if (Global.MovieSession.Movie.IsActive)
 			{
-				StopMovieThenLoadRom(CurrentlyOpenRom);
+				LoadRom(CurrentlyOpenRom);
 				if (Global.MovieSession.Movie.StartsFromSavestate)
 				{
 					// TODO: why does this code exist twice??

BizHawk.Client.EmuHawk/MainForm.cs

@@ -262,7 +262,7 @@ namespace BizHawk.Client.EmuHawk
 			if (cmdRom != null)
 			{
 				// Commandline should always override auto-load
-				StopMovieThenLoadRom(cmdRom);
+				LoadRom(cmdRom);
 				if (Global.Game == null)
 				{
 					MessageBox.Show("Failed to load " + cmdRom + " specified on commandline");
@@ -818,7 +818,7 @@ namespace BizHawk.Client.EmuHawk
 
 		public void RebootCore()
 		{
-			StopMovieThenLoadRom(CurrentlyOpenRom);
+			LoadRom(CurrentlyOpenRom);
 		}
 
 		public void PauseEmulator()
@@ -1208,8 +1208,6 @@ namespace BizHawk.Client.EmuHawk
 		// Resources
 		Bitmap StatusBarDiskLightOnImage, StatusBarDiskLightOffImage;
 
-		private object _syncSettingsHack;
-
 		#endregion
 
 		#region Private methods
@@ -1569,7 +1567,7 @@ namespace BizHawk.Client.EmuHawk
 
 		private void LoadRomFromRecent(string rom)
 		{
-			if (!StopMovieThenLoadRom(rom))
+			if (!LoadRom(rom))
 			{
 				Global.Config.RecentRoms.HandleLoadError(rom);
 			}
@@ -1833,7 +1831,7 @@ namespace BizHawk.Client.EmuHawk
 			var file = new FileInfo(ofd.FileName);
 			Global.Config.LastRomPath = file.DirectoryName;
 			_lastOpenRomFilter = ofd.FilterIndex;
-			StopMovieThenLoadRom(file.FullName);
+			LoadRom(file.FullName);
 		}
 
 		private void CoreSyncSettings(object sender, RomLoader.SettingsLoadArgs e)
@@ -1842,7 +1840,7 @@ namespace BizHawk.Client.EmuHawk
 			// A movie is loaded, then load rom is called, which closes the current rom which closes the current movie (which is the movie just loaded)
 			// As such the movie is "inactive".  So instead we load the movie and populate the _syncSettingsHack
 			// Then let the rom logic work its magic, then use it here, as such it will be null unless a movie invoked the load rom call
-			e.Settings = _syncSettingsHack ?? Global.Config.GetCoreSyncSettings(e.Core);
+			e.Settings = Global.MovieSession.SyncSettingsHack ?? Global.Config.GetCoreSyncSettings(e.Core);
 		}
 
 		private static void CoreSettings(object sender, RomLoader.SettingsLoadArgs e)
@@ -3024,6 +3022,8 @@ namespace BizHawk.Client.EmuHawk
 
 		private void ShowLoadError(object sender, RomLoader.RomErrorArgs e)
 		{
+			Global.MovieSession.SyncSettingsHack = null; // ensure subsequent calls to LoadRom won't get the settings object created here
+
 			if (e.Type == RomLoader.LoadErrorType.MissingFirmware)
 			{
 				var result = MessageBox.Show(
@@ -3064,17 +3064,6 @@ namespace BizHawk.Client.EmuHawk
 			return platformChooser.PlatformChoice;
 		}
 
-		// TODO: a better name for this method, but this is the one that should be called, in general
-		public bool StopMovieThenLoadRom(string path, bool? deterministicemulation = null)
-		{
-			if (Global.MovieSession.Movie.IsActive)
-			{
-				Global.MovieSession.Movie.Stop();
-			}
-
-			return LoadRom(path, deterministicemulation);
-		}
-
 		// Still needs a good bit of refactoring
 		public bool LoadRom(string path, bool? deterministicemulation = null)
 		{
@@ -3097,7 +3086,6 @@ namespace BizHawk.Client.EmuHawk
 				};
 
 			loader.OnLoadError += ShowLoadError;
-
 			loader.OnLoadSettings += CoreSettings;
 			loader.OnLoadSyncSettings += CoreSyncSettings;
 
@@ -3106,8 +3094,6 @@ namespace BizHawk.Client.EmuHawk
 			// the new settings objects
 			CommitCoreSettingsToConfig(); // adelikat: I Think by reordering things, this isn't necessary anymore
 			CloseGame();
-			
-			//Global.Emulator.Dispose(); // CloseGame() already killed and disposed the emulator; this is killing the new one; that's bad
 
 			var nextComm = CreateCoreComm();
 			CoreFileProvider.SyncCoreCommInputSignals(nextComm);
@@ -3116,12 +3102,9 @@ namespace BizHawk.Client.EmuHawk
 
 			if (result)
 			{
-				if (loader.LoadedEmulator is TI83)
+				if (loader.LoadedEmulator is TI83 && Global.Config.TI83autoloadKeyPad)
 				{
-					if (Global.Config.TI83autoloadKeyPad)
+					GlobalWin.Tools.Load<TI83KeyPad>();
-					{
-						GlobalWin.Tools.Load<TI83KeyPad>();
-					}
 				}
 
 				Global.Emulator = loader.LoadedEmulator;
@@ -3153,8 +3136,6 @@ namespace BizHawk.Client.EmuHawk
 					}
 				}
 
-				SetWindowText();
-
 				Global.Rewinder.ResetRewindBuffer();
 
 				if (Global.Emulator.CoreComm.RomStatusDetails == null && loader.Rom != null)
@@ -3180,7 +3161,9 @@ namespace BizHawk.Client.EmuHawk
 
 				Global.Config.RecentRoms.Add(loader.CanonicalFullPath);
 				JumpLists.AddRecentItem(loader.CanonicalFullPath);
-				if (File.Exists(PathManager.SaveRamPath(loader.Game)))
+
+				// Don't load Save Ram if a movie is being loaded
+				if (!Global.MovieSession.MovieIsQueued && File.Exists(PathManager.SaveRamPath(loader.Game)))
 				{
 					LoadSaveRam();
 				}
@@ -3195,6 +3178,7 @@ namespace BizHawk.Client.EmuHawk
 					}
 				}
 
+				SetWindowText();
 				CurrentlyOpenRom = loader.CanonicalFullPath;
 				HandlePlatformMenus();
 				_stateSlots.Clear();
@@ -3303,6 +3287,11 @@ namespace BizHawk.Client.EmuHawk
 			StopAv();
 
 			CommitCoreSettingsToConfig();
+			if (Global.MovieSession.Movie.IsActive) // Note: this must be called after CommitCoreSettingsToConfig()
+			{
+				StopMovie(true);
+			}
+
 
 			Global.Emulator.Dispose();
 			Global.CoreComm = CreateCoreComm();

BizHawk.Client.EmuHawk/Program.cs

@@ -241,7 +241,7 @@ namespace BizHawk.Client.EmuHawk
 
 			void this_StartupNextInstance(object sender, StartupNextInstanceEventArgs e)
 			{
-				(MainForm as MainForm).StopMovieThenLoadRom(e.CommandLine[0]);
+				(MainForm as MainForm).LoadRom(e.CommandLine[0]);
 			}
 
 			protected override void OnCreateMainForm()

BizHawk.Client.EmuHawk/tools/GB/DualGBXMLCreator.cs

@@ -169,7 +169,7 @@ namespace BizHawk.Client.EmuHawk
 
 				DialogResult = DialogResult.OK;
 				Close();
-				GlobalWin.MainForm.StopMovieThenLoadRom(textBoxOutputDir.Text);
+				GlobalWin.MainForm.LoadRom(textBoxOutputDir.Text);
 			}
 		}
 

BizHawk.Client.EmuHawk/tools/Lua/Libraries/EmuLuaLibrary.Client.cs

@@ -140,7 +140,7 @@ namespace BizHawk.Client.EmuHawk
 		)]
 		public static void OpenRom(string path)
 		{
-			GlobalWin.MainForm.StopMovieThenLoadRom(path);
+			GlobalWin.MainForm.LoadRom(path);
 		}
 
 		[LuaMethodAttributes(