From 9f609b5e2c0683935c3992d0f1cff194ce8e0369 Mon Sep 17 00:00:00 2001 From: Yarvin Date: Mon, 19 May 2025 11:00:02 +0200 Subject: [PATCH 1/4] Document runtime class in editor requirement. --- godot-core/src/classes/class_runtime.rs | 24 ++++++++++++++++++++++++ godot-core/src/obj/raw_gd.rs | 12 +++++------- godot-macros/src/lib.rs | 4 +++- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/godot-core/src/classes/class_runtime.rs b/godot-core/src/classes/class_runtime.rs index ba487b03d..f3b48294f 100644 --- a/godot-core/src/classes/class_runtime.rs +++ b/godot-core/src/classes/class_runtime.rs @@ -102,6 +102,30 @@ pub(crate) fn ensure_object_inherits(derived: ClassName, base: ClassName, instan ) } +#[cfg(debug_assertions)] +pub(crate) fn ensure_binding_not_null(binding: sys::GDExtensionClassInstancePtr) +where + T: GodotClass + Bounds, +{ + if !binding.is_null() { + return; + } + + // Non-runtime classes can't be instantiated in the editor. + if crate::classes::Engine::singleton().is_editor_hint() { + panic!( + "Class {} -- null instance; does the class have a Godot creator function? \ + Ensure that given Class is runtime class (#[class_tool]) if it is being accessed in the editor.", + std::any::type_name::() + ) + } else { + panic!( + "Class {} -- null instance; does the class have a Godot creator function?", + std::any::type_name::() + ); + } +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Implementation of this file diff --git a/godot-core/src/obj/raw_gd.rs b/godot-core/src/obj/raw_gd.rs index 6befef0c7..2c686b267 100644 --- a/godot-core/src/obj/raw_gd.rs +++ b/godot-core/src/obj/raw_gd.rs @@ -438,15 +438,13 @@ where let callbacks = crate::storage::nop_instance_callbacks(); let token = sys::get_library() as *mut std::ffi::c_void; - let binding = interface_fn!(object_get_instance_binding)(self.obj_sys(), token, &callbacks); + let binding = interface_fn!(object_get_instance_binding)(self.obj_sys(), token, &callbacks) + as sys::GDExtensionClassInstancePtr; - debug_assert!( - !binding.is_null(), - "Class {} -- null instance; does the class have a Godot creator function?", - std::any::type_name::() - ); + #[cfg(debug_assertions)] + crate::classes::ensure_binding_not_null::(binding); - let ptr = binding as sys::GDExtensionClassInstancePtr; + let ptr = binding; self.cached_storage_ptr.set(ptr); ptr } diff --git a/godot-macros/src/lib.rs b/godot-macros/src/lib.rs index 2ac093d52..3f23d122b 100644 --- a/godot-macros/src/lib.rs +++ b/godot-macros/src/lib.rs @@ -325,7 +325,9 @@ use crate::util::{bail, ident, KvParser}; /// See [`ExtensionLibrary::editor_run_behavior()`](../init/trait.ExtensionLibrary.html#method.editor_run_behavior) /// for more information and further customization. /// -/// This is very similar to [GDScript's `@tool` feature](https://docs.godotengine.org/en/stable/tutorials/plugins/running_code_in_the_editor.html). +/// This behaves similarly to [GDScript's `@tool` feature](https://docs.godotengine.org/en/stable/tutorials/plugins/running_code_in_the_editor.html). +/// +/// **Note**: As in GDScript, the class must be marked as a `tool` to be accessible in the editor (e.g., for use by editor plugins and inspectors). /// /// ## Editor plugins /// From 3d77b0188884aa8b1e1c7180651d754b0f739186 Mon Sep 17 00:00:00 2001 From: Yarvin Date: Mon, 19 May 2025 15:41:40 +0200 Subject: [PATCH 2/4] Document runtime class in editor requirement. - Add safety note. - Make `resolve_instance_ptr` safe. Returning (and casting) null ptrs is totally safe. --- godot-core/src/classes/class_runtime.rs | 4 ++-- godot-core/src/obj/raw_gd.rs | 29 +++++++++++++++++++------ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/godot-core/src/classes/class_runtime.rs b/godot-core/src/classes/class_runtime.rs index f3b48294f..0f77323f5 100644 --- a/godot-core/src/classes/class_runtime.rs +++ b/godot-core/src/classes/class_runtime.rs @@ -111,11 +111,11 @@ where return; } - // Non-runtime classes can't be instantiated in the editor. + // Non-tool classes can't be instantiated in the editor. if crate::classes::Engine::singleton().is_editor_hint() { panic!( "Class {} -- null instance; does the class have a Godot creator function? \ - Ensure that given Class is runtime class (#[class_tool]) if it is being accessed in the editor.", + Ensure that the given class is a tool class with #[class(tool)], if it is being accessed in the editor.", std::any::type_name::() ) } else { diff --git a/godot-core/src/obj/raw_gd.rs b/godot-core/src/obj/raw_gd.rs index 2c686b267..873dbcadf 100644 --- a/godot-core/src/obj/raw_gd.rs +++ b/godot-core/src/obj/raw_gd.rs @@ -425,8 +425,19 @@ where } } - // TODO: document unsafety in this function, and double check that it actually needs to be unsafe. - unsafe fn resolve_instance_ptr(&self) -> sys::GDExtensionClassInstancePtr { + /// Retrieves and caches pointer to this class instance if `self.obj` is non-null. + /// Returns a null pointer otherwise. + /// + /// Note: The returned pointer to the GDExtensionClass instance (even when `self.obj` is non-null) + /// might still be null when: + /// - The class isn't instantiable in the current context. + /// - The instance is a placeholder (e.g., non-`tool` classes in the editor). + /// + /// However, null pointers might also occur in other, undocumented contexts. + /// + /// # Panics + /// In Debug mode, if binding is null. + fn resolve_instance_ptr(&self) -> sys::GDExtensionClassInstancePtr { if self.is_null() { return ptr::null_mut(); } @@ -437,14 +448,18 @@ where } let callbacks = crate::storage::nop_instance_callbacks(); - let token = sys::get_library() as *mut std::ffi::c_void; - let binding = interface_fn!(object_get_instance_binding)(self.obj_sys(), token, &callbacks) - as sys::GDExtensionClassInstancePtr; + // SAFETY: library is already initialized. + let token = unsafe { sys::get_library() }; + // SAFETY: ensured that `self.obj` is non-null and valid. + let binding = unsafe { + interface_fn!(object_get_instance_binding)(self.obj_sys(), token.cast(), &callbacks) + }; + + let ptr: sys::GDExtensionClassInstancePtr = binding.cast(); #[cfg(debug_assertions)] - crate::classes::ensure_binding_not_null::(binding); + crate::classes::ensure_binding_not_null::(ptr); - let ptr = binding; self.cached_storage_ptr.set(ptr); ptr } From 09c04e122438c940d7db4019b0dceeb5722c893c Mon Sep 17 00:00:00 2001 From: Yarvin Date: Mon, 19 May 2025 16:51:44 +0200 Subject: [PATCH 3/4] Document runtime class in editor requirement. - Explicitly declare type while `cast()`ing, so type inference won't try anything funny --- godot-core/src/obj/raw_gd.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/godot-core/src/obj/raw_gd.rs b/godot-core/src/obj/raw_gd.rs index 873dbcadf..8d8a105f6 100644 --- a/godot-core/src/obj/raw_gd.rs +++ b/godot-core/src/obj/raw_gd.rs @@ -448,11 +448,14 @@ where } let callbacks = crate::storage::nop_instance_callbacks(); + // SAFETY: library is already initialized. let token = unsafe { sys::get_library() }; + let token: *mut std::ffi::c_void = token.cast(); + // SAFETY: ensured that `self.obj` is non-null and valid. let binding = unsafe { - interface_fn!(object_get_instance_binding)(self.obj_sys(), token.cast(), &callbacks) + interface_fn!(object_get_instance_binding)(self.obj_sys(), token, &callbacks) }; let ptr: sys::GDExtensionClassInstancePtr = binding.cast(); From 0ec83e949248ce3713ffddb79be56d6b8a375411 Mon Sep 17 00:00:00 2001 From: Yarvin Date: Tue, 20 May 2025 10:52:36 +0200 Subject: [PATCH 4/4] Document runtime class in editor requirement. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Code tweaks – improve readability when specifying a type we cast our pointer to --- godot-core/src/obj/raw_gd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/godot-core/src/obj/raw_gd.rs b/godot-core/src/obj/raw_gd.rs index 8d8a105f6..8a7e8930b 100644 --- a/godot-core/src/obj/raw_gd.rs +++ b/godot-core/src/obj/raw_gd.rs @@ -451,7 +451,7 @@ where // SAFETY: library is already initialized. let token = unsafe { sys::get_library() }; - let token: *mut std::ffi::c_void = token.cast(); + let token = token.cast::(); // SAFETY: ensured that `self.obj` is non-null and valid. let binding = unsafe {