avm2: Add #[native] macro to allow defining methods with native types

[Aaron1011]

This allows you to define a native method like:

#[native]
fn my_method(activation: &mut Activation, this: DisplayObject<'gc>, arg:
bool)

instead of needing to use Option<Object<'gc>> and &[Value<'gc>]
and manual coercions.

See the doc comments for more details.

[relrelb]

I wonder why not implement this instead in build_playerglobal, in the same manner of nativegen.py in avmplus - Each native function will have a generated Rust wrapper function that coerces this: Option<Object<'gc>> and args: &[Value<'gc>] to the appropriate parameters, then calls the actual implementation. This way we'll have access to ActionScript's type system while generating the wrappers, which I guess might help at producing better conversions.

[adrian17]

FWIW I agree; "rust-dependent" paths could be handled by Into impl, and/or by extra annotations on the AS side (stripped after codegen).
Talked about it a bit on Discord too; some of the current type checking is also redundant after calling resolve_parameters().

[Herschel]

Thank you! I'm AFK this weekend so am not able to test this deeply, but it looks fine at a glance, so please feel free to merge if you feel its ready -- otherwise I'll get to it on Monday.

Re: redundant checks in extract_from_vm, I think this will be cleaned up when we make a smarter Value type (union, tagged pointer, NaN-boxed, whatever), which will let us easily do the get_unchecked equivalent inside extract_from_vm. In the meantime, maybe std::hint::unreachable_unchecked() could let us extract the enum variants in a branchless fashion?

[kmeisthax]

I think this has been sitting on the PR queue for far too long; mind giving it one more rebase/merge resolution cycle?

[Herschel]

Sorry that I didn't merge this way back! I agree, assuming this is still useful, let's rebase this.

[Aaron1011]

@Herschel @kmeisthax rebased

[Bale001]

One thing I'm worried about is whether or not "extracting" the type directly is the right approach. From my perspective, it makes more sense to coerce the value to the specified type instead.

For example, currently if I have a parameter like my_param: f64, it will fail if I pass a Value::Integer as that parameter. It will also fail if I pass a string that contains a number, like "0". I do not see this behavior if Flash though, it appears to always coerce the value, even for built-in methods.

[adrian17]

my_param: f64, it will fail if I pass a Value::Integer as that parameter

Note that Number/int/uint are tricky in general. Just check out Value::is_number, Value::is_u32 etc.
But also... the thing is, the interpreter already does these coercions, in resolve_parameters() (assuming the method is correctly typed in AS). So we don't want to duplicate work, in fact ideally I'd love to be able to unsafe-coerce the type to say String in Rust if we can guarantee that the AS typing already coerced it to a String. But again, for numbers this is tricky as type annotation x: Number will still accept either an integer of float Value - the spot to convert the Value to a concrete number type must be on Rust side.

[Aaron1011]

I believe we already do those coercions (based on the type declared in the signature) in the Activation code that sets up a method call.

[adrian17]

I believe we already do those coercions (based on the type declared in the signature) in the Activation code that sets up a method call.

More specifically, for numbers, (even if Ruffle doesn't do it perfectly in line with FP, eventually we should), this is still awkward:

function f(x: Number) {
	trace(getQualifiedClassName(x));
}

var a = f;
a(1);    // int
a(1.5);  // Number
a("12"); // int

So even if you want an f64 in rust, even after AVM-layer coercions, you still need to handle both Value::Integer and Value::Number.

[Dinnerbone]

@Aaron1011 wanna give it one last rebase and see if we can finally land this?

[Aaron1011]

So even if you want an f64 in rust, even after AVM-layer coercions, you still need to handle both Value::Integer and Value::Number.

I think the vast majority of native methods will always be coercing to some internal type like Twips, so this won't come up very often. When it does, the native method can just use Valuefor the affected parameter.