Revert a series of questionable zwinder changes

These all seem to have been made from the point of view of testing every possible value the UI allowed, and then fixing them all with as little effort as possible, with the fixes going deep in core logic instead of in the validation layer, and the fixes not making anything really "work"; just making it stop complaining.

ZWinderBuffers now require TargetFrameLength >= 1 again: A value of zero doesn't make much sense here, and didn't actually behave differently than 1.
ZWinderBuffers now require Size > 0 again: A size 0 buffer will never capture anything and has no value.  If you don't want a buffer, don't make one at all.  I believe that omitting some buffers might make sense for the state manager, maybe; those more familiar with all of its uses will have to chime in.  If that is the case, then the state manager should not instantiate buffers it does not plan to use.
ZWinderBuffers now throw exceptions again when a single state is bigger than the entire buffer:  If you're in this situation, things are phenomenally broken and your buffer is never going to capture anything successfully.  Users need to fix their settings in this case; they don't need to have a completely non functional system silently claim to work while not doing anything, leaving them befuddled as to why seeking is taking forever.

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

@@ -281,10 +281,11 @@ namespace BizHawk.Client.Common
 				return;
 			}
 
-			bool currentCaptured =_current.Capture(frame,
+			_current.Capture(frame,
 				s =>
 				{
 					source.SaveStateBinary(new BinaryWriter(s));
+					StateCache.Add(frame);
 				},
 				index =>
 				{
@@ -298,10 +299,11 @@ namespace BizHawk.Client.Common
 						return;
 					}
 
-					bool recentCaptured = _recent.Capture(state.Frame,
+					_recent.Capture(state.Frame,
 						s =>
 						{
 							state.GetReadStream().CopyTo(s);
+							StateCache.Add(state.Frame);
 						},
 						index2 => 
 						{
@@ -316,12 +318,8 @@ namespace BizHawk.Client.Common
 								AddToReserved(state2);
 							}
 						});
-					if (recentCaptured)
-						StateCache.Add(state.Frame);
 				},
 				force);
-			if (currentCaptured)
-				StateCache.Add(frame);
 		}
 
 		// Returns whether or not a frame has a reserved state within the frame interval on either side of it
@@ -358,14 +356,13 @@ namespace BizHawk.Client.Common
 
 		private void CaptureGap(int frame, IStatable source)
 		{
-			bool captured = _gapFiller.Capture(
+			_gapFiller.Capture(
 				frame, s =>
 				{
+					StateCache.Add(frame);
 					source.SaveStateBinary(new BinaryWriter(s));
 				},
 				index => StateCache.Remove(index));
-			if (captured)
-				StateCache.Add(frame);
 		}
 
 		public void Clear()

src/BizHawk.Client.Common/rewind/ZwinderBuffer.cs

@@ -20,22 +20,23 @@ namespace BizHawk.Client.Common
 		public ZwinderBuffer(IRewindSettings settings)
 		{
 			long targetSize = settings.BufferSize * 1024 * 1024;
+			if (settings.TargetFrameLength < 1)
+			{
+				throw new ArgumentOutOfRangeException(nameof(settings.TargetFrameLength));
+			}
 
 			Size = 1L << (int)Math.Floor(Math.Log(targetSize, 2));
 			_sizeMask = Size - 1;
+			_buffer = new MemoryBlock((ulong)Size);
+			_buffer.Protect(_buffer.Start, _buffer.Size, MemoryBlock.Protection.RW);
 			_targetFrameLength = settings.TargetFrameLength;
+			_states = new StateInfo[STATEMASK + 1];
 			_useCompression = settings.UseCompression;
-			if (Size > 1)
-			{
-				_buffer = new MemoryBlock((ulong)Size);
-				_buffer.Protect(_buffer.Start, _buffer.Size, MemoryBlock.Protection.RW);
-				_states = new StateInfo[STATEMASK + 1];
-			}
 		}
 
 		public void Dispose()
 		{
-			_buffer?.Dispose();
+			_buffer.Dispose();
 		}
 
 
@@ -134,16 +135,15 @@ namespace BizHawk.Client.Common
 		/// Maybe captures a state, if the conditions are favorable
 		/// </summary>
 		/// <param name="frame">frame number to capture</param>
-		/// <param name="callback">will be called with the stream if capture is to be attempted</param>
+		/// <param name="callback">will be called with the stream if capture is to be performed</param>
 		/// <param name="indexInvalidated">
-		/// If provided, will be called with the index of states that are about to be removed. This will happen during
+		/// If provided, will be called with the index of states that are about to be removed.  This will happen during
 		/// calls to Write() inside `callback`, and any reuse of the old state will have to happen immediately
 		/// </param>
-		/// <returns>Returns true if the state was sucesfully captured; otherwise false.</returns>
-		public bool Capture(int frame, Action<Stream> callback, Action<int> indexInvalidated = null, bool force = false)
+		public void Capture(int frame, Action<Stream> callback, Action<int> indexInvalidated = null, bool force = false)
 		{
-			if ((!force && !ShouldCapture(frame)) || _buffer == null)
-				return false;
+			if (!force && !ShouldCapture(frame))
+				return;
 
 			if (Count == STATEMASK)
 			{
@@ -158,7 +158,7 @@ namespace BizHawk.Client.Common
 			Func<long> notifySizeReached = () =>
 			{
 				if (Count == 0)
-					return 0;
+					throw new IOException("A single state must not be larger than the buffer");
 				indexInvalidated?.Invoke(0);
 				_firstStateIndex = (_firstStateIndex + 1) & STATEMASK;
 				return Count > 0
@@ -177,19 +177,12 @@ namespace BizHawk.Client.Common
 				callback(stream);
 			}
 
-			if (stream.Length > Size)
-			{
-				Util.DebugWriteLine("Failed to capture; state size exceeds buffer size.");
-				return false;
-			}
-
 			_states[_nextStateIndex].Frame = frame;
 			_states[_nextStateIndex].Start = start;
 			_states[_nextStateIndex].Size = (int)stream.Length;
 			_nextStateIndex = (_nextStateIndex + 1) & STATEMASK;
 
 			//Util.DebugWriteLine($"Size: {Size >> 20}MiB, Used: {Used >> 20}MiB, States: {Count}");
-			return true;
 		}
 
 		private Stream MakeLoadStream(int index)
@@ -293,11 +286,8 @@ namespace BizHawk.Client.Common
 				nextByte += _states[i].Size;
 			}
 			// TODO: Use spans to avoid this extra copy in .net core
-			if (nextByte > 0)
-			{
-				var dest = _buffer.GetStream(_buffer.Start, (ulong)nextByte, true);
-				WaterboxUtils.CopySome(reader.BaseStream, dest, nextByte);
-			}
+			var dest = _buffer.GetStream(_buffer.Start, (ulong)nextByte, true);
+			WaterboxUtils.CopySome(reader.BaseStream, dest, nextByte);
 		}
 
 		public static ZwinderBuffer Create(BinaryReader reader)
@@ -376,16 +366,7 @@ namespace BizHawk.Client.Common
 			{
 				long requestedSize = _position + buffer.Length;
 				while (requestedSize > _notifySize)
-				{
 					_notifySize = _notifySizeReached();
-					// 0 means abort, because the buffer is too small
-					if (_notifySize == 0)
-					{
-						// we set _position so that we can check later if the attempted write length was larger than the available buffer size
-						_position += buffer.Length;
-						return;
-					}
-				}
 				long n = buffer.Length;
 				if (n > 0)
 				{

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

@@ -71,8 +71,8 @@ namespace BizHawk.Tests.Client.Common.Movie
 			var stateCount = 0;
 			for (int i = 0; i < 1000000; i++)
 			{
-				if (zb.Capture(i, s => ss.SaveStateBinary(new BinaryWriter(s)), j => stateCount--, true))
-					stateCount++;
+				zb.Capture(i, s => ss.SaveStateBinary(new BinaryWriter(s)), j => stateCount--, true);
+				stateCount++;
 			}
 			Assert.AreEqual(zb.Count, stateCount);
 		}