Add classdb_construct_object3 and classdb_register_extension_class6 (refcount-aware inits)#118214
Conversation
d5d7917 to
239d0a5
Compare
dsnopek
left a comment
There was a problem hiding this comment.
Thanks! I'm super excited to get this into GDExtension :-)
ae8a530 to
7602c4c
Compare
…6`, which are refcount-aware initialization functions (establishing `RefCounted` objects with a refcount of 1).
7602c4c to
c9279c1
Compare
| { | ||
| "name": "GDExtensionClassCreationInfo6", | ||
| "kind": "alias", | ||
| "type": "GDExtensionClassCreationInfo5" | ||
| }, |
There was a problem hiding this comment.
Doesn't make any difference – but should it really be set as an alias for GDExtensionClassCreationInfo4?
GDExtensionClassCreationInfo4 defines create_instance_func as:
{
"name": "create_instance_func",
"type": "GDExtensionClassCreateInstance2",
"description": [
"(Default) constructor; mandatory. If the class is not instantiable, consider making it virtual or abstract."
]
},
while GDExtensionClassCreationInfo6 uses GDExtensionClassCreateInstance3.
As I said it makes no difference - same signature, different semantics, you set it accordingly ( extension->gdextension.create_instance3 = p_extension_funcs->create_instance_func;) if caller uses classdb_register_extension_class6, so effectively it is a nitpick
|
I adjusted rust bindings to this branch (https://github.com/Yarwin/gdext/tree/feature/support-new-initialization, WIP ofc) and ran it against simple project: rust part#[derive(GodotClass)]
#[class(base = RefCounted)]
pub struct MyClass {
base: Base<RefCounted>,
}
#[godot_api]
impl IRefCounted for MyClass {
fn init(base: Base<Self::Base>) -> Self {
godot_print!("henlo from myclass constructor!");
let me = Self { base };
// Cloning the base increases the refcount and drops it afterwards.
// Note that it is not possible with safeguards enabled on current master; we prohibit such operation.
godot_print!("I am: {}", me.base().clone());
me
}
}Gdscript partextends Node2D
var weak: WeakRef
var other_weak: WeakRef
func other_fn():
print("some time passed!")
print("my class weak: ", weak.get_ref())
print("other class weak: ", other_weak.get_ref())
pass
func _ready() -> void:
var other_refcounted = RefCounted.new()
self.other_weak = weakref(other_refcounted)
var my_class = MyClass.new()
self.weak = weakref(my_class)
get_tree().create_timer(0.5).timeout.connect(other_fn)Godot 4.6: panic/(UB without safeguards) after print ("I am ..."). This branch + gdext with Additionally I ran current version of godot-rust test suite against Godot built from this branch (godot-rust (API v4.6.stable.official, runtime v4.7.dev.custom_build, safeguards strict)) and everything seems to be fine 👍 (in other words - we are probably good on backward compatibility front). Finally I ran full adjusted godot-rust test suite against this branch ((API v4.7.dev.custom_build, runtime v4.7.dev.custom_build, safeguards strict)) and it works as well (well, after addressing one UB due to missing type check 😁). |
dsnopek
left a comment
There was a problem hiding this comment.
Thanks! This looks good to me
|
Thanks! |
Since godotengine#118214 / 8f7bdc1, ObjectGDExtension only has `create_instance3` when building with `deprecated=no`. So we can only assign `create_instance2` when `deprecated=yes` Also noticed: A comparison checking for `create_instance2` against null twice, where I am pretty sure the intent was to check for *any* of the create_instance methods.
Since godotengine#118214 / 8f7bdc1, ObjectGDExtension has ONLY `create_instance3` when building with `deprecated=no`. So we should only assign `create_instance2` when `deprecated=yes` Also noticed: A comparison checking for `create_instance2` against null twice, where I am pretty sure the intent was to check for *any* of the create_instance methods.
memnew(RefCounted)returnRef, to improve ownership safety #111965Brings the gdextension interface on the same level as core, by offering functions that create objects with a refcount already established (and expecting callers to assume ownership).
This can be used to address #111075, and may in the future lead to the ability to remove the
refcount_inithack, which is a significant performance cost.Hoping to establish this new interface before a 4.7 release, such that gdextensions can start adopting it already.