make reserving states on branch frames optional

src/BizHawk.Client.Common/movie/interfaces/ITasMovie.cs

@@ -20,6 +20,8 @@ public interface ITasMovie : IMovie, IDisposable
 		int LastEditedFrame { get; }
 		bool LastEditWasRecording { get; }
 
+		bool ReserveBranchFrames { get; set; }
+
 		/// <summary>
 		/// Called whenever the movie is modified in a way that could invalidate savestates in the movie's state history.
 		/// Called regardless of whether any states were actually invalidated.

src/BizHawk.Client.Common/movie/tasproj/IStateManager.cs

@@ -6,14 +6,31 @@ namespace BizHawk.Client.Common
 {
 	public interface IStateManager : IDisposable
 	{
+		public enum CaptureType
+		{
+			/// <summary>
+			/// Let the manager decide if capturing is appropriate and when the state should be removed.
+			/// </summary>
+			Normal,
+
+			/// <summary>
+			/// Force the state to be captured, but allow this state to be removed as soon as the next <see cref="LastEditedFrame"/> capture happens.
+			/// </summary>
+			LastEditedFrame,
+
+			/// <summary>
+			/// Force the state to be captured as a reserved state.
+			/// </summary>
+			Reserve,
+		}
+
 		IStateManagerSettings Settings { get; }
 
 		/// <summary>
 		/// Requests that the current emulator state be captured
-		/// Unless force is true, the state may or may not be captured depending on the logic employed by "green-zone" management
+		/// With <see cref="CaptureType.Normal"/>, the state may or may not be captured depending on the logic employed by "green-zone" management
 		/// </summary>
-		/// <param name="force">If true, the state will be temporarily captured. If it would not have otherwise been captured, it may be deleted as soon as another state is force captured.</param>
-		void Capture(int frame, IStatable source, bool force = false);
+		void Capture(int frame, IStatable source, CaptureType type = CaptureType.Normal);
 
 		/// <summary>
 		/// Tell the state manager we no longer wish to reserve the state for the given frame.

src/BizHawk.Client.Common/movie/tasproj/TasBranch.cs

@@ -87,8 +87,7 @@ public void Replace(TasBranch old, TasBranch newBranch)
 			newBranch.Uuid = old.Uuid;
 			if (newBranch.UserText.Length is 0) newBranch.UserText = old.UserText;
 			this[index] = newBranch;
-			if (!_movie.IsReserved(old.Frame))
-				_movie.TasStateManager.Unreserve(old.Frame);
+			DoUnreserve(old);
 
 			_movie.FlagChanges();
 		}
@@ -121,8 +120,7 @@ public void Replace(TasBranch old, TasBranch newBranch)
 			var result = base.Remove(item);
 			if (result)
 			{
-				if (!_movie.IsReserved(item!.Frame))
-					_movie.TasStateManager.Unreserve(item.Frame);
+				DoUnreserve(item);
 
 				_movie.FlagChanges();
 			}
@@ -277,6 +275,16 @@ public void Load(ZipStateLoader bl, ITasMovie movie)
 				nusertext.Increment();
 			}
 		}
+
+		private void DoUnreserve(TasBranch branch)
+		{
+			// The regularly reserved frame
+			if (!_movie.IsReserved(branch.Frame - 1))
+				_movie.TasStateManager.Unreserve(branch.Frame - 1);
+			// The potentially semi-reserved frame from when a branch was loaded
+			if (!_movie.IsReserved(branch.Frame))
+				_movie.TasStateManager.Unreserve(branch.Frame);
+		}
 	}
 
 	public static class TasBranchExtensions

src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs

@@ -84,6 +84,26 @@ public override bool StartsFromSavestate
 
 		public Action<int> GreenzoneInvalidated { get; set; }
 
+		private bool _reserveBranchFrames;
+		public bool ReserveBranchFrames
+		{
+			get => _reserveBranchFrames;
+			set
+			{
+				_reserveBranchFrames = value;
+				if (!value)
+				{
+					foreach (TasBranch branch in Branches)
+					{
+						if (!IsReserved(branch.Frame - 1))
+						{
+							TasStateManager.Unreserve(branch.Frame - 1);
+						}
+					}
+				}
+			}
+		}
+
 		public ITasMovieRecord this[int index]
 		{
 			get
@@ -191,7 +211,10 @@ public void GreenzoneCurrentFrame()
 			LagLog[Emulator.Frame] = _inputPollable.IsLagFrame;
 
 			// We will forcibly capture a state for the last edited frame (requested by https://github.com/TASEmulators/BizHawk/issues/916 for case of "platforms with analog stick")
-			TasStateManager.Capture(Emulator.Frame, Emulator.AsStatable(), Emulator.Frame == LastEditedFrame - 1);
+			TasStateManager.Capture(
+				Emulator.Frame,
+				Emulator.AsStatable(),
+				Emulator.Frame == LastEditedFrame - 1 ? IStateManager.CaptureType.LastEditedFrame : IStateManager.CaptureType.Normal);
 		}
 
 
@@ -327,7 +350,7 @@ public bool IsReserved(int frame)
 			// because we always navigate to the frame before and emulate 1 frame so that we ensure a proper frame buffer on the screen
 			// users want instant navigation to markers, so to do this, we need to reserve the frame before the marker, not the marker itself
 			return Markers.Exists(m => m.WantsState && m.Frame - 1 == frame)
-				|| Branches.Any(b => b.Frame == frame); // Branches should already be in the reserved list, but it doesn't hurt to check
+				|| (ReserveBranchFrames && Branches.Any(b => b.Frame - 1 == frame));
 		}
 
 		public void Dispose()

src/BizHawk.Client.Common/movie/tasproj/ZwinderStateManager.cs

@@ -355,15 +355,15 @@ private bool ShouldKeepForAncient(int frame)
 			return nextState - previousState > _ancientInterval;
 		}
 
-		public void Capture(int frame, IStatable source, bool force = false)
+		public void Capture(int frame, IStatable source, IStateManager.CaptureType type = IStateManager.CaptureType.Normal)
 		{
 			// We already have this state, no need to capture
 			if (StateCache.Contains(frame))
 			{
 				return;
 			}
 
-			if (_reserveCallback(frame))
+			if (type == IStateManager.CaptureType.Reserve || _reserveCallback(frame))
 			{
 				CaptureReserved(frame, source);
 				return;
@@ -376,7 +376,7 @@ public void Capture(int frame, IStatable source, bool force = false)
 			}
 
 			// We use the gap buffer for forced capture to avoid crowding the "current" buffer and thus reducing it's actual span of covered frames.
-			if (NeedsGap(frame) || force)
+			if (NeedsGap(frame) || type == IStateManager.CaptureType.LastEditedFrame)
 			{
 				CaptureGap(frame, source);
 				return;

src/BizHawk.Client.Common/tools/TAStudio/PagedStateManager.cs

@@ -161,6 +161,8 @@ public Stream MakeReadStream(PagedStateManager manager)
 		private readonly SortedSet<StateInfo> _midStates = new();
 		private readonly SortedSet<StateInfo> _newStates = new();
 
+		private readonly Queue<StateInfo> _statesPendingRemoval = new();
+
 		private readonly Func<int, bool> _reserveCallback;
 
 		private bool _bufferIsFull = false;
@@ -350,6 +352,16 @@ private int FreePage(int frame)
 
 			_bufferIsFull = true;
 
+			while (_statesPendingRemoval.Count > 0)
+			{
+				StateInfo si = _statesPendingRemoval.Dequeue();
+				if (_states.Contains(si))
+				{
+					RemoveState(si);
+					return si.FirstPage;
+				}
+			}
+
 			while (true)
 			{
 				// A very special case: We have no mid or new states.
@@ -468,25 +480,25 @@ private void InternalCapture(int frame, IStatable source, StateGroup destination
 			}
 		}
 
-		public void Capture(int frame, IStatable source, bool force = false)
+		public void Capture(int frame, IStatable source, IStateManager.CaptureType type = IStateManager.CaptureType.Normal)
 		{
 			Debug.Assert(_states.Contains(new(0)), "State manager cannot be used until engaged.");
 
 			if (HasState(frame)) return;
 
-			if (force)
+			if (type == IStateManager.CaptureType.LastEditedFrame)
 			{
 				if (HasState(_lastForceCapture))
 				{
 					StateInfo state = _states.GetViewBetween(new(_lastForceCapture), new(_lastForceCapture)).Min;
 					if (!_reserveCallback(_lastForceCapture) && !ShouldKeepForOld(_lastForceCapture))
-						RemoveState(state);
+						_statesPendingRemoval.Enqueue(state);
 				}
 				_lastForceCapture = -1; // This will get set if the frame is actually captured because of force.
 			}
 
 			StateGroup group = StateGroup.None;
-			if (_reserveCallback(frame))
+			if (type == IStateManager.CaptureType.Reserve || _reserveCallback(frame))
 			{
 				group = StateGroup.Old;
 			}
@@ -542,7 +554,7 @@ public void Capture(int frame, IStatable source, bool force = false)
 
 			if (group != StateGroup.None)
 				InternalCapture(frame, source, group);
-			else if (force)
+			else if (type == IStateManager.CaptureType.LastEditedFrame)
 			{
 				_lastForceCapture = frame;
 				InternalCapture(frame, source, StateGroup.Old);
@@ -608,7 +620,7 @@ public void Unreserve(int frame)
 			// Remove the state if it's an old state we don't need.
 			if (!_newStates.Contains(state) && !_midStates.Contains(state) && !ShouldKeepForOld(frame))
 			{
-				RemoveState(state);
+				_statesPendingRemoval.Enqueue(state);
 			}
 		}
 

src/BizHawk.Client.EmuHawk/tools/TAStudio/BookmarksBranchesBox.cs

@@ -172,7 +172,6 @@ public void Branch()
 			BranchView.RowCount = Branches.Count;
 			Branches.Current = Branches.Count - 1;
 			Movie.TasSession.UpdateValues(Tastudio.Emulator.Frame, Branches.Current);
-			Movie.TasStateManager.Capture(Tastudio.Emulator.Frame, new BufferedStatable(branch.CoreData));
 			BranchView.ScrollToIndex(Branches.Current);
 			BranchView.DeselectAll();
 			Select(Branches.Current, true);
@@ -281,7 +280,6 @@ private void UpdateBranchToolStripMenuItem_Click(object sender, EventArgs e)
 			BranchView.ScrollToIndex(Branches.Current);
 			var branch = CreateBranch();
 			Branches.Replace(SelectedBranch, branch);
-			Movie.TasStateManager.Capture(Tastudio.Emulator.Frame, new BufferedStatable(branch.CoreData));
 			Tastudio.RefreshDialog();
 			Tastudio.BranchSavedCallback?.Invoke(Branches.Current);
 			Tastudio.MainForm.AddOnScreenMessage($"Saved branch {Branches.Current + 1}");

src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.ListView.cs

@@ -1089,12 +1089,12 @@ public bool FrameEdited(int frame)
 				{
 					// In this case our regular capture logic won't get the chance
 					// to do a force capture for this edited frame. So do it here.
-					CurrentTasMovie.TasStateManager.Capture(Emulator.Frame, Emulator.AsStatable(), true);
+					CurrentTasMovie.TasStateManager.Capture(Emulator.Frame, Emulator.AsStatable(), IStateManager.CaptureType.LastEditedFrame);
 				}
 				else if (!CurrentTasMovie.TasStateManager.HasState(frame - 1) && Emulator.Frame == frame)
 				{
 					// A less-than-ideal frame to be captured, but still useful for autorestore.
-					CurrentTasMovie.TasStateManager.Capture(Emulator.Frame, Emulator.AsStatable(), true);
+					CurrentTasMovie.TasStateManager.Capture(Emulator.Frame, Emulator.AsStatable(), IStateManager.CaptureType.LastEditedFrame);
 				}
 				_batchEditMinFrame = -1;
 			}

src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.MenuItems.cs

@@ -1098,6 +1098,8 @@ private void TAStudioSettingsToolStripMenuItem_Click(object sender, EventArgs e)
 						_inputRolls[i].ScrollSpeed = Settings.ScrollSpeed;
 					}
 
+					CurrentTasMovie.ReserveBranchFrames = Settings.StateOnBranchFrame;
+
 					UpdateAutoFire();
 
 					if (CurrentTasMovie.TasStateManager.Settings != s.CurrentStateManagerSettings)

src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.cs

@@ -105,6 +105,7 @@ public TAStudioSettings()
 				LoadBranchOnDoubleClick = true;
 				CopyIncludesFrameNo = false;
 				AutoadjustInput = false;
+				StateOnBranchFrame = false;
 
 				// default to taseditor fashion
 				DenoteStatesWithIcons = false;
@@ -144,6 +145,7 @@ public TAStudioSettings()
 			public int RewindStepFast { get; set; } = 4;
 			public bool ScrollSync { get; set; } = true;
 			public bool StatesForMarkers { get; set; } = true;
+			public bool StateOnBranchFrame { get; set; }
 			public PatternPaintModeEnum PatternPaintMode { get; set; } = TAStudioSettings.PatternPaintModeEnum.Never;
 			public PatternSelectionEnum PatternSelection { get; set; } = TAStudioSettings.PatternSelectionEnum.Hold;
 			public Font TasViewFont { get; set; } = new Font("Arial", 8.25F, FontStyle.Bold, GraphicsUnit.Point, 0);
@@ -711,6 +713,7 @@ private bool StartNewMovieWrapper(ITasMovie movie, bool isNew)
 			movie.BindMarkersToInput = Settings.BindMarkersToInput;
 			movie.GreenzoneInvalidated = (f) => _ = FrameEdited(f);
 			movie.ChangeLog.MaxSteps = Settings.MaxUndoSteps;
+			movie.ReserveBranchFrames = Settings.StateOnBranchFrame;
 
 			movie.ChangesChanged += TasMovie_OnChangesChanged;
 			System.Collections.Specialized.NotifyCollectionChangedEventHandler refreshOnMarker = (_, _) => RefreshDialog();
@@ -1335,7 +1338,11 @@ public void LoadBranch(TasBranch branch)
 			_suspendEditLogic = false;
 			LoadState(new(branch.Frame, new MemoryStream(branch.CoreData, false)), CurrentTasMovie.Branches.IndexOf(branch));
 
-			CurrentTasMovie.TasStateManager.Capture(Emulator.Frame, Emulator.AsStatable());
+			// We capture this state to ensure edits at or after the branch frame will not require seeking from prior greenzone
+			// (which might be very far back). Zwinder manager requires it be reserved since it cannot capture out of order.
+			// There may be benefits to reserve capture with Paged manager too.
+			// (Note: IsReserved will say not reserved. We won't need it reserved on re-greenzoning, but we still want to capture it here.)
+			CurrentTasMovie.TasStateManager.Capture(Emulator.Frame, Emulator.AsStatable(), IStateManager.CaptureType.Reserve);
 			QuickBmpFile.Copy(new BitmapBufferVideoProvider(branch.CoreFrameBuffer), VideoProvider);
 
 			if (Settings.OldControlSchemeForBranches && TasPlaybackBox.RecordingMode)

src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudioSettingsForm.cs

@@ -102,6 +102,7 @@ private void TAStudioSettingsForm_Load(object sender, EventArgs e)
 			FastRewindNum.Value = _settings.GeneralClientSettings.RewindStepFast;
 			ScrollSpeedNum.Value = _settings.GeneralClientSettings.ScrollSpeed;
 			StatesForMarkersCheckbox.Checked = _settings.GeneralClientSettings.StatesForMarkers;
+			StateOnBranchFrameCheckbox.Checked = _settings.GeneralClientSettings.StateOnBranchFrame;
 
 			// patterns
 			foreach (var button in _controllerDef.BoolButtons)
@@ -504,6 +505,7 @@ private void ApplyButton_Click(object sender, EventArgs e)
 			_settings.GeneralClientSettings.RewindStep = (int)RewindNum.Value;
 			_settings.GeneralClientSettings.RewindStepFast = (int)FastRewindNum.Value;
 			_settings.GeneralClientSettings.StatesForMarkers = StatesForMarkersCheckbox.Checked;
+			_settings.GeneralClientSettings.StateOnBranchFrame = StateOnBranchFrameCheckbox.Checked;
 
 			if (ScrollToViewRadio.Checked) _settings.GeneralClientSettings.FollowCursorScrollMethod = "near";
 			else if (ScrollToTopRadio.Checked) _settings.GeneralClientSettings.FollowCursorScrollMethod = "top";

src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudioSettingsForm.resx

@@ -131,6 +131,11 @@ Note: The savestate needs to be on the frame before the marker. Thus, making
 a marker on the current frame will not automatically create a state.
 (The reason for the state being on the frame before is so that TAStudio
 does not need to store a screenshot with the savestate.)</value>
+  </data>
+  <data name="StateOnBranchFrameCheckbox.ToolTip" xml:space="preserve">
+    <value>When enabled, TAStudio will keep savestates for each branch the same way
+it does for markers, allowing you to quickly seek to a branch frame.
+This is separate from the savestate that's loaded when you load a branch.</value>
   </data>
   <data name="EditInvisibleColumnsCheckbox.ToolTip" xml:space="preserve">
     <value>When disabled, inputs that are not visible on the active input roll cannot be edited.

src/BizHawk.Tests.Client.Common/Movie/FakeStateManager.cs

@@ -14,7 +14,7 @@ internal class FakeStateManager : IStateManager
 
 		public int Last => throw new NotImplementedException();
 
-		public void Capture(int frame, IStatable source, bool force) => throw new NotImplementedException();
+		public void Capture(int frame, IStatable source, IStateManager.CaptureType force = IStateManager.CaptureType.Normal) => throw new NotImplementedException();
 		public void Clear() => throw new NotImplementedException();
 		public void Dispose() => throw new NotImplementedException();
 		public void Engage(byte[] frameZeroState) { /* nothing */ }