core: Improve execution order of AS

core/src/avm1.rs

@@ -62,6 +62,7 @@ pub struct ActionContext<'a, 'gc, 'gc_context> {
     pub target_path: Value<'gc>,
 
     pub rng: &'a mut SmallRng,
+    pub action_queue: &'a mut crate::player::ActionQueue<'gc>,
     pub audio: &'a mut dyn crate::backend::audio::AudioBackend,
     pub navigator: &'a mut dyn crate::backend::navigator::NavigatorBackend,
 }
@@ -1130,7 +1131,12 @@ impl<'gc> Avm1<'gc> {
             let mut display_object = clip.write(context.gc_context);
             if let Some(clip) = display_object.as_movie_clip_mut() {
                 // The frame on the stack is 0-based, not 1-based.
-                clip.goto_frame(frame + 1, true);
+                clip.goto_frame(
+                    context.active_clip,
+                    &mut context.action_queue,
+                    frame + 1,
+                    true,
+                );
             } else {
                 log::error!("GotoFrame failed: Target is not a MovieClip");
             }
@@ -1154,11 +1160,21 @@ impl<'gc> Avm1<'gc> {
                 match self.pop()? {
                     Value::Number(frame) => {
                         // The frame on the stack is 1-based, not 0-based.
-                        clip.goto_frame(scene_offset + (frame as u16), !set_playing)
+                        clip.goto_frame(
+                            context.active_clip,
+                            &mut context.action_queue,
+                            scene_offset + (frame as u16),
+                            !set_playing,
+                        )
                     }
                     Value::String(frame_label) => {
                         if let Some(frame) = clip.frame_label_to_number(&frame_label) {
-                            clip.goto_frame(scene_offset + frame, !set_playing)
+                            clip.goto_frame(
+                                context.active_clip,
+                                &mut context.action_queue,
+                                scene_offset + frame,
+                                !set_playing,
+                            )
                         } else {
                             log::warn!(
                                 "GotoFrame2: MovieClip {} does not contain frame label '{}'",
@@ -1183,7 +1199,7 @@ impl<'gc> Avm1<'gc> {
             let mut display_object = clip.write(context.gc_context);
             if let Some(clip) = display_object.as_movie_clip_mut() {
                 if let Some(frame) = clip.frame_label_to_number(label) {
-                    clip.goto_frame(frame, true);
+                    clip.goto_frame(context.active_clip, &mut context.action_queue, frame, true);
                 } else {
                     log::warn!("GoToLabel: Frame label '{}' not found", label);
                 }
@@ -1340,7 +1356,7 @@ impl<'gc> Avm1<'gc> {
         if let Some(clip) = context.target_clip {
             let mut display_object = clip.write(context.gc_context);
             if let Some(clip) = display_object.as_movie_clip_mut() {
-                clip.next_frame();
+                clip.next_frame(context.active_clip, context.action_queue);
             } else {
                 log::warn!("NextFrame: Target is not a MovieClip");
             }
@@ -1397,7 +1413,7 @@ impl<'gc> Avm1<'gc> {
         if let Some(clip) = context.target_clip {
             let mut display_object = clip.write(context.gc_context);
             if let Some(clip) = display_object.as_movie_clip_mut() {
-                clip.prev_frame();
+                clip.prev_frame(context.active_clip, context.action_queue);
             } else {
                 log::warn!("PrevFrame: Target is not a MovieClip");
             }

core/src/avm1/movie_clip.rs

@@ -1,5 +1,6 @@
 use crate::avm1::object::{Attribute::*, Object};
 use crate::avm1::{ActionContext, Avm1, Value};
+use crate::display_object::DisplayNode;
 use crate::movie_clip::MovieClip;
 use enumset::EnumSet;
 use gc_arena::{GcCell, MutationContext};
@@ -29,14 +30,14 @@ macro_rules! with_movie_clip_mut {
         $(
             $object.force_set_function(
                 $name,
-                |_avm, context, this, args| -> Value<'gc> {
+                |_avm, context: &mut ActionContext<'_, 'gc, '_>, this, args| -> Value<'gc> {
                     if let Some(display_object) = this.read().display_node() {
                         if let Some(movie_clip) = display_object.write(context.gc_context).as_movie_clip_mut() {
-                            return $fn(movie_clip, args);
+                            return $fn(movie_clip, context, display_object, args);
                         }
                     }
                     Value::Undefined
-                },
+                } as crate::avm1::function::NativeFunction<'gc>,
                 $gc_context,
                 DontDelete | ReadOnly | DontEnum,
             );
@@ -82,19 +83,19 @@ pub fn create_movie_object<'gc>(gc_context: MutationContext<'gc, '_>) -> Object<
     with_movie_clip_mut!(
         gc_context,
         object,
-        "nextFrame" => |movie_clip: &mut MovieClip, _args| {
+        "nextFrame" => |movie_clip: &mut MovieClip<'gc>, context: &mut ActionContext<'_, 'gc, '_>, cell: DisplayNode<'gc>, _args| {
-            movie_clip.next_frame();
+            movie_clip.next_frame(cell, &mut context.action_queue);
             Value::Undefined
         },
-        "prevFrame" => |movie_clip: &mut MovieClip, _args| {
+        "prevFrame" =>  |movie_clip: &mut MovieClip<'gc>, context: &mut ActionContext<'_, 'gc, '_>, cell: DisplayNode<'gc>, _args| {
-            movie_clip.prev_frame();
+            movie_clip.prev_frame(cell, &mut context.action_queue);
             Value::Undefined
         },
-        "play" => |movie_clip: &mut MovieClip, _args| {
+        "play" => |movie_clip: &mut MovieClip<'gc>, _context: &mut ActionContext<'_, 'gc, '_>, _cell: DisplayNode<'gc>, _args| {
             movie_clip.play();
             Value::Undefined
         },
-        "stop" => |movie_clip: &mut MovieClip, _args| {
+        "stop" => |movie_clip: &mut MovieClip<'gc>, _context: &mut ActionContext<'_, 'gc, '_>, _cell: DisplayNode<'gc>, _args| {
             movie_clip.stop();
             Value::Undefined
         }
@@ -103,11 +104,11 @@ pub fn create_movie_object<'gc>(gc_context: MutationContext<'gc, '_>) -> Object<
     with_movie_clip!(
         gc_context,
         object,
-        "getBytesLoaded" => |_movie_clip: &MovieClip, _args| {
+        "getBytesLoaded" => |_movie_clip: &MovieClip<'gc>, _args| {
             // TODO find a correct value
             Value::Number(1.0)
         },
-        "getBytesTotal" => |_movie_clip: &MovieClip, _args| {
+        "getBytesTotal" => |_movie_clip: &MovieClip<'gc>, _args| {
             // TODO find a correct value
             Value::Number(1.0)
         }

core/src/avm1/object.rs

@@ -401,6 +401,7 @@ mod tests {
                 target_clip: Some(root),
                 target_path: Value::Undefined,
                 rng: &mut SmallRng::from_seed([0u8; 16]),
+                action_queue: &mut crate::player::ActionQueue::new(),
                 audio: &mut NullAudioBackend::new(),
                 navigator: &mut NullNavigatorBackend::new(),
             };

core/src/avm1/test_utils.rs

@@ -30,6 +30,7 @@ where
             target_path: Value::Undefined,
             rng: &mut SmallRng::from_seed([0u8; 16]),
             audio: &mut NullAudioBackend::new(),
+            action_queue: &mut crate::player::ActionQueue::new(),
             navigator: &mut NullNavigatorBackend::new(),
         };
 

core/src/button.rs

@@ -137,7 +137,9 @@ impl<'gc> Button<'gc> {
             for action in &self.static_data.actions {
                 if action.condition == condition && action.key_code == key_code {
                     // Note that AVM1 buttons run actions relative to their parent, not themselves.
-                    context.actions.push((parent, action.action_data.clone()));
+                    context
+                        .action_queue
+                        .queue_actions(parent, action.action_data.clone());
                 }
             }
         }
@@ -190,13 +192,6 @@ impl<'gc> DisplayObject<'gc> for Button<'gc> {
         }
     }
 
-    fn run_post_frame(&mut self, context: &mut UpdateContext<'_, 'gc, '_>) {
-        for child in self.children.values_mut() {
-            context.active_clip = *child;
-            child.write(context.gc_context).run_post_frame(context);
-        }
-    }
-
     fn render(&self, context: &mut RenderContext<'_, 'gc>) {
         context.transform_stack.push(self.transform());
 

core/src/display_object.rs

@@ -14,6 +14,20 @@ pub struct DisplayObjectBase<'gc> {
     transform: Transform,
     name: String,
     clip_depth: Depth,
+
+    /// The first child of this display object in order of execution.
+    /// This is differen than render order.
+    first_child: Option<DisplayNode<'gc>>,
+
+    /// The previous sibling of this display object in order of execution.
+    prev_sibling: Option<DisplayNode<'gc>>,
+
+    /// The next sibling of this display object in order of execution.
+    next_sibling: Option<DisplayNode<'gc>>,
+
+    /// Whether this child has been removed from the display list.
+    /// Necessary in AVM1 to throw away queued actions from removed movie clips.
+    removed: bool,
 }
 
 impl<'gc> Default for DisplayObjectBase<'gc> {
@@ -25,6 +39,10 @@ impl<'gc> Default for DisplayObjectBase<'gc> {
             transform: Default::default(),
             name: Default::default(),
             clip_depth: Default::default(),
+            first_child: None,
+            prev_sibling: None,
+            next_sibling: None,
+            removed: false,
         }
     }
 }
@@ -79,6 +97,30 @@ impl<'gc> DisplayObject<'gc> for DisplayObjectBase<'gc> {
     fn set_parent(&mut self, parent: Option<DisplayNode<'gc>>) {
         self.parent = parent;
     }
+    fn first_child(&self) -> Option<DisplayNode<'gc>> {
+        self.first_child
+    }
+    fn set_first_child(&mut self, node: Option<DisplayNode<'gc>>) {
+        self.first_child = node;
+    }
+    fn prev_sibling(&self) -> Option<DisplayNode<'gc>> {
+        self.prev_sibling
+    }
+    fn set_prev_sibling(&mut self, node: Option<DisplayNode<'gc>>) {
+        self.prev_sibling = node;
+    }
+    fn next_sibling(&self) -> Option<DisplayNode<'gc>> {
+        self.next_sibling
+    }
+    fn set_next_sibling(&mut self, node: Option<DisplayNode<'gc>>) {
+        self.next_sibling = node;
+    }
+    fn removed(&self) -> bool {
+        self.removed
+    }
+    fn set_removed(&mut self, removed: bool) {
+        self.removed = removed;
+    }
     fn box_clone(&self) -> Box<dyn DisplayObject<'gc>> {
         Box::new(self.clone())
     }
@@ -109,9 +151,22 @@ pub trait DisplayObject<'gc>: 'gc + Collect + Debug {
     fn set_clip_depth(&mut self, depth: Depth);
     fn parent(&self) -> Option<DisplayNode<'gc>>;
     fn set_parent(&mut self, parent: Option<DisplayNode<'gc>>);
-
+    fn first_child(&self) -> Option<DisplayNode<'gc>>;
+    fn set_first_child(&mut self, node: Option<DisplayNode<'gc>>);
+    fn prev_sibling(&self) -> Option<DisplayNode<'gc>>;
+    fn set_prev_sibling(&mut self, node: Option<DisplayNode<'gc>>);
+    fn next_sibling(&self) -> Option<DisplayNode<'gc>>;
+    fn set_next_sibling(&mut self, node: Option<DisplayNode<'gc>>);
+    /// Iterates over the children of this display object in execution order.
+    /// This is different than render order.
+    fn children(&self) -> ChildIter<'gc> {
+        ChildIter {
+            cur_child: self.first_child(),
+        }
+    }
+    fn removed(&self) -> bool;
+    fn set_removed(&mut self, removed: bool);
     fn run_frame(&mut self, _context: &mut UpdateContext<'_, 'gc, '_>) {}
-    fn run_post_frame(&mut self, _context: &mut UpdateContext<'_, 'gc, '_>) {}
     fn render(&self, _context: &mut RenderContext<'_, 'gc>) {}
 
     fn as_button(&self) -> Option<&crate::button::Button<'gc>> {
@@ -132,15 +187,15 @@ pub trait DisplayObject<'gc>: 'gc + Collect + Debug {
     fn as_morph_shape_mut(&mut self) -> Option<&mut crate::morph_shape::MorphShape<'gc>> {
         None
     }
-    fn apply_place_object(&mut self, place_object: swf::PlaceObject) {
+    fn apply_place_object(&mut self, place_object: &swf::PlaceObject) {
-        if let Some(matrix) = place_object.matrix {
+        if let Some(matrix) = &place_object.matrix {
-            self.set_matrix(&matrix.into());
+            self.set_matrix(&matrix.clone().into());
         }
-        if let Some(color_transform) = place_object.color_transform {
+        if let Some(color_transform) = &place_object.color_transform {
-            self.set_color_transform(&color_transform.into());
+            self.set_color_transform(&color_transform.clone().into());
         }
-        if let Some(name) = place_object.name {
+        if let Some(name) = &place_object.name {
-            self.set_name(&name);
+            self.set_name(name);
         }
         if let Some(clip_depth) = place_object.clip_depth {
             self.set_clip_depth(clip_depth);
@@ -251,6 +306,30 @@ macro_rules! impl_display_object {
         fn set_parent(&mut self, parent: Option<crate::display_object::DisplayNode<'gc>>) {
             self.$field.set_parent(parent)
         }
+        fn first_child(&self) -> Option<DisplayNode<'gc>> {
+            self.$field.first_child()
+        }
+        fn set_first_child(&mut self, node: Option<DisplayNode<'gc>>) {
+            self.$field.set_first_child(node);
+        }
+        fn prev_sibling(&self) -> Option<DisplayNode<'gc>> {
+            self.$field.prev_sibling()
+        }
+        fn set_prev_sibling(&mut self, node: Option<DisplayNode<'gc>>) {
+            self.$field.set_prev_sibling(node);
+        }
+        fn next_sibling(&self) -> Option<DisplayNode<'gc>> {
+            self.$field.next_sibling()
+        }
+        fn set_next_sibling(&mut self, node: Option<DisplayNode<'gc>>) {
+            self.$field.set_next_sibling(node);
+        }
+        fn removed(&self) -> bool {
+            self.$field.removed()
+        }
+        fn set_removed(&mut self, value: bool) {
+            self.$field.set_removed(value)
+        }
         fn box_clone(&self) -> Box<dyn crate::display_object::DisplayObject<'gc>> {
             Box::new(self.clone())
         }
@@ -301,3 +380,18 @@ pub fn render_children<'gc>(
 /// TODO(Herschel): The extra Box here is necessary to hold the trait object inside a GC pointer,
 /// but this is an extra allocation... Can we avoid this, maybe with a DST?
 pub type DisplayNode<'gc> = GcCell<'gc, Box<dyn DisplayObject<'gc>>>;
+
+pub struct ChildIter<'gc> {
+    cur_child: Option<DisplayNode<'gc>>,
+}
+
+impl<'gc> Iterator for ChildIter<'gc> {
+    type Item = DisplayNode<'gc>;
+    fn next(&mut self) -> Option<Self::Item> {
+        let cur = self.cur_child;
+        self.cur_child = self
+            .cur_child
+            .and_then(|display_cell| display_cell.read().next_sibling());
+        cur
+    }
+}

core/src/movie_clip.rs

@@ -8,7 +8,7 @@ use crate::font::Font;
 use crate::graphic::Graphic;
 use crate::matrix::Matrix;
 use crate::morph_shape::MorphShapeStatic;
-use crate::player::{RenderContext, UpdateContext};
+use crate::player::{ActionQueue, RenderContext, UpdateContext};
 use crate::prelude::*;
 use crate::tag_utils::{self, DecodeResult, SwfStream};
 use crate::text::Text;
@@ -26,7 +26,6 @@ pub struct MovieClip<'gc> {
     static_data: Gc<'gc, MovieClipStatic>,
     tag_stream_pos: u64,
     is_playing: bool,
-    goto_queue: Vec<FrameNumber>,
     current_frame: FrameNumber,
     audio_stream: Option<AudioStreamHandle>,
     children: BTreeMap<Depth, DisplayNode<'gc>>,
@@ -41,7 +40,6 @@ impl<'gc> MovieClip<'gc> {
             static_data: Gc::allocate(gc_context, MovieClipStatic::default()),
             tag_stream_pos: 0,
             is_playing: false,
-            goto_queue: Vec::new(),
             current_frame: 0,
             audio_stream: None,
             children: BTreeMap::new(),
@@ -73,7 +71,6 @@ impl<'gc> MovieClip<'gc> {
             ),
             tag_stream_pos: 0,
             is_playing: true,
-            goto_queue: Vec::new(),
             current_frame: 0,
             audio_stream: None,
             children: BTreeMap::new(),
@@ -85,9 +82,9 @@ impl<'gc> MovieClip<'gc> {
         self.is_playing
     }
 
-    pub fn next_frame(&mut self) {
+    pub fn next_frame(&mut self, self_cell: DisplayNode<'gc>, action_queue: &mut ActionQueue<'gc>) {
         if self.current_frame() < self.total_frames() {
-            self.goto_frame(self.current_frame + 1, true);
+            self.goto_frame(self_cell, action_queue, self.current_frame + 1, true);
         }
     }
 
@@ -98,9 +95,9 @@ impl<'gc> MovieClip<'gc> {
         }
     }
 
-    pub fn prev_frame(&mut self) {
+    pub fn prev_frame(&mut self, self_cell: DisplayNode<'gc>, action_queue: &mut ActionQueue<'gc>) {
         if self.current_frame > 1 {
-            self.goto_frame(self.current_frame - 1, true);
+            self.goto_frame(self_cell, action_queue, self.current_frame - 1, true);
         }
     }
 
@@ -110,9 +107,15 @@ impl<'gc> MovieClip<'gc> {
 
     /// Queues up a goto to the specified frame.
     /// `frame` should be 1-based.
-    pub fn goto_frame(&mut self, frame: FrameNumber, stop: bool) {
+    pub fn goto_frame(
+        &mut self,
+        self_cell: DisplayNode<'gc>,
+        action_queue: &mut ActionQueue<'gc>,
+        frame: FrameNumber,
+        stop: bool,
+    ) {
         if frame != self.current_frame {
-            self.goto_queue.push(frame);
+            action_queue.queue_goto(self_cell, frame);
         }
 
         if stop {
@@ -203,19 +206,6 @@ impl<'gc> MovieClip<'gc> {
         self.static_data.frame_labels.get(frame_label).copied()
     }
 
-    pub fn run_goto_queue(&mut self, context: &mut UpdateContext<'_, 'gc, '_>) {
-        let mut i = 0;
-        while i < self.goto_queue.len() {
-            let frame = self.goto_queue[i];
-            if self.current_frame != frame {
-                self.run_goto(context, frame);
-            }
-            i += 1;
-        }
-
-        self.goto_queue.clear();
-    }
-
     fn tag_stream_start(&self) -> u64 {
         self.static_data.tag_stream_start
     }
@@ -235,6 +225,7 @@ impl<'gc> MovieClip<'gc> {
         cursor.set_position(self.tag_stream_pos);
         swf::read::Reader::new(cursor, context.swf_version)
     }
+
     fn run_frame_internal(
         &mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
@@ -292,13 +283,20 @@ impl<'gc> MovieClip<'gc> {
         depth: Depth,
         copy_previous_properties: bool,
     ) -> Option<DisplayNode<'gc>> {
-        if let Ok(child) = context
+        if let Ok(child_cell) = context
             .library
             .instantiate_display_object(id, context.gc_context)
         {
-            let prev_child = self.children.insert(depth, child);
+            // Remove previous child from children list,
+            // and add new childonto front of the list.
+            let prev_child = self.children.insert(depth, child_cell);
+            if let Some(prev_child) = prev_child {
+                self.remove_child_from_exec_list(context.gc_context, prev_child);
+            }
+            self.add_child_to_exec_list(context.gc_context, child_cell);
             {
-                let mut child = child.write(context.gc_context);
+                let mut child = child_cell.write(context.gc_context);
+                // Set initial properties for child.
                 child.set_parent(Some(context.active_clip));
                 child.set_place_frame(self.current_frame);
                 if copy_previous_properties {
@@ -306,28 +304,72 @@ impl<'gc> MovieClip<'gc> {
                         child.copy_display_properties_from(prev_child);
                     }
                 }
+                let prev_clip = context.active_clip;
+                // Run first frame.
+                context.active_clip = child_cell;
+                child.run_frame(context);
+                context.active_clip = prev_clip;
             }
-            Some(child)
+            Some(child_cell)
         } else {
             log::error!("Unable to instantiate display node id {}", id);
             None
         }
     }
-
+    /// Adds a child to the front of the execution list.
-    fn run_goto(&mut self, context: &mut UpdateContext<'_, 'gc, '_>, frame: FrameNumber) {
+    /// This does not affect the render list.
+    fn add_child_to_exec_list(
+        &mut self,
+        gc_context: MutationContext<'gc, '_>,
+        child_cell: DisplayNode<'gc>,
+    ) {
+        if let Some(head) = self.first_child() {
+            head.write(gc_context).set_prev_sibling(Some(child_cell));
+            child_cell.write(gc_context).set_next_sibling(Some(head));
+        }
+        self.set_first_child(Some(child_cell));
+    }
+    /// Removes a child from the execution list.
+    /// This does not affect the render list.
+    fn remove_child_from_exec_list(
+        &mut self,
+        gc_context: MutationContext<'gc, '_>,
+        child_cell: DisplayNode<'gc>,
+    ) {
+        let mut child = child_cell.write(gc_context);
+        // Remove from children linked list.
+        let prev = child.prev_sibling();
+        let next = child.next_sibling();
+        if let Some(prev) = prev {
+            prev.write(gc_context).set_next_sibling(next);
+        }
+        if let Some(next) = next {
+            next.write(gc_context).set_prev_sibling(prev);
+        }
+        if let Some(head) = self.first_child() {
+            if GcCell::ptr_eq(head, child_cell) {
+                self.set_first_child(next);
+            }
+        }
+        // Flag child as removed.
+        child.set_removed(true);
+    }
+    pub fn run_goto(&mut self, context: &mut UpdateContext<'_, 'gc, '_>, frame: FrameNumber) {
         // Flash gotos are tricky:
-        // 1) MovieClip timelines are stored as deltas from frame to frame,
+        // 1) Conceptually, a goto should act like the playhead is advancing forward or
-        //    so we have to step through the intermediate frames to goto a target frame.
+        //    backward to a frame.
-        //    For rewinds, this means starting from frame 1.
+        // 2) However, MovieClip timelines are stored as deltas from frame to frame,
-        // 2) Objects that would persist over the goto should not be recreated and destroyed,
+        //    so for rewinds, we must restart to frame 1 and play forward.
-        //    they should keep their properties.
+        // 3) Objects that would persist over the goto conceptually should not be
-        //    Particularly for rewinds, the object should persist if it  was create
+        //    destroyed and recreated; they should keep their properties.
+        //    Particularly for rewinds, the object should persist if it was created
         //      *before* the frame we are going to. (DisplayNode::place_frame).
-        // 3) We want to avoid creating objects just to destroy them if they aren't on
+        // 4) We want to avoid creating objects just to destroy them if they aren't on
-        //    the goto frame, so we should instead aggregate the deltas into a list
+        //    the goto frame, so we should instead aggregate the deltas into a final list
-        //    of commands at the end of the goto, and THEN create the needed objects.
+        //    of commands, and THEN modify the children as necessary.
 
         // This map will maintain a map of depth -> placement commands.
+        // TODO: Move this to UpdateContext to avoid allocations.
         let mut goto_commands = fnv::FnvHashMap::default();
 
         let is_rewind = if frame < self.current_frame() {
@@ -335,6 +377,25 @@ impl<'gc> MovieClip<'gc> {
             // when rewinding.
             self.tag_stream_pos = 0;
             self.current_frame = 0;
+
+            // Remove all display objects that were created after the desination frame.
+            // TODO: We want to do something like self.children.retain here,
+            // but BTreeMap::retain does not exist.
+            let children: smallvec::SmallVec<[_; 16]> = self
+                .children
+                .iter()
+                .filter_map(|(depth, clip)| {
+                    if clip.read().place_frame() > frame {
+                        Some((*depth, *clip))
+                    } else {
+                        None
+                    }
+                })
+                .collect();
+            for (depth, child) in children {
+                self.children.remove(&depth);
+                self.remove_child_from_exec_list(context.gc_context, child);
+            }
             true
         } else {
             false
@@ -343,6 +404,7 @@ impl<'gc> MovieClip<'gc> {
         // Step through the intermediate frames, and aggregate the deltas of each frame.
         let mut frame_pos = self.tag_stream_pos;
         let mut reader = self.reader(context);
+        let gc_context = context.gc_context;
         while self.current_frame < frame {
             self.current_frame += 1;
             frame_pos = reader.get_inner().position();
@@ -362,36 +424,33 @@ impl<'gc> MovieClip<'gc> {
                     self.goto_place_object(reader, tag_len, 4, &mut goto_commands)
                 }
                 TagCode::RemoveObject => {
-                    self.goto_remove_object(reader, 1, &mut goto_commands, is_rewind)
+                    self.goto_remove_object(reader, 1, gc_context, &mut goto_commands, is_rewind)
                 }
                 TagCode::RemoveObject2 => {
-                    self.goto_remove_object(reader, 2, &mut goto_commands, is_rewind)
+                    self.goto_remove_object(reader, 2, gc_context, &mut goto_commands, is_rewind)
                 }
                 _ => Ok(()),
             };
             let _ = tag_utils::decode_tags(&mut reader, tag_callback, TagCode::ShowFrame);
         }
 
-        let prev_active_clip = context.active_clip;
+        // Run the list of goto commands to actually create and update the display objects.
-
+        let run_goto_command =
-        // Run the final list of commands.
+            |clip: &mut MovieClip<'gc>,
-        if is_rewind {
+             context: &mut UpdateContext<'_, 'gc, '_>,
-            // TODO: We want to do something like self.children.retain here,
+             (&depth, params): (&Depth, &GotoPlaceObject)| {
-            // but BTreeMap::retain does not exist.
+                let (was_instantiated, child) = match clip.children.get_mut(&depth).copied() {
-            let mut children = std::mem::replace(&mut self.children, BTreeMap::new());
-            goto_commands.into_iter().for_each(|(depth, params)| {
-                let (was_instantiated, child) = match children.get_mut(&depth).copied() {
                     // For rewinds, if an object was created before the final frame,
                     // it will exist on the final frame as well. Re-use this object
                     // instead of recreating.
-                    Some(prev_child) if prev_child.read().place_frame() <= frame => {
+                    Some(prev_child) => (false, prev_child),
-                        self.children.insert(depth, prev_child);
+                    None => {
-                        (false, prev_child)
+                        if let Some(child) = clip.instantiate_child(
-                    }
+                            context,
-                    _ => {
+                            params.id(),
-                        if let Some(child) =
+                            depth,
-                            self.instantiate_child(context, params.id(), depth, false)
+                            params.modifies_original_item(),
-                        {
+                        ) {
                             (true, child)
                         } else {
                             return;
@@ -400,61 +459,34 @@ impl<'gc> MovieClip<'gc> {
                 };
 
                 // Apply final delta to display pamareters.
-                let child_node = child;
                 let mut child = child.write(context.gc_context);
-                child.apply_place_object(params.place_object);
+                child.apply_place_object(&params.place_object);
                 if was_instantiated {
                     // Set the placement frame for the new object to the frame
                     // it is actually created on.
                     child.set_place_frame(params.frame);
-                    // We must run newly created objects for one frame
-                    // to ensure they place any children objects.
-                    // TODO: This will probably move as our order-of-execution
-                    // becomes more accurate.
-                    context.active_clip = child_node;
-                    child.run_frame(context);
-                    context.active_clip = prev_active_clip;
                 }
-            });
-        } else {
-            goto_commands.into_iter().for_each(|(depth, params)| {
-                let id = params.id();
-                let child = if id != 0 {
-                    if let Some(child) =
-                        self.instantiate_child(context, id, depth, params.modifies_original_item())
-                    {
-                        child
-                    } else {
-                        return;
-                    }
-                } else if let Some(child) = self.children.get_mut(&depth) {
-                    *child
-                } else {
-                    return;
             };
 
-                // Apply final delta to display pamareters.
+        // We have to be sure that queued actions are generated in the same order
-                let child_node = child;
+        // as if the playhead had reached this frame normally.
-                let mut child = child.write(context.gc_context);
+        // First, run frames for children that were created before this frame.
-                child.apply_place_object(params.place_object);
+        goto_commands
-                if id != 0 {
+            .iter()
-                    // Set the placement frame for the new object to the frame
+            .filter(|(_, params)| params.frame < frame)
-                    // it is actually created on.
+            .for_each(|goto| run_goto_command(self, context, goto));
-                    child.set_place_frame(params.frame);
+
-                    // We must run newly created objects for one frame
+        // Next, run the final frame for the parent clip.
-                    // to ensure they place any children objects.
+        // Re-run the final frame without display tags (DoAction, StartSound, etc.)
-                    // TODO: This will probably move as our order-of-execution
-                    // becomes more accurate.
-                    context.active_clip = child_node;
-                    child.run_frame(context);
-                    context.active_clip = prev_active_clip;
-                }
-            });
-        }
-        // Re-run the final frame to run all other tags (DoAction, StartSound, etc.)
         self.current_frame = frame - 1;
         self.tag_stream_pos = frame_pos;
         self.run_frame_internal(context, false);
+
+        // Finally, run frames for children that are placed on this frame.
+        goto_commands
+            .iter()
+            .filter(|(_, params)| params.frame >= frame)
+            .for_each(|goto| run_goto_command(self, context, goto));
     }
 
     /// Handles a PlaceObject tag when running a goto action.
@@ -492,6 +524,7 @@ impl<'gc> MovieClip<'gc> {
         &mut self,
         reader: &mut SwfStream<&'a [u8]>,
         version: u8,
+        gc_context: MutationContext<'gc, '_>,
         goto_commands: &mut fnv::FnvHashMap<Depth, GotoPlaceObject>,
         is_rewind: bool,
     ) -> DecodeResult {
@@ -507,7 +540,9 @@ impl<'gc> MovieClip<'gc> {
             // Don't do this for rewinds, because they conceptually
             // start from an empty display list, and we also want to examine
             // the old children to decide if they persist (place_frame <= goto_frame).
-            self.children.remove(&remove_object.depth);
+            if let Some(child) = self.children.remove(&remove_object.depth) {
+                self.remove_child_from_exec_list(gc_context, child);
+            }
         }
         Ok(())
     }
@@ -521,24 +556,17 @@ impl<'gc> DisplayObject<'gc> for MovieClip<'gc> {
     }
 
     fn run_frame(&mut self, context: &mut UpdateContext<'_, 'gc, '_>) {
-        if self.is_playing {
+        // Children must run first.
-            self.run_frame_internal(context, true);
+        let prev_clip = context.active_clip;
-        }
+        for child in self.children() {
-
+            context.active_clip = child;
-        // TODO(Herschel): Verify order of execution for parent/children.
-        // Parent first? Children first? Sorted by depth?
-        for child in self.children.values_mut() {
-            context.active_clip = *child;
             child.write(context.gc_context).run_frame(context);
         }
-    }
+        context.active_clip = prev_clip;
 
-    fn run_post_frame(&mut self, context: &mut UpdateContext<'_, 'gc, '_>) {
+        // Run myself.
-        self.run_goto_queue(context);
+        if self.is_playing {
-
+            self.run_frame_internal(context, true);
-        for child in self.children.values() {
-            context.active_clip = *child;
-            child.write(context.gc_context).run_post_frame(context);
         }
     }
 
@@ -1207,7 +1235,9 @@ impl<'gc, 'a> MovieClip<'gc> {
             start,
             end,
         };
-        context.actions.push((context.active_clip, slice));
+        context
+            .action_queue
+            .queue_actions(context.active_clip, slice);
         Ok(())
     }
 
@@ -1238,7 +1268,7 @@ impl<'gc, 'a> MovieClip<'gc> {
                 ) {
                     child
                         .write(context.gc_context)
-                        .apply_place_object(place_object);
+                        .apply_place_object(&place_object);
                     child
                 } else {
                     return Ok(());
@@ -1248,7 +1278,7 @@ impl<'gc, 'a> MovieClip<'gc> {
                 if let Some(child) = self.children.get_mut(&place_object.depth) {
                     child
                         .write(context.gc_context)
-                        .apply_place_object(place_object);
+                        .apply_place_object(&place_object);
                     *child
                 } else {
                     return Ok(());
@@ -1272,7 +1302,7 @@ impl<'gc, 'a> MovieClip<'gc> {
             reader.read_remove_object_2()
         }?;
         if let Some(child) = self.children.remove(&remove_object.depth) {
-            child.write(context.gc_context).set_parent(None);
+            self.remove_child_from_exec_list(context.gc_context, child);
         }
         Ok(())
     }

core/src/player.rs

@@ -6,6 +6,7 @@ use crate::events::{ButtonEvent, PlayerEvent};
 use crate::library::Library;
 use crate::movie_clip::MovieClip;
 use crate::prelude::*;
+use crate::tag_utils::SwfSlice;
 use crate::transform::TransformStack;
 use gc_arena::{make_arena, ArenaParameters, Collect, GcCell, MutationContext};
 use log::info;
@@ -25,6 +26,7 @@ struct GcRoot<'gc> {
     root: DisplayNode<'gc>,
     mouse_hover_node: GcCell<'gc, Option<DisplayNode<'gc>>>, // TODO: Remove GcCell wrapped inside GcCell.
     avm: GcCell<'gc, Avm1<'gc>>,
+    action_queue: GcCell<'gc, ActionQueue<'gc>>,
 }
 
 type Error = Box<dyn std::error::Error>;
@@ -158,6 +160,7 @@ impl<Audio: AudioBackend, Renderer: RenderBackend, Navigator: NavigatorBackend>
                 ),
                 mouse_hover_node: GcCell::allocate(gc_context, None),
                 avm: GcCell::allocate(gc_context, Avm1::new(gc_context, NEWEST_PLAYER_VERSION)),
+                action_queue: GcCell::allocate(gc_context, ActionQueue::new()),
             }),
 
             frame_rate: header.frame_rate.into(),
@@ -304,7 +307,7 @@ impl<Audio: AudioBackend, Renderer: RenderBackend, Navigator: NavigatorBackend>
                 renderer,
                 audio,
                 navigator,
-                actions: vec![],
+                action_queue: gc_root.action_queue.write(gc_context),
                 gc_context,
                 active_clip: gc_root.root,
             };
@@ -379,7 +382,7 @@ impl<Audio: AudioBackend, Renderer: RenderBackend, Navigator: NavigatorBackend>
                     renderer,
                     audio,
                     navigator,
-                    actions: vec![],
+                    action_queue: gc_root.action_queue.write(gc_context),
                     gc_context,
                     active_clip: gc_root.root,
                 };
@@ -446,7 +449,7 @@ impl<Audio: AudioBackend, Renderer: RenderBackend, Navigator: NavigatorBackend>
                 renderer,
                 audio,
                 navigator,
-                actions: vec![],
+                action_queue: gc_root.action_queue.write(gc_context),
                 gc_context,
                 active_clip: gc_root.root,
             };
@@ -506,7 +509,7 @@ impl<Audio: AudioBackend, Renderer: RenderBackend, Navigator: NavigatorBackend>
                 renderer,
                 audio,
                 navigator,
-                actions: vec![],
+                action_queue: gc_root.action_queue.write(gc_context),
                 gc_context,
                 active_clip: gc_root.root,
             };
@@ -578,9 +581,16 @@ impl<Audio: AudioBackend, Renderer: RenderBackend, Navigator: NavigatorBackend>
         // I think this will eventually be cleaned up;
         // Need to figure out the proper order of operations between ticking a clip
         // and running the actions.
-        let mut actions = std::mem::replace(&mut update_context.actions, vec![]);
+        while let Some(clip_action) = update_context.action_queue.pop() {
-        while !actions.is_empty() {
+            match clip_action {
-            {
+                Action::Action {
+                    clip,
+                    actions: action,
+                } => {
+                    // We don't run the action f the clip was removed after it queued the action.
+                    if clip.read().removed() {
+                        continue;
+                    }
                     let mut action_context = crate::avm1::ActionContext {
                         gc_context: update_context.gc_context,
                         global_time: update_context.global_time,
@@ -590,14 +600,14 @@ impl<Audio: AudioBackend, Renderer: RenderBackend, Navigator: NavigatorBackend>
                         active_clip: root,
                         target_clip: Some(root),
                         target_path: crate::avm1::Value::Undefined,
+                        action_queue: &mut update_context.action_queue,
                         rng: update_context.rng,
                         audio: update_context.audio,
                         navigator: update_context.navigator,
                     };
-                for (active_clip, action) in actions {
+                    action_context.start_clip = clip;
-                    action_context.start_clip = active_clip;
+                    action_context.active_clip = clip;
-                    action_context.active_clip = active_clip;
+                    action_context.target_clip = Some(clip);
-                    action_context.target_clip = Some(active_clip);
                     update_context.avm.insert_stack_frame_for_action(
                         update_context.swf_version,
                         action,
@@ -605,14 +615,19 @@ impl<Audio: AudioBackend, Renderer: RenderBackend, Navigator: NavigatorBackend>
                     );
                     let _ = update_context.avm.run_stack_till_empty(&mut action_context);
                 }
+
+                Action::Goto { clip, frame } => {
+                    update_context.active_clip = clip;
+                    let mut clip = clip.write(update_context.gc_context);
+                    // We don't run the action if the clip was removed after it queued the action.
+                    if clip.removed() {
+                        continue;
+                    }
+                    if let Some(movie_clip) = clip.as_movie_clip_mut() {
+                        movie_clip.run_goto(update_context, frame);
+                    }
+                }
             }
-
-            // Run goto queues.
-            update_context.active_clip = root;
-            root.write(update_context.gc_context)
-                .run_post_frame(update_context);
-
-            actions = std::mem::replace(&mut update_context.actions, vec![]);
         }
     }
 
@@ -682,7 +697,7 @@ pub struct UpdateContext<'a, 'gc, 'gc_context> {
     pub audio: &'a mut dyn AudioBackend,
     pub navigator: &'a mut dyn NavigatorBackend,
     pub rng: &'a mut SmallRng,
-    pub actions: Vec<(DisplayNode<'gc>, crate::tag_utils::SwfSlice)>,
+    pub action_queue: std::cell::RefMut<'a, ActionQueue<'gc>>,
     pub active_clip: DisplayNode<'gc>,
 }
 
@@ -693,3 +708,54 @@ pub struct RenderContext<'a, 'gc> {
     pub view_bounds: BoundingBox,
     pub clip_depth_stack: Vec<Depth>,
 }
+
+pub enum Action<'gc> {
+    Action {
+        clip: DisplayNode<'gc>,
+        actions: SwfSlice,
+    },
+    Goto {
+        clip: DisplayNode<'gc>,
+        frame: u16,
+    },
+}
+
+/// Action and gotos need to be queued up to execute at the end of the frame.
+pub struct ActionQueue<'gc> {
+    queue: std::collections::VecDeque<Action<'gc>>,
+}
+
+impl<'gc> ActionQueue<'gc> {
+    const DEFAULT_CAPACITY: usize = 32;
+
+    pub fn new() -> Self {
+        Self {
+            queue: std::collections::VecDeque::with_capacity(Self::DEFAULT_CAPACITY),
+        }
+    }
+
+    pub fn queue_actions(&mut self, clip: DisplayNode<'gc>, actions: SwfSlice) {
+        self.queue.push_back(Action::Action { clip, actions })
+    }
+
+    pub fn queue_goto(&mut self, clip: DisplayNode<'gc>, frame: u16) {
+        self.queue.push_back(Action::Goto { clip, frame })
+    }
+
+    pub fn pop(&mut self) -> Option<Action<'gc>> {
+        self.queue.pop_front()
+    }
+}
+
+impl<'gc> Default for ActionQueue<'gc> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+unsafe impl<'gc> Collect for ActionQueue<'gc> {
+    #[inline]
+    fn trace(&self, cc: gc_arena::CollectionContext) {
+        self.queue.iter().for_each(|o| o.trace(cc));
+    }
+}