Skip to content

[avm1] Better handling of preload/suppress functions flags #7239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 29, 2022

Conversation

moulins
Copy link
Contributor

@moulins moulins commented Jun 16, 2022

  • Handle the case where both preload aud suppress flags are set for the same variable;
  • Remove arguments field in Activation; instead use a normal local definition;
  • When suppress_this is set, inherit the this value from parent activation. (This isn't entirely correct, as FP's this is mutable and seems to be part of the scope chain, but this would require a larger refactoring)

The added test contains hand-edited bytecode, so there's no accompanying .as file.

@Toad06
Copy link
Member

Toad06 commented Jun 23, 2022

This will close #6204.

I also noticed a few regressions on files that are already known to have issues:

Powerpuff Girls: Attack of the Puppybots

Zelda Lampshade

Maybe that these cases are just a side effect and would require this to be mutable?

@moulins
Copy link
Contributor Author

moulins commented Jun 26, 2022

Hmm, it's interesting that both of the original issues are caused by #1513. I should try merging #5492 on top of this to see if it fixes these games.

moulins added 2 commits June 30, 2022 00:31
- reduce rightwards drift by exiting early
- outline some code into separate methods on Avm1Function
@moulins moulins force-pushed the avm1-fn-suppress-preload branch from 7fc133a to f15a546 Compare June 29, 2022 22:34
- Handle the case where both preload aud suppress flags are
set for the same variable;
- Remove `arguments` field in `Activation`; instead use a normal
local definition;
- When `suppress_this` is set, inherit the `this` value from parent
activation. (This isn't entirely correct, as FP's `this` is mutable
and seems to be part of the scope chain, but this would require a
larger refactoring)
@moulins moulins force-pushed the avm1-fn-suppress-preload branch from f15a546 to d74a7d2 Compare June 29, 2022 22:39
Copy link
Member

@Herschel Herschel left a comment

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants