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

Add better jni error reporting inside the godot editor #627

Merged
merged 9 commits into from
May 22, 2024

Conversation

chippmann
Copy link
Contributor

This improves the jni error reporting inside the godot editor by grabbing the stacktrace from the jvm and printing them using the godot print commands. Only applies to tool mode. In all other cases, we still print the error using the jni provided method which just prints to the console.

2024-05-10T12:05:06,018362263+02:00
2024-05-10T11:57:53,693923600+02:00
2024-05-10T12:04:09,201022038+02:00
2024-05-10T12:00:49,301472094+02:00

@CedNaru
Copy link
Member

CedNaru commented May 10, 2024

There is a partial conflict with my new PR.
I made the JNI check optional for method calls and instead use a try catch directly on the Kotlin side. It's more efficient to catch exception when possible directly from the JVM instead of doing it from the C++ side.
See:
fd5fbb2

@chippmann
Copy link
Contributor Author

There is a partial conflict with my new PR. I made the JNI check optional for method calls and instead use a try catch directly on the Kotlin side. It's more efficient to catch exception when possible directly from the JVM instead of doing it from the C++ side. See: fd5fbb2

Ok for me. I'll just wait with this PR until yours is merged and then fix and rebase

src/jni/env.cpp Outdated Show resolved Hide resolved
@chippmann chippmann requested a review from CedNaru May 13, 2024 19:45
CedNaru
CedNaru previously approved these changes May 14, 2024
src/jni/env.cpp Outdated
void Env::exception_describe() {
String Env::exception_describe() {
#ifdef DEBUG_ENABLED
jthrowable e = env->ExceptionOccurred();
Copy link
Member

Choose a reason for hiding this comment

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

Simplification: define it in following if

src/jni/env.cpp Outdated
#ifdef DEBUG_ENABLED
jthrowable e = env->ExceptionOccurred();

if (e != nullptr) {
Copy link
Member

Choose a reason for hiding this comment

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

if (ptr != nullptr) is redundant

@chippmann chippmann merged commit ccef2c5 into master May 22, 2024
44 of 45 checks passed
@chippmann chippmann deleted the feature/better_jni_error_reporting branch May 22, 2024 20:22
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.

None yet

3 participants