core: Replace direct container manipulation with `TDisplayObjectContainer` methods wherever possible.

The purpose of this refactor is twofold:

1. Ensure `TDisplayObjectContainer` holds all the container methods we need
2. Ensure that future adjustments to trait methods automatically apply to the display object's own use of the container, in case we want to do things in those methods that can't be done in a borrowed container

There are two places where we cannot use the new trait methods:

1. `Button.set_state` as it holds a borrow at the point we want to clear the container
2. `MovieClipData.goto_remove_object`, since it's a method on the data and thus cannot access trait methods

This particular commit generates a lot of noise as several `DisplayObject` methods were incorrectly marked as non-mutating.

core/src/avm1/activation.rs

@@ -2769,7 +2769,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> {
         if let Some(level) = self.context.levels.get(&level_id) {
             *level
         } else {
-            let level: DisplayObject<'_> = MovieClip::new(
+            let mut level: DisplayObject<'_> = MovieClip::new(
                 SwfSlice::empty(self.base_clip().movie().unwrap()),
                 self.context.gc_context,
             )

core/src/avm1/globals/movie_clip.rs

@@ -512,7 +512,7 @@ fn attach_movie<'gc>(
         return Ok(Value::Undefined);
     }
 
-    if let Ok(new_clip) = activation
+    if let Ok(mut new_clip) = activation
         .context
         .library
         .library_for_movie(movie_clip.movie().unwrap())
@@ -568,7 +568,7 @@ fn create_empty_movie_clip<'gc>(
         .movie()
         .or_else(|| activation.base_clip().movie())
         .unwrap();
-    let new_clip = MovieClip::new(SwfSlice::empty(swf_movie), activation.context.gc_context);
+    let mut new_clip = MovieClip::new(SwfSlice::empty(swf_movie), activation.context.gc_context);
 
     // Set name and attach to parent.
     new_clip.set_name(activation.context.gc_context, &new_instance_name);
@@ -617,7 +617,7 @@ fn create_text_field<'gc>(
         .unwrap_or(Value::Undefined)
         .coerce_to_f64(activation)?;
 
-    let text_field: DisplayObject<'gc> =
+    let mut text_field: DisplayObject<'gc> =
         EditText::new(&mut activation.context, movie, x, y, width, height).into();
     text_field.set_name(
         activation.context.gc_context,
@@ -687,7 +687,7 @@ pub fn duplicate_movie_clip_with_bias<'gc>(
         return Ok(Value::Undefined);
     }
 
-    if let Ok(new_clip) = activation
+    if let Ok(mut new_clip) = activation
         .context
         .library
         .library_for_movie(movie_clip.movie().unwrap())

core/src/avm1/object/script_object.rs

@@ -874,7 +874,7 @@ mod tests {
             let mut avm1 = Avm1::new(gc_context, swf_version);
             let mut avm2 = Avm2::new(gc_context);
             let swf = Arc::new(SwfMovie::empty(swf_version));
-            let root: DisplayObject<'_> =
+            let mut root: DisplayObject<'_> =
                 MovieClip::new(SwfSlice::empty(swf.clone()), gc_context).into();
             root.set_depth(gc_context, 0);
             let mut levels = BTreeMap::new();

core/src/avm1/test_utils.rs

@@ -36,7 +36,7 @@ where
         let mut avm1 = Avm1::new(gc_context, swf_version);
         let mut avm2 = Avm2::new(gc_context);
         let swf = Arc::new(SwfMovie::empty(swf_version));
-        let root: DisplayObject<'gc> =
+        let mut root: DisplayObject<'gc> =
             MovieClip::new(SwfSlice::empty(swf.clone()), gc_context).into();
         root.set_depth(gc_context, 0);
         let mut levels = BTreeMap::new();

core/src/display_object.rs

@@ -730,14 +730,14 @@ pub trait TDisplayObject<'gc>:
     /// Events execute inside-out; the deepest child will react first, followed by its parent, and
     /// so forth.
     fn handle_clip_event(
-        &self,
+        &mut self,
         _context: &mut UpdateContext<'_, 'gc, '_>,
         _event: ClipEvent,
     ) -> ClipEventResult {
         ClipEventResult::NotHandled
     }
 
-    fn run_frame(&self, _context: &mut UpdateContext<'_, 'gc, '_>) {}
+    fn run_frame(&mut self, _context: &mut UpdateContext<'_, 'gc, '_>) {}
     fn render(&self, _context: &mut RenderContext<'_, 'gc>) {}
 
     fn unload(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
@@ -870,7 +870,7 @@ pub trait TDisplayObject<'gc>:
     }
 
     fn post_instantiation(
-        &self,
+        &mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         _display_object: DisplayObject<'gc>,
         _init_object: Option<Avm1Object<'gc>>,

core/src/display_object/bitmap.rs

@@ -80,7 +80,7 @@ impl<'gc> TDisplayObject<'gc> for Bitmap<'gc> {
         }
     }
 
-    fn run_frame(&self, _context: &mut UpdateContext) {
+    fn run_frame(&mut self, _context: &mut UpdateContext) {
         // Noop
     }
 

core/src/display_object/button.rs

@@ -115,7 +115,7 @@ impl<'gc> Button<'gc> {
     /// This function instantiates children and thus must not be called whilst
     /// the caller is holding a write lock on the button data.
     fn set_state(
-        &self,
+        &mut self,
         self_display_object: DisplayObject<'gc>,
         context: &mut crate::context::UpdateContext<'_, 'gc, '_>,
         state: ButtonState,
@@ -153,18 +153,11 @@ impl<'gc> Button<'gc> {
 
         drop(write);
 
-        //TODO: This code originally reinserted children onto the execution list
+        for (mut child, depth) in new_children {
-        //"backwards" (compared to standard behavior) - do we still want that?
-        for (child, depth) in new_children {
             // Initialize child.
             child.post_instantiation(context, child, None, Instantiator::Movie, false);
             child.run_frame(context);
-            self.0.write(context.gc_context).container.replace_at_depth(
+            self.replace_at_depth(context, child, depth.into());
-                context,
-                (*self).into(),
-                child,
-                depth.into(),
-            );
         }
     }
 }
@@ -181,7 +174,7 @@ impl<'gc> TDisplayObject<'gc> for Button<'gc> {
     }
 
     fn post_instantiation(
-        &self,
+        &mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         display_object: DisplayObject<'gc>,
         _init_object: Option<Object<'gc>>,
@@ -207,7 +200,7 @@ impl<'gc> TDisplayObject<'gc> for Button<'gc> {
         }
     }
 
-    fn run_frame(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
+    fn run_frame(&mut self, context: &mut UpdateContext<'_, 'gc, '_>) {
         let self_display_object = (*self).into();
         let initialized = self.0.read().initialized;
 
@@ -247,7 +240,7 @@ impl<'gc> TDisplayObject<'gc> for Button<'gc> {
 
             drop(read);
 
-            for (child, depth) in new_children {
+            for (mut child, depth) in new_children {
                 child.post_instantiation(context, child, None, Instantiator::Movie, false);
                 self.0
                     .write(context.gc_context)
@@ -256,7 +249,7 @@ impl<'gc> TDisplayObject<'gc> for Button<'gc> {
             }
         }
 
-        for child in self.iter_execution_list() {
+        for mut child in self.iter_execution_list() {
             child.run_frame(context);
         }
     }
@@ -322,19 +315,19 @@ impl<'gc> TDisplayObject<'gc> for Button<'gc> {
     }
 
     fn allow_as_mask(&self) -> bool {
-        !self.0.read().container.is_empty()
+        !self.is_empty()
     }
 
     /// Executes and propagates the given clip event.
     /// Events execute inside-out; the deepest child will react first, followed by its parent, and
     /// so forth.
     fn handle_clip_event(
-        &self,
+        &mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         event: ClipEvent,
     ) -> ClipEventResult {
         if event.propagates() {
-            for child in self.iter_execution_list() {
+            for mut child in self.iter_execution_list() {
                 if child.handle_clip_event(context, event) == ClipEventResult::Handled {
                     return ClipEventResult::Handled;
                 }

core/src/display_object/container.rs

@@ -51,6 +51,10 @@ pub trait TDisplayObjectContainer<'gc>:
     /// Returns the number of children on the render list.
     fn num_children(self) -> usize;
 
+    /// Returns the highest depth on the render list, or `None` if no children
+    /// exist on the depth list.
+    fn highest_depth(self) -> Option<Depth>;
+
     /// Insert a child display object into the container at a specific position
     /// in the depth list, removing any child already at that position.
     ///
@@ -125,6 +129,9 @@ pub trait TDisplayObjectContainer<'gc>:
     /// Clear all three lists in the container.
     fn clear(&mut self, context: MutationContext<'gc, '_>);
 
+    /// Determine if the container is empty.
+    fn is_empty(self) -> bool;
+
     /// Iterates over the children of this display object in execution order.
     /// This is different than render or depth order.
     ///
@@ -175,6 +182,10 @@ macro_rules! impl_display_object_container {
             self.0.read().$field.num_children()
         }
 
+        fn highest_depth(self) -> Option<Depth> {
+            self.0.read().$field.highest_depth()
+        }
+
         fn replace_at_depth(
             &mut self,
             context: &mut UpdateContext<'_, 'gc, '_>,
@@ -258,6 +269,10 @@ macro_rules! impl_display_object_container {
         fn clear(&mut self, gc_context: MutationContext<'gc, '_>) {
             self.0.write(gc_context).$field.clear(gc_context)
         }
+
+        fn is_empty(self) -> bool {
+            self.0.read().$field.is_empty()
+        }
     };
 }
 

core/src/display_object/edit_text.rs

@@ -1093,7 +1093,7 @@ impl<'gc> TDisplayObject<'gc> for EditText<'gc> {
         Some(self.0.read().static_data.swf.clone())
     }
 
-    fn run_frame(&self, _context: &mut UpdateContext) {
+    fn run_frame(&mut self, _context: &mut UpdateContext) {
         // Noop
     }
 
@@ -1102,7 +1102,7 @@ impl<'gc> TDisplayObject<'gc> for EditText<'gc> {
     }
 
     fn post_instantiation(
-        &self,
+        &mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         display_object: DisplayObject<'gc>,
         _init_object: Option<Object<'gc>>,
@@ -1387,7 +1387,7 @@ impl<'gc> TDisplayObject<'gc> for EditText<'gc> {
     }
 
     fn handle_clip_event(
-        &self,
+        &mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         event: ClipEvent,
     ) -> ClipEventResult {

core/src/display_object/graphic.rs

@@ -55,7 +55,7 @@ impl<'gc> TDisplayObject<'gc> for Graphic<'gc> {
         bounds
     }
 
-    fn run_frame(&self, _context: &mut UpdateContext) {
+    fn run_frame(&mut self, _context: &mut UpdateContext) {
         // Noop
     }
 

core/src/display_object/morph_shape.rs

@@ -52,7 +52,7 @@ impl<'gc> TDisplayObject<'gc> for MorphShape<'gc> {
         Some(*self)
     }
 
-    fn run_frame(&self, _context: &mut UpdateContext) {
+    fn run_frame(&mut self, _context: &mut UpdateContext) {
         // Noop
     }
 

core/src/display_object/movie_clip.rs

@@ -921,17 +921,6 @@ impl<'gc> MovieClip<'gc> {
         }
     }
 
-    /// Returns the highest depth in use by this movie clip, or `None` if there
-    /// are no children.
-    pub fn highest_depth(self) -> Option<Depth> {
-        self.0.read().container.highest_depth()
-    }
-
-    /// Returns the number of children on the render list.
-    pub fn num_children(self) -> usize {
-        self.0.read().container.num_children()
-    }
-
     /// Gets the clip events for this movieclip.
     pub fn clip_actions(&self) -> Ref<[ClipAction]> {
         Ref::map(self.0.read(), |mc| mc.clip_actions())
@@ -1115,7 +1104,7 @@ impl<'gc> MovieClip<'gc> {
     /// Instantiate a given child object on the timeline at a given depth.
     #[allow(clippy::too_many_arguments)]
     fn instantiate_child(
-        self,
+        mut self,
         self_display_object: DisplayObject<'gc>,
         context: &mut UpdateContext<'_, 'gc, '_>,
         id: CharacterId,
@@ -1123,18 +1112,14 @@ impl<'gc> MovieClip<'gc> {
         place_object: &swf::PlaceObject,
         copy_previous_properties: bool,
     ) -> Option<DisplayObject<'gc>> {
-        if let Ok(child) = context
+        if let Ok(mut child) = context
             .library
             .library_for_movie_mut(self.movie().unwrap()) //TODO
             .instantiate_by_id(id, context.gc_context)
         {
             // Remove previous child from children list,
             // and add new child onto front of the list.
-            let prev_child = {
+            let prev_child = self.replace_at_depth(context, child, depth);
-                let mut mc = self.0.write(context.gc_context);
-                mc.container
-                    .replace_at_depth(context, self.into(), child, depth)
-            };
             {
                 // Set initial properties for child.
                 child.set_depth(context.gc_context, depth);
@@ -1176,7 +1161,7 @@ impl<'gc> MovieClip<'gc> {
     }
 
     pub fn run_goto(
-        self,
+        mut self,
         self_display_object: DisplayObject<'gc>,
         context: &mut UpdateContext<'_, 'gc, '_>,
         frame: FrameNumber,
@@ -1226,8 +1211,7 @@ impl<'gc> MovieClip<'gc> {
                 .collect();
             for (_depth, child) in children {
                 if !child.placed_by_script() {
-                    let mut mc = self.0.write(context.gc_context);
+                    self.remove_child(context, child);
-                    mc.container.remove_child(context, child);
                 }
             }
             true
@@ -1333,7 +1317,7 @@ impl<'gc> MovieClip<'gc> {
         let run_goto_command = |clip: MovieClip<'gc>,
                                 context: &mut UpdateContext<'_, 'gc, '_>,
                                 params: &GotoPlaceObject| {
-            let child_entry = clip.0.read().container.get_depth(params.depth());
+            let child_entry = clip.child_by_depth(params.depth());
             match child_entry {
                 // Apply final delta to display parameters.
                 // For rewinds, if an object was created before the final frame,
@@ -1395,7 +1379,7 @@ impl<'gc> MovieClip<'gc> {
     }
 
     fn construct_as_avm1_object(
-        self,
+        mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         display_object: DisplayObject<'gc>,
         init_object: Option<Avm1Object<'gc>>,
@@ -1668,9 +1652,9 @@ impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> {
         self.0.read().movie().version()
     }
 
-    fn run_frame(&self, context: &mut UpdateContext<'_, 'gc, '_>) {
+    fn run_frame(&mut self, context: &mut UpdateContext<'_, 'gc, '_>) {
         // Children must run first.
-        for child in self.iter_execution_list() {
+        for mut child in self.iter_execution_list() {
             child.run_frame(context);
         }
 
@@ -1781,12 +1765,12 @@ impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> {
     }
 
     fn handle_clip_event(
-        &self,
+        &mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         event: ClipEvent,
     ) -> ClipEventResult {
         if event.propagates() {
-            for child in self.iter_execution_list() {
+            for mut child in self.iter_execution_list() {
                 if child.handle_clip_event(context, event) == ClipEventResult::Handled {
                     return ClipEventResult::Handled;
                 }
@@ -1805,7 +1789,7 @@ impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> {
     }
 
     fn post_instantiation(
-        &self,
+        &mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         display_object: DisplayObject<'gc>,
         init_object: Option<Avm1Object<'gc>>,
@@ -1880,7 +1864,7 @@ impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> {
     }
 
     fn allow_as_mask(&self) -> bool {
-        !self.0.read().container.is_empty()
+        !self.is_empty()
     }
 
     fn is_focusable(&self) -> bool {
@@ -2829,7 +2813,7 @@ impl<'gc, 'a> MovieClip<'gc> {
                 }
             }
             PlaceObjectAction::Modify => {
-                if let Some(child) = self.0.read().container.get_depth(place_object.depth.into()) {
+                if let Some(child) = self.child_by_depth(place_object.depth.into()) {
                     child.apply_place_object(context.gc_context, &place_object);
                     child
                 } else {
@@ -2843,7 +2827,7 @@ impl<'gc, 'a> MovieClip<'gc> {
 
     #[inline]
     fn remove_object(
-        self,
+        mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         reader: &mut SwfStream<&'a [u8]>,
         version: u8,
@@ -2853,12 +2837,10 @@ impl<'gc, 'a> MovieClip<'gc> {
         } else {
             reader.read_remove_object_2()
         }?;
-
+        
-        let read = self.0.read();
+        if let Some(child) = self.child_by_depth(remove_object.depth.into()) {
-        if let Some(child) = read.container.get_depth(remove_object.depth.into()) {
             if !child.placed_by_script() {
-                drop(read);
+                self.remove_child(context, child);
-                self.0.write(context.gc_context).container.remove_child(context, child);
             }
         }
 

core/src/display_object/text.rs

@@ -53,7 +53,7 @@ impl<'gc> TDisplayObject<'gc> for Text<'gc> {
         Some(self.0.read().static_data.swf.clone())
     }
 
-    fn run_frame(&self, _context: &mut UpdateContext) {
+    fn run_frame(&mut self, _context: &mut UpdateContext) {
         // Noop
     }
 

core/src/player.rs

@@ -290,7 +290,7 @@ impl Player {
 
         player.mutate_with_update_context(|context| {
             // Instantiate an empty root before the main movie loads.
-            let fake_root = MovieClip::from_movie(context.gc_context, fake_movie);
+            let mut fake_root = MovieClip::from_movie(context.gc_context, fake_movie);
             fake_root.post_instantiation(
                 context,
                 fake_root.into(),
@@ -358,7 +358,7 @@ impl Player {
                 .library_for_movie_mut(context.swf.clone())
                 .set_avm2_domain(domain);
 
-            let root: DisplayObject =
+            let mut root: DisplayObject =
                 MovieClip::from_movie(context.gc_context, context.swf.clone()).into();
 
             root.set_depth(context.gc_context, 0);
@@ -608,7 +608,7 @@ impl Player {
         if button_event.is_some() {
             self.mutate_with_update_context(|context| {
                 let levels: Vec<DisplayObject<'_>> = context.levels.values().copied().collect();
-                for level in levels {
+                for mut level in levels {
                     if let Some(button_event) = button_event {
                         let state = level.handle_clip_event(context, button_event);
                         if state == ClipEventResult::Handled {
@@ -658,7 +658,7 @@ impl Player {
             // Fire clip event on all clips.
             if let Some(clip_event) = clip_event {
                 let levels: Vec<DisplayObject<'_>> = context.levels.values().copied().collect();
-                for level in levels {
+                for mut level in levels {
                     level.handle_clip_event(context, clip_event);
                 }
             }
@@ -689,7 +689,7 @@ impl Player {
                 PlayerEvent::MouseDown { .. } => {
                     is_mouse_down = true;
                     needs_render = true;
-                    if let Some(node) = context.mouse_hovered_object {
+                    if let Some(mut node) = context.mouse_hovered_object {
                         node.handle_clip_event(context, ClipEvent::Press);
                     }
                 }
@@ -697,7 +697,7 @@ impl Player {
                 PlayerEvent::MouseUp { .. } => {
                     is_mouse_down = false;
                     needs_render = true;
-                    if let Some(node) = context.mouse_hovered_object {
+                    if let Some(mut node) = context.mouse_hovered_object {
                         node.handle_clip_event(context, ClipEvent::Release);
                     }
                 }
@@ -766,7 +766,7 @@ impl Player {
 
             if cur_hovered.map(|d| d.as_ptr()) != new_hovered.map(|d| d.as_ptr()) {
                 // RollOut of previous node.
-                if let Some(node) = cur_hovered {
+                if let Some(mut node) = cur_hovered {
                     if !node.removed() {
                         node.handle_clip_event(context, ClipEvent::RollOut);
                     }
@@ -774,7 +774,7 @@ impl Player {
 
                 // RollOver on new node.I still
                 new_cursor = MouseCursor::Arrow;
-                if let Some(node) = new_hovered {
+                if let Some(mut node) = new_hovered {
                     new_cursor = node.mouse_cursor();
                     node.handle_clip_event(context, ClipEvent::RollOver);
                 }
@@ -828,7 +828,7 @@ impl Player {
             // want to run frames on
             let levels: Vec<_> = update_context.levels.values().copied().collect();
 
-            for level in levels {
+            for mut level in levels {
                 level.run_frame(update_context);
             }
         });