Merge pull request #1662 from godot-rust/mtnc/callback-names

Name virtual-callback layers after user-facing `I*` virtuals

Author: Bromeon

godot-core/src/obj/traits.rs

@@ -830,6 +830,8 @@ where
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 /// Capability traits, providing dedicated functionalities for Godot classes
+///
+/// The `__godot_*` methods are named after the user-facing `I*` virtual; see [`crate::registry`] for the naming rule across all layers.
 pub mod cap {
     use std::any::Any;
 
@@ -904,7 +906,7 @@ pub mod cap {
     #[doc(hidden)]
     pub trait GodotNotification: GodotClass {
         #[doc(hidden)]
-        fn __godot_notification(&mut self, what: i32);
+        fn __godot_on_notification(&mut self, what: i32);
     }
 
     // TODO Evaluate whether we want this public or not
@@ -920,7 +922,7 @@ pub mod cap {
         type Recv: IntoVirtualMethodReceiver<Self>;
 
         #[doc(hidden)]
-        fn __godot_get_property(
+        fn __godot_on_get(
             this: VirtualMethodReceiver<Self>,
             property: StringName,
         ) -> Option<Variant>;
@@ -932,7 +934,7 @@ pub mod cap {
         type Recv: IntoVirtualMethodReceiver<Self>;
 
         #[doc(hidden)]
-        fn __godot_set_property(
+        fn __godot_on_set(
             this: VirtualMethodReceiver<Self>,
             property: StringName,
             value: Variant,
@@ -945,7 +947,7 @@ pub mod cap {
         type Recv: IntoVirtualMethodReceiver<Self>;
 
         #[doc(hidden)]
-        fn __godot_get_property_list(
+        fn __godot_on_get_property_list(
             this: VirtualMethodReceiver<Self>,
         ) -> Vec<crate::registry::info::PropertyInfo>;
     }
@@ -956,7 +958,7 @@ pub mod cap {
         type Recv: IntoVirtualMethodReceiver<Self>;
 
         #[doc(hidden)]
-        fn __godot_property_get_revert(
+        fn __godot_on_property_get_revert(
             this: VirtualMethodReceiver<Self>,
             property: StringName,
         ) -> Option<Variant>;
@@ -968,7 +970,7 @@ pub mod cap {
         type Recv: IntoVirtualMethodReceiver<Self>;
 
         #[doc(hidden)]
-        fn __godot_validate_property(
+        fn __godot_on_validate_property(
             this: VirtualMethodReceiver<Self>,
             property: &mut PropertyInfo,
         );

godot-core/src/registry/callbacks.rs

@@ -27,7 +27,7 @@ use crate::storage::{InstanceStorage, Storage, StorageRefCounted, as_storage};
 
 /// Invokes `code` -- a callback that calls into user code -- and catches any panic, so it does not unwind across the FFI boundary.
 ///
-/// `method` names the callback in the error context, e.g. `"to_string"` is reported as `MyClass::to_string()`.
+/// `method` is the user-facing `I*` virtual name, not the Godot slot name -- `"on_get"` is reported as `MyClass::on_get()`.
 /// The caller decides how to degrade on `Err`, since each Godot callback has its own failure representation.
 fn handle_method_panic<T: GodotClass, R>(
     method: &str,
@@ -301,24 +301,24 @@ pub unsafe extern "C" fn to_string<T: cap::GodotToString>(
 }
 
 #[expect(unsafe_op_in_unsafe_fn)] // Pointer validity asserted by Godot.
-pub unsafe extern "C" fn on_notification<T: cap::GodotNotification>(
+pub unsafe extern "C" fn notification<T: cap::GodotNotification>(
     instance: sys::GDExtensionClassInstancePtr,
     what: i32,
     _reversed: sys::GDExtensionBool,
 ) {
-    // `get_mut()` can also panic on borrow conflicts, in addition to `__godot_notification` itself.
+    // `get_mut()` can also panic on borrow conflicts, in addition to `__godot_on_notification` itself.
     let code = || {
         let storage = as_storage::<T>(instance);
         let mut instance = storage.get_mut();
 
-        T::__godot_notification(&mut *instance, what);
+        T::__godot_on_notification(&mut *instance, what);
     };
 
     let _ = handle_method_panic::<T, _>("on_notification", code);
 }
 
 #[expect(unsafe_op_in_unsafe_fn)] // Pointer validity asserted by Godot.
-pub unsafe extern "C" fn get_property<T: cap::GodotGet>(
+pub unsafe extern "C" fn get<T: cap::GodotGet>(
     instance: sys::GDExtensionClassInstancePtr,
     name: sys::GDExtensionConstStringNamePtr,
     ret: sys::GDExtensionVariantPtr,
@@ -328,7 +328,7 @@ pub unsafe extern "C" fn get_property<T: cap::GodotGet>(
         let instance = T::Recv::instance(storage);
         let property = StringName::new_from_string_sys(name);
 
-        match T::__godot_get_property(instance, property) {
+        match T::__godot_on_get(instance, property) {
             Some(value) => {
                 value.move_into_var_ptr(ret);
                 true
@@ -337,11 +337,11 @@ pub unsafe extern "C" fn get_property<T: cap::GodotGet>(
         }
     };
 
-    handle_method_panic_bool::<T>("get_property", code)
+    handle_method_panic_bool::<T>("on_get", code)
 }
 
 #[expect(unsafe_op_in_unsafe_fn)] // Pointer validity asserted by Godot.
-pub unsafe extern "C" fn set_property<T: cap::GodotSet>(
+pub unsafe extern "C" fn set<T: cap::GodotSet>(
     instance: sys::GDExtensionClassInstancePtr,
     name: sys::GDExtensionConstStringNamePtr,
     value: sys::GDExtensionConstVariantPtr,
@@ -353,10 +353,10 @@ pub unsafe extern "C" fn set_property<T: cap::GodotSet>(
         let property = StringName::new_from_string_sys(name);
         let value = Variant::new_from_var_sys(value);
 
-        T::__godot_set_property(instance, property, value)
+        T::__godot_on_set(instance, property, value)
     };
 
-    handle_method_panic_bool::<T>("set_property", code)
+    handle_method_panic_bool::<T>("on_set", code)
 }
 
 pub unsafe extern "C" fn reference<T: GodotClass>(instance: sys::GDExtensionClassInstancePtr) {
@@ -381,7 +381,7 @@ pub unsafe extern "C" fn get_property_list<T: cap::GodotGetPropertyList>(
         let storage = as_storage::<T>(instance);
         let instance = T::Recv::instance(storage);
 
-        let property_list = T::__godot_get_property_list(instance);
+        let property_list = T::__godot_on_get_property_list(instance);
         let property_list_sys: Box<[sys::GDExtensionPropertyInfo]> = property_list
             .into_iter()
             .map(|prop| prop.into_owned_property_sys())
@@ -397,7 +397,7 @@ pub unsafe extern "C" fn get_property_list<T: cap::GodotGetPropertyList>(
         Box::leak(property_list_sys).as_mut_ptr().cast_const()
     };
 
-    handle_method_panic::<T, _>("get_property_list", code).unwrap_or_else(|_| {
+    handle_method_panic::<T, _>("on_get_property_list", code).unwrap_or_else(|_| {
         // On panic, report an empty list -- `count` comes uninitialized, so it must be set in any case.
         *count = 0;
 
@@ -453,7 +453,7 @@ unsafe fn raw_property_get_revert<T: cap::GodotPropertyGetRevert>(
     let instance = T::Recv::instance(storage);
 
     let property = StringName::borrow_string_sys(property_name);
-    T::__godot_property_get_revert(instance, property.clone())
+    T::__godot_on_property_get_revert(instance, property.clone())
 }
 
 /// # Safety
@@ -466,7 +466,7 @@ pub unsafe extern "C" fn property_can_revert<T: cap::GodotPropertyGetRevert>(
 ) -> sys::GDExtensionBool {
     let code = || raw_property_get_revert::<T>(instance, property_name).is_some();
 
-    handle_method_panic_bool::<T>("property_can_revert", code)
+    handle_method_panic_bool::<T>("on_property_get_revert", code)
 }
 
 /// # Safety
@@ -487,7 +487,7 @@ pub unsafe extern "C" fn property_get_revert<T: cap::GodotPropertyGetRevert>(
         true
     };
 
-    handle_method_panic_bool::<T>("property_get_revert", code)
+    handle_method_panic_bool::<T>("on_property_get_revert", code)
 }
 
 /// Callback for `validate_property`.
@@ -509,14 +509,14 @@ pub unsafe extern "C" fn validate_property<T: cap::GodotValidateProperty>(
         let instance = T::Recv::instance(storage);
 
         let mut property_info = PropertyInfo::new_from_sys(property_info_ptr);
-        T::__godot_validate_property(instance, &mut property_info);
+        T::__godot_on_validate_property(instance, &mut property_info);
 
         // `property_info_ptr` remains valid and unchanged by the user callback.
         property_info.move_into_property_info_ptr(property_info_ptr);
         true
     };
 
-    handle_method_panic_bool::<T>("validate_property", code)
+    handle_method_panic_bool::<T>("on_validate_property", code)
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------

godot-core/src/registry/class.rs

@@ -165,7 +165,7 @@ pub(crate) fn register_class<
 
     let godot_params = GodotCreationInfo {
         to_string_func: Some(callbacks::to_string::<T>),
-        notification_func: Some(callbacks::on_notification::<T>),
+        notification_func: Some(callbacks::notification::<T>),
         reference_func: Some(callbacks::reference::<T>),
         unreference_func: Some(callbacks::unreference::<T>),
         create_instance_func: Some(callbacks::create::<T>),
@@ -520,7 +520,7 @@ fn fill_class_info(item: ShardItem, c: &mut ClassRegistrationInfo) {
             user_create_fn,
             user_recreate_fn,
             user_to_string_fn,
-            user_on_notification_fn,
+            user_notification_fn,
             user_set_fn,
             user_get_fn,
             get_virtual_fn,
@@ -542,7 +542,7 @@ fn fill_class_info(item: ShardItem, c: &mut ClassRegistrationInfo) {
                 .expect("duplicate: recreate_instance_func (i)");
 
             c.godot_params.to_string_func = user_to_string_fn;
-            c.godot_params.notification_func = user_on_notification_fn;
+            c.godot_params.notification_func = user_notification_fn;
             c.godot_params.set_func = user_set_fn;
             c.godot_params.get_func = user_get_fn;
             c.godot_params.get_property_list_func = user_get_property_list_fn;

godot-core/src/registry/mod.rs

@@ -5,6 +5,22 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+//! Class registration: turns a user's `#[godot_api] impl I* for MyClass` into the callbacks Godot invokes.
+//!
+//! # Naming of lifecycle callbacks
+//! Layers godot-rust owns are named after the user-facing `I*` virtual; those 1:1 with GDExtension use Godot's slot names.
+//! Example `on_get`, from outermost to innermost:
+//!
+//! - `godot-macros`, `interface_trait_impl::TraitImpl::handle_on_get()` -- generates the code below.
+//! - `godot-macros`, `interface_trait_impl::Decls::on_get_impl` -- holds the generated `cap` trait impl.
+//! - [`shard::ITraitImpl::with_on_get()`] -- registers the callback; the sole point where the naming switches sides.
+//! - `callbacks::get()` -- the `extern "C" fn` filling `GDExtensionClassCreationInfo::get_func`, via field `ITraitImpl::user_get_fn`.
+//! - `handle_method_panic()`'s context, reported as `MyClass::on_get()` -- diagnostics, so user-facing again.
+//! - [`cap::GodotGet::__godot_on_get()`][crate::obj::cap::GodotGet] -- calls the user's `on_get()`.
+//!
+//! Both sides are needed since the mapping isn't 1:1: `on_property_get_revert` installs the `property_can_revert` and
+//! `property_get_revert` slots, `on_get_property_list` additionally `free_property_list`.
+
 // Note: final re-exports from godot-core are in lib.rs, mod register::private.
 // These are public here for simplicity, but many are not imported by the main crate.
 

godot-core/src/registry/shard.rs

@@ -362,7 +362,7 @@ pub struct ITraitImpl {
     >,
 
     /// User-defined `on_notification` function.
-    pub(crate) user_on_notification_fn: Option<
+    pub(crate) user_notification_fn: Option<
         unsafe extern "C" fn(
             p_instance: sys::GDExtensionClassInstancePtr,
             p_what: i32,
@@ -474,30 +474,27 @@ impl ITraitImpl {
         self
     }
 
-    pub fn with_string<T: GodotClass + cap::GodotToString>(mut self) -> Self {
+    pub fn with_to_string<T: GodotClass + cap::GodotToString>(mut self) -> Self {
         set(&mut self.user_to_string_fn, callbacks::to_string::<T>);
         self
     }
 
     pub fn with_on_notification<T: GodotClass + cap::GodotNotification>(mut self) -> Self {
-        set(
-            &mut self.user_on_notification_fn,
-            callbacks::on_notification::<T>,
-        );
+        set(&mut self.user_notification_fn, callbacks::notification::<T>);
         self
     }
 
-    pub fn with_get_property<T: GodotClass + cap::GodotGet>(mut self) -> Self {
-        set(&mut self.user_get_fn, callbacks::get_property::<T>);
+    pub fn with_on_get<T: GodotClass + cap::GodotGet>(mut self) -> Self {
+        set(&mut self.user_get_fn, callbacks::get::<T>);
         self
     }
 
-    pub fn with_set_property<T: GodotClass + cap::GodotSet>(mut self) -> Self {
-        set(&mut self.user_set_fn, callbacks::set_property::<T>);
+    pub fn with_on_set<T: GodotClass + cap::GodotSet>(mut self) -> Self {
+        set(&mut self.user_set_fn, callbacks::set::<T>);
         self
     }
 
-    pub fn with_get_property_list<T: GodotClass + cap::GodotGetPropertyList>(mut self) -> Self {
+    pub fn with_on_get_property_list<T: GodotClass + cap::GodotGetPropertyList>(mut self) -> Self {
         set(
             &mut self.user_get_property_list_fn,
             callbacks::get_property_list::<T>,
@@ -511,7 +508,9 @@ impl ITraitImpl {
         self
     }
 
-    pub fn with_property_get_revert<T: GodotClass + cap::GodotPropertyGetRevert>(mut self) -> Self {
+    pub fn with_on_property_get_revert<T: GodotClass + cap::GodotPropertyGetRevert>(
+        mut self,
+    ) -> Self {
         set(
             &mut self.user_property_get_revert_fn,
             callbacks::property_get_revert::<T>,
@@ -523,7 +522,7 @@ impl ITraitImpl {
         self
     }
 
-    pub fn with_validate_property<T: GodotClass + cap::GodotValidateProperty>(mut self) -> Self {
+    pub fn with_on_validate_property<T: GodotClass + cap::GodotValidateProperty>(mut self) -> Self {
         set(
             &mut self.validate_property_fn,
             callbacks::validate_property::<T>,