Skip to content

Commit 0abdcbd

Browse files
committed
avm1: Remove is_slash_path argument in display_object::get_property
Having it set to `true` prevented the '_parent fallback' behavior in some cases where it shouldn't have (see added test), and it didn't have any other effect. Removing it also simplifies a lot of code, so this is a win/win!
1 parent faa3e95 commit 0abdcbd

File tree

17 files changed

+161
-122
lines changed

17 files changed

+161
-122
lines changed

core/src/avm1/activation.rs

Lines changed: 18 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,7 +1070,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
10701070
let object_val = self.context.avm1.pop();
10711071
let object = object_val.coerce_to_object(self);
10721072

1073-
let result = object.get_non_slash_path(name, self)?;
1073+
let result = object.get(name, self)?;
10741074
self.stack_push(result);
10751075

10761076
Ok(FrameControl::Continue)
@@ -2495,7 +2495,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
24952495
let root = start.avm1_root();
24962496
let start = start.object1_or_bare(self.gc());
24972497
Ok(self
2498-
.resolve_target_path(root, start, &path, false, true)?
2498+
.resolve_target_path(root, start, &path, false)?
24992499
.and_then(|o| o.as_display_object()))
25002500
}
25012501

@@ -2514,7 +2514,6 @@ impl<'a, 'gc> Activation<'a, 'gc> {
25142514
start: Object<'gc>,
25152515
mut path: &WStr,
25162516
mut first_element: bool,
2517-
path_has_slash: bool,
25182517
) -> Result<Option<Object<'gc>>, Error<'gc>> {
25192518
// Empty path resolves immediately to start clip.
25202519
if path.is_empty() {
@@ -2589,26 +2588,15 @@ impl<'a, 'gc> Activation<'a, 'gc> {
25892588
.and_then(|o| o.as_container())
25902589
.and_then(|o| o.child_by_name(name, case_sensitive))
25912590
{
2592-
if path_has_slash {
2593-
child.object1_or_undef()
2594-
} else if child.object1().is_none() {
2595-
// If an object doesn't have an object representation,
2596-
// e.g. Graphic, then trying to access it returns
2597-
// the parent instead
2598-
child
2599-
.parent()
2600-
.map(|p| p.object1_or_undef())
2601-
.unwrap_or(Value::Undefined)
2602-
} else {
2603-
child.object1_or_undef()
2604-
}
2591+
child
2592+
.object1()
2593+
// If an object doesn't have an object representation, e.g. Graphic,
2594+
// then trying to access it returns the parent instead
2595+
.or_else(|| child.parent().and_then(|p| p.object1()))
2596+
.map_or(Value::Undefined, Value::from)
26052597
} else {
26062598
let name = AvmString::new(self.gc(), name);
2607-
if path_has_slash {
2608-
object.get(name, self).unwrap()
2609-
} else {
2610-
object.get_non_slash_path(name, self).unwrap()
2611-
}
2599+
object.get(name, self).unwrap()
26122600
}
26132601
}
26142602
};
@@ -2636,8 +2624,6 @@ impl<'a, 'gc> Activation<'a, 'gc> {
26362624
start: DisplayObject<'gc>,
26372625
path: &'s WStr,
26382626
) -> Result<Option<(Object<'gc>, &'s WStr)>, Error<'gc>> {
2639-
let path_has_slash = path.contains(b'/');
2640-
26412627
// Find the right-most : or . in the path.
26422628
// If we have one, we must resolve as a target path.
26432629
if let Some(separator) = path.rfind(b":.".as_ref()) {
@@ -2648,13 +2634,9 @@ impl<'a, 'gc> Activation<'a, 'gc> {
26482634
for scope in Scope::ancestors(self.scope()) {
26492635
let avm1_root = start.avm1_root();
26502636

2651-
if let Some(object) = self.resolve_target_path(
2652-
avm1_root,
2653-
*scope.locals(),
2654-
path,
2655-
true,
2656-
path_has_slash,
2657-
)? {
2637+
if let Some(object) =
2638+
self.resolve_target_path(avm1_root, *scope.locals(), path, true)?
2639+
{
26582640
return Ok(Some((object, var_name)));
26592641
}
26602642
}
@@ -2692,8 +2674,6 @@ impl<'a, 'gc> Activation<'a, 'gc> {
26922674
// Resolve a variable path for a GetVariable action.
26932675
let start = self.target_clip_or_root();
26942676

2695-
let path_has_slash = path.contains(b'/');
2696-
26972677
// Find the right-most : or . in the path.
26982678
// If we have one, we must resolve as a target path.
26992679
if let Some(separator) = path.rfind(b":.".as_ref()) {
@@ -2704,13 +2684,9 @@ impl<'a, 'gc> Activation<'a, 'gc> {
27042684
for scope in Scope::ancestors(self.scope()) {
27052685
let avm1_root = start.avm1_root();
27062686

2707-
if let Some(object) = self.resolve_target_path(
2708-
avm1_root,
2709-
*scope.locals(),
2710-
path,
2711-
true,
2712-
path_has_slash,
2713-
)? {
2687+
if let Some(object) =
2688+
self.resolve_target_path(avm1_root, *scope.locals(), path, true)?
2689+
{
27142690
let var_name = AvmString::new(self.gc(), var_name);
27152691
if object.has_property(self, var_name) {
27162692
return Ok(CallableValue::Callable(object, object.get(var_name, self)?));
@@ -2727,7 +2703,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
27272703
let avm1_root = start.avm1_root();
27282704

27292705
if let Some(object) =
2730-
self.resolve_target_path(avm1_root, *scope.locals(), &path, false, true)?
2706+
self.resolve_target_path(avm1_root, *scope.locals(), &path, false)?
27312707
{
27322708
return Ok(CallableValue::UnCallable(object.into()));
27332709
}
@@ -2793,7 +2769,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
27932769
let avm1_root = start.avm1_root();
27942770

27952771
if let Some(object) =
2796-
self.resolve_target_path(avm1_root, *scope.locals(), path, true, true)?
2772+
self.resolve_target_path(avm1_root, *scope.locals(), path, true)?
27972773
{
27982774
let var_name = AvmString::new(self.gc(), var_name);
27992775
object.set(var_name, value, self)?;
@@ -3011,7 +2987,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
30112987
if target.is_empty() {
30122988
new_target_clip = Some(base_clip);
30132989
} else if let Some(clip) = self
3014-
.resolve_target_path(root, start, target, false, true)?
2990+
.resolve_target_path(root, start, target, false)?
30152991
.and_then(|o| o.as_display_object())
30162992
.filter(|_| !self.base_clip.avm1_removed())
30172993
// All properties invalid if base clip is removed.

core/src/avm1/globals/array.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -643,10 +643,10 @@ fn sort_on_compare<'a, 'gc>(fields: &'a [(AvmString<'gc>, SortOptions)]) -> Comp
643643
if let [Value::Object(a), Value::Object(b)] = [a, b] {
644644
for (field_name, options) in fields {
645645
let a_prop = a
646-
.get_local_stored(*field_name, activation, false)
646+
.get_local_stored(*field_name, activation)
647647
.unwrap_or(Value::Undefined);
648648
let b_prop = b
649-
.get_local_stored(*field_name, activation, false)
649+
.get_local_stored(*field_name, activation)
650650
.unwrap_or(Value::Undefined);
651651

652652
let result = sort_compare(activation, &a_prop, &b_prop, *options)?;

core/src/avm1/globals/bitmap_data.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ fn clone<'gc>(
410410
};
411411

412412
Ok(new_bitmap_data(
413-
this.get_local_stored(istr!("__proto__"), activation, false),
413+
this.get_local_stored(istr!("__proto__"), activation),
414414
bitmap_data.clone_data(activation.context.gc_context, activation.context.renderer),
415415
activation,
416416
)
@@ -795,8 +795,8 @@ fn hit_test<'gc>(
795795

796796
let first_point = args.get_object(activation, 0);
797797
let top_left = if let (Some(x), Some(y)) = (
798-
first_point.get_local_stored(istr!("x"), activation, false),
799-
first_point.get_local_stored(istr!("y"), activation, false),
798+
first_point.get_local_stored(istr!("x"), activation),
799+
first_point.get_local_stored(istr!("y"), activation),
800800
) {
801801
(x.coerce_to_i32(activation)?, y.coerce_to_i32(activation)?)
802802
} else {
@@ -816,8 +816,8 @@ fn hit_test<'gc>(
816816

817817
let second_point = args.get_object(activation, 3);
818818
let second_point = if let (Some(x), Some(y)) = (
819-
second_point.get_local_stored(istr!("x"), activation, false),
820-
second_point.get_local_stored(istr!("y"), activation, false),
819+
second_point.get_local_stored(istr!("x"), activation),
820+
second_point.get_local_stored(istr!("y"), activation),
821821
) {
822822
(x.coerce_to_i32(activation)?, y.coerce_to_i32(activation)?)
823823
} else if args.len() > 3 {
@@ -842,10 +842,10 @@ fn hit_test<'gc>(
842842
// Determine what kind of Object we have, point or rectangle.
843843
// Duck-typed dumb objects are allowed.
844844
let compare_fields = (
845-
compare_object.get_local_stored(istr!("x"), activation, false),
846-
compare_object.get_local_stored(istr!("y"), activation, false),
847-
compare_object.get_local_stored(istr!("width"), activation, false),
848-
compare_object.get_local_stored(istr!("height"), activation, false),
845+
compare_object.get_local_stored(istr!("x"), activation),
846+
compare_object.get_local_stored(istr!("y"), activation),
847+
compare_object.get_local_stored(istr!("width"), activation),
848+
compare_object.get_local_stored(istr!("height"), activation),
849849
);
850850
match compare_fields {
851851
// BitmapData vs. point
@@ -1276,7 +1276,7 @@ fn compare<'gc>(
12761276
other_bitmap_data,
12771277
) {
12781278
Some(bitmap_data) => Ok(new_bitmap_data(
1279-
this.get_local_stored(istr!("__proto__"), activation, false),
1279+
this.get_local_stored(istr!("__proto__"), activation),
12801280
bitmap_data,
12811281
activation,
12821282
)
@@ -1322,7 +1322,7 @@ fn load_bitmap<'gc>(
13221322
.collect(),
13231323
);
13241324
Ok(new_bitmap_data(
1325-
this.get_local_stored(istr!("prototype"), activation, false),
1325+
this.get_local_stored(istr!("prototype"), activation),
13261326
bitmap_data,
13271327
activation,
13281328
)

core/src/avm1/globals/bitmap_filter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ pub fn clone<'gc>(
6565
}
6666
_ => return Ok(Value::Undefined),
6767
};
68-
let proto = this.get_local_stored(istr!("__proto__"), activation, false);
68+
let proto = this.get_local_stored(istr!("__proto__"), activation);
6969
Ok(create_instance(activation, native, proto).into())
7070
}
7171

core/src/avm1/globals/displacement_map_filter.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,9 @@ impl<'gc> DisplacementMapFilter<'gc> {
120120
let Some(value) = value else { return Ok(()) };
121121

122122
if let Value::Object(object) = value {
123-
if let Some(x) = object.get_local_stored(istr!("x"), activation, false) {
123+
if let Some(x) = object.get_local_stored(istr!("x"), activation) {
124124
let x = x.coerce_to_f64(activation)?.clamp_to_i32();
125-
if let Some(y) = object.get_local_stored(istr!("y"), activation, false) {
125+
if let Some(y) = object.get_local_stored(istr!("y"), activation) {
126126
let y = y.coerce_to_f64(activation)?.clamp_to_i32();
127127
self.0.map_point.set(Point::new(x, y));
128128
return Ok(());

core/src/avm1/globals/file_reference.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -251,17 +251,15 @@ pub fn browse<'gc>(
251251

252252
for i in 0..length {
253253
if let Value::Object(element) = array.get_element(activation, i) {
254-
let mac_type = if let Some(val) =
255-
element.get_local_stored(istr!("macType"), activation, false)
256-
{
257-
Some(val.coerce_to_string(activation)?.to_string())
258-
} else {
259-
None
260-
};
261-
262-
let description =
263-
element.get_local_stored(istr!("description"), activation, false);
264-
let extension = element.get_local_stored(istr!("extension"), activation, false);
254+
let mac_type =
255+
if let Some(val) = element.get_local_stored(istr!("macType"), activation) {
256+
Some(val.coerce_to_string(activation)?.to_string())
257+
} else {
258+
None
259+
};
260+
261+
let description = element.get_local_stored(istr!("description"), activation);
262+
let extension = element.get_local_stored(istr!("extension"), activation);
265263

266264
if let (Some(description), Some(extension)) = (description, extension) {
267265
let description = description.coerce_to_string(activation)?.to_string();

core/src/avm1/globals/matrix.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,12 +148,12 @@ pub fn object_to_matrix_or_default<'gc>(
148148
) -> Result<Matrix, Error<'gc>> {
149149
if let (Some(a), Some(b), Some(c), Some(d), Some(tx), Some(ty)) = (
150150
// These lookups do not search the prototype chain and ignore virtual properties.
151-
object.get_local_stored(istr!("a"), activation, false),
152-
object.get_local_stored(istr!("b"), activation, false),
153-
object.get_local_stored(istr!("c"), activation, false),
154-
object.get_local_stored(istr!("d"), activation, false),
155-
object.get_local_stored(istr!("tx"), activation, false),
156-
object.get_local_stored(istr!("ty"), activation, false),
151+
object.get_local_stored(istr!("a"), activation),
152+
object.get_local_stored(istr!("b"), activation),
153+
object.get_local_stored(istr!("c"), activation),
154+
object.get_local_stored(istr!("d"), activation),
155+
object.get_local_stored(istr!("tx"), activation),
156+
object.get_local_stored(istr!("ty"), activation),
157157
) {
158158
let a = a.coerce_to_f64(activation)? as f32;
159159
let b = b.coerce_to_f64(activation)? as f32;

core/src/avm1/globals/movie_clip.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ pub fn object_to_rectangle<'gc>(
153153
let mut values = [0; 4];
154154

155155
for (&name, value) in names.iter().zip(&mut values) {
156-
*value = match object.get_local_stored(name, activation, false) {
156+
*value = match object.get_local_stored(name, activation) {
157157
Some(value) => value.coerce_to_i32(activation)?,
158158
None => return Ok(None),
159159
}
@@ -1378,10 +1378,10 @@ fn local_to_global<'gc>(
13781378
// It does not search the prototype chain and ignores virtual properties.
13791379
if let (Value::Number(x), Value::Number(y)) = (
13801380
point
1381-
.get_local_stored(istr!("x"), activation, false)
1381+
.get_local_stored(istr!("x"), activation)
13821382
.unwrap_or(Value::Undefined),
13831383
point
1384-
.get_local_stored(istr!("y"), activation, false)
1384+
.get_local_stored(istr!("y"), activation)
13851385
.unwrap_or(Value::Undefined),
13861386
) {
13871387
let local = Point::from_pixels(x, y);
@@ -1560,10 +1560,10 @@ fn global_to_local<'gc>(
15601560
// It does not search the prototype chain and ignores virtual properties.
15611561
if let (Value::Number(x), Value::Number(y)) = (
15621562
point
1563-
.get_local_stored(istr!("x"), activation, false)
1563+
.get_local_stored(istr!("x"), activation)
15641564
.unwrap_or(Value::Undefined),
15651565
point
1566-
.get_local_stored(istr!("y"), activation, false)
1566+
.get_local_stored(istr!("y"), activation)
15671567
.unwrap_or(Value::Undefined),
15681568
) {
15691569
let global = Point::from_pixels(x, y);

core/src/avm1/object.rs

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -144,36 +144,18 @@ impl<'gc> NativeObject<'gc> {
144144
}
145145

146146
impl<'gc> Object<'gc> {
147-
/// Retrieve a named property from the object, or its prototype.
148-
pub fn get_non_slash_path(
149-
self,
150-
name: impl Into<AvmString<'gc>>,
151-
activation: &mut Activation<'_, 'gc>,
152-
) -> Result<Value<'gc>, Error<'gc>> {
153-
self.lookup(name, activation, false)
154-
}
155-
156147
/// Retrieve a named property from the object, or its prototype.
157148
pub fn get(
158149
self,
159150
name: impl Into<AvmString<'gc>>,
160151
activation: &mut Activation<'_, 'gc>,
161-
) -> Result<Value<'gc>, Error<'gc>> {
162-
self.lookup(name, activation, true)
163-
}
164-
165-
fn lookup(
166-
self,
167-
name: impl Into<AvmString<'gc>>,
168-
activation: &mut Activation<'_, 'gc>,
169-
is_slash_path: bool,
170152
) -> Result<Value<'gc>, Error<'gc>> {
171153
let (this, proto) = if let Some(super_object) = self.as_super_object() {
172154
(super_object.this(), super_object.proto(activation))
173155
} else {
174156
(self, Value::Object(self))
175157
};
176-
match search_prototype(proto, name.into(), activation, this, is_slash_path, true)? {
158+
match search_prototype(proto, name.into(), activation, this, true)? {
177159
Some((value, _depth)) => Ok(value),
178160
None => Ok(Value::Undefined),
179161
}
@@ -193,7 +175,7 @@ impl<'gc> Object<'gc> {
193175
return Err(Error::PrototypeRecursionLimit);
194176
}
195177

196-
if let Some(value) = p.get_local_stored(name, activation, true) {
178+
if let Some(value) = p.get_local_stored(name, activation) {
197179
return Ok(value);
198180
}
199181

@@ -280,7 +262,7 @@ impl<'gc> Object<'gc> {
280262
}
281263

282264
let (method, depth) =
283-
match search_prototype(Value::Object(self), name, activation, self, false, true)? {
265+
match search_prototype(Value::Object(self), name, activation, self, true)? {
284266
Some((Value::Object(method), depth)) => (method, depth),
285267
_ => return Ok(Value::Undefined),
286268
};
@@ -394,7 +376,6 @@ pub fn search_prototype<'gc>(
394376
name: AvmString<'gc>,
395377
activation: &mut Activation<'_, 'gc>,
396378
this: Object<'gc>,
397-
is_slash_path: bool,
398379
call_resolve_fn: bool,
399380
) -> Result<Option<(Value<'gc>, u8)>, Error<'gc>> {
400381
let mut depth = 0;
@@ -425,7 +406,7 @@ pub fn search_prototype<'gc>(
425406
}
426407
}
427408

428-
if let Some(value) = p.get_local_stored(name, activation, is_slash_path) {
409+
if let Some(value) = p.get_local_stored(name, activation) {
429410
return Ok(Some((value, depth)));
430411
}
431412

@@ -456,7 +437,7 @@ pub fn find_resolve_method<'gc>(
456437
return Err(Error::PrototypeRecursionLimit);
457438
}
458439

459-
let resolve = p.get_local_stored(istr!("__resolve"), activation, false);
440+
let resolve = p.get_local_stored(istr!("__resolve"), activation);
460441
// FP completely skips over primitives (but not over non-function objects).
461442
if let Some(Value::Object(value)) = resolve {
462443
return Ok(Some(value));

0 commit comments

Comments
 (0)