avm2: Implement support for native methods in playerglobal #7244

Aaron1011 · 2022-06-16T23:54:31Z

This commit adds support for marking methods as native
in ActionScript classes defined in playerglobal. The
build_playerglobal now checks for native methods, and
generates Rust code linking them to a corresponding Rust
function definition in the codebase.

To test this functionality, I've reimplemented several
functions as native methods (and moved related code to
pure ActionScript).

kmeisthax

I'd mark this as approved but the prior PR this depends on hasn't been merged in first. But this is absolutely fantastic work. Much appreciated.

Aaron1011 · 2022-06-23T22:55:53Z

@kmeisthax Now that the other PR has been merged, do you think that this is ready to merge?

core/src/avm2.rs

relrelb · 2022-07-09T10:37:34Z

core/build_playerglobal/src/lib.rs

+    // instance methods, class methods, and freestanding functions.
+    // We're going to insert them into an `FnvHashMap`, so the order
+    // that we visit them in doesn't matter.
+    for (i, instance) in abc.instances.iter().enumerate() {


I'm not entirely sure how it's matters, but it seems like nativegen.py recursively traverses through Script -> Method/Class, rather than a linear Instance scan.

Nothing depends on the iteration order, since we just use the method ids.

core/build_playerglobal/src/lib.rs

relrelb · 2022-07-09T11:05:38Z

core/build_playerglobal/src/lib.rs

+    let mut reader = swf::avm2::read::Reader::new(data);
+    let abc = reader.read()?;
+
+    let mut rust_paths = FnvHashMap::default();


How about turning this FnvHashMap into a simple Vec? This way we can get rid of the fnv dependency.

I suggest turning this into a native_methods: Vec<&Method>, which will be gathered in check_trait.
Then the Rust-path logic of check_trait will be moved out of it, next to the Rust code generation.

We need the parent in order to construct the Rust path, so I think it would be simpler to keep that in check_trait. However, I can change it to directly construct a Vec<Option<TokenStream>> with 'holes' for missing methods.

core/build_playerglobal/Cargo.toml

core/build_playerglobal/src/lib.rs

Aaron1011 · 2022-07-09T17:40:47Z

@relrelb I've addressed your comments

core/src/avm2/globals/flash/system/security.rs

Aaron1011 · 2022-07-09T20:40:41Z

@relrelb Are there any other changes that you'd like me to make?

core/build_playerglobal/src/lib.rs

core/src/avm2/globals.rs

core/src/avm2/globals/flash/net/url_loader.rs

relrelb · 2022-07-10T05:22:14Z

core/src/avm2/script.rs

+
+        let mut method = None;
+        if is_global {
+            if let Some(native) = activation.avm2().native_table[method_index.0 as usize] {


Maybe this can be included in the generated NATIVE_TABLE itself? Though it would require Method to be constructible at a const context.

I don't think this is nearly as easy, with GC in the way.
One minor thing I think we could do would be move the props (esp signature) from BytecodeMethod to the NativeMethod without cloning them.

I've refactored this to move out of signature.

Aaron1011 · 2022-07-10T16:16:26Z

@relrelb I've addressed your most recent comments.

core/build_playerglobal/src/lib.rs

Herschel

Thanks for your work! Looks good here, okay with you @relrelb ?

Herschel · 2022-07-14T23:13:18Z

core/build_playerglobal/src/lib.rs

+    let make_native_table = quote! {
+        const NATIVE_TABLE: &[Option<crate::avm2::method::NativeMethodImpl>] = &[
+            #(#rust_paths,)*
+        ];
+    }
+    .to_string();
+
+    // Each table entry ends with ') ,' - insert a newline so that
+    // each entry is on its own line. This makes error messages more readable.
+    let make_native_table = make_native_table.replace(") ,", ") ,\n");


I wonder if it's worth using, for example, prettyplease to make the output nicer in the future. Overkill for this specific case, but I expect we will be generating more and more code that it might be worthwhile at some point.

Yeah, it might be worth adding along with custom instance allocator support (which I'm working on in a follow-up PR).

core/src/avm2/globals/flash/net/URLLoader.as

core/src/avm2/globals/flash/net/URLRequest.as

relrelb

Looks good, able to merge in my opinion, just please rebase/square before merging.

This commit adds support for marking methods as `native` in ActionScript classes defined in playerglobal. The `build_playerglobal` now checks for native methods, and generates Rust code linking them to a corresponding Rust function definition in the codebase. To test this functionality, I've reimplemented several functions as native methods (and moved related code to pure ActionScript).

Aaron1011 · 2022-07-15T16:42:39Z

The Nightly failure is due to the regression rust-lang/rust#99286

Aaron1011 force-pushed the playerglobal-native branch 2 times, most recently from cc2fe1d to 3653144 Compare June 18, 2022 15:56

relrelb mentioned this pull request Jun 18, 2022

avm2: Convert filters to ActionScript #7252

Merged

Aaron1011 force-pushed the playerglobal-native branch from 3653144 to 9c6fe81 Compare June 19, 2022 19:45

CUB3D mentioned this pull request Jun 21, 2022

AVM2: Implement escape() toplevel #7281

Merged

kmeisthax reviewed Jun 22, 2022

View reviewed changes

Aaron1011 force-pushed the playerglobal-native branch from 9c6fe81 to e7f8fac Compare June 23, 2022 16:35

adrian17 reviewed Jun 24, 2022

View reviewed changes

core/src/avm2.rs Outdated Show resolved Hide resolved

Aaron1011 force-pushed the playerglobal-native branch 2 times, most recently from 7b80213 to 9974af4 Compare June 24, 2022 18:18

adrian17 approved these changes Jun 27, 2022

View reviewed changes

Aaron1011 force-pushed the playerglobal-native branch 4 times, most recently from 4610a11 to 33d2a95 Compare July 2, 2022 21:12

relrelb reviewed Jul 9, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

relrelb reviewed Jul 9, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

relrelb reviewed Jul 9, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

relrelb reviewed Jul 9, 2022

View reviewed changes

core/build_playerglobal/Cargo.toml Outdated Show resolved Hide resolved

relrelb reviewed Jul 9, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

Aaron1011 force-pushed the playerglobal-native branch 2 times, most recently from 008df66 to dec04b1 Compare July 9, 2022 17:40

Aaron1011 force-pushed the playerglobal-native branch 2 times, most recently from 00f8367 to 6ad342e Compare July 9, 2022 17:49

relrelb reviewed Jul 9, 2022

View reviewed changes

core/src/avm2/globals/flash/system/security.rs Outdated Show resolved Hide resolved

relrelb reviewed Jul 10, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

relrelb reviewed Jul 10, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Show resolved Hide resolved

relrelb reviewed Jul 10, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

relrelb reviewed Jul 10, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

relrelb reviewed Jul 10, 2022

View reviewed changes

core/src/avm2/globals.rs Outdated Show resolved Hide resolved

relrelb reviewed Jul 10, 2022

View reviewed changes

core/src/avm2/globals/flash/net/url_loader.rs Outdated Show resolved Hide resolved

relrelb reviewed Jul 10, 2022

View reviewed changes

Aaron1011 force-pushed the playerglobal-native branch from d00a061 to 07c4195 Compare July 10, 2022 15:56

Herschel self-assigned this Jul 10, 2022

Herschel reviewed Jul 14, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

Herschel reviewed Jul 14, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

Herschel reviewed Jul 14, 2022

View reviewed changes

core/build_playerglobal/src/lib.rs Outdated Show resolved Hide resolved

Herschel approved these changes Jul 15, 2022

View reviewed changes

Herschel removed their assignment Jul 15, 2022

Aaron1011 force-pushed the playerglobal-native branch from 5431644 to 72db741 Compare July 15, 2022 03:46

relrelb approved these changes Jul 15, 2022

View reviewed changes

Aaron1011 force-pushed the playerglobal-native branch from 72db741 to 880ea71 Compare July 15, 2022 15:48

Aaron1011 merged commit af4f181 into ruffle-rs:master Jul 15, 2022

Aaron1011 deleted the playerglobal-native branch July 15, 2022 22:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

avm2: Implement support for native methods in playerglobal #7244

avm2: Implement support for native methods in playerglobal #7244

Aaron1011 commented Jun 16, 2022 •

edited

Loading

kmeisthax left a comment

Aaron1011 commented Jun 23, 2022

relrelb Jul 9, 2022

Aaron1011 Jul 9, 2022

relrelb Jul 9, 2022 •

edited

Loading

Aaron1011 Jul 9, 2022

Aaron1011 commented Jul 9, 2022

Aaron1011 commented Jul 9, 2022

relrelb Jul 10, 2022

adrian17 Jul 10, 2022

Aaron1011 Jul 10, 2022

Aaron1011 commented Jul 10, 2022

Herschel left a comment

Herschel Jul 14, 2022 •

edited

Loading

Aaron1011 Jul 15, 2022

relrelb left a comment

Aaron1011 commented Jul 15, 2022

avm2: Implement support for native methods in playerglobal #7244

avm2: Implement support for native methods in playerglobal #7244

Conversation

Aaron1011 commented Jun 16, 2022 • edited Loading

kmeisthax left a comment

Choose a reason for hiding this comment

Aaron1011 commented Jun 23, 2022

relrelb Jul 9, 2022

Choose a reason for hiding this comment

Aaron1011 Jul 9, 2022

Choose a reason for hiding this comment

relrelb Jul 9, 2022 • edited Loading

Choose a reason for hiding this comment

Aaron1011 Jul 9, 2022

Choose a reason for hiding this comment

Aaron1011 commented Jul 9, 2022

Aaron1011 commented Jul 9, 2022

relrelb Jul 10, 2022

Choose a reason for hiding this comment

adrian17 Jul 10, 2022

Choose a reason for hiding this comment

Aaron1011 Jul 10, 2022

Choose a reason for hiding this comment

Aaron1011 commented Jul 10, 2022

Herschel left a comment

Choose a reason for hiding this comment

Herschel Jul 14, 2022 • edited Loading

Choose a reason for hiding this comment

Aaron1011 Jul 15, 2022

Choose a reason for hiding this comment

relrelb left a comment

Choose a reason for hiding this comment

Aaron1011 commented Jul 15, 2022

Aaron1011 commented Jun 16, 2022 •

edited

Loading

relrelb Jul 9, 2022 •

edited

Loading

Herschel Jul 14, 2022 •

edited

Loading