Additional fixes and improvements to JavaClassWrapper

[m4gr3d]

Follow up to #96182

• Fix crashing bug when invoking Java class constructor with parameters
• Add support for accessing class constants
• Add support for Godot Callable arguments. A Godot Callable can be wrapped by a Java Runnable to allow Java logic to run arbitrary Godot lambdas
• Automatically convert java.lang.CharSequence to Godot String as needed
• Code cleanup

Those fixes make godotengine/godot-proposals#11189 feasible out of the box.

[dsnopek]

The high-level idea of this looks great!

However, I suspect that some of the memory management around callables (particularly the Variant * that it hangs on to) isn't quite right. More details in the code comments.

I haven't had a chance to properly test it yet, though.

[dsnopek]

I'm nervous about the p_callable pointer: do we know for sure that the memory it points to will remain alive?

Like, the org.godotengine.godot.variant.Callable class is only going to hang on to the pointer, and we aren't managing our own Variant * (I would imagine memnew() one here, and then have Callable do a memdelete() when it's cleaned up), so we're basically dependent on the caller to callable_to_jcallable() ensuring that the Variant stays alive.

I think the simplest solution to this problem is exactly what I described above: memnew()ing our own Variant *, assigning *p_callable to it, and having the Callable class memdelete() it up when it is destroyed.

[m4gr3d]

@dsnopek Thanks for the suggestion! I've updated the PR, and that also fixes the issue @syntaxerror247 was seeing where the toast doesn't show again. @syntaxerror247 Can you test again and check if the callable is called and the toast shows?

and having the Callable class memdelete() it up when it is destroyed.

This part is more complex to do because java objects don't get callbacks when they're destroyed, so we don't have a mechanism to know when to clean up. I've omitted it for now, which means we're allocating Variant * but not freeing, so looking for suggestions.

[syntaxerror247]

@m4gr3d I tested again, now it works!

[m4gr3d]

This part is more complex to do because java objects don't get callbacks when they're destroyed, so we don't have a mechanism to know when to clean up. I've omitted it for now, which means we're allocating Variant * but not freeing, so looking for suggestions.

I ended up using the Object::finalize() method in order to detect when to deallocate the native memory.
As marked in the documentation, that method has been deprecated; unfortunately its replacement, java.lang.ref.Cleaner is only supported on Android api 33 and higher, so we'll need to update that logic when our min api target catches up.

[dsnopek]

The solution to this issue in the latest version of the PR looks good to me!

[dsnopek]

Thanks!

The latest code changes look good to me as far as fixing the memory management issue that I commented on in my last review! However, while looking at the PR again with memory management on my mind, I think I may have found another issue - a memory leak this time. See the code comments below for more details.

I also did some quick testing: basically, just trying the example given in the code comments in this PR. It worked great!

[llama-nl]

I have a question:
based on this https://developer.android.com/reference/android/os/Build

I tried this code:

var android_runtime = Engine.get_singleton("AndroidRuntime")
if android_runtime:
	var build_class = JavaClassWrapper.wrap("android.os.Build") #JavaCalss  <android.os.Build>
	var model = build_class.BRAND
	print("Device Brand: %s (%s)" % [model])
else:
	printerr("Couldn't find AndroidRuntime singleton.")

And it was showing this error:

Invalid access to property or key 'BRAND' on a base object of type 'JavaClass'.

[dsnopek]

@llama-nl While there is some old code for doing fields and constants in JavaClassWrapper, we haven't fixed it yet (I had a note about that on PR #96182 - that PR focussed on fixing methods)

It shouldn't be too hard to do, though. I'll try and take a look at it soon!

[m4gr3d]

@dsnopek I've addressed your feedback!

@llama-nl I've also fixed access to the class constants. Your code sample should now work with the latest version of this PR.

[dsnopek]

Thanks! This looks great to me :-)

I've also fixed access to the class constants.

Oh, awesome! I hadn't found the time to dig into it myself yet

[akien-mga]

Thanks!