utopia-rise / godot-kotlin-jvm

Godot Kotlin JVM Module
MIT License
562 stars 38 forks source link

`GraphEdit._on_connection_request` seems to have changing parameter types depending on connection direction. #596

Closed RobertZenz closed 3 months ago

RobertZenz commented 3 months ago

I have a Java project and extending a class from GraphEdit:

@RegisterClass
public class WorkflowGraphEdit extends GraphEdit {

That works, but when trying to use _on_connection_request I'm encountering the phenomenon that the parameters seem to change types depending on which direction the connection is being dragged from/to. To be exact, the node names which are provided switch between String and StringName as types.

When dragged from "right port" to "left port" the signature is:

public void _on_connection_request(String fromNodeName, int fromPort, StringName toNodeName, int toPort) {

When dragged from "left port" to "right port" it is:

public void _on_connection_request(StringName fromNodeName, int fromPort, String toNodeName, int toPort) {

The resulting error is always:

Exception in thread "main" kotlin.TypeCastException: Cannot match 21 to 4
    at godot.core.VariantKt$getToKotlinLambdaToExecute$2.invoke(Variant.kt:784)
    at godot.core.VariantKt$getToKotlinLambdaToExecute$2.invoke(Variant.kt:773)
    at godot.core.memory.TransferContext.readArguments(TransferContext.kt:64)
    at godot.core.callable.KtCallable.invoke(KtCallable.kt:19)
ERROR: Godot-JVM: An exception has occurred!
   at: check_exceptions (modules/kotlin_jvm/src/jni/env.cpp:69)

This is with 0.8.2-4.2.1.

CedNaru commented 3 months ago

According to the API, the 2 name parameters are StringNames: image

And if I check the generated Kotlin binding, it is properly done:

image

I suspect that the issue is on the Godot side, they probably forgot to transform a String into a StringName and emitted the signal like this (emitting a signal takes an array of Variant so there is not type safety).

We have to check if there is an ongoing issue about that on the official Godot repo. It's also possible it got undetected because GDScript got an automatic conversion String<>StringName.

CedNaru commented 3 months ago

Found it. It was what I said, one parameter was a String and the other a StringName. It was fixed 2 months with this commit: https://github.com/godotengine/godot/commit/9d7c2978f4799e84bcaa4c5692c58391ea7448eb

image

So the solution is just to wait for the next Godot release. I'm not sure if the fix is going to be part of 4.2.2 or only for 4.3.

CedNaru commented 3 months ago

Looking at the state of the master and 4.2 branch, it seems the fix will only be for Godot 4.3. The wait is going to be longer then.

RobertZenz commented 3 months ago

Thanks for the fast response. I already thought as much, but didn't really know where to start looking for it.

I guess then waiting is in order. Though I just thought that it might be possible to accept Object instead and perform the type checks wnd cast myself as a workaround. But I'm not in front my PC right now to test that.

On a related note, I'm very impressed and happy with how godot-jvm works. With the exception of a few head scratchers it is working as advertised and easy to use. Thank you for creating it.

RobertZenz commented 3 months ago

Yes, the following simple workaround is working:

@RegisterFunction
public void _on_connection_request(Object fromNodeUnknown, int fromPort, Object toNodeUnknown, int toPort) {
    String fromNodeName = fromNodeUnknown.toString();
    String toNodeName = toNodeUnknown.toString();

If one needs the StringName then appropriate checks and casts would be necessary, of course.

Not sure why I didn't think of that sooner.