Merge branch 'main' into daylily-zeleen/implement-reload_scripts-for-debugging

Author: Daylily-Zeleen

SCsub

@@ -184,7 +184,7 @@ if quickjs_support is None and jsc_support is None and is_library_supported(v8_p
         download_dependency("lws", deps_lws_version, "lws")
 
 v8_support = validate_library_support(v8_prebuilt_libs) if quickjs_support is None else None
-lws_support = validate_library_support(lws_prebuilt_libs) if v8_support is None else None
+lws_support = validate_library_support(lws_prebuilt_libs) if v8_support is not None else None
 
 jsb_defines = [
     CompileDefines("JSB_USE_TYPESCRIPT", 1 if env["use_typescript"] else 0),

bridge/jsb_environment.cpp

@@ -542,6 +542,7 @@ namespace jsb
                 v8::Isolate::Scope isolate_scope(isolate_);
                 v8::HandleScope handle_scope(isolate_);
                 const v8::Local<v8::Context> context = context_.Get(isolate_);
+                v8::Context::Scope context_scope(context);
 
                 for (const Message& message : messages)
                 {
@@ -639,7 +640,7 @@ namespace jsb
         //TODO 0. HOW TO HANDLE COMPLICATED SITUATIONS? SUCH AS NESTED OBJECTS?
         jsb_nop();
 
-        transfer_in(*p_data);
+        transfer_in(p_context, *p_data);
 
         // call 'ontransfer'
         {
@@ -681,6 +682,13 @@ namespace jsb
         v8::Local<v8::Value> value;
         if (p_message)
         {
+            const std::vector<TransferData>& transfers = p_message->get_transfers();
+
+            for (const auto& transfer : transfers)
+            {
+                p_env->transfer_in(p_context, transfer);
+            }
+
 #if JSB_WITH_V8
             Serialization::VariantDeserializerDelegate delegate(p_env, p_message->get_transfers());
             v8::ValueDeserializer deserializer(isolate, p_message->get_buffer().ptr(), p_message->get_buffer().size(), &delegate);
@@ -952,6 +960,8 @@ namespace jsb
         // hold it in a local variable to avoid gc too early
         v8::Global<v8::Object> obj_ref = std::move(object_handle->ref_);
 
+        // TODO: Look into if we ought to be calling obj->free_instance_binding(this)
+
         //TODO do not clear the internal field if calling from JS GC
         // if (p_finalize != FinalizationType::None)
         // {
@@ -1963,10 +1973,11 @@ namespace jsb
         return _call(isolate, context, js_func.object_.Get(isolate), v8::Undefined(isolate), p_args, p_argcount, r_error);
     }
 
-    void Environment::transfer_out(NativeObjectID p_worker_handle_id, int transfer_index, const Variant& p_variant, TransferData& r_transfer_data)
+    void Environment::prepare_transfer_out(NativeObjectID p_worker_handle_id, int transfer_index, const Variant& p_variant, TransferData& r_transfer_data)
     {
         r_transfer_data.source_worker_id = p_worker_handle_id;
         r_transfer_data.transfer_index = transfer_index;
+        r_transfer_data.variant = p_variant;
 
         if (p_variant.get_type() == Variant::OBJECT)
         {
@@ -1981,22 +1992,28 @@ namespace jsb
 
                 script_instance->get_property_state(r_transfer_data.state);
                 r_transfer_data.script_path = script->get_path();
-
-                obj->set_script_instance(nullptr);
             }
+        }
+    }
+
+    void Environment::finalize_transfer_out(const TransferData& p_transfer_data)
+    {
+        const Variant& variant = p_transfer_data.variant;
 
+        if (variant.get_type() == Variant::OBJECT)
+        {
+            Object* obj = variant;
+            obj->set_script_instance(nullptr);
             free_object(obj, FinalizationType::None);
         }
 
         // For now, we don't do anything special with variants since Godot's variant types are thread safe and use
         // copy-on-write semantics. Technically, it may be advantageous for us to (in future?) clear the variant in the
         // source environment since Godot has optimizations in place that will skip reallocation of the underlying data
         // upon mutation if there's a single reference to that data.
-
-        r_transfer_data.variant = p_variant;
     }
 
-    void Environment::transfer_in(const TransferData& p_data)
+    void Environment::transfer_in(const v8::Local<v8::Context>& p_context, const TransferData& p_data)
     {
         if (p_data.variant.get_type() == Variant::Type::OBJECT)
         {
@@ -2009,21 +2026,38 @@ namespace jsb
                 return;
             }
 
-            // restore the object state if it's a GodotJSScript
+            jsb_checkf(!object_db_.has_object(instance), "transferred in an object already registered in the environment");
+
+            if (!object_db_.has_object(instance))
+            {
+                v8::Local<v8::Object> obj;
+                jsb_check(TypeConvert::gd_obj_to_js(isolate_, p_context, instance, obj));
+
+                if (instance->is_ref_counted())
+                {
+                    RefCounted *reference = static_cast<RefCounted *>(instance);
+
+                    if (reference->unreference())
+                    {
+                        // Uh, we really shouldn't end up here. This can only occur if another thread is doing something it
+                        // really shouldn't be doing. I guess we don't want to be responsible for a leak, but this is bad.
+                        memdelete(reference);
+                        JSB_LOG(Error, "transferred object unexpectedly freed: %d", (uint64_t) object_id);
+                        return;
+                    }
+                }
+            }
+
             if (!p_data.script_path.is_empty())
             {
-                // 1. create a script and script instance
-                // 2. attach the script & script instance to the object
-                const Ref<GodotJSScript> script = ResourceLoader::load(p_data.script_path);
-                jsb_check(script.is_valid());
-                jsb_unused(script->can_instantiate());
-                ScriptInstance* script_instance = script->instance_construct(instance, false);
-                jsb_check(script_instance);
+                ScriptInstance *script_instance = instance->get_script_instance();
 
-                // 3. restore the object state
-                for (const Pair<StringName, Variant>& pair : p_data.state)
+                if (script_instance)
                 {
-                    script_instance->set(pair.first, pair.second);
+                    for (const Pair<StringName, Variant>& pair : p_data.state)
+                    {
+                        script_instance->set(pair.first, pair.second);
+                    }
                 }
             }
         }
@@ -2034,7 +2068,8 @@ namespace jsb
         if (p_variant.get_type() == Variant::OBJECT)
         {
             TransferData* transfer_data = memnew(TransferData);
-            p_from->transfer_out(p_worker_handle_id, 0, p_variant, *transfer_data);
+            p_from->prepare_transfer_out(p_worker_handle_id, 0, p_variant, *transfer_data);
+            p_from->finalize_transfer_out(*transfer_data);
             p_to->add_async_call(AsyncCall::TYPE_TRANSFER_, transfer_data);
         }
         else

bridge/jsb_environment.h

@@ -322,8 +322,9 @@ namespace jsb
          */
         Variant call_script_method(ScriptClassID p_script_class_id, NativeObjectID p_object_id, const StringName& p_method, const Variant** p_argv, int p_argc, Callable::CallError& r_error);
 
-        void transfer_out(NativeObjectID p_worker_handle_id, int transfer_index, const Variant& p_variant, TransferData& r_transfer_data);
-        void transfer_in(const TransferData& p_data);
+        void prepare_transfer_out(NativeObjectID p_worker_handle_id, int transfer_index, const Variant& p_variant, TransferData& r_transfer_data);
+        void finalize_transfer_out(const TransferData& p_data);
+        void transfer_in(const v8::Local<v8::Context>& p_context, const TransferData& p_data);
 
         // [EXPERIMENTAL] transfer object between environments.
         // call this method of the source environment in the source environment thread.

bridge/jsb_object_bindings.cpp

@@ -355,6 +355,7 @@ namespace jsb
         // prepare argv
         const int method_argc = method_bind->get_argument_count();
         const bool method_is_vararg = method_bind->is_vararg();
+
         if (!internal::VariantUtil::check_argc(method_is_vararg, argc, method_bind->get_default_argument_count(), method_argc))
         {
             const String error_message = jsb_errorf("Failed to call: %s. %d arguments are required", method_bind->get_name(), method_argc - method_bind->get_default_argument_count());

bridge/jsb_worker.cpp

@@ -130,6 +130,7 @@ namespace jsb
                                 v8::Isolate::Scope isolate_scope(isolate);
                                 v8::HandleScope handle_scope(isolate);
                                 const v8::Local<v8::Context> context = env->get_context();
+                                v8::Context::Scope context_scope(context);
                                 const v8::Local<v8::Object> context_obj = context_obj_handle.Get(isolate);
 
                                 for (const WorkerMessage& message : messages)
@@ -240,7 +241,7 @@ namespace jsb
 
                 for (const auto& transfer : p_message.get_transfers())
                 {
-                    p_env->transfer_in(transfer);
+                    p_env->transfer_in(p_context, transfer);
                 }
             }
 
@@ -685,21 +686,28 @@ namespace jsb
 
                     Variant variant;
 
-                    if (!TypeConvert::js_to_gd_var(isolate, context, item.As<v8::Object>(), Variant::Type::ARRAY, variant))
+                    if (!TypeConvert::js_to_gd_var(isolate, context, item.As<v8::Object>(), variant))
                     {
                         jsb_throw(isolate, "transfer list must contain Godot object/variant types only");
                         return {nullptr, 0};
                     }
 
                     TransferData transfer_data;
-                    from_env->transfer_out(NativeObjectID::none(), transfers.size(), variant, transfer_data);
+                    from_env->prepare_transfer_out(NativeObjectID::none(), transfers.size(), variant, transfer_data);
                     transfers.insert(variant, transfer_data);
                 }
             }
             else
             {
                 Variant transfer_var;
-                if (!TypeConvert::js_to_gd_var(isolate, context, transfer_arg.As<v8::Object>(), Variant::Type::ARRAY, transfer_var))
+
+                if (!TypeConvert::js_to_gd_var(isolate, context, transfer_arg.As<v8::Object>(), Variant::Type::OBJECT, transfer_var))
+                {
+                    jsb_throw(isolate, "transfer list must be an array");
+                    return std::pair<uint8_t*, size_t>();
+                }
+
+                if (!transfer_var.is_array())
                 {
                     jsb_throw(isolate, "transfer list must be an array");
                     return std::pair<uint8_t*, size_t>();
@@ -711,7 +719,7 @@ namespace jsb
                 {
                     Variant& variant = transfer_arr[i];
                     TransferData transfer_data;
-                    from_env->transfer_out(NativeObjectID::none(), i, variant, transfer_data);
+                    from_env->prepare_transfer_out(NativeObjectID::none(), i, variant, transfer_data);
                     transfers.insert(variant, transfer_data);
                 }
             }
@@ -736,6 +744,11 @@ namespace jsb
             return {nullptr, 0};
         }
 
+        for (const auto& entry : transfers)
+        {
+            from_env->finalize_transfer_out(entry.value);
+        }
+
         return serializer.Release();
     }
 }