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 Progression Events #3177

Merged
merged 32 commits into from
Mar 3, 2021
Merged

Conversation

kmeisthax
Copy link
Member

This is a first attempt at implementing all of the clip events typically issued by a MovieClip (or, really, any DisplayObject). This covers events for adding and removing objects, as well as frame progression events, which were extraordinarily tricky to get even a handful of tests to pass. I'm confident that I at least have events occurring in the correct order, though I'm pretty sure I don't have properties changing at the correct time relative to those events.

Particularly problematic is AS3's frame construction behavior. In AS3, all timeline children of a MovieClip are constructed and added to the display list before the parent's constructor is called. This necessitated tearing run_frame apart into multiple phases which need to be called at the appropriate time, which causes a whole host of other issues. I suspect my current code is a kludge.

The AVM2 frame progression is implemented as follows in this PR:

  1. exit_frame - broadcasts exitFrame when called on any display object
  2. enter_frame - broadcasts enterFrame when called on any display object
  3. construct_frame - execute all PlaceObject tags for the next frame, child-first
  4. run_frame - determine frame progression, run all other tags, and queue frame scripts
  5. run_frame_scripts - execute queued frame scripts, parent-first
  6. update_sounds

These have to be in separate methods because they need to descend into children in different orders. For example, frame scripts always run parent-first, even though construct_frame and run_frame are child-first.

AS3 display object construction is also multi-part now: since we need an AS3 object to store constructed children onto, we allocate the object first thing in construct_frame. post_instantiation now only calls the constructor, which feels very wrong, but if I don't do this then constructors won't see their children.

Further AS3 quirks I've noticed:

  1. Gotos trigger recursive frame progression, but only a subset of the above events (as it effectively is an alternate construct_frame). They will broadcast frameConstructed and exitFrame, but not enterFrame. (This is implemented and tested for.)
  2. Frame numbers are advanced before enterFrame is broadcast. I am not testing for this; as my first attempt at implementing this behavior caused many AS3 tests to fail with off-by-one errors in frame progression. It also breaks AS2 but I can gate that. Due to this, however, construct_frame has to be very wary to "correct" broken place_frames else gotos will not remove children that don't exist on a given frame in all cases.
  3. Broadcast events trigger on unreachable objects. AVM2 appears to maintain a list of broadcast listeners; but it uses weak pointers to reference them. This allows dead objects to continue broadcasting (and logging trace output) until the garbage collector destroys them. However, Adobe's GC is also very lazy and nondeterministic. I worked around this in tests by explicitly unregistered event handlers of objects scheduled to be destroyed and then calling System.gc.

Copy link

@seanpm2001 seanpm2001 left a comment

Choose a reason for hiding this comment

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

Looks good. Very clean and optimized. Might need further review, as I am not as experienced in Rust as I am in other languages.

Copy link
Member

@Herschel Herschel left a comment

Choose a reason for hiding this comment

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

Thanks! Seems good (and I suspect some of this will simplify as we get a better idea of how things might fit together between AVM1/AVM2).

It does seem like some of these event dispatchers fire even in an AVM1 movie (DisplayObject::enter_frame, exit_frame, etc.). That should only happen in an AVM2 movie.

Pending @relrelb's work in #3104, I wonder if we could use the same singular execution list for both AVM1 and broadcast events in AVM2.

self.root(context)
.unwrap_or_else(|| self.into())
.run_frame_scripts(context);
self.exit_frame(context);
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this will still fire AVM2 events in an AVM1 movie?

@kmeisthax
Copy link
Member Author

run_frame_scripts will only actually try to run any code iff there's an AVM2 object attached to the clip. In AVM1 mode it'll track frames but that's about it. I've pushed code to make sure even the queued frame variable isn't checked.

The other events won't hit AVM1 objects primarily because they don't have an AVM2 side, and thus cannot get registered in the AVM2 broadcast list. All the methods do is trigger a broadcast from that list; they don't even try to limit the event to just the object that this method was called on. The way we keep AVM1 from seeing these events is that the only code that actually registers broadcast receivers is the constructor for EventDispatcher, which is a superclass of DisplayObject.

Wait, why? Well, it turns out there's non-display-list broadcasts! EventDispatcher defines at least two, activate and deactivate, which fire on anything that leverages native event handling. This also means we can't use the #3104 list, because it only holds actual DisplayObjects. Furthermore, AVM2's broadcast list consists of lazily-collected weak pointers that broadcast events to unreachable objects; using the #3104 list would mean strictly-collected strong pointers, which will produce different behaviors. It's fairly trivial to discover this - it was actually the first thing I noticed when implementing broadcasts; so I suspect there's at least one movie out there that actually relies on weak objects sticking around for a while.

In the future, most of the events I added here should probably be moved onto some kind of a Stage object that the player pumps events into, but I'm already stubbing Shape in this PR and I figured I was going out of scope if I went further.

@Herschel
Copy link
Member

Herschel commented Feb 20, 2021

I mean that the #3104 should also be changed to use this Weak pointer list as well -- the difference being that each time we iterate through to run a frame, in AVM1 land, any removed clips will get removed from the list while iterating, vs. AVM2 where these will get removed from the list when they are GCd and the weak pointers are no longer valid. They can still be the same pointer type and the same list for both VMs, assuming the clips fire in the same execution order (based on creation order).

@Herschel
Copy link
Member

Herschel commented Feb 20, 2021

Okay, I did some more testing and am convinced now that the display object/AVM1 exec order list has to be different than the AVM2 event dispatch list (the AVM2 order is based on the order of the listeners being registered, and naturally this makes this easier to implement the non-DO broadcast events, as you mention). I still think it's possible that #3104 may need to use weak pointers, as well.

@Herschel
Copy link
Member

Here's a test that doesn't run in the proper order:
test.zip

After two frames, Flash Player outputs:

mc exitframe
root exitframe
root exitframe 2
root enterframe
root enterframe 2
mc enterframe
mc exitframe
root exitframe
root exitframe 2

Ruffle outputs:

root exitframe
root exitframe 2
mc exitframe
root enterframe
root enterframe 2
mc enterframe
root exitframe
root exitframe 2
mc exitframe

@kmeisthax
Copy link
Member Author

Merely reversing the order of exitFrame breaks a whole load of tests... which seems to imply that each event has it's own, separate broadcast list. Implementing this appears to make the test pass (and I've added your code as another test), so I'm going with that approach.

…ous events in the presence of programmed display tree manipulations.
Our current garbage collector design precludes the ability to actually collect garbage during player updates, so this is a no-op.
This appears to only be in use for AVM2. Objects placed on a given frame are constructed before anything else happens with it's parent - even it's constructor being called. This involves splitting AVM2 up into a bunch of steps that really don't make sense for AVM1 content. Hence, `construct_frame` is a no-op for AVM1 and pre-running the first frame when instantiated is AVM1 exclusive now.
…cript queues up additional frame scripts via gotos
…rame lifecycle events.

For some reason, only *some* of the events actually trigger; notably programmatic gotos do not trigger `enterFrame`.

Implicit gotos (like looping around to frame 1) also do not trigger frame scripts; they instead run at the usual time.
…han the frame number advances.

This is most certainly *not* the correct behavior; though it does work. If I track the frame number in event handlers we can see it change before `enterFrame` is broadcast. However, when I tried to do that, all hell broke loose and every AVM1 and AVM2 test failed (gating the behavior to AVM2 did *not* help).
@Herschel
Copy link
Member

Herschel commented Mar 3, 2021

Thanks!

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

3 participants