migueldeicaza / SwiftGodot

New Godot bindings for Swift
https://migueldeicaza.github.io/SwiftGodotDocs/tutorials/swiftgodot-tutorials/
MIT License
1.19k stars 77 forks source link

Improvements, warn when users declare @Callable with invalid types. #539

Open atlasapplications opened 2 months ago

atlasapplications commented 2 months ago

Describe the bug When a C# array is passed as a parameter to a Swift method, a crash occurs with no error or logs produced.

To Reproduce C# Script

GodotObject singleton = ClassDB.Instantiate("CoolObject")
string[] myStringArr = new string[] { "noice", "ok" };
singleton.Call("printCoolWords", myStringArr);

Swift Script

// Class: CoolObject
@Callable
public func printCoolWords(coolWords:[String]) {
  for coolWord in coolWords {
    GD.print(coolWord)
  }
}

Expected behavior Any variant compatible type should be able to be passed as a parameter, per the documentation.

OS: tested on macOS, and iOS.

Additional context There is a work around to use arrays provided by Godot.Collections. So, one could use:

Godot.Collections.Array<string> myStringArr = new();
myStringArr.Add("noice");
myStringArr.Add("ok");

While this makes the plugin functional, it certainly isn't ideal as Godot.Collections should be avoided in C# as it requires some potentially expensive marshaling.

migueldeicaza commented 2 months ago

I suspect the reason is that the above should probably be coolWords: GArray or coolWords: VariantCollection<GString>

atlasapplications commented 2 months ago

Maybe I'm not understanding how some types are mapped. I know obvious ones like C#'s System.String would map to Swift's String. So, wouldn't C#'s System.Array map to Swift's arrays? I thought GArray implemented Godot arrays which are designed to hold Variants. In the case of VariantCollection I thought that just applied strict typing to the Godot array but is still a variant under the hood.

migueldeicaza commented 1 month ago

There is no automatic conversion to a Swift array from a GArray or a variant collection, worth exploring as a new feature.

migueldeicaza commented 1 month ago

We should have the Callable macro catch this particular case '[String]' and point the user on the right direction, not a bug in the binding itself, but would reduce the agony for users.

atlasapplications commented 1 month ago

There is no automatic conversion to a Swift array from a GArray or a variant collection, worth exploring as a new feature.

I understand more now, thank you for explaining that.

atlasapplications commented 1 month ago

We should have the Callable macro catch this particular case '[String]' and point the user on the right direction, not a bug in the binding itself, but would reduce the agony for users.

I think this is an excellent idea! To me, it seemed like this conversion would be automatic so I think for other C# users this could be helpful in preventing some head scratching.

migueldeicaza commented 1 month ago

That's also a possibility, I will try to see if we can do that.