From ad07eb85742c6ef89c74642f1363369758d611fa Mon Sep 17 00:00:00 2001
From: RetroEdit <30182911+RetroEdit@users.noreply.github.com>
Date: Tue, 11 Aug 2020 17:53:27 +0000
Subject: [PATCH] Clean up truncate behavior and docs. Fixes #2285 (#2286)
* Clean up truncate behavior and docs. Fixes #2285
To start with, 770ddc26dbd987262790158206d475c6e5abe3ee (6 years ago, prior to 1.7.0) made the documentation of how truncate is supposed to work. However, judging by my testing of 1.9.0 and 2.4.2, it probably hasn't ever worked that way in practice. It seems justified to change the documentation to match the behavior that the user is familiar with, and what I see as the commonsense behavior anyway (I can justify this further if anyone is curious).
Note this is not changing the behavior of truncate itself; it is simply documenting what the behavior has always been.
Secondly, Invalidate was modified to become InvalidateAfter. This modifies how it gets called, but otherwise remains mostly identical internal to the Zwinder. This was done to make it easier to reason about relative to the input log, which should be the foundation for an understanding of related components.
* zwinder: "frame after" interpretation in all Invalidate methods
---
src/BizHawk.Client.Common/movie/interfaces/IMovie.cs | 3 +--
.../movie/tasproj/IStateManager.cs | 4 ++--
.../movie/tasproj/TasMovie.Editing.cs | 7 +++++--
src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs | 4 ++--
.../movie/tasproj/ZwinderStateManager.cs | 12 ++++++------
5 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs b/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs
index c68baaead9..1f65873238 100644
--- a/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs
+++ b/src/BizHawk.Client.Common/movie/interfaces/IMovie.cs
@@ -212,8 +212,7 @@ namespace BizHawk.Client.Common
void RecordFrame(int frame, IController source);
///
- /// Instructs the movie to remove all input from its input log after frame,
- /// After truncating, frame will be the last frame of input in the movie's input log
+ /// Instructs the movie to remove all input from its input log starting with the input at frame.
///
/// The frame at which to truncate
void Truncate(int frame);
diff --git a/src/BizHawk.Client.Common/movie/tasproj/IStateManager.cs b/src/BizHawk.Client.Common/movie/tasproj/IStateManager.cs
index 6e044bbb83..2b7087e74e 100644
--- a/src/BizHawk.Client.Common/movie/tasproj/IStateManager.cs
+++ b/src/BizHawk.Client.Common/movie/tasproj/IStateManager.cs
@@ -25,9 +25,9 @@ namespace BizHawk.Client.Common
bool HasState(int frame);
///
- /// Clears out all savestates after or at the given frame number
+ /// Clears out all savestates after the given frame number
///
- bool Invalidate(int frame);
+ bool InvalidateAfter(int frame);
// Remove all states, but not the frame 0 state
void Clear();
diff --git a/src/BizHawk.Client.Common/movie/tasproj/TasMovie.Editing.cs b/src/BizHawk.Client.Common/movie/tasproj/TasMovie.Editing.cs
index 7fd2dc21eb..d74b71751b 100644
--- a/src/BizHawk.Client.Common/movie/tasproj/TasMovie.Editing.cs
+++ b/src/BizHawk.Client.Common/movie/tasproj/TasMovie.Editing.cs
@@ -11,6 +11,9 @@ namespace BizHawk.Client.Common
public override void RecordFrame(int frame, IController source)
{
+ // RetroEdit: This check is questionable; recording at frame 0 is valid and should be reversible.
+ // Also, frame - 1, why?
+ // Is the precondition compensating for frame - 1 reindexing?
if (frame != 0)
{
ChangeLog.AddGeneralUndo(frame - 1, frame - 1, $"Record Frame: {frame}");
@@ -26,7 +29,7 @@ namespace BizHawk.Client.Common
if (this.IsRecording())
{
- TasStateManager.Invalidate(frame + 1);
+ TasStateManager.InvalidateAfter(frame);
GreenzoneInvalidated(frame + 1);
}
@@ -49,7 +52,7 @@ namespace BizHawk.Client.Common
base.Truncate(frame);
LagLog.RemoveFrom(frame);
- TasStateManager.Invalidate(frame);
+ TasStateManager.InvalidateAfter(frame);
GreenzoneInvalidated(frame);
Markers.TruncateAt(frame);
diff --git a/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs b/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs
index ebe5dcf32e..bed621527b 100644
--- a/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs
+++ b/src/BizHawk.Client.Common/movie/tasproj/TasMovie.cs
@@ -124,7 +124,7 @@ namespace BizHawk.Client.Common
private void InvalidateAfter(int frame)
{
var anyLagInvalidated = LagLog.RemoveFrom(frame);
- var anyStateInvalidated = TasStateManager.Invalidate(frame + 1);
+ var anyStateInvalidated = TasStateManager.InvalidateAfter(frame);
GreenzoneInvalidated(frame + 1);
if (anyLagInvalidated || anyStateInvalidated)
{
@@ -284,7 +284,7 @@ namespace BizHawk.Client.Common
if (timelineBranchFrame.HasValue)
{
LagLog.RemoveFrom(timelineBranchFrame.Value);
- TasStateManager.Invalidate(timelineBranchFrame.Value);
+ TasStateManager.InvalidateAfter(timelineBranchFrame.Value);
GreenzoneInvalidated(timelineBranchFrame.Value);
}
diff --git a/src/BizHawk.Client.Common/movie/tasproj/ZwinderStateManager.cs b/src/BizHawk.Client.Common/movie/tasproj/ZwinderStateManager.cs
index a3de6f728b..9a6d99bfac 100644
--- a/src/BizHawk.Client.Common/movie/tasproj/ZwinderStateManager.cs
+++ b/src/BizHawk.Client.Common/movie/tasproj/ZwinderStateManager.cs
@@ -237,7 +237,7 @@ namespace BizHawk.Client.Common
{
for (var i = 0; i < _highPriority.Count; i++)
{
- if (_highPriority.GetState(i).Frame >= frame)
+ if (_highPriority.GetState(i).Frame > frame)
{
_highPriority.InvalidateEnd(i);
return true;
@@ -250,7 +250,7 @@ namespace BizHawk.Client.Common
{
for (var i = 0; i < _ancient.Count; i++)
{
- if (_ancient[i].Key >= frame)
+ if (_ancient[i].Key > frame)
{
_ancient.RemoveRange(i, _ancient.Count - i);
_recent.InvalidateEnd(0);
@@ -260,7 +260,7 @@ namespace BizHawk.Client.Common
}
for (var i = 0; i < _recent.Count; i++)
{
- if (_recent.GetState(i).Frame >= frame)
+ if (_recent.GetState(i).Frame > frame)
{
_recent.InvalidateEnd(i);
_current.InvalidateEnd(0);
@@ -269,7 +269,7 @@ namespace BizHawk.Client.Common
}
for (var i = 0; i < _current.Count; i++)
{
- if (_current.GetState(i).Frame >= frame)
+ if (_current.GetState(i).Frame > frame)
{
_current.InvalidateEnd(i);
return true;
@@ -280,9 +280,9 @@ namespace BizHawk.Client.Common
public void UpdateSettings(ZwinderStateManagerSettings settings) => Settings = settings;
- public bool Invalidate(int frame)
+ public bool InvalidateAfter(int frame)
{
- if (frame <= 0)
+ if (frame < 0)
throw new ArgumentOutOfRangeException(nameof(frame));
var b1 = InvalidateNormal(frame);
var b2 = InvalidateHighPriority(frame);