From a6611026c2e6f2d842382d9930550d39441c7474 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sun, 7 Dec 2025 18:20:34 -0800 Subject: [PATCH 01/14] avm2: Make `Stage3DObject::new` take an `UpdateContext` ...instead of `Activation` --- core/src/avm2/object/stage3d_object.rs | 8 ++++---- core/src/display_object/stage.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/avm2/object/stage3d_object.rs b/core/src/avm2/object/stage3d_object.rs index 99cb11e513ff..f7713ac676cb 100644 --- a/core/src/avm2/object/stage3d_object.rs +++ b/core/src/avm2/object/stage3d_object.rs @@ -1,8 +1,8 @@ //! Object representation for Stage3D objects -use crate::avm2::activation::Activation; use crate::avm2::object::script_object::ScriptObjectData; use crate::avm2::object::{Object, TObject}; +use crate::context::UpdateContext; use core::fmt; use gc_arena::barrier::unlock; use gc_arena::lock::Lock; @@ -27,10 +27,10 @@ impl fmt::Debug for Stage3DObject<'_> { } impl<'gc> Stage3DObject<'gc> { - pub fn new(activation: &mut Activation<'_, 'gc>) -> Self { - let class = activation.avm2().classes().stage3d; + pub fn new(context: &mut UpdateContext<'gc>) -> Self { + let class = context.avm2.classes().stage3d; Stage3DObject(Gc::new( - activation.gc(), + context.gc(), Stage3DObjectData { base: ScriptObjectData::new(class), context3d: Lock::new(None), diff --git a/core/src/display_object/stage.rs b/core/src/display_object/stage.rs index 4ee7387fff48..9414838857fb 100644 --- a/core/src/display_object/stage.rs +++ b/core/src/display_object/stage.rs @@ -832,7 +832,7 @@ impl<'gc> TDisplayObject<'gc> for Stage<'gc> { Ok(avm2_stage) => { // Always create 4 Stage3D instances for now, which matches the flash projector behavior let stage3ds: Vec> = (0..4) - .map(|_| Stage3DObject::new(&mut activation).into()) + .map(|_| Stage3DObject::new(activation.context).into()) .collect(); let write = Gc::write(activation.gc(), self.0); From 6b6ae1c441e1608deb963eaf7fc63f6b51f7262c Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sun, 7 Dec 2025 18:26:49 -0800 Subject: [PATCH 02/14] core: Clean up `Stage::post_instantiation` - Remove an old comment - Use `Avm2StageObject::for_display_object` and avoid constructing an `Activation` --- core/src/display_object/stage.rs | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/core/src/display_object/stage.rs b/core/src/display_object/stage.rs index 9414838857fb..ce4e73cc9c0a 100644 --- a/core/src/display_object/stage.rs +++ b/core/src/display_object/stage.rs @@ -817,30 +817,18 @@ impl<'gc> TDisplayObject<'gc> for Stage<'gc> { ) { let stage_constr = context.avm2.classes().stage; - // TODO: Replace this when we have a convenience method for constructing AVM2 native objects. // TODO: We should only do this if the movie is actually an AVM2 movie. // This is necessary for DisplayObject and EventDispatcher super-constructors to run. - let global_domain = context.avm2.stage_domain(); - let mut activation = Avm2Activation::from_domain(context, global_domain); - let avm2_stage = Avm2StageObject::for_display_object_childless( - &mut activation, - self.into(), - stage_constr, - ); - - match avm2_stage { - Ok(avm2_stage) => { - // Always create 4 Stage3D instances for now, which matches the flash projector behavior - let stage3ds: Vec> = (0..4) - .map(|_| Stage3DObject::new(activation.context).into()) - .collect(); - - let write = Gc::write(activation.gc(), self.0); - unlock!(write, StageData, avm2_object).set(Some(avm2_stage)); - unlock!(write, StageData, stage3ds).replace(stage3ds); - } - Err(e) => tracing::error!("Unable to construct AVM2 Stage: {}", e), - } + let avm2_stage = + Avm2StageObject::for_display_object(context.gc(), self.into(), stage_constr); + + // Always create 4 Stage3D instances for now, which matches the flash projector behavior + let stage3ds: Vec> = + (0..4).map(|_| Stage3DObject::new(context).into()).collect(); + + let write = Gc::write(context.gc(), self.0); + unlock!(write, StageData, avm2_object).set(Some(avm2_stage)); + unlock!(write, StageData, stage3ds).replace(stage3ds); } fn id(self) -> CharacterId { From 43f9633c2dbd41560f65df583f63852a4c75b4ea Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sun, 7 Dec 2025 22:55:50 -0800 Subject: [PATCH 03/14] avm2: Make `sound_object::play_queued` and its callers infallible `sound_object::play_queued` returned a `Result`, but never returned the error variant --- core/src/avm2/globals/flash/media/sound.rs | 6 ++--- core/src/avm2/object/sound_object.rs | 27 ++++++++-------------- core/src/loader.rs | 4 +--- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/core/src/avm2/globals/flash/media/sound.rs b/core/src/avm2/globals/flash/media/sound.rs index 7545505f72d9..1ca76b1e773e 100644 --- a/core/src/avm2/globals/flash/media/sound.rs +++ b/core/src/avm2/globals/flash/media/sound.rs @@ -40,7 +40,7 @@ pub fn init<'gc>( .library_for_movie_mut(movie) .character_by_id(symbol) { - sound_object.set_sound(activation.context, sound)?; + sound_object.set_sound(activation.context, sound); } else { tracing::warn!("Attempted to construct subclass of Sound, {}, which is associated with non-Sound character {}", class_def.name().local_name(), symbol); } @@ -178,7 +178,7 @@ pub fn play<'gc>( sound_transform, sound_channel, }; - if sound_object.play(queued_play, activation)? { + if sound_object.play(queued_play, activation) { return Ok(sound_channel.into()); } // If we start playing a loaded sound with an invalid position, @@ -297,7 +297,7 @@ pub fn load_compressed_data_from_byte_array<'gc>( Avm2::dispatch_event(activation.context, progress_evt, this_object); this.read_and_call_id3_event(activation, bytes); - this.set_sound(activation.context, handle)?; + this.set_sound(activation.context, handle); Ok(Value::Undefined) } diff --git a/core/src/avm2/object/sound_object.rs b/core/src/avm2/object/sound_object.rs index 2a74a511af12..987c1b2286a8 100644 --- a/core/src/avm2/object/sound_object.rs +++ b/core/src/avm2/object/sound_object.rs @@ -140,11 +140,7 @@ impl<'gc> SoundObject<'gc> { } /// Returns `true` if a `SoundChannel` should be returned back to the AVM2 caller. - pub fn play( - self, - queued: QueuedPlay<'gc>, - activation: &mut Activation<'_, 'gc>, - ) -> Result> { + pub fn play(self, queued: QueuedPlay<'gc>, activation: &mut Activation<'_, 'gc>) -> bool { let mut sound_data = unlock!( Gc::write(activation.gc(), self.0), SoundObjectData, @@ -156,29 +152,26 @@ impl<'gc> SoundObject<'gc> { // Avoid to enqueue more unloaded sounds than the maximum allowed to be played if queued_plays.len() >= AudioManager::MAX_SOUNDS { tracing::warn!("Sound.play: too many unloaded sounds queued"); - return Ok(false); + return false; } queued_plays.push(queued); + // We don't know the length yet, so return the `SoundChannel` - Ok(true) + true } SoundData::Loaded { sound } => play_queued(queued, *sound, activation.context), } } - pub fn set_sound( - self, - context: &mut UpdateContext<'gc>, - sound: SoundHandle, - ) -> Result<(), Error<'gc>> { + pub fn set_sound(self, context: &mut UpdateContext<'gc>, sound: SoundHandle) { let mut sound_data = unlock!(Gc::write(context.gc(), self.0), SoundObjectData, sound_data).borrow_mut(); match &mut *sound_data { SoundData::NotLoaded { queued_plays } => { for queued in std::mem::take(queued_plays) { - play_queued(queued, sound, context)?; + play_queued(queued, sound, context); } *sound_data = SoundData::Loaded { sound }; } @@ -187,7 +180,6 @@ impl<'gc> SoundObject<'gc> { } } self.set_loading_state(SoundLoadingState::Loaded); - Ok(()) } pub fn id3(self) -> Option> { @@ -280,7 +272,7 @@ fn play_queued<'gc>( queued: QueuedPlay<'gc>, sound: SoundHandle, context: &mut UpdateContext<'gc>, -) -> Result> { +) -> bool { if let Some(duration) = context.audio.get_sound_duration(sound) { if queued.position > duration { tracing::error!( @@ -288,7 +280,7 @@ fn play_queued<'gc>( queued.position, duration ); - return Ok(false); + return false; } } @@ -303,7 +295,8 @@ fn play_queued<'gc>( context.attach_avm2_sound_channel(instance, queued.sound_channel); } - Ok(true) + + true } impl<'gc> TObject<'gc> for SoundObject<'gc> { diff --git a/core/src/loader.rs b/core/src/loader.rs index 9dd36bb1b704..06a59966da8f 100644 --- a/core/src/loader.rs +++ b/core/src/loader.rs @@ -1344,9 +1344,7 @@ pub fn load_sound_avm2<'gc>( match response { Ok((body, _, _, _)) => { let handle = uc.audio.register_mp3(&body)?; - if let Err(e) = sound.set_sound(uc, handle) { - tracing::error!("Encountered AVM2 error when setting sound: {}", e); - } + sound.set_sound(uc, handle); let total_len = body.len(); From 2468836a85fd769433a1e6bbf03fd2c298de8589 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sun, 7 Dec 2025 22:33:36 -0800 Subject: [PATCH 04/14] core/avm2: Introduce `Avm2::uncaught_error` This function should be called when an uncaught AVM2 error needs to be logged. Currently, it just prints the error, but in the future it should also dispatch an `uncaughtError` event. --- core/src/avm2.rs | 31 +++++------ core/src/display_object/movie_clip.rs | 78 +++++++++++++++++++-------- 2 files changed, 71 insertions(+), 38 deletions(-) diff --git a/core/src/avm2.rs b/core/src/avm2.rs index be81a86c9b7c..77429c2ebe82 100644 --- a/core/src/avm2.rs +++ b/core/src/avm2.rs @@ -19,7 +19,7 @@ use crate::avm2::script::{Script, TranslationUnit}; use crate::avm2::stack::Stack; use crate::character::Character; use crate::context::UpdateContext; -use crate::display_object::{MovieClip, TDisplayObject}; +use crate::display_object::{DisplayObject, MovieClip, TDisplayObject}; use crate::string::{AvmString, StringContext}; use crate::tag_utils::SwfMovie; use crate::PlayerRuntime; @@ -470,20 +470,6 @@ impl<'gc> Avm2<'gc> { .retain(|x| x.upgrade(context.gc_context).is_some()); } - pub fn run_stack_frame_for_callable( - callable: Object<'gc>, - receiver: Value<'gc>, - domain: Domain<'gc>, - context: &mut UpdateContext<'gc>, - ) -> Result<(), String> { - let mut evt_activation = Activation::from_domain(context, domain); - Value::from(callable) - .call(&mut evt_activation, receiver, FunctionArgs::empty()) - .map_err(|e| format!("{e:?}"))?; - - Ok(()) - } - pub fn lookup_class_for_character( activation: &mut Activation<'_, 'gc>, movie_clip: MovieClip<'gc>, @@ -703,4 +689,19 @@ impl<'gc> Avm2<'gc> { pub fn set_optimizer_enabled(&mut self, value: bool) { self.optimizer_enabled = value; } + + // Report an uncaught AVM2 error. + // TODO should the `display_object` parameter be optional or not? + #[cold] + #[inline(never)] + pub fn uncaught_error( + _activation: &mut Activation<'_, 'gc>, + _display_object: Option>, + error: Error<'gc>, + info: &str, + ) { + tracing::error!("{}: {:?}", info, error); + + // TODO: push the error onto `loaderInfo.uncaughtErrorEvents` + } } diff --git a/core/src/display_object/movie_clip.rs b/core/src/display_object/movie_clip.rs index e6b838c2c246..fc5c49cf82b0 100644 --- a/core/src/display_object/movie_clip.rs +++ b/core/src/display_object/movie_clip.rs @@ -8,7 +8,7 @@ use crate::avm2::script::Script; use crate::avm2::Activation as Avm2Activation; use crate::avm2::{ Avm2, ClassObject as Avm2ClassObject, FunctionArgs as Avm2FunctionArgs, LoaderInfoObject, - Object as Avm2Object, StageObject as Avm2StageObject, + Object as Avm2Object, StageObject as Avm2StageObject, Value as Avm2Value, }; use crate::backend::audio::{AudioManager, SoundInstanceHandle}; use crate::backend::navigator::Request; @@ -671,8 +671,16 @@ impl<'gc> MovieClip<'gc> { self.movie(), ) { Ok(res) => return Ok(res), - Err(e) => { - tracing::warn!("Error loading ABC file: {e:?}"); + Err(err) => { + // TODO can we skip this Activation construction? + let mut temp_activation = Avm2Activation::from_nothing(context); + + Avm2::uncaught_error( + &mut temp_activation, + Some(self.into()), + err, + "Error loading AVM2 ABC", + ); return Ok(None); } } @@ -707,8 +715,16 @@ impl<'gc> MovieClip<'gc> { self.movie(), ) { Ok(res) => return Ok(res), - Err(e) => { - tracing::warn!("Error loading ABC file: {e:?}"); + Err(err) => { + // TODO can we skip this Activation construction? + let mut temp_activation = Avm2Activation::from_nothing(context); + + Avm2::uncaught_error( + &mut temp_activation, + Some(self.into()), + err, + "Error loading AVM2 ABC", + ); } } } @@ -2037,13 +2053,11 @@ impl<'gc> MovieClip<'gc> { class_object.call_init(object.into(), Avm2FunctionArgs::empty(), &mut activation); if let Err(e) = result { - tracing::error!( - "Got \"{:?}\" when constructing AVM2 side of movie clip of type {}", + Avm2::uncaught_error( + &mut activation, + Some(self.into()), e, - class_object - .inner_class_definition() - .name() - .to_qualified_name(context.gc()) + "Error running AVM2 construction for movie clip", ); } } @@ -2369,6 +2383,8 @@ impl<'gc> MovieClip<'gc> { if is_fresh_frame { if let Some(callable) = self.frame_script(frame_id) { + let callable = Avm2Value::from(callable); + self.0.last_queued_script_frame.set(Some(frame_id)); self.0.has_pending_script.set(false); self.0 @@ -2381,15 +2397,18 @@ impl<'gc> MovieClip<'gc> { .unwrap() .avm2_domain(); - if let Err(e) = Avm2::run_stack_frame_for_callable( - callable, + let mut activation = Avm2Activation::from_domain(context, domain); + + if let Err(e) = callable.call( + &mut activation, avm2_object.into(), - domain, - context, + Avm2FunctionArgs::empty(), ) { - tracing::error!( - "Error occurred when running AVM2 frame script: {}", - e + Avm2::uncaught_error( + &mut activation, + Some(self.into()), + e, + "Error running AVM2 frame script", ); } @@ -4273,15 +4292,28 @@ impl<'gc, 'a> MovieClip<'gc> { } } } - Err(e) => tracing::error!( - "Got AVM2 error when attempting to lookup symbol class: {e:?}", - ), + Err(err) => { + Avm2::uncaught_error( + &mut activation, + Some(self.into()), + err, + "Error attempting to lookup AVM2 symbol class", + ); + } } } } for script in eager_scripts { - if let Err(e) = script.globals(context) { - tracing::error!("Error running eager script: {:?}", e); + if let Err(err) = script.globals(context) { + // TODO can we skip this Activation construction? + let mut temp_activation = Avm2Activation::from_nothing(context); + + Avm2::uncaught_error( + &mut temp_activation, + Some(self.into()), + err, + "Error running AVM2 eager script", + ); } } Ok(()) From 63698805bdcc59dc16b1f6c64d5cae7ab1f1e961 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 08:56:17 -0800 Subject: [PATCH 05/14] avm2: Make `dispatch_event_to_target` and its caller infallible The function never returned an error --- core/src/avm2.rs | 29 ++++--------------- core/src/avm2/events.rs | 27 +++++++---------- .../globals/flash/events/event_dispatcher.rs | 2 +- 3 files changed, 17 insertions(+), 41 deletions(-) diff --git a/core/src/avm2.rs b/core/src/avm2.rs index 77429c2ebe82..9818a18e2294 100644 --- a/core/src/avm2.rs +++ b/core/src/avm2.rs @@ -361,20 +361,8 @@ impl<'gc> Avm2<'gc> { simulate_dispatch: bool, ) -> bool { let mut activation = Activation::from_nothing(context); - match events::dispatch_event(&mut activation, target, event, simulate_dispatch) { - Err(err) => { - let event_name = event.event().event_type(); - - tracing::error!( - "Encountered AVM2 error when dispatching `{}` event: {:?}", - event_name, - err, - ); - // TODO: push the error onto `loaderInfo.uncaughtErrorEvents` - false - } - Ok(handled) => handled, - } + + events::dispatch_event(&mut activation, target, event, simulate_dispatch) } /// Add an object to the broadcast list. @@ -447,17 +435,10 @@ impl<'gc> Avm2<'gc> { .copied(); if let Some(object) = object.and_then(|obj| obj.upgrade(context.gc())) { - let mut activation = Activation::from_nothing(context); - if object.is_of_type(on_type.inner_class_definition()) { - if let Err(err) = events::broadcast_event(&mut activation, object, event) { - tracing::error!( - "Encountered AVM2 error when broadcasting `{}` event: {:?}", - event_name, - err, - ); - // TODO: push the error onto `loaderInfo.uncaughtErrorEvents` - } + let mut activation = Activation::from_nothing(context); + + events::broadcast_event(&mut activation, object, event); } } } diff --git a/core/src/avm2/events.rs b/core/src/avm2/events.rs index 348aa2966ca1..a7d3f771f15c 100644 --- a/core/src/avm2/events.rs +++ b/core/src/avm2/events.rs @@ -4,7 +4,6 @@ use crate::avm2::activation::Activation; use crate::avm2::function::FunctionArgs; use crate::avm2::globals::slots::flash_events_event_dispatcher as slots; use crate::avm2::object::{EventObject, FunctionObject, Object, TObject as _}; -use crate::avm2::Error; use crate::display_object::TDisplayObject; use crate::string::AvmString; use fnv::FnvHashMap; @@ -364,7 +363,7 @@ fn dispatch_event_to_target<'gc>( current_target: Object<'gc>, event: EventObject<'gc>, simulate_dispatch: bool, -) -> Result<(), Error<'gc>> { +) { avm_debug!( activation.context.avm2, "Event dispatch: {} to {current_target:?}", @@ -375,7 +374,7 @@ fn dispatch_event_to_target<'gc>( if dispatch_list.is_none() { // Objects with no dispatch list act as if they had an empty one - return Ok(()); + return; } let dispatch_list = dispatch_list.unwrap(); @@ -398,7 +397,7 @@ fn dispatch_event_to_target<'gc>( drop(evtmut); if simulate_dispatch { - return Ok(()); + return; } for handler in handlers.iter() { @@ -419,8 +418,6 @@ fn dispatch_event_to_target<'gc>( ); } } - - Ok(()) } pub fn dispatch_event<'gc>( @@ -428,7 +425,7 @@ pub fn dispatch_event<'gc>( this: Object<'gc>, event: EventObject<'gc>, simulate_dispatch: bool, -) -> Result> { +) -> bool { let target = this.get_slot(slots::TARGET).as_object().unwrap_or(this); let mut ancestor_list = Vec::new(); @@ -460,7 +457,7 @@ pub fn dispatch_event<'gc>( *ancestor, event, simulate_dispatch, - )?; + ); } event @@ -468,7 +465,7 @@ pub fn dispatch_event<'gc>( .set_phase(EventPhase::AtTarget); if !event.event().is_propagation_stopped() { - dispatch_event_to_target(activation, this, target, target, event, simulate_dispatch)?; + dispatch_event_to_target(activation, this, target, target, event, simulate_dispatch); } event @@ -488,12 +485,12 @@ pub fn dispatch_event<'gc>( *ancestor, event, simulate_dispatch, - )?; + ); } } - let handled = event.event().target.is_some(); - Ok(handled) + // If the target is set, the event was handled + event.event().target.is_some() } /// Like `dispatch_event`, but does not run the Capturing and Bubbling phases, @@ -503,14 +500,12 @@ pub fn broadcast_event<'gc>( activation: &mut Activation<'_, 'gc>, this: Object<'gc>, event: EventObject<'gc>, -) -> Result<(), Error<'gc>> { +) { let target = this.get_slot(slots::TARGET).as_object().unwrap_or(this); event .event_mut(activation.gc()) .set_phase(EventPhase::AtTarget); - dispatch_event_to_target(activation, this, target, target, event, false)?; - - Ok(()) + dispatch_event_to_target(activation, this, target, target, event, false); } diff --git a/core/src/avm2/globals/flash/events/event_dispatcher.rs b/core/src/avm2/globals/flash/events/event_dispatcher.rs index 227d60532cf9..3e059caddbbf 100644 --- a/core/src/avm2/globals/flash/events/event_dispatcher.rs +++ b/core/src/avm2/globals/flash/events/event_dispatcher.rs @@ -131,7 +131,7 @@ pub fn dispatch_event_internal<'gc>( // AS3-side typing guarantees that the event is actually an Event let event = event.as_event_object().unwrap(); - events::dispatch_event(activation, this, event, false)?; + events::dispatch_event(activation, this, event, false); let not_canceled = !event.event().is_cancelled(); Ok(not_canceled.into()) From 300618e165c369fdf742018052193701d0b6eec2 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 09:05:20 -0800 Subject: [PATCH 06/14] avm2: Use `Avm2::uncaught_event` in event dispatch code --- core/src/avm2/events.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/core/src/avm2/events.rs b/core/src/avm2/events.rs index a7d3f771f15c..7864490cdf55 100644 --- a/core/src/avm2/events.rs +++ b/core/src/avm2/events.rs @@ -4,6 +4,7 @@ use crate::avm2::activation::Activation; use crate::avm2::function::FunctionArgs; use crate::avm2::globals::slots::flash_events_event_dispatcher as slots; use crate::avm2::object::{EventObject, FunctionObject, Object, TObject as _}; +use crate::avm2::Avm2; use crate::display_object::TDisplayObject; use crate::string::AvmString; use fnv::FnvHashMap; @@ -410,11 +411,13 @@ fn dispatch_event_to_target<'gc>( let args = &[event.into()]; let result = handler.call(activation, global.into(), FunctionArgs::from_slice(args)); if let Err(err) = result { - tracing::error!( - "Error dispatching event {:?} to handler {:?} : {:?}", - event, - handler, + let event_name = event.event().event_type(); + + Avm2::uncaught_error( + activation, + None, // TODO we need to set this, but how? err, + &format!("Error dispatching event \"{}\"", event_name), ); } } From fb6fd47286242efe64ca2991440e67943d7578ff Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 09:36:12 -0800 Subject: [PATCH 07/14] core: Use `Avm2::uncaught_event` in `NetConnection` code --- core/src/avm2/object/responder_object.rs | 8 +++----- core/src/net_connection.rs | 11 +++++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/core/src/avm2/object/responder_object.rs b/core/src/avm2/object/responder_object.rs index c531cba97f88..f43446528583 100644 --- a/core/src/avm2/object/responder_object.rs +++ b/core/src/avm2/object/responder_object.rs @@ -2,7 +2,6 @@ use crate::avm2::function::FunctionArgs; use crate::avm2::object::script_object::ScriptObjectData; use crate::avm2::object::{ClassObject, FunctionObject, Object, TObject}; use crate::avm2::{Activation, Error}; -use crate::context::UpdateContext; use crate::net_connection::ResponderCallback; use flash_lso::types::Value as AMFValue; use gc_arena::barrier::unlock; @@ -64,7 +63,7 @@ impl<'gc> ResponderObject<'gc> { pub fn send_callback( self, - context: &mut UpdateContext<'gc>, + activation: &mut Activation<'_, 'gc>, callback: ResponderCallback, message: &AMFValue, ) -> Result<(), Error<'gc>> { @@ -74,10 +73,9 @@ impl<'gc> ResponderObject<'gc> { }; if let Some(function) = function { - let mut activation = Activation::from_nothing(context); - let value = crate::avm2::amf::deserialize_value(&mut activation, message)?; + let value = crate::avm2::amf::deserialize_value(activation, message)?; let args = &[value]; - function.call(&mut activation, self.into(), FunctionArgs::from_slice(args))?; + function.call(activation, self.into(), FunctionArgs::from_slice(args))?; } Ok(()) diff --git a/core/src/net_connection.rs b/core/src/net_connection.rs index 6b4e252e2521..2b89f1fd1e32 100644 --- a/core/src/net_connection.rs +++ b/core/src/net_connection.rs @@ -54,8 +54,15 @@ impl ResponderHandle { match self { ResponderHandle::Avm2(handle) => { let object = context.dynamic_root.fetch(handle); - if let Err(e) = object.send_callback(context, callback, &message) { - tracing::error!("Unhandled error sending {callback:?} callback: {e}"); + let mut activation = Avm2Activation::from_nothing(context); + + if let Err(err) = object.send_callback(&mut activation, callback, &message) { + Avm2::uncaught_error( + &mut activation, + None, // TODO we need to set this, but how? + err, + "Error running AVM2 NetConnection callback", + ); } } ResponderHandle::Avm1(handle) => { From d7e23f0dd21736a4056cebf6c091f1b51aebc97c Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 10:08:59 -0800 Subject: [PATCH 08/14] avm2: Use `Avm2::uncaught_error` in Bitmap and BitmapData construction This fixes a panic when the `Bitmap` constructor throws an error --- core/src/display_object/bitmap.rs | 34 +++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/core/src/display_object/bitmap.rs b/core/src/display_object/bitmap.rs index 4181e03b8cf3..886e1bf3f4cd 100644 --- a/core/src/display_object/bitmap.rs +++ b/core/src/display_object/bitmap.rs @@ -2,7 +2,7 @@ use crate::avm1::Object as Avm1Object; use crate::avm2::{ - Activation as Avm2Activation, BitmapDataObject as Avm2BitmapDataObject, + Activation as Avm2Activation, Avm2, BitmapDataObject as Avm2BitmapDataObject, ClassObject as Avm2ClassObject, FunctionArgs as Avm2FunctionArgs, StageObject as Avm2StageObject, }; @@ -331,13 +331,24 @@ impl<'gc> TDisplayObject<'gc> for Bitmap<'gc> { let mut activation = Avm2Activation::from_nothing(context); - let bitmap = Avm2StageObject::for_display_object_childless( + let bitmap_obj = + Avm2StageObject::for_display_object(activation.gc(), self.into(), bitmap_cls); + + let call_result = bitmap_cls.call_init( + bitmap_obj.into(), + Avm2FunctionArgs::empty(), &mut activation, - self.into(), - bitmap_cls, - ) - .expect("can't throw from post_instantiation -_-"); - self.set_avm2_object(activation.gc(), Some(bitmap)); + ); + if let Err(err) = call_result { + Avm2::uncaught_error( + &mut activation, + Some(self.into()), + err, + "Error running AVM2 construction for bitmap", + ); + }; + + self.set_avm2_object(activation.gc(), Some(bitmap_obj)); // Use a dummy BitmapData when calling the constructor on the user subclass // - the constructor should see an invalid BitmapData before calling 'super', @@ -361,8 +372,13 @@ impl<'gc> TDisplayObject<'gc> for Bitmap<'gc> { Avm2FunctionArgs::from_slice(args), &mut activation, ); - if let Err(e) = call_result { - tracing::error!("Got error when constructing AVM2 side of bitmap: {}", e); + if let Err(err) = call_result { + Avm2::uncaught_error( + &mut activation, + Some(self.into()), + err, + "Error running AVM2 construction for bitmap data", + ); } self.set_bitmap_data(activation.context, bitmap_data_obj.get_bitmap_data()); From f789b703693fe92a89d927465b35280505ced509 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 10:22:23 -0800 Subject: [PATCH 09/14] avm2: Use `Avm2::uncaught_error` in `Graphic` DO construction --- core/src/display_object/graphic.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/core/src/display_object/graphic.rs b/core/src/display_object/graphic.rs index c4b0e117de9f..28f259febbdb 100644 --- a/core/src/display_object/graphic.rs +++ b/core/src/display_object/graphic.rs @@ -1,6 +1,7 @@ use crate::avm1::Object as Avm1Object; use crate::avm2::{ - Activation as Avm2Activation, ClassObject as Avm2ClassObject, StageObject as Avm2StageObject, + Activation as Avm2Activation, Avm2, ClassObject as Avm2ClassObject, + StageObject as Avm2StageObject, }; use crate::context::{RenderContext, UpdateContext}; use crate::display_object::DisplayObjectBase; @@ -159,8 +160,13 @@ impl<'gc> TDisplayObject<'gc> for Graphic<'gc> { class_object, ) { Ok(object) => self.set_object2(activation.context, object), - Err(e) => { - tracing::error!("Got error when constructing AVM2 side of shape: {}", e) + Err(err) => { + Avm2::uncaught_error( + &mut activation, + Some(self.into()), + err, + "Error running AVM2 construction for shape", + ); } } From 7417116277b8c3c672c5bdc070936caa8cdfc2ae Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 10:28:46 -0800 Subject: [PATCH 10/14] avm2: Use `Avm2::uncaught_error` in `EditText` DO construction --- core/src/display_object/edit_text.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/core/src/display_object/edit_text.rs b/core/src/display_object/edit_text.rs index cbefc946a487..8fd24beab50d 100644 --- a/core/src/display_object/edit_text.rs +++ b/core/src/display_object/edit_text.rs @@ -2191,10 +2191,14 @@ impl<'gc> EditText<'gc> { Ok(object) => { self.set_object(Some(object.into()), context.gc()); } - Err(e) => tracing::error!( - "Got error when constructing AVM2 side of dynamic text field: {}", - e - ), + Err(err) => { + Avm2::uncaught_error( + &mut activation, + Some(self.into()), + err, + "Error running AVM2 construction for dynamic text", + ); + } } } From cad408d4342605302f54183eb945b61aa79e321d Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 11:01:09 -0800 Subject: [PATCH 11/14] avm2: Use `Avm2::uncaught_error` in `SimpleButton` DO construction --- core/src/display_object/avm2_button.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/src/display_object/avm2_button.rs b/core/src/display_object/avm2_button.rs index 33be3b8b5d75..167a3083a927 100644 --- a/core/src/display_object/avm2_button.rs +++ b/core/src/display_object/avm2_button.rs @@ -3,8 +3,8 @@ use super::dispatch_added_event_only; use super::interactive::Avm2MousePick; use crate::avm1::Object as Avm1Object; use crate::avm2::{ - Activation as Avm2Activation, ClassObject as Avm2ClassObject, FunctionArgs as Avm2FunctionArgs, - StageObject as Avm2StageObject, + Activation as Avm2Activation, Avm2, ClassObject as Avm2ClassObject, + FunctionArgs as Avm2FunctionArgs, StageObject as Avm2StageObject, }; use crate::backend::audio::AudioManager; use crate::backend::ui::MouseCursor; @@ -545,8 +545,13 @@ impl<'gc> TDisplayObject<'gc> for Avm2Button<'gc> { &mut activation, ); - if let Err(e) = result { - tracing::error!("Got {} when constructing AVM2 side of button", e); + if let Err(err) = result { + Avm2::uncaught_error( + &mut activation, + Some(self.into()), + err, + "Error running AVM2 construction for button", + ); } } From 525d61decb8e4cc98464e23ada00fa0f08c2e727 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 12:45:19 -0800 Subject: [PATCH 12/14] core: Use `Avm2::uncaught_error` in AVM2 child property setting code Also: - Remove some old TODO comments - Don't log errors that appear when getting original value of property --- core/src/display_object.rs | 24 +++++++++++++----------- core/src/display_object/container.rs | 26 +++++++++++++++++++++----- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/core/src/display_object.rs b/core/src/display_object.rs index f8a31e07c831..403e4e4a2398 100644 --- a/core/src/display_object.rs +++ b/core/src/display_object.rs @@ -2,8 +2,9 @@ use crate::avm1::{ ActivationIdentifier as Avm1ActivationIdentifier, Object as Avm1Object, Value as Avm1Value, }; use crate::avm2::{ - Activation as Avm2Activation, Error as Avm2Error, LoaderInfoObject, Multiname as Avm2Multiname, - Object as Avm2Object, StageObject as Avm2StageObject, TObject as _, Value as Avm2Value, + Activation as Avm2Activation, Avm2, Error as Avm2Error, LoaderInfoObject, + Multiname as Avm2Multiname, Object as Avm2Object, StageObject as Avm2StageObject, TObject as _, + Value as Avm2Value, }; use crate::context::{RenderContext, UpdateContext}; use crate::drawing::Drawing; @@ -2331,8 +2332,6 @@ pub trait TDisplayObject<'gc>: #[no_dynamic] fn set_on_parent_field(self, context: &mut UpdateContext<'gc>) { - //TODO: Don't report missing property errors. - //TODO: Don't attempt to set properties if object was placed without a name. if self.has_explicit_name() { if let Some(parent) = self.parent().and_then(|p| p.object2()) { let parent = Avm2Value::from(parent); @@ -2344,16 +2343,19 @@ pub trait TDisplayObject<'gc>: .library_for_movie(self.movie()) .unwrap() .avm2_domain(); + let mut activation = Avm2Activation::from_domain(context, domain); let multiname = Avm2Multiname::new(activation.avm2().find_public_namespace(), name); - if let Err(e) = - parent.init_property(&multiname, child.into(), &mut activation) - { - tracing::error!( - "Got error when setting AVM2 child named \"{}\": {}", - &name, - e + let set_result = + parent.init_property(&multiname, child.into(), &mut activation); + + if let Err(err) = set_result { + Avm2::uncaught_error( + &mut activation, + Some(self), + err, + &format!("Error setting AVM2 child named \"{}\"", name), ); } } diff --git a/core/src/display_object/container.rs b/core/src/display_object/container.rs index a112c92ac0d8..f8c2439ded84 100644 --- a/core/src/display_object/container.rs +++ b/core/src/display_object/container.rs @@ -758,19 +758,35 @@ impl<'gc> ChildContainer<'gc> { let current_val = parent_obj.get_property(&multiname, &mut activation); match current_val { - Ok(Avm2Value::Null) | Ok(Avm2Value::Undefined) => {} + Ok(Avm2Value::Null) | Ok(Avm2Value::Undefined) => { + // When the `get_property` returns null + // or undefined, FP doesn't attempt to + // set it to `null`. This is observable: + // a setter method won't get called. + } Ok(_other) => { let res = parent_obj.set_property( &multiname, Avm2Value::Null, &mut activation, ); - if let Err(e) = res { - tracing::error!("Failed to set child {} ({:?}) to null on parent obj {:?}: {:?}", name, child, parent_obj, e); + if let Err(err) = res { + Avm2::uncaught_error( + &mut activation, + Some(child), + err, + &format!( + "Error setting AVM2 child named \"{}\" to null", + name + ), + ); } } - Err(e) => { - tracing::error!("Failed to get current value of child {} ({:?}) on parent obj {:?}: {:?}", name, child, parent_obj, e); + Err(_) => { + // In FP, errors when accessing the + // original value are completely + // swallowed. They don't make it to + // flashlog or to uncaught error events. } } } From 891ba011640898bbda686c3af43665704ad8340d Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 12:52:06 -0800 Subject: [PATCH 13/14] core: Use `Avm2::uncaught_error` in timer code --- core/src/timer.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/core/src/timer.rs b/core/src/timer.rs index 453986fa8288..a437f8cc1bd3 100644 --- a/core/src/timer.rs +++ b/core/src/timer.rs @@ -9,7 +9,7 @@ use crate::avm1::{Activation, ActivationIdentifier, Object as Avm1Object, Value use crate::avm2::error::make_null_or_undefined_error; use crate::avm2::object::FunctionObject as Avm2FunctionObject; use crate::avm2::{ - Activation as Avm2Activation, Error as Avm2Error, FunctionArgs, Value as Avm2Value, + Activation as Avm2Activation, Avm2, Error as Avm2Error, FunctionArgs, Value as Avm2Value, }; use crate::context::UpdateContext; use crate::display_object::{DisplayObject, TDisplayObject}; @@ -159,8 +159,13 @@ impl<'gc> Timers<'gc> { match run_closure() { Ok(v) => v.coerce_to_boolean(), - Err(e) => { - tracing::error!("Unhandled AVM2 error in timer callback: {e:?}",); + Err(err) => { + Avm2::uncaught_error( + &mut avm2_activation, + None, // TODO do we need to set this? + err, + "Error in AVM2 timer callback", + ); false } } From da8fe7d1333f869e3f94633fb0ec82e771853ab3 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 8 Dec 2025 13:27:55 -0800 Subject: [PATCH 14/14] avm2: Use `Avm2::uncaught_error` in `external.rs` --- core/src/external.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/core/src/external.rs b/core/src/external.rs index 7563d61d5c61..e92dfdd85ddd 100644 --- a/core/src/external.rs +++ b/core/src/external.rs @@ -8,7 +8,7 @@ use crate::avm2::object::{ ArrayObject as Avm2ArrayObject, FunctionObject as Avm2FunctionObject, Object as Avm2Object, ScriptObject as Avm2ScriptObject, TObject as _, }; -use crate::avm2::{FunctionArgs, Value as Avm2Value}; +use crate::avm2::{Avm2, FunctionArgs, Value as Avm2Value}; use crate::context::UpdateContext; use crate::string::AvmString; use gc_arena::Collect; @@ -324,10 +324,12 @@ impl<'gc> Callback<'gc> { ); match result.and_then(|value| Value::from_avm2(&mut activation, value)) { Ok(result) => result, - Err(e) => { - tracing::error!( - "Unhandled error in External Interface callback {name}: {:?}", - e + Err(err) => { + Avm2::uncaught_error( + &mut activation, + None, // TODO do we need to set this? + err, + "Error in AVM2 external interface callback", ); Value::Null }