utopia-rise / godot-kotlin-jvm

Godot Kotlin JVM Module
MIT License
614 stars 44 forks source link

Registered properties of type VariantArray throw exception when declared as lateinit #569

Closed MartinHaeusler closed 1 month ago

MartinHaeusler commented 8 months ago

The following property declaration works without any issues and behaves as expected:

@Export
@RegisterProperty
lateinit var myNode: Node3D

However, the following property declaration:

@Export
@RegisterProperty
lateinit var playerAFrontRowPaths: VariantArray<NodePath>

... will throw an exception:

ERROR: Godot-JVM: An exception has occurred!
   at: check_exceptions (modules/kotlin_jvm/src/jni/env.cpp:69)
Exception in thread "main" kotlin.UninitializedPropertyAccessException: lateinit property playerAFrontRowPaths has not been initialized
    at com.cardsmith.client.ui.components.field.FieldManager.getPlayerAFrontRowPaths(FieldManager.kt:29)
    at godot.entry.FieldManagerRegistrar$register$1$1$4.get(FieldManagerRegistrar.kt:41)
    at godot.core.KtProperty.callGet(Properties.kt:28)

I can only assume that the VariantArray has something to do with it; it's the only difference I could spot. It doesn't make a difference if the property is ever used or not, the exception happens always when the scene loads.

As a workaround, the following declaration works again without issues:

@Export
@RegisterProperty
var playerAFrontRowPaths: VariantArray<NodePath>? = null

... but forces the programmer to deal with the null value at all call sites.

piiertho commented 8 months ago

lateinit variables works, see tests I suppose you call your variable too early and godot did not set it yet.

chippmann commented 8 months ago

The lifecycle info can be found in the danger box here: https://godot-kotl.in/en/stable/user-guide/properties/#exporting-properties

@MartinHaeusler was this the issue and this can be closed?

MartinHaeusler commented 8 months ago

@chippmann unfortunately no. Please consider the following class:

@RegisterClass
class ArrayCrashTest: Node3D() {

    @Export
    @RegisterProperty
    lateinit var testArray: VariantArray<Node3D>

}

The steps to reproduce the issue are quite simple:

You will be faced with the following error immediately upon scene load:

image

This only happens if all of the following conditions are met:

This happens for me with no autoloads and no other scripts in the scene. As you can see, my script never attempts to access the testArray (frankly, it has no methods at all) and the error still happens 100% of the time. I can only assume that it has something to do with the array content from the editor being assigned to the property.

MartinHaeusler commented 8 months ago

By the way, it would be super cool if this "Godot-JVM: An Exception has occurred!" would actually print the exception as well :sweat_smile:

CedNaru commented 6 months ago

I have to investigate but knowing Godot's internal, I have my theory. Outside the case of Object and all its child class, Godot doesn't have the notion of null for the other types of property. It always expects a default value to be already set. When it's primitive or some math type like Vector2, Godot will simply override the current value, null or not, so it doesn't cause any danger. The issues start when using a container type of property (VariantArray, Dictionary, PackedXArray), when setting the value, Godot probably doesn't set the container directly but fill the existing one. But when using lateinit on such container, there is no existing instance to fill.

If that's true, the crash will happen with the other container types as well. And there is not really any solution to allow lateinit like this to work. The rule would be to not use lateinit on containers in the first place and directly instantiate an empty one instead.

chippmann commented 6 months ago

By the way, it would be super cool if this "Godot-JVM: An Exception has occurred!" would actually print the exception as well 😅

@MartinHaeusler the exception which caused this, should actually be printed right before that. We get the exception in cpp land and let jni print it to the console. After that depending on whether we are in editor, in debug mode or in release mode we either print it, or let the program crash with that exception.

So the root cause should always be printed right before the line "Godot-JVM: An Exception has occurred!"

If that's not the case, this is a bug and I would be more than happy if it should occur, you file a separate issue so we can track it :-)

CedNaru commented 1 month ago

Following #668, only Object (and children classes like Resource and Node) can be declared as lateinit. If you want to registerd a VariantArray as a property, you will have to directly initialize an empty one. Keeping lateinit for coretype was too error prone and caused several bugs.