ruffle icon indicating copy to clipboard operation
ruffle copied to clipboard

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

Open Aaron1011 opened this issue 2 years ago • 2 comments

This allows you to define a native method like:

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.

Aaron1011 avatar Sep 06 '22 00:09 Aaron1011

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.

relrelb avatar Sep 06 '22 17:09 relrelb

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().

adrian17 avatar Sep 07 '22 09:09 adrian17

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

Herschel avatar Dec 19 '22 18:12 Herschel

@Herschel @kmeisthax rebased

Aaron1011 avatar Dec 23 '22 00:12 Aaron1011

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.

Bale001 avatar Dec 23 '22 00:12 Bale001

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.

adrian17 avatar Dec 23 '22 00:12 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.

Aaron1011 avatar Dec 23 '22 00:12 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.

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.

adrian17 avatar Dec 23 '22 00:12 adrian17

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

Dinnerbone avatar Jan 10 '23 20:01 Dinnerbone

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.

Aaron1011 avatar Jan 10 '23 23:01 Aaron1011