AVM2: RemoveObject runs at the start of a frame & frame phase tracking

core/src/avm2/globals/flash/display/displayobject.rs

@@ -8,6 +8,7 @@ use crate::avm2::object::{stage_allocator, LoaderInfoObject, Object, TObject};
 use crate::avm2::value::Value;
 use crate::avm2::Error;
 use crate::display_object::{DisplayObject, HitTestOptions, TDisplayObject};
+use crate::frame_lifecycle::catchup_display_object_to_frame;
 use crate::types::{Degrees, Percent};
 use crate::vminterface::Instantiator;
 use gc_arena::{GcCell, MutationContext};
@@ -52,7 +53,7 @@ pub fn native_instance_init<'gc>(
                 child.set_object2(activation.context.gc_context, this);
 
                 child.post_instantiation(&mut activation.context, None, Instantiator::Avm2, false);
-                child.construct_frame(&mut activation.context);
+                catchup_display_object_to_frame(&mut activation.context, child);
             }
         }
     }

core/src/context.rs

@@ -18,6 +18,7 @@ use crate::context_menu::ContextMenuState;
 use crate::display_object::{EditText, InteractiveObject, MovieClip, SoundTransform, Stage};
 use crate::external::ExternalInterface;
 use crate::focus_tracker::FocusTracker;
+use crate::frame_lifecycle::FramePhase;
 use crate::library::Library;
 use crate::loader::LoadManager;
 use crate::player::Player;
@@ -167,6 +168,11 @@ pub struct UpdateContext<'a, 'gc, 'gc_context> {
 
     /// The current stage frame rate.
     pub frame_rate: &'a mut f64,
+
+    /// The current frame processing phase.
+    ///
+    /// If we are not doing frame processing, then this is `FramePhase::Enter`.
+    pub frame_phase: &'a mut FramePhase,
 }
 
 /// Convenience methods for controlling audio.
@@ -325,6 +331,7 @@ impl<'a, 'gc, 'gc_context> UpdateContext<'a, 'gc, 'gc_context> {
             times_get_time_called: self.times_get_time_called,
             time_offset: self.time_offset,
             frame_rate: self.frame_rate,
+            frame_phase: self.frame_phase,
         }
     }
 

core/src/display_object.rs

@@ -1108,22 +1108,10 @@ pub trait TDisplayObject<'gc>:
             .set_instantiated_by_timeline(value);
     }
 
-    /// Emit an `enterFrame` event on this DisplayObject and any children it
-    /// may have.
-    fn enter_frame(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
-        let mut enter_frame_evt = Avm2Event::new("enterFrame", Avm2EventData::Empty);
-        enter_frame_evt.set_bubbles(false);
-        enter_frame_evt.set_cancelable(false);
-
-        let dobject_constr = context.avm2.classes().display_object;
-
-        if let Err(e) = Avm2::broadcast_event(context, enter_frame_evt, dobject_constr) {
-            log::error!(
-                "Encountered AVM2 error when broadcasting enterFrame event: {}",
-                e
-            );
-        }
-    }
+    /// Run any start-of-frame actions for this display object.
+    ///
+    /// When fired on `Stage`, this also emits the AVM2 `enterFrame` broadcast.
+    fn enter_frame(&self, _context: &mut UpdateContext<'_, 'gc, '_>) {}
 
     /// Construct all display objects that the timeline indicates should exist
     /// this frame, and their children.

core/src/display_object/avm2_button.rs

@@ -12,6 +12,7 @@ use crate::display_object::interactive::{
 };
 use crate::display_object::{DisplayObjectBase, DisplayObjectPtr, MovieClip, TDisplayObject};
 use crate::events::{ClipEvent, ClipEventResult};
+use crate::frame_lifecycle::catchup_display_object_to_frame;
 use crate::prelude::*;
 use crate::tag_utils::{SwfMovie, SwfSlice};
 use crate::vminterface::Instantiator;
@@ -224,15 +225,15 @@ impl<'gc> Avm2Button<'gc> {
 
             child.set_parent(context.gc_context, Some(self.into()));
             child.post_instantiation(context, None, Instantiator::Movie, false);
-            child.construct_frame(context);
+            catchup_display_object_to_frame(context, child);
 
             (child, false)
         } else {
             let state_sprite = MovieClip::new(movie, context.gc_context);
 
             state_sprite.set_avm2_class(context.gc_context, Some(sprite_class));
             state_sprite.set_parent(context.gc_context, Some(self.into()));
-            state_sprite.construct_frame(context);
+            catchup_display_object_to_frame(context, state_sprite.into());
 
             for (child, depth) in children {
                 // `parent` returns `null` for these grandchildren during construction time, even though
@@ -242,7 +243,7 @@ impl<'gc> Avm2Button<'gc> {
                 state_sprite.replace_at_depth(context, child, depth.into());
                 child.set_parent(context.gc_context, Some(self.into()));
                 child.post_instantiation(context, None, Instantiator::Movie, false);
-                child.construct_frame(context);
+                catchup_display_object_to_frame(context, child);
                 child.set_parent(context.gc_context, Some(state_sprite.into()));
             }
 
@@ -428,6 +429,28 @@ impl<'gc> TDisplayObject<'gc> for Avm2Button<'gc> {
         self.set_state(context, ButtonState::Up);
     }
 
+    fn enter_frame(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
+        let hit_area = self.0.read().hit_area;
+        if let Some(hit_area) = hit_area {
+            hit_area.enter_frame(context);
+        }
+
+        let up_state = self.0.read().up_state;
+        if let Some(up_state) = up_state {
+            up_state.enter_frame(context);
+        }
+
+        let down_state = self.0.read().down_state;
+        if let Some(down_state) = down_state {
+            down_state.enter_frame(context);
+        }
+
+        let over_state = self.0.read().over_state;
+        if let Some(over_state) = over_state {
+            over_state.enter_frame(context);
+        }
+    }
+
     fn construct_frame(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
         let hit_area = self.0.read().hit_area;
         if let Some(hit_area) = hit_area {

core/src/display_object/movie_clip.rs

@@ -30,6 +30,7 @@ use crate::display_object::{
 use crate::drawing::Drawing;
 use crate::events::{ButtonKeyCode, ClipEvent, ClipEventResult};
 use crate::font::Font;
+use crate::frame_lifecycle::catchup_display_object_to_frame;
 use crate::prelude::*;
 use crate::string::{AvmString, WStr, WString};
 use crate::tag_utils::{self, DecodeResult, SwfMovie, SwfSlice, SwfStream};
@@ -1077,8 +1078,12 @@ impl<'gc> MovieClip<'gc> {
             TagCode::PlaceObject4 if run_display_actions && vm_type == AvmType::Avm1 => {
                 self.place_object(context, reader, tag_len, 4)
             }
-            TagCode::RemoveObject if run_display_actions => self.remove_object(context, reader, 1),
-            TagCode::RemoveObject2 if run_display_actions => self.remove_object(context, reader, 2),
+            TagCode::RemoveObject if run_display_actions && vm_type == AvmType::Avm1 => {
+                self.remove_object(context, reader, 1)
+            }
+            TagCode::RemoveObject2 if run_display_actions && vm_type == AvmType::Avm1 => {
+                self.remove_object(context, reader, 2)
+            }
             TagCode::SetBackgroundColor => self.set_background_color(context, reader),
             TagCode::StartSound => self.start_sound_1(context, reader),
             TagCode::SoundStreamBlock => self.sound_stream_block(context, reader),
@@ -1168,7 +1173,7 @@ impl<'gc> MovieClip<'gc> {
                     // TODO: Missing PlaceObject properties: amf_data, filters
 
                     // Run first frame.
-                    child.construct_frame(context);
+                    catchup_display_object_to_frame(context, child);
                     child.post_instantiation(context, None, Instantiator::Movie, false);
                     // In AVM1, children are added in `run_frame` so this is necessary.
                     // In AVM2 we add them in `construct_frame` so calling this causes
@@ -1733,6 +1738,36 @@ impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> {
         self.0.read().movie().version()
     }
 
+    fn enter_frame(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
+        for child in self.iter_render_list() {
+            child.enter_frame(context);
+        }
+
+        if context.avm_type() == AvmType::Avm2 {
+            let is_playing = self.playing();
+
+            if is_playing {
+                // Frame destruction happens in-line with frame number advance.
+                // If we expect to loop, we do not run `RemoveObject` tags.
+                if self.current_frame() < self.total_frames() {
+                    let mc = self.0.read();
+                    let data = mc.static_data.swf.clone();
+                    let mut reader = data.read_from(mc.tag_stream_pos);
+                    drop(mc);
+
+                    use swf::TagCode;
+                    let tag_callback =
+                        |reader: &mut SwfStream<'_>, tag_code, _tag_len| match tag_code {
+                            TagCode::RemoveObject => self.remove_object(context, reader, 1),
+                            TagCode::RemoveObject2 => self.remove_object(context, reader, 2),
+                            _ => Ok(()),
+                        };
+                    let _ = tag_utils::decode_tags(&mut reader, tag_callback, TagCode::ShowFrame);
+                }
+            }
+        }
+    }
+
     /// Construct objects placed on this frame.
     fn construct_frame(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
         // New children will be constructed when they are instantiated and thus

core/src/display_object/stage.rs

@@ -2,7 +2,7 @@
 
 use crate::avm1::Object as Avm1Object;
 use crate::avm2::{
-    Activation as Avm2Activation, Event as Avm2Event, EventData as Avm2EventData,
+    Activation as Avm2Activation, Avm2, Event as Avm2Event, EventData as Avm2EventData,
     Object as Avm2Object, ScriptObject as Avm2ScriptObject, StageObject as Avm2StageObject,
     Value as Avm2Value,
 };
@@ -711,6 +711,25 @@ impl<'gc> TDisplayObject<'gc> for Stage<'gc> {
         context.renderer.end_frame();
     }
 
+    fn enter_frame(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
+        for child in self.iter_render_list() {
+            child.enter_frame(context);
+        }
+
+        let mut enter_frame_evt = Avm2Event::new("enterFrame", Avm2EventData::Empty);
+        enter_frame_evt.set_bubbles(false);
+        enter_frame_evt.set_cancelable(false);
+
+        let dobject_constr = context.avm2.classes().display_object;
+
+        if let Err(e) = Avm2::broadcast_event(context, enter_frame_evt, dobject_constr) {
+            log::error!(
+                "Encountered AVM2 error when broadcasting enterFrame event: {}",
+                e
+            );
+        }
+    }
+
     fn construct_frame(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
         for child in self.iter_render_list() {
             child.construct_frame(context);

core/src/frame_lifecycle.rs

@@ -0,0 +1,157 @@
+//! Frame events management
+//!
+//! This module aids in keeping track of which frame execution phase we are in.
+//!
+//! For AVM2 code, display objects execute a series of discrete phases, and
+//! each object is notified about the current frame phase in rendering order.
+//! When objects are created, they are 'caught up' to the current frame phase
+//! to ensure correct order of operations.
+//!
+//! AVM1 code (presumably, either on an AVM1 stage or within an `AVM1Movie`)
+//! runs in one phase, with timeline operations executing with all phases
+//! inline in the order that clips were originally created.
+
+use crate::context::UpdateContext;
+use crate::display_object::{DisplayObject, TDisplayObject};
+use crate::vminterface::AvmType;
+
+/// Which phase of the frame we're currently in.
+///
+/// AVM2 frames exist in one of five phases: `Enter`, `Construct`, `Update`,
+/// `FrameScripts`, or `Exit`. An additional `Idle` phase covers rendering and
+/// event processing.
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+pub enum FramePhase {
+    /// We're entering the next frame.
+    ///
+    /// When movie clips enter a new frame, they must do two things:
+    ///
+    ///  - Remove all children that should not exist on the next frame.
+    ///  - Increment their current frame number.
+    ///
+    /// Once this phase ends, we fire `enterFrame` on the broadcast list.
+    Enter,
+
+    /// We're constructing children of existing display objects.
+    ///
+    /// All `PlaceObject` tags should execute at this time.
+    ///
+    /// Once we construct the frame, we fire `frameConstructed` on the
+    /// broadcast list.
+    Construct,
+
+    /// We're updating all display objects on the stage.
+    ///
+    /// This roughly corresponds to `run_frame`; and should encompass all time
+    /// based display object changes that are not encompassed by the other
+    /// phases.
+    ///
+    /// This frame phase also exists in AVM1 frames. In AVM1, it does the work
+    /// of `Enter`, `FrameScripts` (`DoAction` tags), and `Construct`.
+    Update,
+
+    /// We're running all queued frame scripts.
+    ///
+    /// Frame scripts are the AS3 equivalent of old-style `DoAction` tags. They
+    /// are queued in the `Update` phase if the current timeline frame number
+    /// differs from the prior frame's one.
+    FrameScripts,
+
+    /// We're finishing frame processing.
+    ///
+    /// When we exit a completed frame, we fire `exitFrame` on the broadcast
+    /// list.
+    Exit,
+
+    /// We're not currently executing any frame code.
+    ///
+    /// At this point in time, event handlers are expected to run. No frame
+    /// catch-up work should execute.
+    Idle,
+}
+
+impl Default for FramePhase {
+    fn default() -> Self {
+        FramePhase::Idle
+    }
+}
+
+/// Run one frame according to AVM1 frame order.
+pub fn run_all_phases_avm1<'gc>(context: &mut UpdateContext<'_, 'gc, '_>) {
+    // In AVM1, we only ever execute the update phase, and all the work that
+    // would ordinarily be phased is instead run all at once in whatever order
+    // the SWF requests it.
+    *context.frame_phase = FramePhase::Update;
+
+    // AVM1 execution order is determined by the global execution list, based on instantiation order.
+    for clip in context.avm1.clip_exec_iter() {
+        if clip.removed() {
+            // Clean up removed objects from this frame or a previous frame.
+            // Can be safely removed while iterating here, because the iterator advances
+            // to the next node before returning the current node.
+            context.avm1.remove_from_exec_list(context.gc_context, clip);
+        } else {
+            clip.run_frame(context);
+        }
+    }
+
+    // Fire "onLoadInit" events.
+    context
+        .load_manager
+        .movie_clip_on_load(context.action_queue);
+
+    *context.frame_phase = FramePhase::Idle;
+}
+
+/// Run one frame according to AVM2 frame order.
+pub fn run_all_phases_avm2<'gc>(context: &mut UpdateContext<'_, 'gc, '_>) {
+    let stage = context.stage;
+
+    *context.frame_phase = FramePhase::Enter;
+    stage.enter_frame(context);
+
+    *context.frame_phase = FramePhase::Construct;
+    stage.construct_frame(context);
+    stage.frame_constructed(context);
+
+    *context.frame_phase = FramePhase::Update;
+    stage.run_frame_avm2(context);
+
+    *context.frame_phase = FramePhase::FrameScripts;
+    stage.run_frame_scripts(context);
+
+    *context.frame_phase = FramePhase::Exit;
+    stage.exit_frame(context);
+
+    *context.frame_phase = FramePhase::Idle;
+}
+
+/// Run all previously-executed frame phases on a newly-constructed display
+/// object.
+///
+/// This is a no-op on AVM1, which has it's own catch-up logic.
+pub fn catchup_display_object_to_frame<'gc>(
+    context: &mut UpdateContext<'_, 'gc, '_>,
+    dobj: DisplayObject<'gc>,
+) {
+    match (*context.frame_phase, context.avm_type()) {
+        (_, AvmType::Avm1) => {}
+        //NOTE: We currently do not have test coverage to justify `Enter`
+        //running `construct_frame`. However, `Idle` *does* need frame
+        //construction to happen, because event handlers expect to be able to
+        //construct new movie clips and see their grandchildren. So I suspect
+        //that constructing symbols in `enterFrame` works the same way.
+        (FramePhase::Enter, AvmType::Avm2) | (FramePhase::Construct, AvmType::Avm2) => {
+            dobj.enter_frame(context);
+            dobj.construct_frame(context);
+        }
+        (FramePhase::Update, AvmType::Avm2)
+        | (FramePhase::FrameScripts, AvmType::Avm2)
+        | (FramePhase::Exit, AvmType::Avm2)
+        | (FramePhase::Idle, AvmType::Avm2) => {
+            dobj.enter_frame(context);
+            dobj.construct_frame(context);
+            dobj.run_frame_avm2(context);
+        }
+    }
+}