core: Add timeline_debug feature that logs all gotos and asserts if they desync
#7334
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kmeisthax
force-pushed
the
timeline-paranoia
branch
from
8d6bc40
to
c2a23dc
Compare
…imeline_debug` feature is enabled.
…m positions against the list of stream boundaries.
…ing the target frame. If we try to go to a frame that doesn't exist, or hasn't been loaded yet, we will stop on the last available frame, but skip any tags that would have run there. This is technically a desync, but it hasn't caused any problems so far as any further timeline interaction would trigger a rewind (which isn't affected by desyncs). Of course, now that we're actually testing the tag stream position it *does* cause problems. We actually have to fix up the position to be correct even though it will never be used (hopefully). It may be prudent to do this outside of the `timeline_debug` feature as well in the future.
Herschel
force-pushed
the
timeline-paranoia
branch
from
1735467
to
a07c46e
Compare
Herschel
force-pushed
the
timeline-paranoia
branch
from
a07c46e
to
871fda8
Compare
|
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ruffle keeps track of all movie clips' timelines via a struct member called
tag_stream_pos. This is updated whenever a clip runs a frame, loops back to the start, or is given a goto command from script code, and should always point to the end of the current frame's tags (with some exceptions). If this invariant is broken, the tag stream is said to have desynchronized. The timeline no longer works correctly in the presence of fast-forwarding operations until a rewind occurs. The current implementation of AVM2 timeline operations suffers from a number of goto-related bugs that trigger desyncs.This PR adds a new feature called
timeline_debug, analogous to theavm_debugfeature that prints out every stack operation. When enabled,timeline_debugwill print out every goto, as well as assert that the goto started and ended in the correct spot. We also run the entire test suite with this feature enabled so that those tests will assert if they desync.As I mentioned above, there are exceptions to the invariants on
tag_stream_pos. Specifically:For the time being, the first desync on the list is being explicitly checked for and corrected. My assumption is that, at least for a non-progressive-loading player like ours, there is no way to actually trigger a fast-forwarding goto off the back of one that didn't hit its target frame. It may make sense to correct for it without
timeline_debugbut that's probably outside the scope of this PR.The second desync is a huge part of why the frame events fixes PR has been such a long-running project. I have made no attempt to account for it in this PR; as doing so requires the frame phase tracking logic from #7048. The existing suite of AVM2 tests work with the asserts enabled.