Merge pull request #542 from godot-rust/qol/better-double

Make `double-precision` feature more robust

Author: Bromeon

godot-codegen/src/central_generator.rs

@@ -463,7 +463,7 @@ fn make_method_table(info: IndexedMethodTable) -> TokenStream {
             pub fn load() -> Self {
                 // SAFETY: interface and lifecycle tables are initialized at this point, so we can get 'static references to them.
                 let (interface, lifecycle_table) = unsafe {
-                    (crate::get_interface(), crate::method_table())
+                    (crate::get_interface(), crate::builtin_lifecycle_api())
                 };
 
                 Self {

godot-codegen/src/interface_generator.rs

@@ -77,10 +77,10 @@ fn generate_proc_address_funcs(h_path: &Path) -> TokenStream {
 
     // Do not derive Copy -- even though the struct is bitwise-copyable, this is rarely needed and may point to an error.
     let code = quote! {
-        #[path = "../compat/compat_4_1.rs"]
-        mod compat_4_1;
+        #[path = "../compat/compat_4_1plus.rs"]
+        mod compat_4_1plus;
 
-        pub use compat_4_1::InitCompat;
+        pub use compat_4_1plus::InitCompat;
 
         pub struct GDExtensionInterface {
             #( #fptr_decls )*

godot-codegen/src/special_cases.rs

@@ -150,6 +150,13 @@ fn is_class_experimental(class_name: &TyName) -> bool {
 /// Whether a method is available in the method table as a named accessor.
 #[rustfmt::skip]
 pub(crate) fn is_named_accessor_in_table(class_or_builtin_ty: &TyName, godot_method_name: &str) -> bool {
+    // Hand-selected APIs.
+    match (class_or_builtin_ty.godot_ty.as_str(), godot_method_name) {
+        | ("OS", "has_feature")
+
+        => return true, _ => {}
+    }
+
     // Generated methods made private are typically needed internally and exposed with a different API,
     // so make them accessible.
     is_method_private(class_or_builtin_ty, godot_method_name)

godot-core/src/engine/gfile.rs

@@ -397,10 +397,24 @@ impl GFile {
     /// See [`real`][type@real] type for more information.
     ///
     /// Underlying Godot method:
-    /// [`FileAccess::get_real`](https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#class-fileaccess-method-get-real).
+    /// [`FileAccess::get_float`](https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#class-fileaccess-method-get-float) or
+    /// [`FileAccess::get_double`](https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#class-fileaccess-method-get-double)
+    /// (note that [`FileAccess::get_real`](https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#class-fileaccess-method-get-real)
+    /// does not return an actual `real`).
+    ///
+    /// <div class="warning">
+    /// <strong>Warning:</strong>
+    /// Since this involves a configuration-dependent type, you may not be able to read the value back if Godot uses different precision setting
+    /// (single or double) than the one used to write the value.
+    /// </div>
     #[doc(alias = "get_real")]
     pub fn read_real(&mut self) -> IoResult<real> {
-        let val = self.fa.get_real();
+        #[cfg(feature = "double-precision")]
+        let val = self.fa.get_double();
+
+        #[cfg(not(feature = "double-precision"))]
+        let val = self.fa.get_float();
+
         self.check_error()?;
         Ok(val)
     }
@@ -410,7 +424,7 @@ impl GFile {
     /// If `allow_objects` is set to `true`, objects will be decoded.
     ///
     /// <div class="warning">
-    /// **Warning:** Deserialized objects can contain code which gets executed. Do not use this option if the serialized object
+    /// <strong>Warning:</strong> Deserialized objects can contain code which gets executed. Do not use this option if the serialized object
     /// comes from untrusted sources, to avoid potential security threats such as remote code execution.
     /// </div>
     ///
@@ -500,10 +514,26 @@ impl GFile {
     /// See [`real`][type@real] type for more information.
     ///
     /// Underlying Godot method:
-    /// [`FileAccess::store_real`](https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#class-fileaccess-method-store-real).
+    /// [`FileAccess::store_float`](https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#class-fileaccess-method-store-float) or
+    /// [`FileAccess::store_double`](https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#class-fileaccess-method-store-double)
+    /// (note that [`FileAccess::store_real`](https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#class-fileaccess-method-store-real)
+    /// does not accept an actual `real`).
+    ///
+    /// <div class="warning">
+    /// <strong>Warning:</strong>
+    /// Since this involves a configuration-dependent type, you may not be able to read the value back if Godot uses different precision setting
+    /// (single or double) than the one used to write the value.
+    /// </div>
     #[doc(alias = "store_real")]
     pub fn write_real(&mut self, value: real) -> IoResult<()> {
-        self.fa.store_real(value);
+        // FileAccess::store_real() does not accept an actual real_t; work around this.
+
+        #[cfg(feature = "double-precision")]
+        self.fa.store_double(value);
+
+        #[cfg(not(feature = "double-precision"))]
+        self.fa.store_float(value);
+
         self.clear_file_length();
         self.check_error()?;
         Ok(())

godot-core/src/init/mod.rs

@@ -8,6 +8,10 @@
 use godot_ffi as sys;
 
 use std::cell;
+use sys::GodotFfi;
+
+use crate::builtin::{GString, StringName};
+use crate::out;
 
 pub use sys::GdextBuild;
 
@@ -95,6 +99,7 @@ fn gdext_on_level_init(level: InitLevel) {
             }
             InitLevel::Scene => {
                 sys::load_class_method_table(sys::ClassApiLevel::Scene);
+                ensure_godot_features_compatible();
             }
             InitLevel::Editor => {
                 sys::load_class_method_table(sys::ClassApiLevel::Editor);
@@ -244,3 +249,52 @@ impl InitLevel {
         }
     }
 }
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+
+fn ensure_godot_features_compatible() {
+    // The reason why we don't simply call Os::has_feature() here is that we might move the high-level engine classes out of godot-core
+    // later, and godot-core would only depend on godot-sys. This makes future migrations easier. We still have access to builtins though.
+
+    out!("Check Godot precision setting...");
+
+    let os_class = StringName::from("OS");
+    let single = GString::from("single");
+    let double = GString::from("double");
+
+    // SAFETY: main thread, after initialize(), valid string pointers.
+    let gdext_is_double = cfg!(feature = "double-precision");
+    let godot_is_double = unsafe {
+        let is_single = sys::godot_has_feature(os_class.string_sys(), single.sys_const());
+        let is_double = sys::godot_has_feature(os_class.string_sys(), double.sys_const());
+
+        assert_ne!(
+            is_single, is_double,
+            "Godot has invalid configuration: single={is_single}, double={is_double}"
+        );
+
+        is_double
+    };
+
+    let s = |is_double: bool| -> &'static str {
+        if is_double {
+            "double"
+        } else {
+            "single"
+        }
+    };
+
+    out!(
+        "Is double precision: Godot={}, gdext={}",
+        s(godot_is_double),
+        s(gdext_is_double)
+    );
+
+    if godot_is_double != gdext_is_double {
+        panic!(
+            "Godot runs with {} precision, but gdext was compiled with {} precision.\n\
+            Cargo feature `double-precision` must be used if and only if Godot is compiled with `precision=double`.\n",
+            s(godot_is_double), s(gdext_is_double),
+        );
+    }
+}

godot-ffi/src/compat/compat_4_1.rs renamed to godot-ffi/src/compat/compat_4_1plus.rs

@@ -32,7 +32,7 @@ pub(crate) mod gen {
 }
 
 mod compat;
-mod gdextension_plus;
+mod extras;
 mod godot_ffi;
 mod opaque;
 mod plugins;
@@ -41,7 +41,6 @@ mod toolbox;
 
 use compat::BindingCompat;
 use std::cell;
-use std::ffi::CStr;
 
 // See https://github.com/dtolnay/paste/issues/69#issuecomment-962418430
 // and https://users.rust-lang.org/t/proc-macros-using-third-party-crate/42465/4
@@ -66,7 +65,7 @@ pub use gen::table_servers_classes::*;
 pub use gen::table_utilities::*;
 
 // Other
-pub use gdextension_plus::*;
+pub use extras::*;
 pub use gen::central::*;
 pub use gen::gdextension_interface::*;
 pub use gen::interface::*;
@@ -188,12 +187,14 @@ pub unsafe fn initialize(
         out!("Loaded builtin method table (lazily).");
     }
 
-    println!(
-        "Initialize GDExtension API for Rust: {}",
-        CStr::from_ptr(version.string)
-            .to_str()
-            .expect("unknown Godot version")
-    );
+    print_preamble(version);
+}
+
+fn print_preamble(version: GDExtensionGodotVersion) {
+    let api_version: &'static str = GdextBuild::godot_static_version_string();
+    let runtime_version = read_version_string(&version);
+
+    println!("Initialize godot-rust (API {api_version}, runtime {runtime_version})");
 }
 
 /// # Safety
@@ -223,7 +224,7 @@ pub unsafe fn get_library() -> GDExtensionClassLibraryPtr {
 ///
 /// The interface must have been initialised with [`initialize`] before calling this function.
 #[inline(always)]
-pub unsafe fn method_table() -> &'static BuiltinLifecycleTable {
+pub unsafe fn builtin_lifecycle_api() -> &'static BuiltinLifecycleTable {
     &unwrap_ref_unchecked(&BINDING).global_method_table
 }
 
@@ -291,7 +292,7 @@ pub unsafe fn utility_function_table() -> &'static UtilityFunctionTable {
 /// # Safety
 ///
 /// The interface must have been initialised with [`initialize`] before calling this function.
-#[inline(always)]
+#[inline]
 pub unsafe fn load_class_method_table(api_level: ClassApiLevel) {
     let binding = unwrap_ref_unchecked_mut(&mut BINDING);
 
@@ -360,6 +361,35 @@ pub unsafe fn load_class_method_table(api_level: ClassApiLevel) {
     );
 }
 
+/// # Safety
+///
+/// Must be accessed from the main thread, and the interface must have been initialized.
+/// `tag_string` must be a valid type pointer of a `String` instance.
+#[inline]
+pub unsafe fn godot_has_feature(
+    os_class_sname: GDExtensionConstStringNamePtr,
+    tag_string: GDExtensionConstTypePtr,
+) -> bool {
+    // Issue a raw C call to OS.has_feature(tag_string).
+
+    let method_bind = class_scene_api().os__has_feature();
+    let get_singleton = get_interface().global_get_singleton.unwrap();
+    let class_ptrcall = get_interface().object_method_bind_ptrcall.unwrap();
+
+    let object_ptr = get_singleton(os_class_sname);
+    let mut return_ptr = false;
+    let type_ptrs = [tag_string];
+
+    class_ptrcall(
+        method_bind,
+        object_ptr,
+        type_ptrs.as_ptr(),
+        return_ptr.sys_mut(),
+    );
+
+    return_ptr
+}
+
 /// # Safety
 ///
 /// Must be accessed from the main thread, and the interface must have been initialized.
@@ -383,15 +413,15 @@ pub unsafe fn config() -> &'static GdextConfig {
 #[doc(hidden)]
 macro_rules! builtin_fn {
     ($name:ident $(@1)?) => {
-        $crate::method_table().$name
+        $crate::builtin_lifecycle_api().$name
     };
 }
 
 #[macro_export]
 #[doc(hidden)]
 macro_rules! builtin_call {
         ($name:ident ( $($args:expr),* $(,)? )) => {
-            ($crate::method_table().$name)( $($args),* )
+            ($crate::builtin_lifecycle_api().$name)( $($args),* )
         };
     }
 

godot-ffi/src/gdextension_plus.rs renamed to godot-ffi/src/extras.rs

@@ -304,4 +304,18 @@ pub(crate) fn load_utility_function(
     })
 }
 
+pub(crate) fn read_version_string(version_ptr: &sys::GDExtensionGodotVersion) -> String {
+    let char_ptr = version_ptr.string;
+
+    // SAFETY: GDExtensionGodotVersion has the (manually upheld) invariant of a valid string field.
+    let c_str = unsafe { std::ffi::CStr::from_ptr(char_ptr) };
+
+    let full_version = c_str.to_str().unwrap_or("(invalid UTF-8 in version)");
+
+    full_version
+        .strip_prefix("Godot Engine ")
+        .unwrap_or(full_version)
+        .to_string()
+}
+
 const INFO: &str = "\nMake sure gdext and Godot are compatible: https://godot-rust.github.io/book/gdext/advanced/compatibility.html";

godot-ffi/src/lib.rs

@@ -188,6 +188,10 @@ compile_error!("Thread safety for lazy function pointers is not yet implemented.
 #[cfg(all(target_family = "wasm", not(feature = "experimental-wasm")))]
 compile_error!("Must opt-in using `experimental-wasm` Cargo feature; keep in mind that this is work in progress");
 
+// See also https://github.com/godotengine/godot/issues/86346.
+#[cfg(all(feature = "double-precision", not(feature = "custom-godot")))]
+compile_error!("The feature `double-precision` currently requires `custom-godot` due to incompatibilities in the GDExtension API JSON.");
+
 pub mod init {
     pub use godot_core::init::*;