Since it is possible to have virtual properties in the scope chain, overwriting them should trigger their setters.

Define, since it's intended for setting locals only, always uses force-set and does not trigger setters.
This commit is contained in:
David Wendt 2019-10-02 21:18:37 -04:00 committed by Mike Welsh
parent 06d0cf5ed1
commit 5873eefb06
2 changed files with 22 additions and 12 deletions

View File

@ -1399,11 +1399,11 @@ impl<'gc> Avm1<'gc> {
} }
} }
} else { } else {
match self.current_stack_frame().unwrap().scope().overwrite(var_path, value, context.gc_context) { let this = self.current_stack_frame().unwrap().this_cell();
None => {}, let scope = self.current_stack_frame().unwrap().scope_cell();
Some(value) => { let unused_value = scope.read().overwrite(var_path, value, self, context, this);
self.current_stack_frame().unwrap().define(var_path, value, context.gc_context); if let Some(value) = unused_value {
} self.current_stack_frame().unwrap().define(var_path, value, context.gc_context);
} }
} }

View File

@ -2,7 +2,7 @@
use std::cell::{Ref, RefMut}; use std::cell::{Ref, RefMut};
use gc_arena::{GcCell, MutationContext}; use gc_arena::{GcCell, MutationContext};
use crate::avm1::{Object, Value}; use crate::avm1::{Avm1, ActionContext, Object, Value};
/// Indicates what kind of scope a scope is. /// Indicates what kind of scope a scope is.
#[derive(Copy, Clone, #[derive(Copy, Clone,
@ -173,23 +173,33 @@ impl<'gc> Scope<'gc> {
/// ///
/// If the value is currently already defined in this scope, then it will /// If the value is currently already defined in this scope, then it will
/// be overwritten. If it is not defined, then we traverse the scope chain /// be overwritten. If it is not defined, then we traverse the scope chain
/// until we find a defined value to overwrite. If no value can be /// until we find a defined value to overwrite. We do not define a property
/// overwritten, then we return the unwritten value so that it may be used /// if it is not already defined somewhere in the scope chain, and instead
/// in some other way. /// return it so that the caller may manually define the property itself.
pub fn overwrite(&self, name: &str, value: Value<'gc>, mc: MutationContext<'gc, '_>) -> Option<Value<'gc>> { pub fn overwrite(&self,
name: &str,
value: Value<'gc>,
avm: &mut Avm1<'gc>,
context: &mut ActionContext<'_, 'gc, '_>,
this: GcCell<'gc, Object<'gc>>) -> Option<Value<'gc>> {
if self.locals().has_property(name) { if self.locals().has_property(name) {
self.locals_mut(mc).force_set(name, value); self.locals_mut(context.gc_context).set(name, value, avm, context, this);
return None; return None;
} }
if let Some(scope) = self.parent() { if let Some(scope) = self.parent() {
return scope.overwrite(name, value, mc); return scope.overwrite(name, value, avm, context, this);
} }
Some(value) Some(value)
} }
/// Set a particular value in the locals for this scope. /// Set a particular value in the locals for this scope.
///
/// By convention, the locals for a given function are always defined as
/// stored (e.g. not virtual) properties on the lowest object in the scope
/// chain. As a result, this function always force sets a property on the
/// local object and does not traverse the scope chain.
pub fn define(&self, name: &str, value: Value<'gc>, mc: MutationContext<'gc, '_>) { pub fn define(&self, name: &str, value: Value<'gc>, mc: MutationContext<'gc, '_>) {
self.locals_mut(mc).force_set(name, value); self.locals_mut(mc).force_set(name, value);
} }