Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDKotlin Rework: Module initialization #609

Merged
merged 10 commits into from
Apr 26, 2024

Conversation

CedNaru
Copy link
Member

@CedNaru CedNaru commented Apr 15, 2024

Make the module more resilient to invalid states. Many of the previous explicit crashes got replaced by errors that won't stop the engine execution.
If something goes wrong somewhere in the initialization of the module, a helpful message will appear on-screen before falling back to a Godot with only JVM placeholders.
In debug/tool mode, it's even possible to start the game using JVM scripts that are not built yet. In such case, the script is just not created and only the naked native object will remain (Errors are still thrown in such cases).

GDKotlin got explicit initialization states and a custom error message exists for almost each of them (when the cause is likely a basic one) in case an issue was encountered.
If the final state is invalid, GDkotlin will be immediately freed and unloaded. The languages and resource loader are still registered to Godot, even in the case of an invalid state, so users can manipulate JVM scripts in the editor without a JAVA_HOME, embedded JRE, boostrap.jar or main.jar (They will have an alert appearing on-screen when the engine starts).

image

Note that the last part of the initialization still has to be done in GdjLanguage. This final step fetches all the Jvm Scripts from Kotlin to C++ as well as sending all method pointers from C++ to Kotlin.
This latter detail matters because this operation requires all classes to be initialized in Godot, so we can't do it as part of the module initialization itself, even using the latest initialization enum value in it. Sadly Godot doesn't have a "afterAll" phase for module initialization.

@CedNaru CedNaru force-pushed the gd_kotlin/initialization_management branch from ba502c2 to 2373302 Compare April 16, 2024 01:02
@CedNaru CedNaru marked this pull request as ready for review April 16, 2024 01:03
@CedNaru CedNaru requested review from chippmann and piiertho and removed request for chippmann April 16, 2024 01:03
@CedNaru CedNaru force-pushed the gd_kotlin/initialization_management branch 3 times, most recently from e83700b to 2fe7b51 Compare April 16, 2024 01:54
@CedNaru CedNaru force-pushed the gd_kotlin/import_export_and_co branch from 8e5ff25 to 839fbb9 Compare April 16, 2024 03:16
@CedNaru CedNaru force-pushed the gd_kotlin/initialization_management branch 2 times, most recently from e983afe to d3043bb Compare April 16, 2024 03:50
src/binding/kotlin_binding.cpp Show resolved Hide resolved
src/gd_kotlin.cpp Outdated Show resolved Hide resolved
src/jvm_wrapper/bootstrap.cpp Outdated Show resolved Hide resolved
src/lifecycle/jvm_manager.cpp Outdated Show resolved Hide resolved
@CedNaru CedNaru requested a review from chippmann April 17, 2024 17:15
@CedNaru CedNaru force-pushed the gd_kotlin/initialization_management branch from cef05e5 to e48e07c Compare April 17, 2024 17:18
@CedNaru CedNaru force-pushed the gd_kotlin/import_export_and_co branch from 839fbb9 to b561986 Compare April 17, 2024 23:19
@CedNaru CedNaru force-pushed the gd_kotlin/initialization_management branch from e48e07c to ad08b33 Compare April 17, 2024 23:20
Copy link
Contributor

@chippmann chippmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent job. Not much to complain.

Just briefly tested build, startup and reloading on the macos arm editor.
A simple error case was also tested.

#endif

#ifdef DEBUG_ENABLED
void GDKotlin::validate_state() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably move this to a separate class no? we could centralize the checks once we add more. Also for once we add back the editor plugin we could do some checks there.

But I'm also fine if we think about this later

Copy link
Member Author

@CedNaru CedNaru Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to keep it there. GDKotlin is now responsible for all the high level management of the module, so it's right where it should be.
Also I don't recommend having checks in the editor plugin, because it means we won't be able to use them when doing an export debug.

@CedNaru CedNaru force-pushed the gd_kotlin/import_export_and_co branch from c514de1 to c173356 Compare April 23, 2024 16:14
@CedNaru CedNaru force-pushed the gd_kotlin/initialization_management branch 3 times, most recently from 78a1ccb to d46573a Compare April 24, 2024 01:56
@CedNaru CedNaru force-pushed the gd_kotlin/import_export_and_co branch from c173356 to 7e7e6e8 Compare April 26, 2024 10:13
@CedNaru CedNaru force-pushed the gd_kotlin/initialization_management branch from 747f1f1 to e92b2f5 Compare April 26, 2024 10:43
Base automatically changed from gd_kotlin/import_export_and_co to gd_kotlin_rework April 26, 2024 11:46
@CedNaru CedNaru force-pushed the gd_kotlin/initialization_management branch from e92b2f5 to 23cb5d3 Compare April 26, 2024 11:51
@CedNaru CedNaru merged commit 9377746 into gd_kotlin_rework Apr 26, 2024
45 checks passed
@CedNaru CedNaru deleted the gd_kotlin/initialization_management branch April 26, 2024 17:14
CedNaru added a commit that referenced this pull request Apr 30, 2024
* Initialize GDKotlin outside language.
* Add proper initialization step for the module and remove many explicit crashes
* Create JvmScriptManager
* Remove NO_USE_STDLIB preprocessor, it has been removed from the engine since Godot 4
CedNaru added a commit that referenced this pull request May 7, 2024
* Initialize GDKotlin outside language.
* Add proper initialization step for the module and remove many explicit crashes
* Create JvmScriptManager
* Remove NO_USE_STDLIB preprocessor, it has been removed from the engine since Godot 4
CedNaru added a commit that referenced this pull request May 8, 2024
* Initialize GDKotlin outside language.
* Add proper initialization step for the module and remove many explicit crashes
* Create JvmScriptManager
* Remove NO_USE_STDLIB preprocessor, it has been removed from the engine since Godot 4
CedNaru added a commit that referenced this pull request May 9, 2024
* Initialize GDKotlin outside language.
* Add proper initialization step for the module and remove many explicit crashes
* Create JvmScriptManager
* Remove NO_USE_STDLIB preprocessor, it has been removed from the engine since Godot 4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants