Skip to content

Commit 2f8dde8

Browse files
authored
avm2: Include class name in ScriptObject debug (ruffle-rs#7512)
* avm2: Include class name in ScriptObject debug Currently, the `ScriptObject` debug impl is almost useless - while you determine if two printed objects are the same by comparing the pointer value, you'll have no idea what kind of object it actually is. This PR now formats the `ScriptObject` output as a struct, printing a (fake) "class" field containing the class name. Before/after: ``` [ERROR ruffle_core::avm2::activation] AVM2 error: Cannot coerce Object(ScriptObject(ScriptObject(GcCell(Gc { ptr: 0x55f863936db8 })))) to an QName { ns: Private("Test.as$38"), name: "Second" } [ERROR ruffle_core::avm2::activation] AVM2 error: Cannot coerce Object(ScriptObject(ScriptObject { class: "Object", ptr: 0x55ee0ad161e0 })) to an QName { ns: Private("Test.as$38"), name: "Second" } ``` Getting access to the class name from a `Debug` impl is tricky: Developers can (and should be able to) insert logging statements whereever they want, so any `GcCell` may be mutably borrowed. Panics in debug impls are extremely frustrating to deal with, so I've ensured that we only use `try_borrow` at each step. If any of the attempted borrows fail, we print out an error message in the "class_name" field, but we're still able to print the rest of the `ScriptObject`. Additionally, we have no access to a `MutationContext`, so we cannot allocate a new `AvmString`. To get around this, I've created a new method `QName::to_qualified_name_no_mc`, which uses an `Either` to return a `WString` instead of allocating an `AvmString`. This is more cumbersome to work with than the nrmal `QName::to_qualified_name`, so we'll only want to use it when we have no other choice.
1 parent 2383e68 commit 2f8dde8

File tree

5 files changed

+84
-9
lines changed

5 files changed

+84
-9
lines changed

core/src/avm2/names.rs

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ use crate::avm2::activation::Activation;
44
use crate::avm2::script::TranslationUnit;
55
use crate::avm2::value::Value;
66
use crate::avm2::Error;
7+
use crate::either::Either;
78
use crate::string::{AvmString, WStr, WString};
89
use gc_arena::{Collect, MutationContext};
10+
use std::fmt::Debug;
911
use swf::avm2::types::{
1012
AbcFile, Index, Multiname as AbcMultiname, Namespace as AbcNamespace,
1113
NamespaceSet as AbcNamespaceSet,
@@ -135,7 +137,7 @@ impl<'gc> Namespace<'gc> {
135137
/// `QName`. All other forms of names and multinames are either versions of
136138
/// `QName` with unspecified parameters, or multiple names to be checked in
137139
/// order.
138-
#[derive(Clone, Copy, Collect, Debug, Hash)]
140+
#[derive(Clone, Copy, Collect, Hash)]
139141
#[collect(no_drop)]
140142
pub struct QName<'gc> {
141143
ns: Namespace<'gc>,
@@ -222,14 +224,32 @@ impl<'gc> QName<'gc> {
222224

223225
/// Converts this `QName` to a fully qualified name.
224226
pub fn to_qualified_name(self, mc: MutationContext<'gc, '_>) -> AvmString<'gc> {
227+
match self.to_qualified_name_no_mc() {
228+
Either::Left(avm_string) => avm_string,
229+
Either::Right(wstring) => AvmString::new(mc, wstring),
230+
}
231+
}
232+
233+
/// Like `to_qualified_name`, but avoids the need for a `MutationContext`
234+
/// by returning `Either::Right(wstring)` when it would otherwise
235+
/// be necessary to allocate a new `AvmString`.
236+
///
237+
/// This method is intended for contexts like `Debug` impls where
238+
/// a `MutationContext` is not available. Normally, you should
239+
/// use `to_qualified_name`
240+
pub fn to_qualified_name_no_mc(self) -> Either<AvmString<'gc>, WString> {
225241
let uri = self.namespace().as_uri();
226242
let name = self.local_name();
227-
uri.is_empty().then_some(name).unwrap_or_else(|| {
228-
let mut buf = WString::from(uri.as_wstr());
229-
buf.push_str(WStr::from_units(b"::"));
230-
buf.push_str(&name);
231-
AvmString::new(mc, buf)
232-
})
243+
if uri.is_empty() {
244+
Either::Left(name)
245+
} else {
246+
Either::Right({
247+
let mut buf = WString::from(uri.as_wstr());
248+
buf.push_str(WStr::from_units(b"::"));
249+
buf.push_str(&name);
250+
buf
251+
})
252+
}
233253
}
234254

235255
pub fn local_name(&self) -> AvmString<'gc> {
@@ -264,6 +284,15 @@ impl<'gc> QName<'gc> {
264284
}
265285
}
266286

287+
impl<'gc> Debug for QName<'gc> {
288+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
289+
match self.to_qualified_name_no_mc() {
290+
Either::Left(name) => write!(f, "{}", name),
291+
Either::Right(name) => write!(f, "{}", name),
292+
}
293+
}
294+
}
295+
267296
/// A `Multiname` consists of a name which could be resolved in one or more
268297
/// potential namespaces.
269298
///

core/src/avm2/object/class_object.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::avm2::TranslationUnit;
1717
use crate::string::AvmString;
1818
use fnv::FnvHashMap;
1919
use gc_arena::{Collect, GcCell, MutationContext};
20-
use std::cell::{Ref, RefMut};
20+
use std::cell::{BorrowError, Ref, RefMut};
2121
use std::hash::{Hash, Hasher};
2222

2323
/// An Object which can be called to execute its function code.
@@ -706,6 +706,15 @@ impl<'gc> ClassObject<'gc> {
706706
self.0.read().class_vtable
707707
}
708708

709+
/// Like `inner_class_definition`, but returns an `Err(BorrowError)` instead of panicking
710+
/// if our `GcCell` is already mutably borrowed. This is useful
711+
/// in contexts where panicking would be extremely undesirable,
712+
/// and there's a fallback if we cannot obtain the `Class`
713+
/// (such as `Debug` impls),
714+
pub fn try_inner_class_definition(&self) -> Result<GcCell<'gc, Class<'gc>>, BorrowError> {
715+
self.0.try_read().map(|c| c.class)
716+
}
717+
709718
pub fn inner_class_definition(self) -> GcCell<'gc, Class<'gc>> {
710719
self.0.read().class
711720
}

core/src/avm2/object/script_object.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ pub fn scriptobject_allocator<'gc>(
2424
}
2525

2626
/// Default implementation of `avm2::Object`.
27-
#[derive(Clone, Collect, Debug, Copy)]
27+
#[derive(Clone, Collect, Copy)]
2828
#[collect(no_drop)]
2929
pub struct ScriptObject<'gc>(GcCell<'gc, ScriptObjectData<'gc>>);
3030

@@ -429,3 +429,35 @@ impl<'gc> ScriptObjectData<'gc> {
429429
self.vtable = Some(vtable);
430430
}
431431
}
432+
433+
impl<'gc> Debug for ScriptObject<'gc> {
434+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
435+
let mut fmt = f.debug_struct("ScriptObject");
436+
437+
let class_name = self
438+
.0
439+
.try_read()
440+
.map(|obj| obj.instance_of())
441+
.transpose()
442+
.map(|class_obj| {
443+
class_obj
444+
.and_then(|class_obj| class_obj.try_inner_class_definition())
445+
.and_then(|class| class.try_read().map(|c| c.name()))
446+
});
447+
448+
match class_name {
449+
Some(Ok(class_name)) => {
450+
fmt.field("class", &class_name);
451+
}
452+
Some(Err(err)) => {
453+
fmt.field("class", &err);
454+
}
455+
None => {
456+
fmt.field("class", &"<None>");
457+
}
458+
}
459+
460+
fmt.field("ptr", &self.0.as_ptr());
461+
fmt.finish()
462+
}
463+
}

core/src/either.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
pub enum Either<A, B> {
2+
Left(A),
3+
Right(B),
4+
}

core/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub mod context;
2323
pub mod context_menu;
2424
mod drawing;
2525
mod ecma_conversions;
26+
pub(crate) mod either;
2627
pub mod events;
2728
pub mod focus_tracker;
2829
mod font;

0 commit comments

Comments
 (0)