Rework logging

src/editor/godot_kotlin_jvm_editor.cpp

@@ -16,7 +16,7 @@ void GodotKotlinJvmEditor::on_file_system_dock_file_moved(// NOLINT(readability-
   const String& new_file
 ) {
     if (file.ends_with(String(".") + GODOT_JVM_REGISTRATION_FILE_EXTENSION)) {
-        LOG_WARNING(
+        JVM_WARNING(
           vformat("You should not move registration files in the godot editor! Use the IDE for that. File moved: %s -> %s", file, new_file)
         );
     }
@@ -26,7 +26,7 @@ void GodotKotlinJvmEditor::on_file_system_dock_file_removed(// NOLINT(readabilit
   const String& file
 ) {
     if (file.ends_with(String(".") + GODOT_JVM_REGISTRATION_FILE_EXTENSION)) {
-        LOG_WARNING(vformat("You should not remove registration files in the godot editor! Use the IDE for that. File removed: %s", file));
+        JVM_WARNING("You should not remove registration files in the godot editor! Use the IDE for that. File removed: %s", file);
     }
 }
 
@@ -38,8 +38,7 @@ void GodotKotlinJvmEditor::on_file_system_dock_folder_moved(// NOLINT(readabilit
     String file_path = dir_access->get_next();
     while (!file_path.is_empty()) {
         if (file_path.ends_with(String(".") + GODOT_JVM_REGISTRATION_FILE_EXTENSION)) {
-            LOG_WARNING(vformat("You should not move folders with registration files in the godot editor! Use the IDE for that. Folder moved: %s", folder)
-            );
+            JVM_WARNING("You should not move folders with registration files in the godot editor! Use the IDE for that. Folder moved: %s", folder);
             break;
         }
         file_path = dir_access->get_next();
@@ -52,7 +51,7 @@ void GodotKotlinJvmEditor::on_menu_option_pressed(int menu_option) {
             about_dialog->popup_centered();
             break;
         default:
-            LOG_ERROR("Invalid menu option. Please file a bugreport to "
+            JVM_ERR_FAIL_MSG("Invalid menu option. Please file a bugreport to "
                       "https://github.com/utopia-rise/godot-kotlin-jvm/issues");
     }
 }

src/gd_kotlin.cpp

@@ -36,14 +36,14 @@ void GDKotlin::fetch_user_configuration() {
 
         // The function is going to mutate the provided configuration with the valid values found in the file.
         // If some of the values parsed in the file are invalid, it will return true;
-        LOG_VERBOSE("Parsing JSON configuration file...")
+        JVM_VERBOSE("Parsing JSON configuration file...");
         invalid_file_content = JvmUserConfiguration::parse_configuration_json(content, user_configuration);
         if (invalid_file_content) {
-            LOG_WARNING("Configuration file is malformed. One or several settings might not be applied.");
+            JVM_WARNING("Configuration file is malformed. One or several settings might not be applied.");
         }
     } else {
         // No need for a warning, it's most likely the first time the project is run.
-        LOG_INFO("No JVM user_configuration file found. Default settings for your platform will be used.");
+        JVM_LOG("No JVM user_configuration file found. Default settings for your platform will be used.");
     }
 
 #ifdef TOOLS_ENABLED
@@ -52,15 +52,15 @@ void GDKotlin::fetch_user_configuration() {
     if (invalid_file_content || !configuration_file_exist) {
         Ref<FileAccess> file_access = FileAccess::open(JVM_CONFIGURATION_PATH, FileAccess::WRITE);
         String json = JvmUserConfiguration::export_configuration_to_json(user_configuration);
-        LOG_INFO(vformat("Writing a new configuration file to disk at %s", JVM_CONFIGURATION_PATH));
+        JVM_LOG("Writing a new configuration file to disk at %s", JVM_CONFIGURATION_PATH);
         file_access->store_string(json);
     }
 #endif
 
     HashMap<String, Variant> cmd_argument_map;
-    LOG_VERBOSE("Parsing commandline arguments...")
+    JVM_VERBOSE("Parsing commandline arguments...");
     JvmUserConfiguration::parse_command_line(OS::get_singleton()->get_cmdline_args(), cmd_argument_map);
-    LOG_VERBOSE("Creating final JVM Configuration...")
+    JVM_VERBOSE("Creating final JVM Configuration...");
     JvmUserConfiguration::merge_with_command_line(user_configuration, cmd_argument_map);
     JvmUserConfiguration::sanitize_and_log_configuration(user_configuration);
 }
@@ -83,7 +83,7 @@ void GDKotlin::set_jvm_options() {
     if (user_configuration.jvm_jmx_port >= 0) { jvm_options.add_jmx_option(user_configuration.jvm_jmx_port); }
 
     if (!Engine::get_singleton()->is_editor_hint() && !user_configuration.jvm_args.is_empty()) {
-        LOG_WARNING("You are using custom arguments for the JVM. Make sure they are valid or you risk the JVM to not "
+        JVM_WARNING("You are using custom arguments for the JVM. Make sure they are valid or you risk the JVM to not "
                     "launch properly");
         jvm_options.add_custom_options(user_configuration.jvm_args);
     }
@@ -103,7 +103,7 @@ String GDKotlin::copy_new_file_to_user_dir(const String& file_name) {
 
 #ifndef __ANDROID__
     if (!FileAccess::exists(file_user_path) || FileAccess::get_md5(file_user_path) != FileAccess::get_md5(file_res_path)) {
-        LOG_VERBOSE(vformat("%s file has changed. Copying it from res:// to user://.", file_name));
+        JVM_VERBOSE("%s file has changed. Copying it from res:// to user://.", file_name);
 #else
     // as per suggestion of https://developer.android.com/about/versions/14/behavior-changes-14#safer-dynamic-code-loading, we first delete existing files and then copy them again
     // if we don't do this, subsequent app starts where the files already exist, error out
@@ -115,7 +115,7 @@ String GDKotlin::copy_new_file_to_user_dir(const String& file_name) {
     Error err;
     Ref<DirAccess> dir_access {DirAccess::open(BUILD_DIRECTORY, &err)};
 
-    JVM_ERR_FAIL_COND_V_MSG(err != OK, "", vformat("Cannot open %s file in res://.", file_name));
+    JVM_ERR_FAIL_COND_V_MSG(err != OK, "", "Cannot open %s file in res://.", file_name);
 
     dir_access->copy(file_res_path, file_user_path);
 
@@ -144,7 +144,7 @@ bool GDKotlin::load_bootstrap() {
 #endif
 
     JVM_ERR_FAIL_COND_V_MSG(!FileAccess::exists(bootstrap_jar), false, error_text);
-    LOG_VERBOSE(vformat("Loading bootstrap jar: %s", bootstrap_jar));
+    JVM_VERBOSE("Loading bootstrap jar: %s", bootstrap_jar);
 
     bootstrap_class_loader = ClassLoader::create_instance(env, bootstrap_jar, jni::JObject(nullptr));
     bootstrap_class_loader->set_as_context_loader(env);
@@ -181,7 +181,7 @@ bool GDKotlin::load_user_code() {
 #else
         String user_code_path {copy_new_file_to_user_dir(USER_CODE_FILE)};
 #endif
-        LOG_VERBOSE(vformat("Loading usercode file at: %s", user_code_path));
+        JVM_VERBOSE("Loading usercode file at: %s", user_code_path);
         // TODO: Rework this part when cpp reloading done, can't check what's happening in the Kotlin code from here.
         ClassLoader* user_class_loader = ClassLoader::create_instance(
           env,
@@ -264,7 +264,7 @@ bool GDKotlin::load_dynamic_lib() {
 #ifdef TOOLS_ENABLED
             else if (String environment_jvm = get_path_to_environment_jvm();
                      !environment_jvm.is_empty() && FileAccess::exists(environment_jvm)) {
-                LOG_WARNING(vformat("Godot-JVM: You really should embed a JRE in your project with jlink! See the "
+                JVM_WARNING(vformat("Godot-JVM: You really should embed a JRE in your project with jlink! See the "
                                     "documentation if you don't know how to do that"));
                 path_to_jvm_lib = environment_jvm;
             } else {
@@ -281,7 +281,7 @@ bool GDKotlin::load_dynamic_lib() {
             }
 #else
             else {
-                JVM_ERR_FAIL_V_MSG(false, vformat("No embedded JRE found at: %s!", get_path_to_embedded_jvm()));
+                JVM_ERR_FAIL_V_MSG(false, "No embedded JRE found at: %s!", get_path_to_embedded_jvm());
             }
 #endif
 
@@ -301,11 +301,11 @@ bool GDKotlin::load_dynamic_lib() {
             break;
         default:
             // Sanity check. Should never happen
-            JVM_CRASH_NOW_MSG("Tried to load a VM that's neither the JVM nor Graal Native Image");
+            JVM_DEV_ASSERT(false, "Tried to load a VM that's neither the JVM nor Graal Native Image");
     }
 
     if (OS::get_singleton()->open_dynamic_library(path_to_jvm_lib, jvm_dynamic_library_handle) != OK) {
-        JVM_ERR_FAIL_V_MSG(false, vformat("Failed to load the jvm dynamic library from path %s!", path_to_jvm_lib));
+        JVM_ERR_FAIL_V_MSG(false, "Failed to load the jvm dynamic library from path %s!", path_to_jvm_lib);
     }
     return true;
 }

src/jni/env.cpp

@@ -10,7 +10,7 @@ namespace jni {
 
     void Env::push_local_frame(int capacity) {
         auto result = env->PushLocalFrame(capacity);
-        JVM_CRASH_COND_MSG(result != JNI_OK, "Failed to push local frame!");
+        JVM_ERR_FAIL_COND_MSG(result != JNI_OK, "Failed to push local frame!");
     }
 
     void Env::pop_local_frame() {
@@ -24,7 +24,8 @@ namespace jni {
     //Support fully qualified class names with "a/b/c" and "a.b.c" format
     JClass Env::find_class(const char* name) {
         jclass cls = env->FindClass(String(name).replace(".", "/").utf8());
-        JVM_CRASH_COND_MSG(cls == nullptr, vformat("Class not found: %s", name));
+        JVM_DEV_ASSERT(cls, vformat("Class not found: %s", name));
+        check_exceptions();
         return JClass(cls);
     }
 
@@ -96,7 +97,7 @@ namespace jni {
             if (exception.is_empty()) {
                 exception = "An exception has occurred!";
             }
-            HANDLE_JVM_EXCEPTIONS(true, exception);
+            JVM_ERR_FAIL_MSG(exception);
         }
     }
 

src/jni/env.h

@@ -6,14 +6,6 @@
 #include <core/string/ustring.h>
 #include <jni.h>
 
-#ifdef TOOLS_ENABLED
-#define HANDLE_JVM_EXCEPTIONS(condition, message) JVM_ERR_FAIL_COND_MSG(condition, message)
-#elif DEBUG_ENABLED
-#define HANDLE_JVM_EXCEPTIONS(condition, message) JVM_CRASH_COND_MSG(condition, message)
-#else
-#define HANDLE_JVM_EXCEPTIONS(condition, message) JVM_ERR_FAIL_COND_MSG(condition, message)
-#endif
-
 namespace jni {
 
     class JObject;

src/jni/jvm.cpp

@@ -30,13 +30,13 @@ namespace jni {
 #else
         jint result = _instance->vm->AttachCurrentThreadAsDaemon((void**) &r_env, nullptr);
 #endif
-        JVM_CRASH_COND_MSG(result != JNI_OK, "Failed to attach vm to current thread!");
+        JVM_DEV_ASSERT(result == JNI_OK, "Failed to attach vm to current thread!");
         env = new Env(r_env);
     }
 
     void Jvm::detach() {
         jint result = _instance->vm->DetachCurrentThread();
-        JVM_CRASH_COND_MSG(result != JNI_OK, "Failed to detach vm to current thread!");
+        JVM_DEV_ASSERT(result == JNI_OK, "Failed to detach vm to current thread!");
         delete env;
         env = nullptr;
     }
@@ -45,7 +45,7 @@ namespace jni {
         if (unlikely(!env)) {
             JNIEnv* r_env;
             jint result = _instance->vm->GetEnv((void**) &r_env, _instance->version);
-            JVM_CRASH_COND_MSG(result == JNI_EDETACHED, "Current thread is not attached!");
+            JVM_DEV_ASSERT(result != JNI_EDETACHED, "Current thread is not attached!");
             env = new Env(r_env);
         }
         return *env;

src/jni/types.cpp

@@ -67,21 +67,21 @@ namespace jni {
 
     MethodID JClass::get_method_id(Env& env, const char* name, const char* signature) {
         auto id = env.env->GetMethodID((jclass) obj, name, signature);
-        JVM_CRASH_COND_MSG(id == nullptr, vformat("Method not found: %s with signature: %s", name, signature));
+        JVM_DEV_ASSERT(id, vformat("Method not found: %s with signature: %s", name, signature));
         env.check_exceptions();
         return id;
     }
 
     MethodID JClass::get_static_method_id(Env& env, const char* name, const char* signature) {
         auto id = env.env->GetStaticMethodID((jclass) obj, name, signature);
-        JVM_CRASH_COND_MSG(id == nullptr, vformat("Method not found: %s with signature: %s", name, signature));
+        JVM_DEV_ASSERT(id, vformat("Method not found: %s with signature: %s", name, signature));
         env.check_exceptions();
         return id;
     }
 
     FieldID JClass::get_static_field_id(Env& env, const char* name, const char* signature) {
         auto id = env.env->GetStaticFieldID((jclass) obj, name, signature);
-        JVM_CRASH_COND_MSG(id == nullptr, vformat("Field not found: %s with signature: %s", name, signature));
+        JVM_DEV_ASSERT(id, vformat("Field not found: %s with signature: %s", name, signature));
         env.check_exceptions();
         return id;
     }
@@ -97,7 +97,7 @@ namespace jni {
 
     JObject JClass::new_instance(Env& env, MethodID ctor, jvalue* args) {
         auto ret = env.env->NewObjectA((jclass) obj, ctor, args);
-        JVM_CRASH_COND_MSG(ret == nullptr, "Failed to instantiated object!");
+        JVM_DEV_ASSERT(ret, "Failed to instantiated object!");
         env.check_exceptions();
         return JObject(ret);
     }
@@ -116,7 +116,7 @@ namespace jni {
 
     JObjectArray JClass::new_object_array(Env& env, int size, JObject initial) {
         auto ret = env.env->NewObjectArray(size, (jclass) obj, initial.obj);
-        JVM_CRASH_COND_MSG(ret == nullptr, "Failed to instantiated object array!");
+        JVM_DEV_ASSERT(ret, "Failed to instantiated object array!");
         return JObjectArray(ret);
     }
 

src/jni/types.h

@@ -247,15 +247,10 @@ namespace jni {
 
     class JClass : public JObject {
     public:
-        explicit JClass(jclass cls) : JObject(cls) {}
+        JClass() = default;
+        JClass(jclass clazz): JObject(clazz){};
 
-        explicit JClass(jobject cls) : JObject(cls) {}
-
-        JClass(const JClass&) = default;
-
-        JClass& operator=(const JClass&) = default;
-
-        JClass() : JClass((jclass) nullptr) {}
+        explicit JClass(JObject jObject): JObject(jObject){};
 
         JObject new_instance(Env& env, MethodID ctor, jvalue* args = {});
 

src/jvm_wrapper/bootstrap.cpp

@@ -13,7 +13,7 @@ void Bootstrap::load_classes(JNIEnv* p_env, jobject p_this, jobjectArray p_class
         KtClass* kt_class = new KtClass(env, jni_classes.get(env, i));
         kt_class->fetch_members(env);
         classes.append(kt_class);
-        LOG_DEV_VERBOSE(vformat("Loaded class %s : %s", kt_class->registered_class_name, kt_class->base_godot_class));
+        JVM_DEV_VERBOSE("Loaded class %s : %s", kt_class->registered_class_name, kt_class->base_godot_class);
     }
 
     j_object.delete_local_ref(env);
@@ -24,7 +24,7 @@ void Bootstrap::load_classes(JNIEnv* p_env, jobject p_this, jobjectArray p_class
 
 void Bootstrap::register_engine_type(JNIEnv* p_env, jobject p_this, jobjectArray p_classes_names, jobjectArray p_singleton_names) {
 #ifdef DEV_ENABLED
-    LOG_VERBOSE("Starting to register managed engine types...");
+    JVM_VERBOSE("Starting to register managed engine types...");
 #endif
     jni::Env env(p_env);
 
@@ -39,7 +39,7 @@ void Bootstrap::register_engine_type(JNIEnv* p_env, jobject p_this, jobjectArray
     engine_types.delete_local_ref(env);
     singleton_names.delete_local_ref(env);
 #ifdef DEV_ENABLED
-    LOG_VERBOSE("Done registering managed engine types...");
+    JVM_VERBOSE("Done registering managed engine types...");
 #endif
 }
 
@@ -50,7 +50,7 @@ void Bootstrap::init(jni::Env& p_env, const String& p_project_path, const String
     jni::JObject project_path = p_env.new_string(p_project_path.utf8().get_data());
     jni::JObject jar_file {p_env.new_string(p_jar_file.utf8().get_data())};
     jvalue args[3] = {jni::to_jni_arg(project_path), jni::to_jni_arg(jar_file), jni::to_jni_arg(p_class_loader)};
-    wrapped.call_object_method(p_env, INIT, args);
+    wrapped.call_void_method(p_env, INIT, args);
 }
 
 void Bootstrap::finish(jni::Env& p_env) {

src/jvm_wrapper/bootstrap.h

@@ -7,7 +7,7 @@ JVM_INSTANCE_WRAPPER(Bootstrap, "godot.runtime.Bootstrap") {
     JVM_CLASS(Bootstrap)
 
     // clang-format off
-    JNI_OBJECT_METHOD(INIT)
+    JNI_VOID_METHOD(INIT)
     JNI_VOID_METHOD(FINISH)
 
     INIT_JNI_BINDINGS(

src/jvm_wrapper/jvm_instance_wrapper.h

@@ -9,7 +9,7 @@
     inline constexpr char NAME##QualifiedName[] = FQNAME; \
     class NAME : public JvmInstanceWrapper<NAME, NAME##QualifiedName>
 
-#define JVM_CLASS(NAME) \
+#define JVM_CLASS(NAME)                                         \
     friend class JvmInstanceWrapper<NAME, NAME##QualifiedName>; \
     static inline constexpr const char* fq_name = NAME##QualifiedName;
 
@@ -21,7 +21,6 @@
 #define JNI_DOUBLE_METHOD(var_name) inline static jni::DoubleMethodID var_name;
 #define JNI_OBJECT_METHOD(var_name) inline static jni::ObjectMethodID var_name;
 
-
 #define INIT_JNI_METHOD(var_name, name, signature) var_name.methodId = clazz.get_method_id(p_env, name, signature);
 
 #define INIT_NATIVE_METHOD(string_name, signature, function) \
@@ -57,7 +56,7 @@ public:
 template<class Derived, const char* FqName>
 class JvmInstanceWrapper {
 protected:
-    bool is_weak;
+    bool is_weak = false;
     jni::JObject wrapped;
 
     explicit JvmInstanceWrapper(jni::Env& p_env, jni::JObject p_wrapped);
@@ -76,7 +75,7 @@ class JvmInstanceWrapper {
 };
 
 template<class Derived, const char* FqName>
-JvmInstanceWrapper<Derived, FqName>::JvmInstanceWrapper(jni::Env& p_env, jni::JObject p_wrapped) : is_weak(false) {
+JvmInstanceWrapper<Derived, FqName>::JvmInstanceWrapper(jni::Env& p_env, jni::JObject p_wrapped) {
     // When created, it's a strong reference by default
     wrapped = p_wrapped.new_global_ref<jni::JObject>(p_env);
     p_wrapped.delete_local_ref(p_env);

src/jvm_wrapper/jvm_singleton_wrapper.h

@@ -57,8 +57,7 @@ class JvmSingletonWrapper : public JvmInstanceWrapper<Derived, FqName> {
 
 template<class Derived, const char* FqName>
 Derived* JvmSingletonWrapper<Derived, FqName>::create_instance(jni::Env& p_env) {
-    LOG_ERROR("Can't create a new instance of a this class. Returning the singleton instead");
-    return _instance;
+    JVM_ERR_FAIL_V_MSG(_instance, "Can't create a new instance of a this class. Returning the singleton instead");
 }
 
 template<class Derived, const char* FqName>

src/jvm_wrapper/memory/memory_manager.cpp

@@ -183,9 +183,9 @@ void MemoryManager::sync_memory(jni::Env& p_env) {
 }
 
 void MemoryManager::clean_up(jni::Env& p_env) {
-    LOG_VERBOSE("Cleaning JVM Memory...")
+    JVM_VERBOSE("Cleaning JVM Memory...");
     wrapped.call_void_method(p_env, CLEAN_UP);
-    LOG_VERBOSE("JVM Memory cleaned!")
+    JVM_VERBOSE("JVM Memory cleaned!");
 }
 
 void MemoryManager::queue_dead_object(Object* obj) {