Merge pull request #1664 from godot-rust/qol/validate-registrations

Validate class registrations against `ClassDB`

Author: Bromeon

godot-core/src/init/mod.rs

@@ -125,6 +125,9 @@ unsafe extern "C" fn startup_func<E: ExtensionLibrary>() {
 
     // Now that editor UI is ready, display all warnings/error collected so far.
     sys::print_deferred_startup_messages();
+
+    // If any of the errors were fatal, exit here.
+    abort_if_startup_fatal();
 }
 
 #[cfg(since_api = "4.5")]
@@ -345,6 +348,49 @@ unsafe fn gdext_on_level_init(level: InitLevel, _userdata: &InitUserData) {
     CURRENT_INIT_LEVEL.store(Some(level));
 }
 
+/// Aborts the process if a fatal startup error was collected (depending on runtime target).
+///
+/// Behavior:
+/// - An _interactive_ editor is not aborted, so the developer can read the errors and hot-reload a fix.
+/// - A _headless_ editor (CI, `--export-release`, `--import`) has no one to read them and aborts like a game run.
+/// - A _game_ runs `SceneTree::quit()` (frames may still run with the broken class set, possibly causing follow-up errors)
+/// - A game without a `SceneTree` (e.g. a custom `MainLoop`) calls `process::exit()`.
+#[cfg(since_api = "4.5")]
+fn abort_if_startup_fatal() {
+    if !sys::take_startup_fatal() {
+        return;
+    }
+
+    // Interactive editor: keep running, the developer can read the errors and hot-reload a fix.
+    if is_editor_hint() && !is_headless() {
+        return;
+    }
+
+    // Exit code within Godot's recommended 0..=125 range for `SceneTree::quit()`.
+    let exit_code = 111;
+    if let Some(main_loop) = classes::Engine::singleton().get_main_loop()
+        && let Ok(mut scene_tree) = main_loop.try_cast::<classes::SceneTree>()
+    {
+        scene_tree.quit_ex().exit_code(exit_code).done();
+    } else {
+        std::process::exit(exit_code);
+    }
+}
+
+/// Whether Godot runs without a visual display -- `--headless`, `--display-driver headless`, or an export/import run, which force it.
+#[cfg(since_api = "4.5")]
+fn is_headless() -> bool {
+    // Absent singleton or a failing call -> headless. Dynamic call for minimal codegen.
+    if let Some(mut display_server) = classes::Engine::singleton().get_singleton("DisplayServer")
+        && let Ok(name) = display_server.try_call("get_name", &[])
+        && let Ok(name) = name.try_to::<GString>()
+    {
+        name == "headless"
+    } else {
+        true
+    }
+}
+
 /// Tasks needed to be done by gdext internally upon unloading an initialization level. Called after user code.
 fn gdext_on_level_deinit(level: InitLevel) {
     if level == InitLevel::Editor {

godot-core/src/private.rs

@@ -33,6 +33,7 @@ mod reexport_pub {
     pub use crate::obj::rpc::priv_re_export::*;
     pub use crate::obj::rtti::ObjectRtti;
     pub use crate::registry::callbacks;
+    pub use crate::registry::reg_validation::validate_signal;
     pub use crate::registry::shard::{
         ClassShard, DynTraitImpl, ErasedDynGd, ErasedRegisterFn, ITraitImpl, InherentImpl,
         ShardItem, Struct,

godot-core/src/registry/class.rs

@@ -16,8 +16,8 @@ use crate::meta::ClassId;
 use crate::meta::error::FromGodotError;
 use crate::obj::{DynGd, Gd, GodotClass, Singleton, cap};
 use crate::private::{ClassShard, ShardItem};
-use crate::registry::callbacks;
 use crate::registry::shard::{DynTraitImpl, ErasedRegisterFn, ITraitImpl, InherentImpl, Struct};
+use crate::registry::{callbacks, reg_validation};
 use crate::{godot_error, godot_warn, sys};
 
 /// Returns a lock to a global map of loaded classes, by initialization level.
@@ -585,14 +585,14 @@ fn fill_into<T>(dst: &mut Option<T>, src: Option<T>) -> Result<(), ()> {
 fn register_class_raw(mut info: ClassRegistrationInfo) {
     // Some metadata like dynify fns are already emptied at this point. Only consider registrations for Godot.
 
-    // First register class...
-    validate_class_constraints(&info);
-
     let class_name = info.class_name;
     let parent_class_name = info
         .parent_class_name
         .expect("class defined (parent_class_name)");
 
+    // First register class...
+    precheck_class_registration(&info, parent_class_name);
+
     // Register virtual functions -- if the user provided some via #[godot_api], take those; otherwise, use the
     // ones generated alongside #[derive(GodotClass)]. The latter can also be null, if no OnReady is provided.
     if info.godot_params.get_virtual_func.is_none() {
@@ -622,14 +622,14 @@ fn register_class_raw(mut info: ClassRegistrationInfo) {
             ptr::addr_of!(info.godot_params),
         );
 
-        // ...then see if it worked.
-        // This is necessary because the above registration does not report errors (apart from console output).
+        // ...then see if it worked. The registration above does not report errors (apart from console output), and precheck may miss something.
         let tag = interface_fn!(classdb_get_class_tag)(class_name.string_sys());
         tag.is_null()
     };
 
     // Do not panic here; otherwise lock is poisoned and the whole extension becomes unusable.
     // This can happen during hot reload if a class changes base type in an incompatible way (e.g. RefCounted -> Node).
+    // Also, the pre-validation only runs under strict safeguards, there may be registrations that Godot rejects but godot-rust doesn't catch.
     if registration_failed {
         godot_error!(
             "Failed to register class `{class_name}`; check preceding Godot stderr messages."
@@ -661,8 +661,11 @@ fn register_class_raw(mut info: ClassRegistrationInfo) {
     }
 }
 
-fn validate_class_constraints(_class: &ClassRegistrationInfo) {
-    // TODO: if we add builder API, the proc-macro checks in parse_struct_attributes() etc. should be duplicated here.
+/// Validates a class registration before it is handed to Godot, which reports errors only to stderr.
+fn precheck_class_registration(class_info: &ClassRegistrationInfo, parent_class_id: ClassId) {
+    // TODO(v0.7): if we add builder API, the proc-macro checks in parse_struct_attributes() etc. should be duplicated here.
+
+    reg_validation::validate_class(class_info.class_name, parent_class_id);
 }
 
 fn unregister_class_raw(class: LoadedClass) {

godot-core/src/registry/constant.rs

@@ -31,6 +31,8 @@ impl IntegerConstant {
     }
 
     fn register(&self, class_name: ClassId, enum_name: &StringName, is_bitfield: bool) {
+        crate::registry::reg_validation::validate_constant(class_name, &self.name);
+
         unsafe {
             interface_fn!(classdb_register_extension_class_integer_constant)(
                 sys::get_library(),

godot-core/src/registry/godot_register_wrappers.rs

@@ -84,6 +84,13 @@ fn register_var_or_export_inner(
     let getter_name = StringName::from(getter_name);
     let setter_name = StringName::from(setter_name);
 
+    crate::registry::reg_validation::validate_property(
+        class_name,
+        &info.property_name,
+        &getter_name,
+        &setter_name,
+    );
+
     let property_info_sys = info.property_sys();
 
     unsafe {

godot-core/src/registry/method.rs

@@ -153,6 +153,9 @@ impl ClassMethodInfo {
     }
 
     fn register_nonvirtual_class_method(&self, method_info_sys: sys::GDExtensionClassMethodInfo) {
+        // Only for non-virtual methods. Godot keeps virtual methods in a separate map, which isn't exposed through ClassDB.
+        crate::registry::reg_validation::validate_method(self.class_id, &self.method_name);
+
         // SAFETY: The lifetime of the data we use here is at least as long as this function's scope. So we can
         // safely call this function without issue.
         //

godot-core/src/registry/mod.rs

@@ -30,6 +30,7 @@ pub mod constant;
 pub mod info;
 pub mod method;
 pub mod property;
+pub mod reg_validation;
 pub mod shard;
 
 // RpcConfig uses MultiplayerPeer::TransferMode and MultiplayerApi::RpcMode, which are only enabled in `codegen-full` feature.