Skip to content

Commit 3f11955

Browse files
feat: Native object now guaranteed bound during JS initializaation.
Previously, when a JS class was being instantiated for an existing native Godot object (e.g. ResourceLoader.load('...')) the JS instance wasn't actually being bound to the native Godot object until AFTER the JS instance was constructed/initialized. This created a problem in which field/property initializers and the constructor were unable to call any native functions, but could call JS functions. Now, the Godot object is always bound to our instantiated object as part of our Godot native class constructor. Thus, we can now safely call methods during initialization. Importantly, this also fixes TC39 Stage 3 decorator @bind.signal() support for fields.
1 parent 82e0268 commit 3f11955

File tree

6 files changed

+32
-45
lines changed

6 files changed

+32
-45
lines changed

bridge/jsb_bridge_helper.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ namespace jsb
4242
// read script class id from the javascript Class object (the constructor object)
4343
const v8::Local<v8::Object> prototype = prototype_val.As<v8::Object>();
4444
const v8::Local<v8::Object> dt_base_obj = prototype->Get(context, jsb_name(environment, constructor)).ToLocalChecked().As<v8::Object>();
45-
if (!dt_base_obj->Get(context, jsb_symbol(environment, CrossBind)).ToLocal(&cross_bind_sym) || !cross_bind_sym->IsString())
45+
if (!dt_base_obj->Get(context, jsb_symbol(environment, ClassModuleId)).ToLocal(&cross_bind_sym) || !cross_bind_sym->IsString())
4646
{
4747
break;
4848
}

bridge/jsb_class_info.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ namespace jsb
316316
}
317317

318318
// trick: save godot class id for convenience of getting it in JS class constructor
319-
class_obj->Set(p_context, jsb_symbol(environment, CrossBind), environment->get_string_value(p_module.id)).Check();
319+
class_obj->Set(p_context, jsb_symbol(environment, ClassModuleId), environment->get_string_value(p_module.id)).Check();
320320

321321
const v8::Local<v8::Object> dt_base_obj =
322322
class_obj
@@ -325,7 +325,7 @@ namespace jsb
325325
->Get(p_context, jsb_name(environment, constructor)).ToLocalChecked().As<v8::Object>();
326326
jsb_check(class_obj != dt_base_obj);
327327

328-
const v8::Local<v8::Value> dt_base_tag = dt_base_obj->Get(p_context, jsb_symbol(environment, CrossBind)).ToLocalChecked();
328+
const v8::Local<v8::Value> dt_base_tag = dt_base_obj->Get(p_context, jsb_symbol(environment, ClassModuleId)).ToLocalChecked();
329329
existed_class_info->base_script_module_id = dt_base_tag->IsString() ? environment->get_string_name(dt_base_tag.As<v8::String>()) : StringName();
330330
JSB_LOG(Verbose, "%s script %d inherits script module %s native: %d",
331331
p_module.source_info.source_filepath, p_module.script_class_id, existed_class_info->base_script_module_id, *native_class_id);

bridge/jsb_environment.cpp

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -298,8 +298,6 @@ namespace jsb
298298
context->SetAlignedPointerInEmbedderData(kContextEmbedderData, this);
299299
context_.Reset(isolate_, context);
300300

301-
js_only_constructor_tag_.Reset(isolate_, impl::Helper::new_noop_function(isolate_, context));
302-
303301
// init module cache, and register the global 'require' function
304302
{
305303
const v8::Local<v8::Object> cache_obj = v8::Object::New(isolate_);
@@ -874,8 +872,7 @@ namespace jsb
874872

875873
void* Environment::get_verified_object(const v8::Local<v8::Object>& p_obj, NativeClassType::Type p_type) const
876874
{
877-
if (!TypeConvert::is_object(p_obj, p_type)
878-
|| (NativeClassType::Type) (uintptr_t) p_obj->GetAlignedPointerFromInternalField(IF_ClassType) != p_type)
875+
if (!TypeConvert::is_object(p_obj, p_type))
879876
{
880877
return nullptr;
881878
}
@@ -1200,7 +1197,7 @@ namespace jsb
12001197
v8::Local<v8::Context> context = context_.Get(isolate);
12011198
v8::Context::Scope context_scope(context);
12021199

1203-
// In Editor, the script can be attached to an Object after it created in JS (e.g. 'enter_tree' as a child node of a script attached parent node)
1200+
// Can occur at runtime if object.set_script(...) is used, or in the editor due to hot-reloading etc.
12041201
if (const NativeObjectID object_id = this->try_get_object_id(p_this))
12051202
{
12061203
JSB_LOG(Verbose, "crossbinding on previously bound object %d (addr:%d), rebind it to script class %d", object_id, (uintptr_t) p_this, p_class_id);
@@ -1211,13 +1208,11 @@ namespace jsb
12111208
}
12121209

12131210
StringName js_class_name;
1214-
NativeClassID native_class_id;
12151211
v8::Local<v8::Object> class_obj;
12161212

12171213
{
12181214
const ScriptClassInfoPtr class_info = this->get_script_class(p_class_id);
12191215
js_class_name = class_info->js_class_name;
1220-
native_class_id = class_info->native_class_id;
12211216
class_obj = class_info->js_class.Get(isolate);
12221217
JSB_LOG(VeryVerbose, "crossbind %s %s(%d) %d", class_info->js_class_name, class_info->native_class_name, class_info->native_class_id, (uintptr_t) p_this);
12231218
jsb_check(!class_obj->IsNullOrUndefined());
@@ -1241,17 +1236,18 @@ namespace jsb
12411236

12421237
const impl::TryCatch try_catch_run(isolate);
12431238

1244-
v8::Local<v8::Function> js_only_constructor_tag = js_only_constructor_tag_.Get(isolate);
12451239
v8::Local<v8::Value> class_prototype = class_obj->Get(context, jsb_name(this, prototype)).ToLocalChecked();
1246-
js_only_constructor_tag->Set(context, jsb_name(this, prototype), class_prototype).Check();
1240+
v8::Local<v8::Function> new_target = impl::Helper::new_noop_function(isolate_, context);
1241+
new_target->Set(context, jsb_name(this, prototype), class_prototype).Check();
1242+
new_target->Set(context, jsb_symbol(this, ConstructorBindObject), v8::External::New(isolate, p_this)).Check();
12471243

12481244
v8::Local<v8::Object> reflect = context->Global()->Get(context, jsb_name(this, Reflect)).ToLocalChecked().As<v8::Object>();
12491245
v8::Local<v8::Function> reflect_construct = reflect->Get(context, jsb_name(this, construct)).ToLocalChecked().As<v8::Function>();
12501246

12511247
v8::Local<v8::Value> reflect_args[] = {
12521248
class_obj,
12531249
arguments,
1254-
js_only_constructor_tag
1250+
new_target
12551251
};
12561252

12571253
v8::MaybeLocal<v8::Value> constructed_value = reflect_construct->Call(context, reflect, 3, reflect_args);
@@ -1269,8 +1265,8 @@ namespace jsb
12691265
JSB_LOG(Error, "bad instance '%s", js_class_name);
12701266
return {};
12711267
}
1272-
const NativeObjectID object_id = this->bind_godot_object(native_class_id, p_this, instance.As<v8::Object>());
1273-
return object_id;
1268+
1269+
return this->try_get_object_id(p_this);
12741270
}
12751271

12761272
void Environment::rebind(Object *p_this, ScriptClassID p_class_id)

bridge/jsb_environment.h

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ namespace jsb
4646
Doc,
4747
MemberDocMap,
4848

49-
CrossBind, // a symbol can only be used from C++ to indicate calling from cross-bind
49+
ClassModuleId, // provides quick access from a class' constructor to its corresponding module ID
50+
ConstructorBindObject, // indicates a constructor invocation is binding to an existing Godot native object
5051

5152
// Exposed as properties on the `godot` module
5253
FloatType,
@@ -197,8 +198,6 @@ namespace jsb
197198

198199
internal::VariantInfoCollection variant_info_collection_;
199200

200-
v8::Global<v8::Function> js_only_constructor_tag_;
201-
202201
public:
203202
enum class Type : uint8_t
204203
{
@@ -596,19 +595,6 @@ namespace jsb
596595
// NOTE: you can't get a shadow environment with this method
597596
static std::shared_ptr<Environment> _access();
598597

599-
/**
600-
* Should a constructor call only construct a JavaScript object i.e. skip creation of the native Godot Object?
601-
* This occurs when we have an existing Godot object that we wish to "cross bind" with its JavaScript
602-
* implementation. For example, when instantiating a packed scene, Godot creates the objects first; we must then
603-
* cross bind these existing objects with their JavaScript implementation.
604-
* @param function
605-
* @return
606-
*/
607-
bool is_js_only_constructor_tag(const v8::Local<v8::Function> function)
608-
{
609-
return function == js_only_constructor_tag_;
610-
}
611-
612598
private:
613599
void exec_async_calls();
614600
void exec_async_call(AsyncCall::Type p_type, void* p_binding);

bridge/jsb_transpiler.h

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -306,26 +306,30 @@ namespace jsb
306306

307307
// We need to handle different binding scenarios:
308308
//
309-
// 1. Instantiating a new Godot object (e.g. new SubClass() in JS) with a new JS script class instance.
310-
// 2. Instantiating a JS class to be attached to an existing Godot Object e.g., scene instantiation.
309+
// 1. Instantiating only the script class instance, which is to be bound to an existing Godot Object.
310+
// 2. Instantiating a new native Godot object along with the newly instantiated script class.
311311

312312
jsb_check(info.NewTarget()->IsFunction());
313313
v8::Local<v8::Context> context = isolate->GetCurrentContext();
314314
const v8::Local<v8::Function> new_target = info.NewTarget().As<v8::Function>();
315315

316-
if (environment->is_js_only_constructor_tag(new_target))
316+
const v8::Local<v8::Object> self = info.This();
317+
318+
// 1. binding to an existing Godot object
319+
v8::Local<v8::Value> bind_object_value;
320+
if (new_target->Get(context, jsb_symbol(environment, ConstructorBindObject)).ToLocal(&bind_object_value) && bind_object_value->IsExternal())
317321
{
318-
JSB_LOG(Verbose, "cross binding from C++. %s(%d)", class_name, class_id);
322+
JSB_LOG(Verbose, "binding to JS instance to existing native object. %s(%d)", class_name, class_id);
323+
environment->bind_godot_object(class_id, (Object*) bind_object_value.As<v8::External>()->Value(), self);
319324
return;
320325
}
321326

322-
// (case-0) directly instantiate from an underlying native class (it's usually called from scripts)
327+
// 2a. directly instantiating a Godot native object (via its corresponding JavaScript wrapper class)
323328
if (new_target->HasOwnProperty(context, jsb_symbol(environment, ClassId)).ToChecked())
324329
// if (class_info->clazz.NewTarget(isolate) == new_target)
325330
{
326331
internal::StringNames& names = internal::StringNames::get_singleton();
327332
const StringName original_name = names.get_original_name(class_name);
328-
const v8::Local<v8::Object> self = info.This();
329333
Object* gd_object = ClassDB::instantiate(original_name);
330334

331335
// IS IT A TRUTH that ref_count==1 after creation_func??
@@ -334,16 +338,15 @@ namespace jsb
334338
return;
335339
}
336340

337-
// (case-1) new from scripts
338-
v8::Local<v8::Value> cross_bind_sym;
339-
if (new_target->Get(context, jsb_symbol(environment, CrossBind)).ToLocal(&cross_bind_sym) && cross_bind_sym->IsString())
341+
// 2b. instantiating a sub-class of a native Godot object (JavaScript wrapper class)
342+
v8::Local<v8::Value> module_id_value;
343+
if (new_target->Get(context, jsb_symbol(environment, ClassModuleId)).ToLocal(&module_id_value) && module_id_value->IsString())
340344
{
341-
const StringName script_module_id = environment->get_string_name(cross_bind_sym.As<v8::String>());
345+
const StringName script_module_id = environment->get_string_name(module_id_value.As<v8::String>());
342346
jsb_check(environment->get_module_cache().find(script_module_id));
343347
JSB_LOG(Verbose, "(newbind) constructing %s(%s) which extends %s(%d) from script",
344348
environment->get_script_class(environment->get_module_cache().find(script_module_id)->script_class_id)->js_class_name, script_module_id,
345349
class_name, class_id);
346-
const v8::Local<v8::Object> self = info.This();
347350
ScriptClassInfo::instantiate(environment, script_module_id, self);
348351
return;
349352
}

scripts/jsb.runtime/src/godot.annotations.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,8 @@ export function createClassBinder(): ClassBinder {
944944
throw new Error("The createClassBinder() requires modern decorator support. Disable legacy decorators (experimentalDecorators) in your tsconfig.json");
945945
}
946946

947+
context = proxy.proxy_unwrap_value(context);
948+
947949
const name = context.name;
948950

949951
if (typeof name !== "string") {
@@ -954,18 +956,18 @@ export function createClassBinder(): ClassBinder {
954956

955957
if (context.kind === "accessor") {
956958
return {
957-
get: jsb.internal.create_script_signal_getter(name),
959+
get: proxy.proxy_unwrap_value(jsb.internal.create_script_signal_getter(name)),
958960
set: () => {
959961
throw new Error(`Signal properties cannot be reassigned. Did you mean to .connect() a callback instead?`);
960962
},
961963
} satisfies ClassMemberDecoratorReturn<ClassAccessorDecoratorContext<Godot.Object, Godot.Signal>> as any;
962964
} else if (context.kind === "field") {
963965
context.addInitializer(function (this: Godot.Object) {
964-
context.access.set(this, jsb.internal.create_script_signal_getter(name).call(this));
966+
context.access.set(this, proxy.proxy_unwrap_value(jsb.internal.create_script_signal_getter(name)).call(this));
965967
});
966968
return undefined as any;
967969
} else if (context.kind === "getter") {
968-
return jsb.internal.create_script_signal_getter(name) satisfies
970+
return proxy.proxy_unwrap_value(jsb.internal.create_script_signal_getter(name)) satisfies
969971
ClassMemberDecoratorReturn<ClassGetterDecoratorContext<Godot.Object, Godot.Signal>> as any;
970972
} else {
971973
throw new Error(`The signal decorator can not be used to decorate a ${(context as ClassMemberDecoratorContext).kind}. A \`readonly\` field is recommended.`);

0 commit comments

Comments
 (0)