Merge pull request #1403 from godot-rust/bugfix/version-check

Fix `get_godot_version2` unavailable on older builds

Author: Bromeon

godot-codegen/src/generator/gdext_build_struct.rs

@@ -24,34 +24,29 @@ pub fn make_gdext_build_struct(header: &GodotApiVersion) -> TokenStream {
         pub struct GdextBuild;
 
         impl GdextBuild {
-            /// Godot version against which gdext was compiled.
+            /// Godot version against which godot-rust was compiled.
             ///
             /// Example format: `v4.0.stable.official`
             pub const fn godot_static_version_string() -> &'static str {
                 #version_string
             }
 
-            /// Godot version against which gdext was compiled, as `(major, minor, patch)` triple.
+            /// Godot version against which godot-rust was compiled, as `(major, minor, patch)` triple.
             pub const fn godot_static_version_triple() -> (u8, u8, u8) {
                 (#major, #minor, #patch)
             }
 
-            /// Version of the Godot engine which loaded gdext via GDExtension binding.
+            /// Version of the Godot engine which loaded godot-rust via GDExtension binding.
             pub fn godot_runtime_version_string() -> String {
-                unsafe {
-                    let char_ptr = crate::runtime_metadata().godot_version.string;
-                    let c_str = std::ffi::CStr::from_ptr(char_ptr);
-                    String::from_utf8_lossy(c_str.to_bytes()).to_string()
-                }
+                let rt = unsafe { crate::runtime_metadata() };
+                rt.version_string().to_string()
             }
 
-            /// Version of the Godot engine which loaded gdext via GDExtension binding, as
+            /// Version of the Godot engine which loaded godot-rust via GDExtension binding, as
             /// `(major, minor, patch)` triple.
             pub fn godot_runtime_version_triple() -> (u8, u8, u8) {
-                let version = unsafe {
-                    crate::runtime_metadata().godot_version
-                };
-                (version.major as u8, version.minor as u8, version.patch as u8)
+                let rt = unsafe { crate::runtime_metadata() };
+                rt.version_triple()
             }
 
             // Duplicates code from `before_api` in `godot-bindings/lib.rs`.

godot-ffi/src/interface_init.rs

@@ -70,9 +70,13 @@ pub fn ensure_static_runtime_compatibility(
         // SAFETY: see above.
         let minor = unsafe { data_ptr.offset(1).read() };
         if minor == 0 {
+            let data_ptr = get_proc_address as *const sys::GDExtensionGodotVersion; // Always v1 of the struct.
+
             // SAFETY: at this point it's reasonably safe to say that we are indeed dealing with that version struct; read the whole.
-            let data_ptr = get_proc_address as *const sys::GodotSysVersion;
-            let runtime_version_str = unsafe { read_version_string(&data_ptr.read()) };
+            let runtime_version_str = unsafe {
+                let data_ref = &*data_ptr;
+                read_version_string(data_ref.string)
+            };
 
             panic!(
                 "gdext was compiled against a newer Godot version: {static_version_str}\n\
@@ -99,7 +103,8 @@ pub fn ensure_static_runtime_compatibility(
     );
 
     if runtime_version < static_version {
-        let runtime_version_str = read_version_string(&runtime_version_raw);
+        // SAFETY: valid `runtime_version_raw`.
+        let runtime_version_str = unsafe { read_version_string(runtime_version_raw.string) };
 
         panic!(
             "gdext was compiled against newer Godot version: {static_version_str}\n\
@@ -114,33 +119,75 @@ pub fn ensure_static_runtime_compatibility(
 
 pub unsafe fn runtime_version(
     get_proc_address: sys::GDExtensionInterfaceGetProcAddress,
-) -> sys::GodotSysVersion {
+) -> sys::GDExtensionGodotVersion {
     let get_proc_address = get_proc_address.expect("get_proc_address unexpectedly null");
 
     runtime_version_inner(get_proc_address)
 }
 
+/// Generic helper to fetch and call a version function.
+///
+/// # Safety
+/// - `get_proc_address` must be a valid function pointer from Godot.
+/// - The function pointer associated with `fn_name` must be valid, have signature `unsafe extern "C" fn(*mut V)` and initialize
+///   the version struct.
 #[deny(unsafe_op_in_unsafe_fn)]
-unsafe fn runtime_version_inner(
+unsafe fn fetch_version<V>(
     get_proc_address: unsafe extern "C" fn(
         *const std::ffi::c_char,
     ) -> sys::GDExtensionInterfaceFunctionPtr,
-) -> sys::GodotSysVersion {
-    // SAFETY: `self.0` is a valid `get_proc_address` pointer.
-    let get_godot_version = unsafe { get_proc_address(sys::c_str(sys::GET_GODOT_VERSION_SYS_STR)) }; //.expect("get_godot_version unexpectedly null");
+    fn_name: &std::ffi::CStr,
+) -> Option<V> {
+    // SAFETY: `get_proc_address` is a valid function pointer.
+    let fn_ptr = unsafe { get_proc_address(fn_name.as_ptr()) };
+    let fn_ptr = fn_ptr?;
+
+    // SAFETY: Caller guarantees correct signature (either GDExtensionInterfaceGetGodotVersion or GDExtensionInterfaceGetGodotVersion2).
+    let caller: unsafe extern "C" fn(*mut V) = unsafe {
+        std::mem::transmute::<unsafe extern "C" fn(), unsafe extern "C" fn(*mut V)>(fn_ptr)
+    };
+
+    let mut version = std::mem::MaybeUninit::<V>::zeroed();
 
-    // SAFETY: `GDExtensionInterfaceGetGodotVersion` is an `Option` of an `unsafe extern "C"` function pointer.
-    let get_godot_version =
-        crate::unsafe_cast_fn_ptr!(get_godot_version as sys::GetGodotSysVersion);
+    // SAFETY: `caller` is a valid function pointer from Godot and must be callable.
+    unsafe { caller(version.as_mut_ptr()) };
 
-    let mut version = std::mem::MaybeUninit::<sys::GodotSysVersion>::zeroed();
+    // SAFETY: The version function initializes `version`.
+    Some(unsafe { version.assume_init() })
+}
 
-    // SAFETY: `get_proc_address` with "get_godot_version" does return a valid `GDExtensionInterfaceGetGodotVersion` pointer, and since we have a valid
-    // `get_proc_address` pointer then it must be callable.
-    unsafe { get_godot_version(version.as_mut_ptr()) };
+#[deny(unsafe_op_in_unsafe_fn)]
+unsafe fn runtime_version_inner(
+    get_proc_address: unsafe extern "C" fn(
+        *const std::ffi::c_char,
+    ) -> sys::GDExtensionInterfaceFunctionPtr,
+) -> sys::GDExtensionGodotVersion {
+    // Try get_godot_version first (available in all versions, unless Godot built with deprecated features).
+
+    // SAFETY: `get_proc_address` is valid, function has signature fn(*mut GDExtensionGodotVersion).
+    if let Some(version1) = unsafe { fetch_version(get_proc_address, c"get_godot_version") } {
+        return version1;
+    }
+
+    // Fall back to get_godot_version2 for 4.5+ builds that have removed the original function.
+    #[cfg(since_api = "4.5")]
+    {
+        // SAFETY: `get_proc_address` is valid, function has signature fn(*mut GDExtensionGodotVersion2).
+        let version2: Option<sys::GDExtensionGodotVersion2> =
+            unsafe { fetch_version(get_proc_address, c"get_godot_version2") };
+
+        if let Some(version2) = version2 {
+            // Convert to old "common denominator" struct.
+            return sys::GDExtensionGodotVersion {
+                major: version2.major,
+                minor: version2.minor,
+                patch: version2.patch,
+                string: version2.string,
+            };
+        }
+    }
 
-    // SAFETY: `get_godot_version` initializes `version`.
-    unsafe { version.assume_init() }
+    panic!("None of `get_godot_version`, `get_godot_version2` function pointers available")
 }
 
 pub unsafe fn load_interface(

godot-ffi/src/lib.rs

@@ -112,37 +112,37 @@ use binding::{
 #[cfg(not(wasm_nothreads))]
 static MAIN_THREAD_ID: ManualInitCell<std::thread::ThreadId> = ManualInitCell::new();
 
-#[cfg(before_api = "4.5")]
-mod version_symbols {
-
-    pub type GodotSysVersion = super::GDExtensionGodotVersion;
-    pub type GetGodotSysVersion = super::GDExtensionInterfaceGetGodotVersion;
-    pub const GET_GODOT_VERSION_SYS_STR: &[u8] = b"get_godot_version\0";
-}
-
-#[cfg(since_api = "4.5")]
-mod version_symbols {
-    pub type GodotSysVersion = super::GDExtensionGodotVersion2;
-
-    pub type GetGodotSysVersion = super::GDExtensionInterfaceGetGodotVersion2;
-    pub const GET_GODOT_VERSION_SYS_STR: &[u8] = b"get_godot_version2\0";
-}
-
-use version_symbols::*;
-
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 pub struct GdextRuntimeMetadata {
-    godot_version: GodotSysVersion,
+    version_string: String,
+    version_triple: (u8, u8, u8),
 }
 
 impl GdextRuntimeMetadata {
-    /// # Safety
-    ///
-    /// - The `string` field of `godot_version` must not be written to while this struct exists.
-    /// - The `string` field of `godot_version` must be safe to read from while this struct exists.
-    pub unsafe fn new(godot_version: GodotSysVersion) -> Self {
-        Self { godot_version }
+    pub fn load(sys_version: GDExtensionGodotVersion) -> Self {
+        // SAFETY: GDExtensionGodotVersion always contains valid string.
+        let version_string = unsafe { read_version_string(sys_version.string) };
+
+        let version_triple = (
+            sys_version.major as u8,
+            sys_version.minor as u8,
+            sys_version.patch as u8,
+        );
+
+        Self {
+            version_string,
+            version_triple,
+        }
+    }
+
+    // TODO(v0.5): CowStr, also in GdextBuild.
+    pub fn version_string(&self) -> &str {
+        &self.version_string
+    }
+
+    pub fn version_triple(&self) -> (u8, u8, u8) {
+        self.version_triple
     }
 }
 
@@ -164,10 +164,10 @@ pub unsafe fn initialize(
     library: GDExtensionClassLibraryPtr,
     config: GdextConfig,
 ) {
-    out!("Initialize gdext...");
+    out!("Initialize godot-rust...");
 
     out!(
-        "Godot version against which gdext was compiled: {}",
+        "Godot version against which godot-rust was compiled: {}",
         GdextBuild::godot_static_version_string()
     );
 
@@ -201,8 +201,7 @@ pub unsafe fn initialize(
         unsafe { UtilityFunctionTable::load(&interface, &mut string_names) };
     out!("Loaded utility function table.");
 
-    // SAFETY: We do not touch `version` again after passing it to `new` here.
-    let runtime_metadata = unsafe { GdextRuntimeMetadata::new(version) };
+    let runtime_metadata = GdextRuntimeMetadata::load(version);
 
     let builtin_method_table = {
         #[cfg(feature = "codegen-lazy-fptrs")]
@@ -283,9 +282,11 @@ fn safeguards_level_string() -> &'static str {
     }
 }
 
-fn print_preamble(version: GodotSysVersion) {
+fn print_preamble(version: GDExtensionGodotVersion) {
+    // SAFETY: GDExtensionGodotVersion always contains valid string.
+    let runtime_version = unsafe { read_version_string(version.string) };
+
     let api_version: &'static str = GdextBuild::godot_static_version_string();
-    let runtime_version = read_version_string(&version);
     let safeguards_level = safeguards_level_string();
     println!("Initialize godot-rust (API {api_version}, runtime {runtime_version}, safeguards {safeguards_level})");
 }
@@ -489,6 +490,7 @@ pub unsafe fn classdb_construct_object(
 ) -> GDExtensionObjectPtr {
     #[cfg(before_api = "4.4")]
     return interface_fn!(classdb_construct_object)(class_name);
+
     #[cfg(since_api = "4.4")]
     return interface_fn!(classdb_construct_object2)(class_name);
 }

godot-ffi/src/toolbox.rs

@@ -386,10 +386,14 @@ pub(crate) fn load_utility_function(
     })
 }
 
-pub(crate) fn read_version_string(version_ptr: &sys::GodotSysVersion) -> String {
-    let char_ptr = version_ptr.string;
-
-    // SAFETY: GDExtensionGodotVersion has the (manually upheld) invariant of a valid string field.
+/// Extracts the version string from a Godot version struct.
+///
+/// Works transparently with both `GDExtensionGodotVersion` and `GDExtensionGodotVersion2`.
+///
+/// # Safety
+/// The `char_ptr` must point to a valid C string.
+pub(crate) unsafe fn read_version_string(char_ptr: *const std::ffi::c_char) -> String {
+    // SAFETY: Caller guarantees the pointer is valid.
     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)");