From c2beb017ed56d872837e24d4c457a28b6b3189df Mon Sep 17 00:00:00 2001 From: SuuperW Date: Sat, 24 May 2025 02:34:10 -0500 Subject: [PATCH] Pass new test: ensure we don't create gaps larger than the ancient state interval. --- .../movie/tasproj/IStateManager.cs | 4 +- .../movie/tasproj/TasBranch.cs | 4 +- .../movie/tasproj/TasMovieMarker.cs | 4 +- .../movie/tasproj/ZwinderStateManager.cs | 78 ++++++++++--------- .../Movie/ZwinderStateManagerTests.cs | 2 +- 5 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/BizHawk.Client.Common/movie/tasproj/IStateManager.cs b/src/BizHawk.Client.Common/movie/tasproj/IStateManager.cs index aba852c334..bc9444f612 100644 --- a/src/BizHawk.Client.Common/movie/tasproj/IStateManager.cs +++ b/src/BizHawk.Client.Common/movie/tasproj/IStateManager.cs @@ -23,9 +23,9 @@ namespace BizHawk.Client.Common void Capture(int frame, IStatable source, bool force = false); /// - /// Commands the state manager to remove a reserved state for the given frame, if it is exists + /// Tell the state manager we no longer wish to reserve the state for the given frame. /// - void EvictReserved(int frame); + void Unreserve(int frame); bool HasState(int frame); diff --git a/src/BizHawk.Client.Common/movie/tasproj/TasBranch.cs b/src/BizHawk.Client.Common/movie/tasproj/TasBranch.cs index 6df10f9835..7604e0d816 100644 --- a/src/BizHawk.Client.Common/movie/tasproj/TasBranch.cs +++ b/src/BizHawk.Client.Common/movie/tasproj/TasBranch.cs @@ -88,7 +88,7 @@ namespace BizHawk.Client.Common if (newBranch.UserText.Length is 0) newBranch.UserText = old.UserText; this[index] = newBranch; if (!_movie.IsReserved(old.Frame)) - _movie.TasStateManager.EvictReserved(old.Frame); + _movie.TasStateManager.Unreserve(old.Frame); _movie.FlagChanges(); } @@ -120,7 +120,7 @@ namespace BizHawk.Client.Common if (result) { if (!_movie.IsReserved(item!.Frame)) - _movie.TasStateManager.EvictReserved(item.Frame); + _movie.TasStateManager.Unreserve(item.Frame); _movie.FlagChanges(); } diff --git a/src/BizHawk.Client.Common/movie/tasproj/TasMovieMarker.cs b/src/BizHawk.Client.Common/movie/tasproj/TasMovieMarker.cs index 4bf4e5f9de..04e976d9ca 100644 --- a/src/BizHawk.Client.Common/movie/tasproj/TasMovieMarker.cs +++ b/src/BizHawk.Client.Common/movie/tasproj/TasMovieMarker.cs @@ -206,7 +206,7 @@ namespace BizHawk.Client.Common return; } - _movie.TasStateManager.EvictReserved(item.Frame - 1); + _movie.TasStateManager.Unreserve(item.Frame - 1); _movie.ChangeLog.AddMarkerChange(null, item.Frame, item.Message); base.Remove(item); @@ -221,7 +221,7 @@ namespace BizHawk.Client.Common if (match.Invoke(m)) { _movie.ChangeLog.AddMarkerChange(null, m.Frame, m.Message); - _movie.TasStateManager.EvictReserved(m.Frame - 1); + _movie.TasStateManager.Unreserve(m.Frame - 1); } } diff --git a/src/BizHawk.Client.Common/movie/tasproj/ZwinderStateManager.cs b/src/BizHawk.Client.Common/movie/tasproj/ZwinderStateManager.cs index 8b522b619d..9d760a2bfd 100644 --- a/src/BizHawk.Client.Common/movie/tasproj/ZwinderStateManager.cs +++ b/src/BizHawk.Client.Common/movie/tasproj/ZwinderStateManager.cs @@ -93,22 +93,22 @@ namespace BizHawk.Client.Common if (keepOldStates) { - // For ancients ... lets just make sure we aren't keeping states with a gap below the new interval + // For ancients, let's throw out states if doing so still satisfies the ancient state interval. if (settings.AncientStateInterval > _ancientInterval) { - int lastReserved = 0; - List framesToRemove = new List(); - foreach (int f in _reserved.Keys) + List reservedFrames = _reserved.Keys.ToList(); + reservedFrames.Sort(); + for (int i = 1; i < reservedFrames.Count - 1; i++) { - if (!_reserveCallback(f) && f - lastReserved < settings.AncientStateInterval) - framesToRemove.Add(f); - else - lastReserved = f; - } - foreach (int f in framesToRemove) - { - if (f != 0) - EvictReserved(f); + if (_reserveCallback(reservedFrames[i])) + continue; + + if (reservedFrames[i + 1] - reservedFrames[i - 1] <= settings.AncientStateInterval) + { + EvictReserved(reservedFrames[i]); + reservedFrames.RemoveAt(i); + i--; + } } } } @@ -297,7 +297,7 @@ namespace BizHawk.Client.Common } } - public void EvictReserved(int frame) + private void EvictReserved(int frame) { if (frame == 0) { @@ -311,6 +311,16 @@ namespace BizHawk.Client.Common } } + public void Unreserve(int frame) + { + // Before removing the state, check if it should be still be reserved. + // For now, this just means checking if we need to keep this state to satisfy the ancient interval. + if (ShouldKeepForAncient(frame)) + return; + + EvictReserved(frame); + } + public void Capture(int frame, IStatable source, bool force = false) { // We already have this state, no need to capture @@ -366,41 +376,39 @@ namespace BizHawk.Client.Common index2 => { var state2 = _recent.GetState(index2); - StateCache.Remove(state2.Frame); - var isReserved = _reserveCallback(state2.Frame); // Add to reserved if reserved, or if it matches an "ancient" state consideration - if (isReserved || !HasNearByReserved(state2.Frame)) + if (isReserved || ShouldKeepForAncient(state2.Frame)) { AddToReserved(state2); } + else + StateCache.Remove(state2.Frame); }); }); } - // Returns whether or not a frame has a reserved state within the frame interval on either side of it - private bool HasNearByReserved(int frame) + /// + /// Will removing the state on this leave us with a gap larger than the ancient interval? + /// + private bool ShouldKeepForAncient(int frame) { - // An easy optimization, we know frame 0 always exists - if (frame < _ancientInterval) + int index = StateCache.BinarySearch(frame + 1); + if (index < 0) + index = ~index; + if (index == StateCache.Count) { - return true; + // There is no future state... so why are we wanting to evict this state? + // We aren't decaying from _recent, that's for sure. So, + return false; } + if (index <= 1) + return true; // This should never happen. - // Has nearby before - if (_reserved.Any(kvp => kvp.Key < frame && kvp.Key > frame - _ancientInterval)) - { - return true; - } - - // Has nearby after - if (_reserved.Any(kvp => kvp.Key > frame && kvp.Key < frame + _ancientInterval)) - { - return true; - } - - return false; + int nextState = StateCache[index]; + int previousState = StateCache[index - 2]; // assume StateCache[index - 1] == frame + return nextState - previousState > _ancientInterval; } private bool NeedsGap(int frame) diff --git a/src/BizHawk.Tests/Client.Common/Movie/ZwinderStateManagerTests.cs b/src/BizHawk.Tests/Client.Common/Movie/ZwinderStateManagerTests.cs index a69a382e0e..ae8cf927ea 100644 --- a/src/BizHawk.Tests/Client.Common/Movie/ZwinderStateManagerTests.cs +++ b/src/BizHawk.Tests/Client.Common/Movie/ZwinderStateManagerTests.cs @@ -413,7 +413,7 @@ namespace BizHawk.Tests.Client.Common.Movie for (int i = 400; i < 1000; i += 400) { - zw.EvictReserved(i); + zw.Unreserve(i); } for (int i = 0; i < 10000; i++)