avm1: Delay clip removals when a child has an unload listener

When removing a clip, first check if it has an unload event listener somewhere
it's hierarchy.
If it does, enqueue the removal to happen on the next frame, by moving it to a negative depth.

core/src/avm1/runtime.rs

@@ -392,9 +392,53 @@ impl<'gc> Avm1<'gc> {
         self.registers.get_mut(id)
     }
 
+    /// Find all display objects with negative depth recurisvely
+    ///
+    /// If an object is pending removal due to being removed by a removeObject tag on the previous frame,
+    /// while it had an unload event listener attached, avm1 requires that the object is kept around for one extra frame.
+    ///
+    /// This will be called at the start of each frame, to gather the objects for removal
+    fn find_display_objects_pending_removal(
+        obj: DisplayObject<'gc>,
+        out: &mut Vec<DisplayObject<'gc>>,
+    ) {
+        if let Some(parent) = obj.as_container() {
+            for child in parent.iter_render_list() {
+                if child.pending_removal() {
+                    out.push(child);
+                }
+
+                Self::find_display_objects_pending_removal(child, out);
+            }
+        }
+    }
+
+    /// Remove all display objects pending removal
+    /// See [`find_display_objects_pending_removal`] for details
+    fn remove_pending(context: &mut UpdateContext<'_, 'gc>) {
+        // Storage for objects to remove
+        // Have to do this in two passes to avoid borrow-mut while already borrowed
+        let mut out = Vec::new();
+
+        // Find objects to remove
+        Self::find_display_objects_pending_removal(context.stage.root_clip(), &mut out);
+
+        for child in out {
+            // Get the parent of this object
+            let parent = child.parent().unwrap();
+            let parent_container = parent.as_container().unwrap();
+
+            // Remove it
+            parent_container.remove_child_directly(context, child);
+        }
+    }
+
     // Run a single frame.
     #[instrument(level = "debug", skip_all)]
     pub fn run_frame(context: &mut UpdateContext<'_, 'gc>) {
+        // Remove pending objects
+        Self::remove_pending(context);
+
         // 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.

core/src/display_object.rs

@@ -1096,10 +1096,16 @@ pub trait TDisplayObject<'gc>:
     fn removed(&self) -> bool {
         self.base().removed()
     }
+
     fn set_removed(&self, gc_context: MutationContext<'gc, '_>, value: bool) {
         self.base_mut(gc_context).set_removed(value)
     }
 
+    /// Is this object waiting to be removed on the start of the next frame
+    fn pending_removal(&self) -> bool {
+        self.depth() < 0
+    }
+
     /// Whether this display object is visible.
     /// Invisible objects are not rendered, but otherwise continue to exist normally.
     /// Returned by the `_visible`/`visible` ActionScript properties.

core/src/display_object/container.rs

@@ -6,7 +6,7 @@ use crate::display_object::avm1_button::Avm1Button;
 use crate::display_object::loader_display::LoaderDisplay;
 use crate::display_object::movie_clip::MovieClip;
 use crate::display_object::stage::Stage;
-use crate::display_object::{Depth, DisplayObject, TDisplayObject};
+use crate::display_object::{Depth, DisplayObject, TDisplayObject, TInteractiveObject};
 use crate::string::WStr;
 use gc_arena::{Collect, MutationContext};
 use ruffle_macros::enum_trait_object;
@@ -299,12 +299,29 @@ pub trait TDisplayObjectContainer<'gc>:
     }
 
     /// Remove (and unloads) a child display object from this container's render and depth lists.
+    ///
+    /// Will also handle AVM1 delayed clip removal, when a unload listener is present
     fn remove_child(&mut self, context: &mut UpdateContext<'_, 'gc>, child: DisplayObject<'gc>) {
+        // We should always be the parent of this child
         debug_assert!(DisplayObject::ptr_eq(
             child.parent().unwrap(),
             (*self).into()
         ));
 
+        // Check if this child should have delayed removal
+        if ChildContainer::should_delay_removal(child) {
+            ChildContainer::queue_removal(child, context);
+        } else {
+            self.remove_child_directly(context, child);
+        }
+    }
+
+    /// Remove (and unloads) a child display object from this container's render and depth lists.
+    fn remove_child_directly(
+        &self,
+        context: &mut UpdateContext<'_, 'gc>,
+        child: DisplayObject<'gc>,
+    ) {
         dispatch_removed_event(child, context);
 
         let mut write = self.raw_container_mut(context.gc_context);
@@ -774,9 +791,52 @@ impl<'gc> ChildContainer<'gc> {
     }
 
     /// Yield children in the order they are rendered.
-    fn iter_render_list<'a>(&'a self) -> impl 'a + Iterator<Item = DisplayObject<'gc>> {
+    pub fn iter_render_list<'a>(&'a self) -> impl 'a + Iterator<Item = DisplayObject<'gc>> {
         self.render_list.iter().copied()
     }
+
+    /// Should the removal of this clip be delayed to the start of the next frame
+    ///
+    /// Checks recursively for unload handlers
+    pub fn should_delay_removal(child: DisplayObject<'gc>) -> bool {
+        // Do we have an unload event handler
+        if let Some(mc) = child.as_movie_clip() {
+            if mc.has_unload_handler() {
+                return true;
+            }
+        }
+
+        // Otherwise, check children if we have them
+        if let Some(c) = child.as_container() {
+            for child in c.iter_render_list() {
+                if Self::should_delay_removal(child) {
+                    return true;
+                }
+            }
+        }
+
+        false
+    }
+
+    /// Enqueue the given child and all sub-children for delayed removal at the start of the next frame
+    ///
+    /// This just moves the children to a negative depth
+    /// Will also fire unload events, as they should occur when the removal is queued, not when it actually occurs
+    fn queue_removal(child: DisplayObject<'gc>, context: &mut UpdateContext<'_, 'gc>) {
+        if let Some(c) = child.as_container() {
+            for child in c.iter_render_list() {
+                Self::queue_removal(child, context);
+            }
+        }
+
+        let cur_depth = child.depth();
+        child.set_depth(context.gc_context, -cur_depth);
+
+        if let Some(mc) = child.as_movie_clip() {
+            // Clip events should still fire
+            mc.event_dispatch(context, crate::events::ClipEvent::Unload);
+        }
+    }
 }
 
 pub struct RenderIter<'gc> {

core/src/display_object/movie_clip.rs

@@ -897,6 +897,15 @@ impl<'gc> MovieClip<'gc> {
         self.0.write(context.gc_context).stop(context)
     }
 
+    /// Does this clip have a unload handler
+    pub fn has_unload_handler(&self) -> bool {
+        self.0
+            .read()
+            .clip_event_handlers
+            .iter()
+            .any(|handler| handler.events.contains(ClipEventFlag::UNLOAD))
+    }
+
     /// Queues up a goto to the specified frame.
     /// `frame` should be 1-based.
     ///
@@ -2561,7 +2570,12 @@ impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> {
             let mut mc = self.0.write(context.gc_context);
             mc.stop_audio_stream(context);
         }
+
+        // If this clip is currently pending removal, then it unload event will have already been dispatched
+        if !self.pending_removal() {
             self.event_dispatch(context, ClipEvent::Unload);
+        }
+
         self.set_removed(context.gc_context, true);
     }
 

core/src/player.rs

@@ -1573,8 +1573,8 @@ impl Player {
     pub fn run_actions(context: &mut UpdateContext<'_, '_>) {
         // Note that actions can queue further actions, so a while loop is necessary here.
         while let Some(action) = context.action_queue.pop_action() {
-            // We don't run frame actions if the clip was removed after it queued the action.
-            if !action.is_unload && action.clip.removed() {
+            // We don't run frame actions if the clip was removed (or scheduled to be removed) after it queued the action.
+            if !action.is_unload && (action.clip.removed() || action.clip.pending_removal()) {
                 continue;
             }
 

tests/tests/swfs/avm1/unload/output.txt

@@ -0,0 +1,20 @@
+Frame 1:
+clip = _level0.clip
+clip2 = _level0.clip2
+clip3 = _level0.clip3
+clip4 = _level0.clip4
+End frame 1
+Unload clip4
+unload clipEvent
+Frame 2
+clip = _level0.clip
+clip2 = undefined
+clip3 = undefined
+clip4 = undefined
+End frame 2
+Frame 3
+clip = undefined
+clip2 = undefined
+clip3 = undefined
+clip4 = undefined
+End frame 3

tests/tests/swfs/avm1/unload/test.toml

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

tests/tests/swfs/avm1/unload_nested_child/input.json

@@ -0,0 +1,15 @@
+[
+    {
+        "type": "MouseDown",
+        "pos": [253.0, 231.0],
+        "btn": "Left"
+    },
+    {
+        "type": "Wait"
+    },
+    {
+        "type": "MouseUp",
+        "pos": [253.0, 231.0],
+        "btn": "Left"
+    }
+]

tests/tests/swfs/avm1/unload_nested_child/output.txt

@@ -0,0 +1,5 @@
+go completed
+unload
+frame 2
+_level0.clip
+frame 3

tests/tests/swfs/avm1/unload_nested_child/test.toml

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