thorium-cfx / mono_v2_get_started

Mono v2 runtime for FiveM/RedM
36 stars 3 forks source link

Calling export by omitting a parameter causes System.InvalidCastException #25

Open Kiminaze opened 1 year ago

Kiminaze commented 1 year ago

What happened?

Registering an export that takes any type as the first parameter and an object[] as a second parameter will result in an System.InvalidCastException when called from Lua when omitting the object[] parameter.

When registering an export that takes an object[] as the first parameter and calling it without that parameter will not result in an error and execute the export function as expected.

Error message:

SCRIPT ERROR: Could not invoke reference function DBConnector.DatabaseStartup.ParamTest2
  expected: (String, Object[])
  received: (String)
  * Any type in red is not convertible, castable, or simply missing and requires attention.
Failed with exception:
System.InvalidCastException: Specified cast is not valid.
  at (wrapper dynamic-method) System.Object.DBConnector.DatabaseStartup.ParamTest2(object,CitizenFX.Core.Remote,object[])
  at CitizenFX.Core.ReferenceFunctionManager.Invoke (System.Int32 reference, System.Byte* arguments, System.UInt32 argsSize) [0x00024] in C:\gl\builds\4ff63adb\0\cfx\fivem\code\client\clrcore-v2\Interop\ReferenceFunctionManager.cs:174

Expected result

No error being caused

Reproduction steps

  1. Register two exports:
    
    [Export("ParamTest1")]
    private int ParamTest1(object[] parameter) => 1;

[Export("ParamTest2")] private int ParamTest2(string pad, object[] parameter) => 2;


2. Call exports from lua and omit `object[] parameter` completely:
```lua
-- both work
print("Data1", exports["resourceName"]:ParamTest1({}) )
print("Data2", exports["resourceName"]:ParamTest2("test", {}) )

-- this one works as well although parameter is missing
print("Data3", exports["resourceName"]:ParamTest1() )

-- this one produces the error
print("Data4", exports["resourceName"]:ParamTest2("test") )

Importancy

Slight inconvenience

Specific version

FiveM Server 6645 Windows

Extra

thorium-cfx commented 1 year ago

Registering an export that takes any type as the first parameter and an object[] as a second parameter will result in an System.InvalidCastException when called from Lua when omitting the object[] parameter.

This is intended behavior. C# never allows the caller to omit arguments, otherwise we'll get undefined behavior.

When registering an export that takes an object[] as the first parameter and calling it without that parameter will not result in an error and execute the export function as expected.

That shouldn't work, wonder how that even passes šŸ¤”

Default parameter values is a TODO and will indeed solve these issues once it's implemented. But I'm also making bigger changes to the (de)serialization phase of these calls, which may have this fixed from the start.

Kiminaze commented 1 year ago

This is intended behavior. C# never allows the caller to omit arguments, otherwise we'll get undefined behavior.

Already thought so. But it used to work with List on v1. E.g. this:

private object DB_SelectScalar(string query, List<object> parameter)
{
    return database.SelectScalar(query, parameter?.ToArray());
}

(curently porting my sql connector to v2 for all the improvement and few queries don't need parameters, so why not just omit the table completely when calling from Lua. Just thinking here: One thing that could work would be overloaded methods for exports?

[Export("ParamTest2")]
private int ParamTest2(string pad) => 2;
[Export("ParamTest2")]
private int ParamTest2(string pad, object[] parameter) => 2;

That shouldn't work, wonder how that even passes šŸ¤”

Yea, that's what I was wondering as well šŸ˜…

Default parameter values is a TODO and will indeed solve these issues once it's implemented. But I'm also making bigger changes to the (de)serialization phase of these calls, which may have this fixed from the start.

That would also be perfect!

thorium-cfx commented 7 months ago

Support for argument omission (through default parameters) will be added with the new MsgPack serializer update.

(curently porting my sql connector to v2 for all the improvement and few queries don't need parameters, so why not just omit the table completely when calling from Lua. Just thinking here: One thing that could work would be overloaded methods for exports?

[Export("ParamTest2")]
private int ParamTest2(string pad) => 2;
[Export("ParamTest2")]
private int ParamTest2(string pad, object[] parameter) => 2;

It can work, though it will go through all of these listeners and try to cast the values to the parameters, logging any that won't pass. You can also mute the latter, but that'll mute it for all and makes debugging harder.