diff --git a/core/src/avm1/activation.rs b/core/src/avm1/activation.rs index 659905fbe..c81b79fc5 100644 --- a/core/src/avm1/activation.rs +++ b/core/src/avm1/activation.rs @@ -486,8 +486,8 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { Action::CloneSprite => self.action_clone_sprite(), Action::ConstantPool(action) => self.action_constant_pool(action), Action::Decrement => self.action_decrement(), - Action::DefineFunction(action) => self.action_define_function(action, data), - Action::DefineFunction2(action) => self.action_define_function_2(action, data), + Action::DefineFunction(action) => self.action_define_function(action.into(), data), + Action::DefineFunction2(action) => self.action_define_function(action, data), Action::DefineLocal => self.action_define_local(), Action::DefineLocal2 => self.action_define_local_2(), Action::Delete => self.action_delete(), @@ -863,23 +863,21 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { fn action_define_function( &mut self, - action: DefineFunction, + action: DefineFunction2, parent_data: &SwfSlice, ) -> Result, Error<'gc>> { - let name = action.name.to_str_lossy(self.encoding()); - let name = name.as_ref(); let swf_version = self.swf_version(); + let func_data = parent_data.to_unbounded_subslice(action.actions).unwrap(); let scope = Scope::new_closure_scope(self.scope_cell(), self.context.gc_context); let constant_pool = self.constant_pool(); - let func = Avm1Function::from_df1( + let func = Avm1Function::from_swf_function( self.context.gc_context, swf_version, - parent_data.to_unbounded_subslice(action.actions).unwrap(), - name, - &action.params[..], + func_data, + action, scope, constant_pool, - self.target_clip_or_root()?, + self.base_clip(), ); let name = func.name(); let prototype = ScriptObject::object( @@ -902,46 +900,6 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { Ok(FrameControl::Continue) } - fn action_define_function_2( - &mut self, - action: DefineFunction2, - parent_data: &SwfSlice, - ) -> Result, Error<'gc>> { - let swf_version = self.swf_version(); - let func_data = parent_data.to_unbounded_subslice(action.actions).unwrap(); - let scope = Scope::new_closure_scope(self.scope_cell(), self.context.gc_context); - let constant_pool = self.constant_pool(); - let func = Avm1Function::from_df2( - self.context.gc_context, - swf_version, - func_data, - &action, - scope, - constant_pool, - self.base_clip(), - ); - let prototype = ScriptObject::object( - self.context.gc_context, - Some(self.context.avm1.prototypes.object), - ) - .into(); - let func_obj = FunctionObject::function( - self.context.gc_context, - Gc::allocate(self.context.gc_context, func), - Some(self.context.avm1.prototypes.function), - prototype, - ); - if action.name.is_empty() { - self.context.avm1.push(func_obj.into()); - } else { - let name = action.name.to_str_lossy(self.encoding()); - let name = AvmString::new_utf8(self.context.gc_context, name); - self.define_local(name, func_obj.into())?; - } - - Ok(FrameControl::Continue) - } - fn action_define_local(&mut self) -> Result, Error<'gc>> { // If the property does not exist on the local object's prototype chain, it is created on the local object. // Otherwise, the property is set (including calling virtual setters). diff --git a/core/src/avm1/function.rs b/core/src/avm1/function.rs index 5a5900f2f..b3dfb5b37 100644 --- a/core/src/avm1/function.rs +++ b/core/src/avm1/function.rs @@ -10,8 +10,7 @@ use crate::avm1::{ArrayObject, AvmString, Object, ObjectPtr, ScriptObject, TObje use crate::display_object::{DisplayObject, TDisplayObject}; use crate::tag_utils::SwfSlice; use gc_arena::{Collect, CollectionContext, Gc, GcCell, MutationContext}; -use std::borrow::Cow; -use std::fmt; +use std::{borrow::Cow, fmt, num::NonZeroU8}; use swf::{avm1::types::FunctionFlags, SwfStr}; /// Represents a function defined in Ruffle's code. @@ -62,10 +61,8 @@ pub struct Avm1Function<'gc> { /// set. Any register beyond this ID will be served from the global one. register_count: u8, - /// The names of the function parameters and their register mappings. - /// r0 indicates that no register shall be written and the parameter stored - /// as a Variable instead. - params: Vec<(Option, AvmString<'gc>)>, + /// The parameters of the function. + params: Vec>, /// The scope the function was born into. scope: GcCell<'gc, Scope<'gc>>, @@ -83,62 +80,21 @@ pub struct Avm1Function<'gc> { } impl<'gc> Avm1Function<'gc> { - /// Construct a function from a DefineFunction action. - /// - /// Parameters not specified in DefineFunction are filled with reasonable - /// defaults. - #[allow(clippy::too_many_arguments)] - pub fn from_df1( - gc_context: MutationContext<'gc, '_>, - swf_version: u8, - actions: SwfSlice, - name: &str, - params: &[&'_ SwfStr], - scope: GcCell<'gc, Scope<'gc>>, - constant_pool: GcCell<'gc, Vec>>, - base_clip: DisplayObject<'gc>, - ) -> Self { - let name = if name.is_empty() { - None - } else { - Some(AvmString::new_utf8(gc_context, name)) - }; - - Avm1Function { - swf_version, - data: actions, - name, - register_count: 0, - params: params - .iter() - .map(|&s| { - let name = s.to_str_lossy(SwfStr::encoding_for_version(swf_version)); - (None, AvmString::new_utf8(gc_context, name)) - }) - .collect(), - scope, - constant_pool, - base_clip, - flags: FunctionFlags::empty(), - } - } - /// Construct a function from a DefineFunction2 action. - pub fn from_df2( + pub fn from_swf_function( gc_context: MutationContext<'gc, '_>, swf_version: u8, actions: SwfSlice, - swf_function: &swf::avm1::types::DefineFunction2, + swf_function: swf::avm1::types::DefineFunction2, scope: GcCell<'gc, Scope<'gc>>, constant_pool: GcCell<'gc, Vec>>, base_clip: DisplayObject<'gc>, ) -> Self { + let encoding = SwfStr::encoding_for_version(swf_version); let name = if swf_function.name.is_empty() { None } else { - let name = swf_function - .name - .to_str_lossy(SwfStr::encoding_for_version(swf_version)); + let name = swf_function.name.to_str_lossy(encoding); Some(AvmString::new_utf8(gc_context, name)) }; @@ -146,10 +102,11 @@ impl<'gc> Avm1Function<'gc> { .params .iter() .map(|p| { - let name = p - .name - .to_str_lossy(SwfStr::encoding_for_version(swf_version)); - (p.register_index, AvmString::new_utf8(gc_context, name)) + let name = p.name.to_str_lossy(encoding); + Param { + register: p.register_index, + name: AvmString::new_utf8(gc_context, name), + } }) .collect(); @@ -187,6 +144,22 @@ impl<'gc> Avm1Function<'gc> { } } +#[derive(Debug, Clone, Collect)] +#[collect(no_drop)] +struct Param<'gc> { + /// The register the argument will be preloaded into. + /// + /// If `register` is `None`, then this parameter will be stored in a named variable in the + /// function activation and can be accessed using `GetVariable`/`SetVariable`. + /// Otherwise, the parameter is loaded into a register and must be accessed with + /// `Push`/`StoreRegister`. + #[collect(require_static)] + register: Option, + + /// The name of the parameter. + name: AvmString<'gc>, +} + /// Represents a function that can be defined in the Ruffle runtime or by the /// AVM1 bytecode itself. #[derive(Clone)] @@ -394,9 +367,10 @@ impl<'gc> Executable<'gc> { //TODO: What happens if the argument registers clash with the //preloaded registers? What gets done last? for (param, value) in af.params.iter().zip(args_iter) { - match param { - (Some(argreg), _argname) => frame.set_local_register(*argreg, value), - (None, argname) => frame.force_define_local(*argname, value), + if let Some(register) = param.register { + frame.set_local_register(register.get(), value); + } else { + frame.force_define_local(param.name, value); } } diff --git a/swf/src/avm1/read.rs b/swf/src/avm1/read.rs index 62f4740fc..8dcc59376 100644 --- a/swf/src/avm1/read.rs +++ b/swf/src/avm1/read.rs @@ -1,6 +1,7 @@ use crate::avm1::{opcode::OpCode, types::*}; use crate::error::{Error, Result}; use crate::extensions::ReadSwfExt; +use std::num::NonZeroU8; pub struct Reader<'a> { input: &'a [u8], @@ -235,10 +236,9 @@ impl<'a> Reader<'a> { let flags = FunctionFlags::from_bits_truncate(self.read_u16()?); let mut params = Vec::with_capacity(num_params as usize); for _ in 0..num_params { - let register = self.read_u8()?; params.push(FunctionParam { + register_index: NonZeroU8::new(self.read_u8()?), name: self.read_str()?, - register_index: if register == 0 { None } else { Some(register) }, }); } // code_length isn't included in the DefineFunction's length. diff --git a/swf/src/avm1/types.rs b/swf/src/avm1/types.rs index 95769bfa6..b4db7f461 100644 --- a/swf/src/avm1/types.rs +++ b/swf/src/avm1/types.rs @@ -1,5 +1,6 @@ use crate::string::SwfStr; use bitflags::bitflags; +use std::num::NonZeroU8; #[derive(Clone, Debug, PartialEq)] pub enum Action<'a> { @@ -127,10 +128,31 @@ pub struct DefineFunction2<'a> { pub actions: &'a [u8], } +impl<'a> From> for DefineFunction2<'a> { + #[inline] + fn from(function: DefineFunction<'a>) -> Self { + let params = function + .params + .into_iter() + .map(|param| FunctionParam { + name: param, + register_index: None, + }) + .collect(); + Self { + name: function.name, + register_count: 0, + params, + flags: FunctionFlags::default(), + actions: function.actions, + } + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct FunctionParam<'a> { pub name: &'a SwfStr, - pub register_index: Option, + pub register_index: Option, } bitflags! { @@ -147,6 +169,12 @@ bitflags! { } } +impl Default for FunctionFlags { + fn default() -> Self { + Self::empty() + } +} + #[derive(Clone, Debug, PartialEq)] pub struct GetUrl<'a> { pub url: &'a SwfStr, diff --git a/swf/src/avm1/write.rs b/swf/src/avm1/write.rs index ffa21465a..5a8b92cff 100644 --- a/swf/src/avm1/write.rs +++ b/swf/src/avm1/write.rs @@ -248,11 +248,7 @@ impl Writer { self.write_u8(action.register_count)?; self.write_u16(action.flags.bits())?; for param in &action.params { - self.write_u8(if let Some(n) = param.register_index { - n - } else { - 0 - })?; + self.write_u8(param.register_index.map(|n| n.get()).unwrap_or_default())?; self.write_string(param.name)?; } self.write_u16(action.actions.len() as u16)?;