avm2: Refactor Array.splice to ArrayStorage

core/src/avm2/array.rs

@@ -358,7 +358,7 @@ impl<'gc> ArrayStorage<'gc> {
 
     /// Pop a value from the back of the array.
     ///
-    /// This method preferrentially pops non-holes from the array first. If a
+    /// This method preferentially pops non-holes from the array first. If a
     /// hole is popped, it will become `undefined`.
     pub fn pop(&mut self) -> Value<'gc> {
         match self {
@@ -527,6 +527,77 @@ impl<'gc> ArrayStorage<'gc> {
             }
         }
     }
+
+    pub fn splice(
+        &mut self,
+        start: usize,
+        delete_count: usize,
+        items: &[Value<'gc>],
+    ) -> ArrayStorage<'gc> {
+        let delete_count = delete_count.min(self.length() - start);
+        let end = start + delete_count;
+        match self {
+            ArrayStorage::Dense {
+                storage,
+                occupied_count,
+            } => {
+                let mut occupied_removed = 0;
+                let splice = storage
+                    .splice(start..end, items.iter().map(|i| Some(*i)))
+                    .inspect(|v| {
+                        if v.is_some() {
+                            occupied_removed += 1;
+                        }
+                    })
+                    .collect::<Vec<_>>();
+                *occupied_count = occupied_count.saturating_sub(occupied_removed);
+                ArrayStorage::from_storage(splice)
+            }
+            ArrayStorage::Sparse { storage, length } => {
+                // 1. Split the map into 3 ranges:
+                //      storage, splice, remaining
+                let remaining = storage.split_off(&end);
+                let splice = storage.split_off(&start);
+
+                // 2. Remap indices in remaining elements
+                let rshift = items.len() as isize - delete_count as isize;
+                let mut remaining = if rshift == 0 {
+                    remaining
+                } else {
+                    remaining
+                        .into_iter()
+                        .map(|(i, v)| (i.saturating_add_signed(rshift), v))
+                        .collect::<BTreeMap<_, _>>()
+                };
+
+                // 3. Put remaining elements back in original array
+                storage.append(&mut remaining);
+
+                // 4. Put new items to the original array
+                for (i, item) in items.iter().enumerate() {
+                    storage.insert(start + i, *item);
+                }
+
+                // 5. Remap indices in splice
+                let splice = if start == 0 {
+                    splice
+                } else {
+                    splice
+                        .into_iter()
+                        .map(|(i, v)| (i.saturating_sub(start), v))
+                        .collect::<BTreeMap<_, _>>()
+                };
+
+                // 6. Update length
+                *length = length.saturating_add_signed(rshift);
+
+                ArrayStorage::Sparse {
+                    storage: splice,
+                    length: delete_count,
+                }
+            }
+        }
+    }
 }
 
 impl<'gc, V> FromIterator<V> for ArrayStorage<'gc>

core/src/avm2/globals/array.rs

@@ -11,7 +11,7 @@ use crate::avm2::Error;
 use crate::string::AvmString;
 use bitflags::bitflags;
 use ruffle_macros::istr;
-use std::cmp::{min, Ordering};
+use std::cmp::Ordering;
 use std::mem::swap;
 
 pub use crate::avm2::object::array_allocator;
@@ -643,49 +643,31 @@ pub fn splice<'gc>(
 ) -> Result<Value<'gc>, Error<'gc>> {
     let this = this.as_object().unwrap();
 
-    let array_length = this.as_array_storage().map(|a| a.length());
-
-    if let Some(array_length) = array_length {
-        if let Some(start) = args.get_optional(0) {
-            let actual_start = resolve_index(activation, start, array_length)?;
-            let delete_count = args
-                .get_optional(1)
-                .unwrap_or_else(|| array_length.into())
-                .coerce_to_i32(activation)?;
-
-            let actual_end = min(array_length, actual_start + delete_count as usize);
-            let args_slice = if args.len() > 2 {
-                args[2..].iter().cloned()
-            } else {
-                [].iter().cloned()
-            };
-
-            let contents = this
-                .as_array_storage()
-                .map(|a| a.iter().collect::<Vec<Option<Value<'gc>>>>())
-                .unwrap();
-
-            let mut resolved = Vec::with_capacity(contents.len());
-            for (i, v) in contents.iter().enumerate() {
-                resolved.push(resolve_array_hole(activation, this, i, *v)?);
-            }
+    let Some(mut array_storage) = this.as_array_storage_mut(activation.gc()) else {
+        return Ok(Value::Undefined);
+    };
+    let array_length = array_storage.length();
+    let Some(start) = args.get_optional(0) else {
+        return Ok(Value::Undefined);
+    };
 
-            let removed = resolved
-                .splice(actual_start..actual_end, args_slice)
-                .collect::<Vec<Value<'gc>>>();
-            let removed_array = ArrayStorage::from_args(&removed[..]);
+    let actual_start = resolve_index(activation, start, array_length)?;
+    let delete_count = args
+        .get_optional(1)
+        .unwrap_or_else(|| array_length.into())
+        .as_u32();
 
-            let mut resolved_array = ArrayStorage::from_args(&resolved[..]);
+    let args_slice = if args.len() > 2 { &args[2..] } else { &[] };
 
-            if let Some(mut array) = this.as_array_storage_mut(activation.gc()) {
-                swap(&mut *array, &mut resolved_array)
-            }
-
-            return Ok(build_array(activation, removed_array));
-        }
+    // FIXME Flash does not iterate over those elements like we do, it's too
+    //   inefficient. Flash probably iterates through set properties only.
+    for i in actual_start..array_length {
+        let item = array_storage.get(i);
+        array_storage.set(i, resolve_array_hole(activation, this, i, item)?);
     }
 
-    Ok(Value::Undefined)
+    let ret = array_storage.splice(actual_start, delete_count as usize, args_slice);
+    Ok(build_array(activation, ret))
 }
 
 /// Insert an element into a specific position of an array.

tests/tests/swfs/avm2/array_splice2/Test.as

@@ -0,0 +1,116 @@
+package {
+import flash.display.*;
+
+public class Test extends MovieClip {
+    public function Test() {
+        var array_sparse1:Array = [];
+        array_sparse1[100000] = 5;
+
+        var array_sparse2:Array = [1];
+        array_sparse2[100000] = 5;
+
+        var arrays:Array = [
+            [],
+            [1],
+            [1, 2, 3, 4, 5],
+            array_sparse1,
+            array_sparse2,
+            [[1], [1, 2], [1, 2, 3]],
+            [1, 2, [3], 4, [5], 6],
+        ];
+
+        var i:int = 0;
+        while (i < arrays.length) {
+            testArray(i, arrays[i]);
+            ++i;
+        }
+    }
+
+    private function testArray(i:int, array:Array):void {
+        trace("Testing array " + i + ":");
+        trace("  array: " + arrToString(array));
+
+        trace("  splice():");
+        testSplice(array, function(a:Array):Array {
+            return a.splice();
+        });
+
+        trace("  splice(0):");
+        testSplice(array, function(a:Array):Array {
+            return a.splice(0);
+        });
+
+        trace("  splice(1):");
+        testSplice(array, function(a:Array):Array {
+            return a.splice(1);
+        });
+
+        trace("  splice(a.length):");
+        testSplice(array, function(a:Array):Array {
+            return a.splice(a.length);
+        });
+
+        trace("  splice(a.length-1):");
+        testSplice(array, function(a:Array):Array {
+            return a.splice(a.length-1);
+        });
+
+        trace("  splice(a.length+1):");
+        testSplice(array, function(a:Array):Array {
+            return a.splice(a.length+1);
+        });
+
+        trace("  splice(a.length/2):");
+        testSplice(array, function(a:Array):Array {
+            return a.splice(a.length/2);
+        });
+
+        trace("  splice(1,2):");
+        testSplice(array, function(a:Array):Array {
+            return a.splice(1,2);
+        });
+
+        trace("  splice(1,2,3,4):");
+        testSplice(array, function(a:Array):Array {
+            return a.splice(1,2,3,4);
+        });
+
+        trace("  splice(1,2,3,4,5,6):");
+        testSplice(array, function(a:Array):Array {
+            return a.splice(1,2,3,4,5,6);
+        });
+    }
+
+    private function testSplice(array:Array, f:Function):void {
+        array = array.concat();
+        var ret:Array = f(array);
+
+        trace("    returned: " + arrToString(ret));
+        trace("    after: " + arrToString(array));
+        for (var i in [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 99999, 100000, 100001]) {
+            if (ret != null && ret[i] != null) {
+                trace("    returned[" + i + "]: " + ret[i]);
+            }
+            if (array != null && array[i] != null) {
+                trace("    after[" + i + "]: " + array[i]);
+            }
+        }
+    }
+
+    private function arrToString(array:*):String {
+        if (array == null) {
+            return "null";
+        } else if (!(array is Array)) {
+            return "" + array;
+        } else if (array.length > 1000) {
+            return "<len(" + array.length + ")>";
+        } else if (array.length == 0) {
+            return "<empty>";
+        } else {
+            return "[" + array.map(function(el:*, ...rest):String {
+                return arrToString(el);
+            }) + "]";
+        }
+    }
+}
+}