Skip to content

Conversation

@Lord-McSweeney
Copy link
Collaborator

@Lord-McSweeney Lord-McSweeney commented Oct 18, 2025

Fixes #21007

@Lord-McSweeney Lord-McSweeney added mixed-avm Mixing AVM1 and AVM2 A-core Area: Core player, where no other category fits T-fix Type: Bug fix (in something that's supposed to work already) labels Oct 18, 2025
@Lord-McSweeney Lord-McSweeney added the waiting-on-review Waiting on review from a Ruffle team member label Oct 18, 2025
// Also, we consider AVM2 MovieClips loaded into an AVM1 parent to always
// be orphans, since we know they were placed by AVM1 code and not by
// the timeline.
dobj.placed_by_script() || has_avm1_parent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't such movies be placed by script anyway?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, from what I understand, placed_by_script is only used by AVM2? As in, AVM1 code won't set_placed_by_script on DOs that it creates?

Copy link
Member

@kjarosh kjarosh Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, but in this case you have placed_by_avm1_script. As a side note, I think we should aim at merging these two.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, but actually we don't really care about placed_by_avm1_script? As the comment says, we consider AVM2 MovieClips loaded into an AVM1 parent to be orphans no matter what, even if the AVM1 parent was placed by the timeline, because we need that AVM2 movie to tick its frames no matter what.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm misunderstanding something, but the comment says that we do that exactly because the object couldn't have been placed by the timeline. In which case it makes sense to check placed_by_avm1_script instead of using this assumption, right?

I'm looking into merging placed_by_avm1_script and placed_by_avm2_script into one

@Lord-McSweeney Lord-McSweeney force-pushed the avm2-avm1-orphan branch 2 times, most recently from 2316a64 to 6cec69f Compare October 20, 2025 23:39
@Lord-McSweeney Lord-McSweeney added waiting-on-author Waiting on the PR author to make the requested changes and removed waiting-on-review Waiting on review from a Ruffle team member labels Oct 22, 2025
@Lord-McSweeney Lord-McSweeney added waiting-on-review Waiting on review from a Ruffle team member and removed waiting-on-author Waiting on the PR author to make the requested changes labels Oct 27, 2025
Instead, do it explicitly everywhere it would've been done before. This allows us to make `TDisplayObject::object2` take a `Mutation` instead of an `UpdateContext`

/// The next MovieClip in the AVM1 execution list.
///
/// `None` in an AVM2 movie.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate here when it's None vs Some? IIUC it's Some when either this movie clip or the root MC is from AVM1. Because without a comment I'd assume it's for AVM1 only.

if !self.movie().is_action_script_3() {
// Run my load/enterFrame clip event.
// Run my load/enterFrame clip event.
if !self.movie().is_action_script_3() || !context.root_swf.is_action_script_3() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a comment here explaining this situation? I.e. why it is being run for an AVM2 movie

Comment on lines -81 to +85
// To detect this, we check 'placed_by_avm2_script'. This flag get set to 'true'
// To detect this, we check 'placed_by_script'. This flag get set to 'true'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 2 changes should be reverted

@Lord-McSweeney
Copy link
Collaborator Author

This is the wrong approach- when AVM1 loads an AVM2 movie, we probably want to interpret the loaded movie as actually being AVM1, except for the running of DoAction tags

@Lord-McSweeney Lord-McSweeney removed the waiting-on-review Waiting on review from a Ruffle team member label Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-core Area: Core player, where no other category fits mixed-avm Mixing AVM1 and AVM2 T-fix Type: Bug fix (in something that's supposed to work already)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with mixed avm

2 participants