Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AVM2 Frame Events Ordering Fixes #5876

Closed
wants to merge 64 commits into from

Conversation

kmeisthax
Copy link
Member

@kmeisthax kmeisthax commented Dec 17, 2021

This PR aims to fix remaining order-of-events bugs in AVM2. The previous work to implement frame lifecycle events was deliberately unfinished. Now that some games are getting close to running, these bugs need to get fixed.

So far, I've fixed the following unaccounted-for things:

  • EventDispatcher methods need to work before initialization, so they now lazily initialize the object's event dispatch list. It is possible to gain access to uninitialized display objects (e.g. in parent constructors) and AVM2 movies expect to be able to put event handlers on them. (@adrian17 caught this one, thanks)
  • Frame construction only occurs if MovieClips are actually playing (as cached at enterFrame). Otherwise, if you stopped a MovieClip on a frame with a PlaceObject tag, it would constantly keep creating that tag.
  • The meaning of tag_stream_pos has changed subtly. In AVM1, it's just a pointer to the next frame's tags, because they all run at once. In AVM2, we have to process this multiple times, so we do not update tag_stream_pos until we are done processing the current frame's tags.
    • This also introduces the notion of a "tag stream desync". The goto machinery wants tag_stream_pos to work like it works in AVM1. Any time that it doesn't, we say that the tag stream has desynchronized.
    • If we have intentionally desynchronized the tag stream, run_goto needs to check if we have done so and either skip tags or rewind.
    • If we unintentionally desynchronized the tag stream, then gotos will break.
      • For some reason, fast-forwarding gotos in event handlers (Idle phase) will desync if we run_frame_internal; so we skip that part.
  • Loops work identically to advancing to the next frame in AVM2, unlike in AVM1 where they work like an explicit gotoAndPlay(1). This requires us to maintain various lies about the state of the timeline, especially tag_stream_pos, as the loop's effects are split across the various frame lifecycle phases.
  • When we start a new frame, MovieClip does a whole bunch of extra work before enterFrame is dispatched:
    • Start frames for all our children. For some reason this appears to run in reverse order.
    • Cache self.playing(), which now becomes self.playing_at_frame_advance(). Every playback check that happens after frame advance needs to use the cached flag instead of self.playing(), at least in AVM2.
    • Flag the clip as needing frame construction. This flag exists to prevent recursive frames (see below) from creating more timeline children than should exist.
    • Destroy any children that should not exist on the next frame.
      • For normal frame advance, we execute the next frame's RemoveObject tags now.
      • If we are looping, we remove all children placed after frame 1.
    • TODO: Add nulls to the render list for all children to be placed during frame construction. We don't do this yet.
    • Then, if we are playing, we advance the MovieClip to the next frame and queue that frame's frame scripts.
      • In the case of a loop, we set the frame number to 1 without running a goto. This is one of the timeline lies.
    • Cache determine_next_frame (which, incidentally, no longer exists) as natural_playhead_action. We use this to either revert or commit to the lie above in later frame phases.
    • Dispatch enterFrame.
      • If user code calls our bluff by triggering a goto at this time, commit to the lie by rolling the tag stream position back to 0 as well, treating all gotos as rewinds.
  • Frame construction then creates any children of the current clip for the next frame. It changes as such:
    • Child creation only happens if the MovieClip was playing when we entered the current frame.
    • We still use the implicit goto machinery for loops, but we roll back the lie we made before enterFrame beforehand.
    • We still have to treat all gotos during loops as rewinds, because our tag stream position is still desynchronized. However, we additionally have to skip frame 1's tags since we have already placed its children.
  • AVM2 has the notion of a "recursive frame" - i.e. synchronous operations that are treated as dispatching a frame's frameConstructed and exitFrame events, as well as running queued frame scripts. Gotos are one of them.
  • No-op gotos will trigger a recursive frame and thus cannot be skipped on AVM2.
  • Gotos that do change the frame number need to queue up a new frame script, and disable duplicate frame detection.
  • Gotos do not dispatch their recursive frames until after child manipulation (goto_commands internally) has finished.
  • Recursive frames check if a MovieClip is already running its frame scripts. If so, the script is left on the queue.
    • We also had to fix a related bug in which MovieClips would not drop the EXECUTING_AVM2_FRAME_SCRIPT tag if they tried to execute frame scripts, but couldn't find any. This would lock up the clip timeline until the next frame script.
  • As a result of the above, a number of AVM2 hacks surrounding place_frame got removed. These may become irrelevant if core: Simplifiy goto logic and use ratio field #5431 merges before this PR.
  • SimpleButton has a whole host of oddities to it:
    • When initialized by AVM2 code, SimpleButton and it's child classes initialize as you would expect: i.e. in line with the rest of the code.
    • When placed by PlaceObject, the AVM2 instance initializer for the button will not run until we enter the next frame.
    • Except for buttons on frame 1. These initialize immediately after all child state objects are initialized and run their frames.
    • If a button is on frame 1 and has an Up state, it dispatches a recursive frame on all of it's children (in a slightly different order, no less).
    • The order that frame scripts are run appears to change based on whether or not the parent movie clip is stopped or not. I've decided to #[ignore] the tests that are affected by this for now. The effects are deterministic, but I suspect they are caused by some deeper unimplemented behavior in the Player. Talking with one of our sponsors it sounded like this was a known problem with Flash Player that people programmed around rather than relied upon.
    • Certain tests are supposed to fire extra recursive frames for some reason. I'm ignoring those tests, too.
  • The order of stage events is enter, construct, run, scripts, exit; we were starting from exit and looping back around.
    • We also have to take care not to remove children until the next frame occurs; otherwise they disappear one frame early.
  • We now keep track of which frame phase we're in, and use that to decide what events should be triggered on newly-created display objects. This applies to MovieClip's instantiate_child, symbol class construction, and button state construction.
    • We also add an Idle phase for event handlers (none of which exist in this PR, but I've been testing this alongside avm2: Mouse event support #5767).
    • Regardless of phase, in AVM2 we always enter and construct a frame on an object, because all scripts - even event handlers - expect to be able to see children of clips they create. This happens even if doing so would cause multiple construct_frame calls internally; we use display object flags to prevent duplicate construction.
    • While the catchup process vaguely resembles the normal frame phases, they are not recursive frames. They exclude any event broadcasts or frame script execution.
  • Gotos don't remove any children until after the new timeline position has been written to the parent clip.
    • In fast-forwarding gotos, child removals instead happen before the frame number updates.
    • Children removed during fast-forwarding still run their queued frame scripts when the goto dispatches its recursive frame.
  • The root movie clip can receive added/addedToStage (at least, if you set up a document class); but running construct_frame on the root before those events naturally fire was rather difficult. Instead, I re-fire the event when the root is constructed in the normal frame flow.

This list will grow as I fix more broken tests in the PR, or add new broken tests. I'm calling time on this PR.

@kmeisthax kmeisthax force-pushed the avm2-frameordering branch 2 times, most recently from 700acef to 6c557f7 Compare December 22, 2021 02:52
@kmeisthax kmeisthax marked this pull request as ready for review January 5, 2022 22:34
@kmeisthax kmeisthax changed the title [DRAFT] AVM2 Frame Events Ordering Fixes AVM2 Frame Events Ordering Fixes Jan 5, 2022
@kmeisthax
Copy link
Member Author

I've decided to call time on this PR and #[ignore] the six currently-failing SimpleButton tests. Why?

  1. They rely on behavior that is technically deterministic, in that I get the same output in each Flash Player run; but does not appear to have anything to do with when buttons are initialized, how they are initialized, or what they contain. For example, buttons can run AVM2 instance initializers late, but I can't seem to actually write a test that causes the player to initialize buttons early or late.
  2. Talking with one of our sponsors, it sounds like AS3 developers were at least mildly aware of SimpleButton doing weird things and programmed around them.
  3. The two people on Discord at the time I asked also agreed on ignoring the tests.

@kmeisthax kmeisthax force-pushed the avm2-frameordering branch 2 times, most recently from e565f3e to c10f7f0 Compare January 8, 2022 21:55
@kmeisthax kmeisthax force-pushed the avm2-frameordering branch 3 times, most recently from 9271f9b to 8b537da Compare January 18, 2022 23:40
core/src/frame_lifecycle.rs Outdated Show resolved Hide resolved
core/src/frame_lifecycle.rs Outdated Show resolved Hide resolved
core/src/frame_lifecycle.rs Outdated Show resolved Hide resolved
core/src/frame_lifecycle.rs Outdated Show resolved Hide resolved
core/src/player.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@adrian17 adrian17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to pretend I understood everything in movie_clip.rs... :(

Would also appreciate if @Herschel reviewed this.

core/src/display_object/movie_clip.rs Show resolved Hide resolved
core/src/display_object/movie_clip.rs Outdated Show resolved Hide resolved
@kmeisthax kmeisthax force-pushed the avm2-frameordering branch 3 times, most recently from 0444a3c to 0af8e09 Compare February 2, 2022 23:09
@kmeisthax kmeisthax force-pushed the avm2-frameordering branch 2 times, most recently from f61c0e5 to b78c721 Compare February 18, 2022 00:20
@Toad06
Copy link
Member

Toad06 commented Feb 21, 2022

Done the usual testings with AVM1 files, everything looks good to me.

@ruffle-rs ruffle-rs deleted a comment Feb 21, 2022
@Aaron1011
Copy link
Member

This lets 'Pixel Legions' get further in the startup screen - previously, it would freeze almost immediately due to an error when dispatching an event.

… except `destroy_frame`

If we don't do this, then display objects created in event handlers will have a frame number desynchronized from their tag stream position.

Unfortunately we cannot test for this.
…stily fix up the clip so it looks like the implicit goto already completed.
…lip, even if we already executed that script beforehand.
…ed back and doing so breaks fast-forwarding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants