core/avm2: Minor fix to construction of children that were placed on the first frame

core/src/avm2/globals/flash/display/DisplayObject.as

@@ -17,8 +17,6 @@ package flash.display {
     public class DisplayObject extends EventDispatcher implements IBitmapDrawable {
         private var _accessibilityProperties:AccessibilityProperties;
 
-        public native function DisplayObject();
-
         public function get accessibilityProperties():AccessibilityProperties {
             return this._accessibilityProperties;
         }

core/src/avm2/globals/flash/display/Sprite.as

@@ -6,10 +6,16 @@ package flash.display {
 
     [Ruffle(InstanceAllocator)]
     public class Sprite extends DisplayObjectContainer {
-
         [Ruffle(NativeAccessible)]
         private var _graphics:Graphics;
 
+        public function Sprite() {
+            super();
+            this.constructChildren();
+        }
+
+        private native function constructChildren():void;
+
         public native function get graphics():Graphics;
         public native function get dropTarget():DisplayObject;
         public native function get soundTransform():SoundTransform;

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

@@ -53,35 +53,6 @@ pub fn initialize_for_allocator<'gc>(
     obj.into()
 }
 
-/// Implements `flash.display.DisplayObject`'s native instance constructor.
-pub fn display_object_initializer<'gc>(
-    activation: &mut Activation<'_, 'gc>,
-    this: Value<'gc>,
-    _args: &[Value<'gc>],
-) -> Result<Value<'gc>, Error<'gc>> {
-    // No need to call `super()`, it wouldn't do anything
-
-    let this = this.as_object().unwrap();
-
-    if let Some(dobj) = this.as_display_object() {
-        if let Some(clip) = dobj.as_movie_clip() {
-            clip.set_constructing_frame(true);
-        }
-
-        if let Some(container) = dobj.as_container() {
-            for child in container.iter_render_list() {
-                child.construct_frame(activation.context);
-            }
-        }
-
-        if let Some(clip) = dobj.as_movie_clip() {
-            clip.set_constructing_frame(false);
-        }
-    }
-
-    Ok(Value::Undefined)
-}
-
 /// Implements `alpha`'s getter.
 pub fn get_alpha<'gc>(
     _activation: &mut Activation<'_, 'gc>,

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

@@ -9,7 +9,7 @@ use crate::avm2::globals::slots::{
 use crate::avm2::object::{ClassObject, Object, StageObject, TObject as _};
 use crate::avm2::parameters::ParametersExt;
 use crate::avm2::value::Value;
-use crate::display_object::{MovieClip, SoundTransform, TDisplayObject};
+use crate::display_object::{MovieClip, SoundTransform, TDisplayObject, TDisplayObjectContainer};
 use swf::{Rectangle, Twips};
 
 pub fn sprite_allocator<'gc>(
@@ -58,6 +58,25 @@ pub fn sprite_allocator<'gc>(
     unreachable!("A Sprite subclass should have Sprite in superclass chain");
 }
 
+pub fn construct_children<'gc>(
+    activation: &mut Activation<'_, 'gc>,
+    this: Value<'gc>,
+    _args: &[Value<'gc>],
+) -> Result<Value<'gc>, Error<'gc>> {
+    let this = this.as_object().unwrap();
+    let this = this.as_display_object().unwrap();
+    let clip = this.as_movie_clip().unwrap();
+
+    // Construct children of this Sprite
+    clip.set_constructing_frame(true);
+    for child in clip.iter_render_list() {
+        child.construct_frame(activation.context);
+    }
+    clip.set_constructing_frame(false);
+
+    Ok(Value::Undefined)
+}
+
 /// Implements `dropTarget`'s getter
 pub fn get_drop_target<'gc>(
     _activation: &mut Activation<'_, 'gc>,

core/src/display_object.rs

@@ -2,13 +2,11 @@ use crate::avm1::{
     ActivationIdentifier as Avm1ActivationIdentifier, Object as Avm1Object, Value as Avm1Value,
 };
 use crate::avm2::{
-    Activation as Avm2Activation, Avm2, Error as Avm2Error, EventObject as Avm2EventObject,
-    LoaderInfoObject, Multiname as Avm2Multiname, Object as Avm2Object,
-    StageObject as Avm2StageObject, TObject as _, Value as Avm2Value,
+    Activation as Avm2Activation, Error as Avm2Error, LoaderInfoObject, Multiname as Avm2Multiname,
+    Object as Avm2Object, StageObject as Avm2StageObject, TObject as _, Value as Avm2Value,
 };
 use crate::context::{RenderContext, UpdateContext};
 use crate::drawing::Drawing;
-use crate::loader::LoadManager;
 use crate::prelude::*;
 use crate::string::{AvmString, WString};
 use crate::tag_utils::SwfMovie;
@@ -796,6 +794,14 @@ impl<'gc> DisplayObjectBase<'gc> {
         self.set_flag(DisplayObjectFlags::PLACED_BY_AVM2_SCRIPT, value);
     }
 
+    fn frame_construct_skipped(&self) -> bool {
+        self.contains_flag(DisplayObjectFlags::FRAME_CONSTRUCT_SKIPPED)
+    }
+
+    fn set_frame_construct_skipped(&self, value: bool) {
+        self.set_flag(DisplayObjectFlags::FRAME_CONSTRUCT_SKIPPED, value);
+    }
+
     fn is_bitmap_cached_preference(&self) -> bool {
         self.contains_flag(DisplayObjectFlags::CACHE_AS_BITMAP)
     }
@@ -2194,6 +2200,18 @@ pub trait TDisplayObject<'gc>:
         self.base().set_placed_by_avm2_script(value)
     }
 
+    #[no_dynamic]
+    fn frame_construct_skipped(&self) -> bool {
+        self.base().frame_construct_skipped()
+    }
+
+    /// When this flag is set, the object will not be instantiated in-line with
+    /// normal frame construction by `MovieClip::construct_frame`.
+    #[no_dynamic]
+    fn set_frame_construct_skipped(&self, value: bool) {
+        self.base().set_frame_construct_skipped(value);
+    }
+
     /// Whether this display object has been instantiated by the timeline.
     /// When this flag is set, attempts to change the object's name from AVM2
     /// throw an exception.
@@ -2344,16 +2362,6 @@ pub trait TDisplayObject<'gc>:
         }
     }
 
-    /// Emit a `frameConstructed` event on this DisplayObject and any children it
-    /// may have.
-    #[no_dynamic]
-    fn frame_constructed(self, context: &mut UpdateContext<'gc>) {
-        let frame_constructed_evt =
-            Avm2EventObject::bare_default_event(context, "frameConstructed");
-        let dobject_constr = context.avm2.classes().display_object;
-        Avm2::broadcast_event(context, frame_constructed_evt, dobject_constr);
-    }
-
     /// Run any frame scripts (if they exist and this object needs to run them).
     fn run_frame_scripts(self, context: &mut UpdateContext<'gc>) {
         if let Some(container) = self.as_container() {
@@ -2363,16 +2371,6 @@ pub trait TDisplayObject<'gc>:
         }
     }
 
-    /// Emit an `exitFrame` broadcast event.
-    #[no_dynamic]
-    fn exit_frame(self, context: &mut UpdateContext<'gc>) {
-        let exit_frame_evt = Avm2EventObject::bare_default_event(context, "exitFrame");
-        let dobject_constr = context.avm2.classes().display_object;
-        Avm2::broadcast_event(context, exit_frame_evt, dobject_constr);
-
-        LoadManager::run_exit_frame(context);
-    }
-
     /// Called before the child is about to be rendered.
     /// Note that this happens even if the child is invisible
     /// (as long as the child is still on a render list)
@@ -2886,7 +2884,7 @@ impl<'gc> DisplayObject<'gc> {
 bitflags! {
     /// Bit flags used by `DisplayObject`.
     #[derive(Clone, Copy)]
-    struct DisplayObjectFlags: u16 {
+    struct DisplayObjectFlags: u32 {
         /// Whether this object has been removed from the display list.
         /// Necessary in AVM1 to throw away queued actions from removed movie clips.
         const AVM1_REMOVED             = 1 << 0;
@@ -2947,6 +2945,16 @@ bitflags! {
         /// i.e. attachMovie, createEmptyMovieClip, duplicateMovieClip.
         // TODO [KJ] Can this be merged with PLACED_BY_AVM2_SCRIPT?
         const PLACED_BY_AVM1_SCRIPT    = 1 << 15;
+
+        /// Whether this object was placed by the timeline on a `MovieClip`
+        /// before the `MovieClip` had its AVM2 object constructed. Such objects
+        /// are only instantiated by `Sprite.constructChildren`, which is
+        /// usually called when `super()` is called in a `Sprite` subclass.
+        /// However, if `super()` (and therefore `Sprite.constructChildren()`)
+        /// is never called, the object will never be instantiated. We mark all
+        /// objects placed by the timeline on a load frame with this flag to
+        /// ensure that `MovieClip::construct_frame` does not instantiate them.
+        const FRAME_CONSTRUCT_SKIPPED  = 1 << 16;
     }
 }
 

core/src/display_object/avm2_button.rs

@@ -14,7 +14,9 @@ use crate::display_object::container::{dispatch_added_event, dispatch_removed_ev
 use crate::display_object::interactive::{InteractiveObjectBase, TInteractiveObject};
 use crate::display_object::{DisplayObjectBase, MovieClip};
 use crate::events::{ClipEvent, ClipEventResult};
-use crate::frame_lifecycle::catchup_display_object_to_frame;
+use crate::frame_lifecycle::{
+    broadcast_frame_constructed, broadcast_frame_exited, catchup_display_object_to_frame,
+};
 use crate::prelude::*;
 use crate::tag_utils::{SwfMovie, SwfSlice};
 use crate::vminterface::Instantiator;
@@ -363,15 +365,17 @@ impl<'gc> Avm2Button<'gc> {
                 dispatch_removed_event(old_state_child, context);
             }
 
-            if let Some(child) = child {
-                child.frame_constructed(context);
+            // FIXME is this correct?
+            if child.is_some() {
+                broadcast_frame_constructed(context);
             }
         }
 
         if is_cur_state {
             if let Some(child) = child {
                 child.run_frame_scripts(context);
-                child.exit_frame(context);
+                // FIXME is this correct?
+                broadcast_frame_exited(context);
             }
         }
     }
@@ -527,10 +531,10 @@ impl<'gc> TDisplayObject<'gc> for Avm2Button<'gc> {
 
                     let stage = context.stage;
                     stage.construct_frame(context);
-                    stage.frame_constructed(context);
+                    broadcast_frame_constructed(context);
                     self.set_state(context, ButtonState::Up);
                     stage.run_frame_scripts(context);
-                    stage.exit_frame(context);
+                    broadcast_frame_exited(context);
                 }
 
                 if let Some(avm2_object) = self.0.object.get() {

core/src/display_object/movie_clip.rs

@@ -1409,6 +1409,12 @@ impl<'gc> MovieClip<'gc> {
                     child.set_parent(context, Some(self.into()));
                     child.set_place_frame(self.current_frame());
 
+                    // If this MC hasn't had an `object2` allocated yet, this
+                    // child will be constructed by `Sprite.constructChildren`,
+                    // not by the timeline
+                    let has_object2_allocated = self.object2().is_none();
+                    child.set_frame_construct_skipped(has_object2_allocated);
+
                     // Apply PlaceObject parameters.
                     child.apply_place_object(context, place_object);
                     if let Some(name) = &place_object.name {
@@ -2475,7 +2481,7 @@ impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> {
                 self.construct_as_avm2_object(context);
                 self.on_construction_complete(context);
                 // If we're in the load frame and we were constructed by ActionScript,
-                // then we want to wait for the DisplayObject constructor to run
+                // then we want to wait for the Sprite constructor to run
                 // 'construct_frame' on children. This is observable by ActionScript -
                 // before calling super(), 'this.numChildren' will show a non-zero number
                 // when we have children placed on the load frame, but 'this.getChildAt(0)'
@@ -2485,12 +2491,28 @@ impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> {
                 let running_construct_frame = self
                     .0
                     .contains_flag(MovieClipFlags::RUNNING_CONSTRUCT_FRAME);
-                // The supercall constructor for display objects is responsible
-                // for triggering construct_frame on frame 1.
                 for child in self.iter_render_list() {
-                    if running_construct_frame && child.object2().is_none() {
-                        continue;
+                    // Under some conditions, we won't run `construct_frame` on
+                    // a not-yet-constructed child
+                    if child.object2().is_none() {
+                        // Avoid running recursively- if `Sprite.constructChildren`
+                        // was constructing this clip's children, and somehow
+                        // a child's construction triggered another `construct_frame`
+                        // on this clip, Flash avoids running `construct_frame` on
+                        // the non-constructed children of this clip.
+                        if running_construct_frame {
+                            continue;
+                        }
+
+                        // The supercall constructor for display objects is responsible
+                        // for triggering construct_frame on frame 1 (see above comment).
+                        // However, if the child has already been constructed, we run
+                        // `construct_frame` like normal.
+                        if child.frame_construct_skipped() {
+                            continue;
+                        }
                     }
+
                     child.construct_frame(context);
                 }
             }

core/src/frame_lifecycle.rs

@@ -11,9 +11,11 @@
 //! runs in one phase, with timeline operations executing with all phases
 //! inline in the order that clips were originally created.
 
+use crate::avm2::{Avm2, EventObject};
 use crate::avm2_stub_method_context;
 use crate::context::UpdateContext;
 use crate::display_object::{DisplayObject, MovieClip, TDisplayObject};
+use crate::loader::LoadManager;
 use crate::orphan_manager::OrphanManager;
 use tracing::instrument;
 
@@ -87,7 +89,7 @@ pub fn run_all_phases_avm2(context: &mut UpdateContext<'_>) {
         orphan.construct_frame(context);
     });
     stage.construct_frame(context);
-    stage.frame_constructed(context);
+    broadcast_frame_constructed(context);
 
     *context.frame_phase = FramePhase::FrameScripts;
     OrphanManager::each_orphan_obj(context, |orphan, context| {
@@ -97,7 +99,7 @@ pub fn run_all_phases_avm2(context: &mut UpdateContext<'_>) {
     MovieClip::run_frame_script_cleanup(context);
 
     *context.frame_phase = FramePhase::Exit;
-    stage.exit_frame(context);
+    broadcast_frame_exited(context);
 
     // We cannot easily remove dead `GcWeak` instances from the orphan list
     // inside `each_orphan_movie`, since the callback may modify the orphan list.
@@ -151,7 +153,7 @@ pub fn run_inner_goto_frame<'gc>(
         orphan.construct_frame(context);
     });
     stage.construct_frame(context);
-    stage.frame_constructed(context);
+    broadcast_frame_constructed(context);
 
     *context.frame_phase = FramePhase::FrameScripts;
     stage.run_frame_scripts(context);
@@ -164,7 +166,7 @@ pub fn run_inner_goto_frame<'gc>(
     }
 
     *context.frame_phase = FramePhase::Exit;
-    stage.exit_frame(context);
+    broadcast_frame_exited(context);
 
     // We cannot easily remove dead `GcWeak` instances from the orphan list
     // inside `each_orphan_movie`, since the callback may modify the orphan list.
@@ -176,6 +178,22 @@ pub fn run_inner_goto_frame<'gc>(
     *context.frame_phase = old_phase;
 }
 
+/// Broadcast a `frameConstructed` event to all `DisplayObject`s.
+pub fn broadcast_frame_constructed<'gc>(context: &mut UpdateContext<'gc>) {
+    let frame_constructed_evt = EventObject::bare_default_event(context, "frameConstructed");
+    let dobject_constr = context.avm2.classes().display_object;
+    Avm2::broadcast_event(context, frame_constructed_evt, dobject_constr);
+}
+
+/// Broadcast a `exitFrame` event to all `DisplayObject`s.
+pub fn broadcast_frame_exited<'gc>(context: &mut UpdateContext<'gc>) {
+    let exit_frame_evt = EventObject::bare_default_event(context, "exitFrame");
+    let dobject_constr = context.avm2.classes().display_object;
+    Avm2::broadcast_event(context, exit_frame_evt, dobject_constr);
+
+    LoadManager::run_exit_frame(context);
+}
+
 /// Run all previously-executed frame phases on a newly-constructed display
 /// object.
 ///

tests/tests/swfs/avm2/movieclip_frameconstruct_skipped/output.txt

@@ -0,0 +1,9 @@
+Main allocated
+Frame constructed (theSprite=null, theShape=null)
+Frame exited
+Frame entered
+Frame constructed (theSprite=null, theShape=null)
+Frame exited
+Frame entered
+Frame constructed (theSprite=null, theShape=null)
+Frame exited

tests/tests/swfs/avm2/movieclip_frameconstruct_skipped/test.toml

@@ -0,0 +1 @@
+num_frames = 3