From 5dc6915f334305499f1d3dd5bef5dd1bdfbe0696 Mon Sep 17 00:00:00 2001 From: Lili Zoey Date: Thu, 25 May 2023 23:02:04 +0200 Subject: [PATCH 1/6] Add support for conditional compilation based on gdextension api version. --- godot-bindings/src/godot_exe.rs | 6 +-- godot-bindings/src/godot_version.rs | 19 +-------- godot-bindings/src/lib.rs | 56 +++++++++++++++++++++++++++ godot-core/Cargo.toml | 1 + godot-core/build.rs | 2 + godot-core/src/builtin/basis.rs | 10 +++++ godot-core/src/builtin/transform3d.rs | 9 +++++ godot-core/src/lib.rs | 36 +++++++++++++++++ godot-ffi/build.rs | 2 + 9 files changed, 120 insertions(+), 21 deletions(-) diff --git a/godot-bindings/src/godot_exe.rs b/godot-bindings/src/godot_exe.rs index 6b2814c6d..7a4a409c6 100644 --- a/godot-bindings/src/godot_exe.rs +++ b/godot-bindings/src/godot_exe.rs @@ -6,10 +6,10 @@ //! Commands related to Godot executable -use crate::custom::godot_version::GodotVersion; use crate::godot_version::parse_godot_version; use crate::header_gen::generate_rust_binding; use crate::watch::StopWatch; +use crate::GodotVersion; use regex::Regex; use std::fs; @@ -99,7 +99,7 @@ fn update_version_file(version: &str) { } */ -fn read_godot_version(godot_bin: &Path) -> GodotVersion { +pub(crate) fn read_godot_version(godot_bin: &Path) -> GodotVersion { let output = Command::new(godot_bin) .arg("--version") .output() @@ -259,7 +259,7 @@ fn polyfill_legacy_header(c: &mut String) { ); } -fn locate_godot_binary() -> PathBuf { +pub(crate) fn locate_godot_binary() -> PathBuf { if let Ok(string) = std::env::var("GODOT4_BIN") { println!("Found GODOT4_BIN with path to executable: '{string}'"); println!("cargo:rerun-if-env-changed=GODOT4_BIN"); diff --git a/godot-bindings/src/godot_version.rs b/godot-bindings/src/godot_version.rs index f687f23b3..7abfd858b 100644 --- a/godot-bindings/src/godot_version.rs +++ b/godot-bindings/src/godot_version.rs @@ -6,28 +6,11 @@ //#![allow(unused_variables, dead_code)] +use crate::GodotVersion; use regex::{Captures, Regex}; use std::error::Error; use std::str::FromStr; -#[derive(Debug, Eq, PartialEq)] -pub struct GodotVersion { - /// the original string (trimmed, stripped of text around) - pub full_string: String, - - pub major: u8, - pub minor: u8, - - /// 0 if none - pub patch: u8, - - /// alpha|beta|dev|stable - pub status: String, - - /// Git revision 'custom_build.{rev}' or '{official}.rev', if available - pub custom_rev: Option, -} - pub fn parse_godot_version(version_str: &str) -> Result> { // Format of the string emitted by `godot --version`: // https://github.com/godot-rust/gdext/issues/118#issuecomment-1465748123 diff --git a/godot-bindings/src/lib.rs b/godot-bindings/src/lib.rs index 8cbf4a334..c7c6d6106 100644 --- a/godot-bindings/src/lib.rs +++ b/godot-bindings/src/lib.rs @@ -17,6 +17,26 @@ compile_error!( "At least one of `custom-godot` or `prebuilt-godot` must be specified (none given)." ); +// This is outside of `godot_version` to allow us to use it even when we don't have the `custom-godot` +// feature enabled. +#[derive(Debug, Eq, PartialEq)] +pub struct GodotVersion { + /// the original string (trimmed, stripped of text around) + pub full_string: String, + + pub major: u8, + pub minor: u8, + + /// 0 if none + pub patch: u8, + + /// alpha|beta|dev|stable + pub status: String, + + /// Git revision 'custom_build.{rev}' or '{official}.rev', if available + pub custom_rev: Option, +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Regenerate all files @@ -41,6 +61,10 @@ mod custom { pub fn write_gdextension_headers_from_c(h_path: &Path, rs_path: &Path, watch: &mut StopWatch) { godot_exe::write_gdextension_headers(h_path, rs_path, true, watch); } + + pub(crate) fn get_godot_version() -> GodotVersion { + godot_exe::read_godot_version(&godot_exe::locate_godot_binary()) + } } #[cfg(feature = "custom-godot")] @@ -70,6 +94,20 @@ mod prebuilt { .unwrap_or_else(|e| panic!("failed to write gdextension_interface.rs: {e}")); watch.record("write_header_rs"); } + + pub(crate) fn get_godot_version() -> GodotVersion { + let version: Vec<&str> = godot4_prebuilt::GODOT_VERSION + .split('.') + .collect::>(); + GodotVersion { + full_string: godot4_prebuilt::GODOT_VERSION.into(), + major: version[0].parse().unwrap(), + minor: version[1].parse().unwrap(), + patch: version[2].parse().unwrap(), + status: "stable".into(), + custom_rev: None, + } + } } #[cfg(not(feature = "custom-godot"))] @@ -85,3 +123,21 @@ pub fn clear_dir(dir: &Path, watch: &mut StopWatch) { } std::fs::create_dir_all(dir).unwrap_or_else(|e| panic!("failed to create dir: {e}")); } + +pub fn emit_godot_version_cfg() { + let GodotVersion { + major, + minor, + patch, + .. + } = get_godot_version(); + + println!(r#"cargo:rustc-cfg=gdextension_api_version="{major}.{minor}""#); + + // Godot drops the patch version if it is 0. + if patch != 0 { + println!(r#"cargo:rustc-cfg=gdextension_api_version_full="{major}.{minor}.{patch}""#); + } else { + println!(r#"cargo:rustc-cfg=gdextension_api_version_full="{major}.{minor}""#); + } +} diff --git a/godot-core/Cargo.toml b/godot-core/Cargo.toml index a606a6687..8c96dd95b 100644 --- a/godot-core/Cargo.toml +++ b/godot-core/Cargo.toml @@ -29,4 +29,5 @@ godot = { path = "../godot" } serde_json = "1.0" [build-dependencies] +godot-bindings = { path = "../godot-bindings" } godot-codegen = { path = "../godot-codegen" } diff --git a/godot-core/build.rs b/godot-core/build.rs index a6add2833..16c92c736 100644 --- a/godot-core/build.rs +++ b/godot-core/build.rs @@ -17,4 +17,6 @@ fn main() { godot_codegen::generate_core_files(gen_path); println!("cargo:rerun-if-changed=build.rs"); + + godot_bindings::emit_godot_version_cfg(); } diff --git a/godot-core/src/builtin/basis.rs b/godot-core/src/builtin/basis.rs index 635c6fa39..4feac3039 100644 --- a/godot-core/src/builtin/basis.rs +++ b/godot-core/src/builtin/basis.rs @@ -143,10 +143,20 @@ impl Basis { /// orthonormalized. The `target` and `up` vectors cannot be zero, and /// cannot be parallel to each other. /// + #[cfg(gdextension_api_version = "4.0")] /// _Godot equivalent: `Basis.looking_at()`_ pub fn new_looking_at(target: Vector3, up: Vector3) -> Self { super::inner::InnerBasis::looking_at(target, up) } + #[cfg(not(gdextension_api_version = "4.0"))] + /// If `use_model_front` is true, the +Z axis (asset front) is treated as forward (implies +X is left) + /// and points toward the target position. By default, the -Z axis (camera forward) is treated as forward + /// (implies +X is right). + /// + /// _Godot equivalent: `Basis.looking_at()`_ + pub fn new_looking_at(target: Vector3, up: Vector3, use_model_front: bool) -> Self { + super::inner::InnerBasis::looking_at(target, up, use_model_front) + } /// Creates a `[Vector3; 3]` with the columns of the `Basis`. pub fn to_cols(self) -> [Vector3; 3] { diff --git a/godot-core/src/builtin/transform3d.rs b/godot-core/src/builtin/transform3d.rs index ce48f0702..21cce290d 100644 --- a/godot-core/src/builtin/transform3d.rs +++ b/godot-core/src/builtin/transform3d.rs @@ -145,6 +145,7 @@ impl Transform3D { /// See [`Basis::new_looking_at()`] for more information. /// /// _Godot equivalent: Transform3D.looking_at()_ + #[cfg(gdextension_api_version = "4.0")] #[must_use] pub fn looking_at(self, target: Vector3, up: Vector3) -> Self { Self { @@ -152,6 +153,14 @@ impl Transform3D { origin: self.origin, } } + #[cfg(not(gdextension_api_version = "4.0"))] + #[must_use] + pub fn looking_at(self, target: Vector3, up: Vector3, use_model_front: bool) -> Self { + Self { + basis: Basis::new_looking_at(target - self.origin, up, use_model_front), + origin: self.origin, + } + } /// Returns the transform with the basis orthogonal (90 degrees), and /// normalized axis vectors (scale of 1 or -1). diff --git a/godot-core/src/lib.rs b/godot-core/src/lib.rs index 47e79d174..e959c81b3 100644 --- a/godot-core/src/lib.rs +++ b/godot-core/src/lib.rs @@ -148,3 +148,39 @@ pub mod private { std::io::stdout().flush().expect("flush stdout"); } } + +macro_rules! generate_gdextension_api_version { + ( + $( + $name:ident => { + $($version:literal, )* + } + ),* $(,)? + ) => { + $( + $( + #[cfg(gdextension_api_version = $version)] + #[allow(dead_code)] + const $name: &str = $version; + )* + )* + }; +} + +// If multiple gdextension_api_version's are found then this will generate several structs with the same +// name, causing a compile error. +// +// This includes all versions we're developing for, including unreleased future versions. +generate_gdextension_api_version!( + GDEXTENSION_API_VERSION_FULL => { + "4.0", + "4.0.1", + "4.0.2", + "4.0.3", + "4.1", + }, + GDEXTENSION_API_VERSION => { + "4.0", + "4.1", + }, +); diff --git a/godot-ffi/build.rs b/godot-ffi/build.rs index 4c6283111..c929796ea 100644 --- a/godot-ffi/build.rs +++ b/godot-ffi/build.rs @@ -27,4 +27,6 @@ fn main() { watch.write_stats_to(&gen_path.join("ffi-stats.txt")); println!("cargo:rerun-if-changed=build.rs"); + + godot_bindings::emit_godot_version_cfg(); } From 4b1d208a04f19c05e1e46073a938f4dbfd2eacfe Mon Sep 17 00:00:00 2001 From: Lili Zoey Date: Fri, 26 May 2023 16:58:32 +0200 Subject: [PATCH 2/6] Fix Variant::evaluate to work with both 4.0 and 4.1 Fix impl_variant_traits macro's FromVariant impl to work with both 4.0 and 4.1 --- godot-bindings/src/lib.rs | 11 +++++++---- godot-core/src/builtin/basis.rs | 4 ++-- godot-core/src/builtin/transform3d.rs | 4 ++-- godot-core/src/builtin/variant/impls.rs | 10 +++++++++- godot-core/src/builtin/variant/mod.rs | 16 ++++++++++++++++ godot-core/src/lib.rs | 8 ++++---- 6 files changed, 40 insertions(+), 13 deletions(-) diff --git a/godot-bindings/src/lib.rs b/godot-bindings/src/lib.rs index c7c6d6106..ed8493ea4 100644 --- a/godot-bindings/src/lib.rs +++ b/godot-bindings/src/lib.rs @@ -103,7 +103,10 @@ mod prebuilt { full_string: godot4_prebuilt::GODOT_VERSION.into(), major: version[0].parse().unwrap(), minor: version[1].parse().unwrap(), - patch: version[2].parse().unwrap(), + patch: version + .get(2) + .and_then(|patch| patch.parse().ok()) + .unwrap_or(0), status: "stable".into(), custom_rev: None, } @@ -132,12 +135,12 @@ pub fn emit_godot_version_cfg() { .. } = get_godot_version(); - println!(r#"cargo:rustc-cfg=gdextension_api_version="{major}.{minor}""#); + println!(r#"cargo:rustc-cfg=gdextension_api="{major}.{minor}""#); // Godot drops the patch version if it is 0. if patch != 0 { - println!(r#"cargo:rustc-cfg=gdextension_api_version_full="{major}.{minor}.{patch}""#); + println!(r#"cargo:rustc-cfg=gdextension_exact_api="{major}.{minor}.{patch}""#); } else { - println!(r#"cargo:rustc-cfg=gdextension_api_version_full="{major}.{minor}""#); + println!(r#"cargo:rustc-cfg=gdextension_exact_api="{major}.{minor}""#); } } diff --git a/godot-core/src/builtin/basis.rs b/godot-core/src/builtin/basis.rs index 4feac3039..f7f453a68 100644 --- a/godot-core/src/builtin/basis.rs +++ b/godot-core/src/builtin/basis.rs @@ -143,12 +143,12 @@ impl Basis { /// orthonormalized. The `target` and `up` vectors cannot be zero, and /// cannot be parallel to each other. /// - #[cfg(gdextension_api_version = "4.0")] + #[cfg(gdextension_api = "4.0")] /// _Godot equivalent: `Basis.looking_at()`_ pub fn new_looking_at(target: Vector3, up: Vector3) -> Self { super::inner::InnerBasis::looking_at(target, up) } - #[cfg(not(gdextension_api_version = "4.0"))] + #[cfg(not(gdextension_api = "4.0"))] /// If `use_model_front` is true, the +Z axis (asset front) is treated as forward (implies +X is left) /// and points toward the target position. By default, the -Z axis (camera forward) is treated as forward /// (implies +X is right). diff --git a/godot-core/src/builtin/transform3d.rs b/godot-core/src/builtin/transform3d.rs index 21cce290d..1ed816437 100644 --- a/godot-core/src/builtin/transform3d.rs +++ b/godot-core/src/builtin/transform3d.rs @@ -145,7 +145,7 @@ impl Transform3D { /// See [`Basis::new_looking_at()`] for more information. /// /// _Godot equivalent: Transform3D.looking_at()_ - #[cfg(gdextension_api_version = "4.0")] + #[cfg(gdextension_api = "4.0")] #[must_use] pub fn looking_at(self, target: Vector3, up: Vector3) -> Self { Self { @@ -153,7 +153,7 @@ impl Transform3D { origin: self.origin, } } - #[cfg(not(gdextension_api_version = "4.0"))] + #[cfg(not(gdextension_api = "4.0"))] #[must_use] pub fn looking_at(self, target: Vector3, up: Vector3, use_model_front: bool) -> Self { Self { diff --git a/godot-core/src/builtin/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index f108d585f..304608bbb 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -61,13 +61,21 @@ macro_rules! impl_variant_traits { return Err(VariantConversionError::BadType) } + // For 4.0: // In contrast to T -> Variant, the conversion Variant -> T assumes // that the destination is initialized (at least for some T). For example: // void String::operator=(const String &p_str) { _cowdata._ref(p_str._cowdata); } // does a copy-on-write and explodes if this->_cowdata is not initialized. // We can thus NOT use Self::from_sys_init(). + // + // This was changed in 4.1. let result = unsafe { - Self::from_sys_init(|self_ptr| { + #[cfg(gdextension_api = "4.0")] + let from_sys_init = Self::from_sys_init_default; + #[cfg(not(gdextension_api = "4.0"))] + let from_sys_init = Self::from_sys_init; + + from_sys_init(|self_ptr| { let converter = sys::builtin_fn!($to_fn); converter(self_ptr, variant.var_sys()); }) diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index ae57fc25d..b5e9c40d3 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -132,6 +132,22 @@ impl Variant { let op_sys = op.sys(); let mut is_valid = false as u8; + #[cfg(gdextension_api = "4.0")] + let result = { + #[allow(unused_mut)] + let mut result = Variant::nil(); + unsafe { + interface_fn!(variant_evaluate)( + op_sys, + self.var_sys(), + rhs.var_sys(), + result.var_sys(), + ptr::addr_of_mut!(is_valid), + ) + }; + result + }; + #[cfg(not(gdextension_api = "4.0"))] let result = unsafe { Variant::from_var_sys_init(|variant_ptr| { interface_fn!(variant_evaluate)( diff --git a/godot-core/src/lib.rs b/godot-core/src/lib.rs index e959c81b3..84215d19c 100644 --- a/godot-core/src/lib.rs +++ b/godot-core/src/lib.rs @@ -152,14 +152,14 @@ pub mod private { macro_rules! generate_gdextension_api_version { ( $( - $name:ident => { + ($name:ident, $gdextension_api:ident) => { $($version:literal, )* } ),* $(,)? ) => { $( $( - #[cfg(gdextension_api_version = $version)] + #[cfg($gdextension_api = $version)] #[allow(dead_code)] const $name: &str = $version; )* @@ -172,14 +172,14 @@ macro_rules! generate_gdextension_api_version { // // This includes all versions we're developing for, including unreleased future versions. generate_gdextension_api_version!( - GDEXTENSION_API_VERSION_FULL => { + (GDEXTENSION_API_VERSION_FULL, gdextension_exact_api) => { "4.0", "4.0.1", "4.0.2", "4.0.3", "4.1", }, - GDEXTENSION_API_VERSION => { + (GDEXTENSION_API_VERSION, gdextension_api) => { "4.0", "4.1", }, From 1c19ad00b180e0523ed59c6565a76f7763f98283 Mon Sep 17 00:00:00 2001 From: Lili Zoey Date: Sat, 27 May 2023 20:17:50 +0200 Subject: [PATCH 3/6] Ensure only types that need to be initialized are initialized, but not types that leak when initialized. --- godot-codegen/src/central_generator.rs | 9 ++++ godot-core/src/builtin/variant/impls.rs | 65 +++++++++++++++++-------- godot-core/src/lib.rs | 4 +- 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/godot-codegen/src/central_generator.rs b/godot-codegen/src/central_generator.rs index 7c1373577..11054ab06 100644 --- a/godot-codegen/src/central_generator.rs +++ b/godot-codegen/src/central_generator.rs @@ -272,6 +272,15 @@ fn make_build_config(header: &Header) -> TokenStream { String::from_utf8_lossy(c_str.to_bytes()).to_string() } } + + /// Version of the Godot engine which loaded gdext 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) + } } } } diff --git a/godot-core/src/builtin/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index 304608bbb..d9aa72003 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -26,13 +26,22 @@ macro_rules! impl_variant_metadata { } }; } - +// Certain types need to be passed as initialized pointers in their from_variant implementations in 4.0. Because +// 4.0 uses `*ptr = value` to return the type, and some types in c++ override `operator=` in c++ in a way +// that requires the pointer the be initialized. But some other types will cause a memory leak in 4.1 if +// initialized. +// +// Thus we can use `init` to indicate when it must be initialized in 4.0. macro_rules! impl_variant_traits { ($T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident) => { impl_variant_traits!(@@ $T, $from_fn, $to_fn, $variant_type;); }; - ($T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident, $param_metadata:ident) => { + ($T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident, init) => { + impl_variant_traits!(@@ $T, $from_fn, $to_fn, $variant_type, init;); + }; + + ($T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident, metadata = $param_metadata:ident) => { impl_variant_traits!(@@ $T, $from_fn, $to_fn, $variant_type; fn param_metadata() -> sys::GDExtensionClassMethodArgumentMetadata { sys::$param_metadata @@ -40,7 +49,15 @@ macro_rules! impl_variant_traits { ); }; - (@@ $T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident; $($extra:tt)*) => { + ($T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident, init, metadata = $param_metadata:ident) => { + impl_variant_traits!(@@ $T, $from_fn, $to_fn, $variant_type, init; + fn param_metadata() -> sys::GDExtensionClassMethodArgumentMetadata { + sys::$param_metadata + } + ); + }; + + (@@ $T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident $(, $init:ident)?; $($extra:tt)*) => { impl ToVariant for $T { fn to_variant(&self) -> Variant { let variant = unsafe { @@ -70,10 +87,7 @@ macro_rules! impl_variant_traits { // // This was changed in 4.1. let result = unsafe { - #[cfg(gdextension_api = "4.0")] - let from_sys_init = Self::from_sys_init_default; - #[cfg(not(gdextension_api = "4.0"))] - let from_sys_init = Self::from_sys_init; + impl_variant_traits!(@@from_sys_init, from_sys_init $(, $init)?); from_sys_init(|self_ptr| { let converter = sys::builtin_fn!($to_fn); @@ -87,6 +101,17 @@ macro_rules! impl_variant_traits { impl_variant_metadata!($T, $variant_type; $($extra)*); }; + + (@@from_sys_init, $from_sys_init:ident, init) => { + #[cfg(gdextension_api = "4.0")] + let $from_sys_init = Self::from_sys_init_default; + #[cfg(not(gdextension_api = "4.0"))] + let $from_sys_init = Self::from_sys_init; + }; + + (@@from_sys_init, $from_sys_init:ident) => { + let $from_sys_init = Self::from_sys_init; + }; } macro_rules! impl_variant_traits_int { @@ -154,7 +179,7 @@ mod impls { impl_variant_traits!(Aabb, aabb_to_variant, aabb_from_variant, Aabb); impl_variant_traits!(bool, bool_to_variant, bool_from_variant, Bool); impl_variant_traits!(Basis, basis_to_variant, basis_from_variant, Basis); - impl_variant_traits!(Callable, callable_to_variant, callable_from_variant, Callable); + impl_variant_traits!(Callable, callable_to_variant, callable_from_variant, Callable, init); impl_variant_traits!(Vector2, vector2_to_variant, vector2_from_variant, Vector2); impl_variant_traits!(Vector3, vector3_to_variant, vector3_from_variant, Vector3); impl_variant_traits!(Vector4, vector4_to_variant, vector4_from_variant, Vector4); @@ -162,20 +187,20 @@ mod impls { impl_variant_traits!(Vector3i, vector3i_to_variant, vector3i_from_variant, Vector3i); impl_variant_traits!(Quaternion, quaternion_to_variant, quaternion_from_variant, Quaternion); impl_variant_traits!(Color, color_to_variant, color_from_variant, Color); - impl_variant_traits!(GodotString, string_to_variant, string_from_variant, String); + impl_variant_traits!(GodotString, string_to_variant, string_from_variant, String, init); impl_variant_traits!(StringName, string_name_to_variant, string_name_from_variant, StringName); impl_variant_traits!(NodePath, node_path_to_variant, node_path_from_variant, NodePath); // TODO use impl_variant_traits!, as soon as `Default` is available. Also consider auto-generating. impl_variant_metadata!(Signal, /* signal_to_variant, signal_from_variant, */ Signal); - impl_variant_traits!(PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant, PackedByteArray); - impl_variant_traits!(PackedInt32Array, packed_int32_array_to_variant, packed_int32_array_from_variant, PackedInt32Array); - impl_variant_traits!(PackedInt64Array, packed_int64_array_to_variant, packed_int64_array_from_variant, PackedInt64Array); - impl_variant_traits!(PackedFloat32Array, packed_float32_array_to_variant, packed_float32_array_from_variant, PackedFloat32Array); - impl_variant_traits!(PackedFloat64Array, packed_float64_array_to_variant, packed_float64_array_from_variant, PackedFloat64Array); - impl_variant_traits!(PackedStringArray, packed_string_array_to_variant, packed_string_array_from_variant, PackedStringArray); - impl_variant_traits!(PackedVector2Array, packed_vector2_array_to_variant, packed_vector2_array_from_variant, PackedVector2Array); - impl_variant_traits!(PackedVector3Array, packed_vector3_array_to_variant, packed_vector3_array_from_variant, PackedVector3Array); - impl_variant_traits!(PackedColorArray, packed_color_array_to_variant, packed_color_array_from_variant, PackedColorArray); + impl_variant_traits!(PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant, PackedByteArray, init); + impl_variant_traits!(PackedInt32Array, packed_int32_array_to_variant, packed_int32_array_from_variant, PackedInt32Array, init); + impl_variant_traits!(PackedInt64Array, packed_int64_array_to_variant, packed_int64_array_from_variant, PackedInt64Array, init); + impl_variant_traits!(PackedFloat32Array, packed_float32_array_to_variant, packed_float32_array_from_variant, PackedFloat32Array, init); + impl_variant_traits!(PackedFloat64Array, packed_float64_array_to_variant, packed_float64_array_from_variant, PackedFloat64Array, init); + impl_variant_traits!(PackedStringArray, packed_string_array_to_variant, packed_string_array_from_variant, PackedStringArray, init); + impl_variant_traits!(PackedVector2Array, packed_vector2_array_to_variant, packed_vector2_array_from_variant, PackedVector2Array, init); + impl_variant_traits!(PackedVector3Array, packed_vector3_array_to_variant, packed_vector3_array_from_variant, PackedVector3Array, init); + impl_variant_traits!(PackedColorArray, packed_color_array_to_variant, packed_color_array_from_variant, PackedColorArray, init); impl_variant_traits!(Plane, plane_to_variant, plane_from_variant, Plane); impl_variant_traits!(Projection, projection_to_variant, projection_from_variant, Projection); impl_variant_traits!(Rid, rid_to_variant, rid_from_variant, Rid); @@ -185,7 +210,7 @@ mod impls { impl_variant_traits!(Transform3D, transform_3d_to_variant, transform_3d_from_variant, Transform3D); impl_variant_traits!(Dictionary, dictionary_to_variant, dictionary_from_variant, Dictionary); - impl_variant_traits!(i64, int_to_variant, int_from_variant, Int, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT64); + impl_variant_traits!(i64, int_to_variant, int_from_variant, Int, metadata = GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT64); impl_variant_traits_int!(i8, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT8); impl_variant_traits_int!(i16, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT16); impl_variant_traits_int!(i32, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT32); @@ -195,7 +220,7 @@ mod impls { impl_variant_traits_int!(u32, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_UINT32); // u64 is not supported, because it cannot be represented on GDScript side, and implicitly converting to i64 is error-prone. - impl_variant_traits!(f64, float_to_variant, float_from_variant, Float, GDEXTENSION_METHOD_ARGUMENT_METADATA_REAL_IS_DOUBLE); + impl_variant_traits!(f64, float_to_variant, float_from_variant, Float, metadata = GDEXTENSION_METHOD_ARGUMENT_METADATA_REAL_IS_DOUBLE); impl_variant_traits_float!(f32, GDEXTENSION_METHOD_ARGUMENT_METADATA_REAL_IS_FLOAT); } diff --git a/godot-core/src/lib.rs b/godot-core/src/lib.rs index 84215d19c..b1f0b4783 100644 --- a/godot-core/src/lib.rs +++ b/godot-core/src/lib.rs @@ -172,14 +172,14 @@ macro_rules! generate_gdextension_api_version { // // This includes all versions we're developing for, including unreleased future versions. generate_gdextension_api_version!( - (GDEXTENSION_API_VERSION_FULL, gdextension_exact_api) => { + (GDEXTENSION_EXACT_API, gdextension_exact_api) => { "4.0", "4.0.1", "4.0.2", "4.0.3", "4.1", }, - (GDEXTENSION_API_VERSION, gdextension_api) => { + (GDEXTENSION_API, gdextension_api) => { "4.0", "4.1", }, From 6797f161b7918831d5c44a1f8f3de79b5a0d745c Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Mon, 29 May 2023 11:49:17 +0200 Subject: [PATCH 4/6] Run CI against both Godot stable/nightly versions; remove gcc memcheck For now, disable failing memleak CI job (4.0.3 API, 4.1+ binary) --- .github/composite/godot-install/action.yml | 12 +++- .github/workflows/full-ci.yml | 77 +++++++++++++--------- .github/workflows/minimal-ci.yml | 2 +- 3 files changed, 56 insertions(+), 35 deletions(-) diff --git a/.github/composite/godot-install/action.yml b/.github/composite/godot-install/action.yml index ca4378e7d..d985688d6 100644 --- a/.github/composite/godot-install/action.yml +++ b/.github/composite/godot-install/action.yml @@ -37,13 +37,19 @@ runs: # shell: bash - name: "Download Godot artifact" + env: + ARTIFACT_NAME: ${{ inputs.artifact-name }} # if: steps.cache-godot.outputs.cache-hit != 'true' # If a specific Godot revision should be used, rather than latest, use this: # curl https://nightly.link/Bromeon/godot4-nightly/actions/runs/4910907653/${{ inputs.artifact-name }}.zip \ run: | - curl https://nightly.link/Bromeon/godot4-nightly/workflows/compile-godot/master/${{ inputs.artifact-name }}.zip \ - -Lo artifact.zip \ - --retry 3 + if [[ $ARTIFACT_NAME == *"stable"* ]]; then + url="https://nightly.link/Bromeon/godot4-nightly/workflows/compile-godot-stable/master/$ARTIFACT_NAME.zip" + else + url="https://nightly.link/Bromeon/godot4-nightly/workflows/compile-godot-nightly/master/$ARTIFACT_NAME.zip" + fi + + curl "$url" -Lo artifact.zip --retry 3 unzip artifact.zip -d $RUNNER_DIR/godot_bin shell: bash diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index 6e212c685..9a00ddb76 100644 --- a/.github/workflows/full-ci.yml +++ b/.github/workflows/full-ci.yml @@ -123,26 +123,36 @@ jobs: strategy: fail-fast: false # cancel all jobs as soon as one fails? matrix: + # Naming: {os}[-{runtimeVersion}]-{apiVersion} + # runtimeVersion = version of Godot binary; apiVersion = version of GDExtension API against which gdext is compiled. + # Order this way because macOS typically has the longest duration, followed by Windows, so it benefits total workflow execution time. # Additionally, the 'linux (msrv *)' special case will then be listed next to the other 'linux' jobs. # Note: Windows uses '--target x86_64-pc-windows-msvc' by default as Cargo argument. include: # macOS + - name: macos-stableRt-4.0.3 + os: macos-12 + artifact-name: macos-stable + godot-binary: godot.macos.editor.dev.x86_64 + godot-prebuilt-patch: '4.0.3' + - name: macos-4.0.3 os: macos-12 - artifact-name: macos + artifact-name: macos-nightly godot-binary: godot.macos.editor.dev.x86_64 godot-prebuilt-patch: '4.0.3' - name: macos-double os: macos-12 + artifact-name: macos-double-nightly godot-binary: godot.macos.editor.dev.double.x86_64 rust-extra-args: --features godot/double-precision - name: macos-nightly os: macos-12 - artifact-name: macos + artifact-name: macos-nightly godot-binary: godot.macos.editor.dev.x86_64 rust-extra-args: --features godot/custom-godot with-llvm: true @@ -150,20 +160,27 @@ jobs: # Windows + - name: windows-stableRt-4.0.3 + os: windows-latest + artifact-name: windows-stable + godot-binary: godot.windows.editor.dev.x86_64.exe + godot-prebuilt-patch: '4.0.3' + - name: windows-4.0.3 os: windows-latest - artifact-name: windows + artifact-name: windows-nightly godot-binary: godot.windows.editor.dev.x86_64.exe godot-prebuilt-patch: '4.0.3' - name: windows-double os: windows-latest + artifact-name: windows-double-nightly godot-binary: godot.windows.editor.dev.double.x86_64.exe rust-extra-args: --features godot/double-precision - name: windows-nightly os: windows-latest - artifact-name: windows + artifact-name: windows-nightly godot-binary: godot.windows.editor.dev.x86_64.exe rust-extra-args: --features godot/custom-godot godot-prebuilt-patch: '4.1' @@ -172,44 +189,50 @@ jobs: # Don't use latest Ubuntu (22.04) as it breaks lots of ecosystem compatibility. # If ever moving to ubuntu-latest, need to manually install libtinfo5 for LLVM. + - name: linux-stableRt-4.0.3 + os: ubuntu-20.04 + artifact-name: linux-stable + godot-binary: godot.linuxbsd.editor.dev.x86_64 + - name: linux-4.0.3 os: ubuntu-20.04 - artifact-name: linux + artifact-name: linux-nightly godot-binary: godot.linuxbsd.editor.dev.x86_64 godot-check-header: false # disabled for now - name: linux-4.0.2 os: ubuntu-20.04 - artifact-name: linux + artifact-name: linux-nightly godot-binary: godot.linuxbsd.editor.dev.x86_64 godot-prebuilt-patch: '4.0.2' - name: linux-4.0.1 os: ubuntu-20.04 - artifact-name: linux + artifact-name: linux-nightly godot-binary: godot.linuxbsd.editor.dev.x86_64 godot-prebuilt-patch: '4.0.1' - name: linux-4.0 os: ubuntu-20.04 - artifact-name: linux + artifact-name: linux-nightly godot-binary: godot.linuxbsd.editor.dev.x86_64 godot-prebuilt-patch: '4.0' - name: linux-double os: ubuntu-20.04 + artifact-name: linux-double-nightly godot-binary: godot.linuxbsd.editor.dev.double.x86_64 rust-extra-args: --features godot/double-precision - name: linux-features os: ubuntu-20.04 - artifact-name: linux + artifact-name: linux-nightly godot-binary: godot.linuxbsd.editor.dev.x86_64 rust-extra-args: --features godot/threads,godot/serde - name: linux-nightly os: ubuntu-20.04 - artifact-name: linux + artifact-name: linux-nightly godot-binary: godot.linuxbsd.editor.dev.x86_64 rust-extra-args: --features godot/custom-godot godot-prebuilt-patch: '4.1' @@ -220,35 +243,27 @@ jobs: # The gcc version can possibly be removed later, as it is slower and needs a larger artifact than the clang one. # --disallow-focus: fail if #[itest(focus)] is encountered, to prevent running only a few tests for full CI - - name: linux-memcheck-gcc-4.0.3 - os: ubuntu-20.04 - artifact-name: linux-memcheck-gcc - godot-binary: godot.linuxbsd.editor.dev.x86_64.san - godot-args: -- --disallow-focus - rust-toolchain: nightly - rust-env-rustflags: -Zrandomize-layout - - name: linux-memcheck-clang-4.0.3 + - name: linux-memcheck-stableRt-4.0.3 os: ubuntu-20.04 - artifact-name: linux-memcheck-clang + artifact-name: linux-memcheck-clang-stable godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san godot-args: -- --disallow-focus rust-toolchain: nightly rust-env-rustflags: -Zrandomize-layout - - name: linux-memcheck-gcc-nightly - os: ubuntu-20.04 - artifact-name: linux-memcheck-gcc - godot-binary: godot.linuxbsd.editor.dev.x86_64.san - godot-args: -- --disallow-focus - rust-toolchain: nightly - rust-env-rustflags: -Zrandomize-layout - rust-extra-args: --features godot/custom-godot - godot-prebuilt-patch: '4.1' + # FIXME(#298): memory leaks detected when running 4.0.3 API with 4.1+ binary +# - name: linux-memcheck-4.0.3 +# os: ubuntu-20.04 +# artifact-name: linux-memcheck-clang-nightly +# godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san +# godot-args: -- --disallow-focus +# rust-toolchain: nightly +# rust-env-rustflags: -Zrandomize-layout - - name: linux-memcheck-clang-nightly + - name: linux-memcheck-nightly os: ubuntu-20.04 - artifact-name: linux-memcheck-clang + artifact-name: linux-memcheck-clang-nightly godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san godot-args: -- --disallow-focus rust-toolchain: nightly @@ -263,7 +278,7 @@ jobs: - name: "Run Godot integration test" uses: ./.github/composite/godot-itest with: - artifact-name: godot-${{ matrix.artifact-name || matrix.name }} + artifact-name: godot-${{ matrix.artifact-name }} godot-binary: ${{ matrix.godot-binary }} godot-args: ${{ matrix.godot-args }} godot-prebuilt-patch: ${{ matrix.godot-prebuilt-patch }} diff --git a/.github/workflows/minimal-ci.yml b/.github/workflows/minimal-ci.yml index 93f612ccd..6c148b334 100644 --- a/.github/workflows/minimal-ci.yml +++ b/.github/workflows/minimal-ci.yml @@ -86,7 +86,7 @@ jobs: - name: "Run Godot integration test" uses: ./.github/composite/godot-itest with: - artifact-name: godot-linux + artifact-name: godot-linux-nightly godot-binary: godot.linuxbsd.editor.dev.x86_64 From ede9046f8d2d7e9104680514ef8cb29145a924f3 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Mon, 29 May 2023 19:02:41 +0200 Subject: [PATCH 5/6] Re-introduce from_sys_init_default() for API 4.0 Addresses a regression introduced in fc4a405a2c4400a333c60d226cc5eacf1f008342: > Remove several memory leaks by constructing into uninitialized pointers --- godot-core/src/builtin/array.rs | 2 +- godot-core/src/builtin/callable.rs | 2 +- godot-core/src/builtin/string/godot_string.rs | 4 +- godot-core/src/builtin/string/node_path.rs | 2 +- godot-core/src/builtin/string/string_name.rs | 2 +- godot-core/src/builtin/variant/impls.rs | 5 +++ godot-core/src/builtin/variant/mod.rs | 30 +++++++++++++ godot-ffi/src/godot_ffi.rs | 42 +++++++++++++++---- godot-ffi/src/lib.rs | 4 +- 9 files changed, 79 insertions(+), 14 deletions(-) diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs index 55a102b06..64a81639b 100644 --- a/godot-core/src/builtin/array.rs +++ b/godot-core/src/builtin/array.rs @@ -717,7 +717,7 @@ impl FromVariant for Array { } let array = unsafe { - Self::from_sys_init(|self_ptr| { + sys::from_sys_init_or_init_default::(|self_ptr| { let array_from_variant = sys::builtin_fn!(array_from_variant); array_from_variant(self_ptr, variant.var_sys()); }) diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index ed9e9e6ee..a7652172b 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -45,7 +45,7 @@ impl Callable { // upcast not needed let method = method_name.into(); unsafe { - Self::from_sys_init(|self_ptr| { + sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(callable_from_object_method); let args = [object.sys_const(), method.sys_const()]; ctor(self_ptr, args.as_ptr()); diff --git a/godot-core/src/builtin/string/godot_string.rs b/godot-core/src/builtin/string/godot_string.rs index 85f8f50e6..fbe152551 100644 --- a/godot-core/src/builtin/string/godot_string.rs +++ b/godot-core/src/builtin/string/godot_string.rs @@ -223,7 +223,7 @@ impl FromStr for GodotString { impl From<&StringName> for GodotString { fn from(string: &StringName) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(string_from_string_name); let args = [string.sys_const()]; ctor(self_ptr, args.as_ptr()); @@ -244,7 +244,7 @@ impl From for GodotString { impl From<&NodePath> for GodotString { fn from(path: &NodePath) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(string_from_node_path); let args = [path.sys_const()]; ctor(self_ptr, args.as_ptr()); diff --git a/godot-core/src/builtin/string/node_path.rs b/godot-core/src/builtin/string/node_path.rs index ff4c7ac88..a88e5e623 100644 --- a/godot-core/src/builtin/string/node_path.rs +++ b/godot-core/src/builtin/string/node_path.rs @@ -101,7 +101,7 @@ impl_rust_string_conv!(NodePath); impl From<&GodotString> for NodePath { fn from(string: &GodotString) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(node_path_from_string); let args = [string.sys_const()]; ctor(self_ptr, args.as_ptr()); diff --git a/godot-core/src/builtin/string/string_name.rs b/godot-core/src/builtin/string/string_name.rs index b35b467a8..8add64453 100644 --- a/godot-core/src/builtin/string/string_name.rs +++ b/godot-core/src/builtin/string/string_name.rs @@ -129,7 +129,7 @@ impl_rust_string_conv!(StringName); impl From<&GodotString> for StringName { fn from(string: &GodotString) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(string_name_from_string); let args = [string.sys_const()]; ctor(self_ptr, args.as_ptr()); diff --git a/godot-core/src/builtin/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index d9aa72003..8645ff4dc 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -110,6 +110,11 @@ macro_rules! impl_variant_traits { }; (@@from_sys_init, $from_sys_init:ident) => { + // let $from_sys_init = Self::from_sys_init; + + #[cfg(gdextension_api = "4.0")] + let $from_sys_init = Self::from_sys_init_default; + #[cfg(not(gdextension_api = "4.0"))] let $from_sys_init = Self::from_sys_init; }; } diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index b5e9c40d3..0b3b0890e 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -108,6 +108,26 @@ impl Variant { let args_sys: Vec<_> = args.iter().map(|v| v.var_sys_const()).collect(); let mut error = sys::default_call_error(); + // #[cfg(gdextension_api = "4.0")] + // let result = { + // #[allow(unused_mut)] + // let mut result = Variant::nil(); + // + // use sys::AsUninit; + // unsafe { + // interface_fn!(variant_call)( + // self.var_sys(), + // method.string_sys(), + // args_sys.as_ptr(), + // args_sys.len() as i64, + // result.var_sys().as_uninit(), + // ptr::addr_of_mut!(error), + // ) + // }; + // result + // }; + // + // #[cfg(not(gdextension_api = "4.0"))] let result = unsafe { Variant::from_var_sys_init(|variant_ptr| { interface_fn!(variant_call)( @@ -210,6 +230,16 @@ impl Variant { fn var_sys = sys; } + // #[doc(hidden)] + // pub unsafe fn from_var_sys_init_default( + // init_fn: impl FnOnce(sys::GDExtensionVariantPtr), + // ) -> Self { + // #[allow(unused_mut)] + // let mut variant = Variant::nil(); + // init_fn(variant.var_sys()); + // variant + // } + #[doc(hidden)] pub fn var_sys_const(&self) -> sys::GDExtensionConstVariantPtr { sys::to_const_ptr(self.var_sys()) diff --git a/godot-ffi/src/godot_ffi.rs b/godot-ffi/src/godot_ffi.rs index 974f61a21..4bfb2f7c3 100644 --- a/godot-ffi/src/godot_ffi.rs +++ b/godot-ffi/src/godot_ffi.rs @@ -96,6 +96,32 @@ pub unsafe trait GodotFfi { unsafe fn move_return_ptr(self, dst: sys::GDExtensionTypePtr, call_type: PtrcallType); } +// In Godot 4.0.x, a lot of that are "constructed into" require a default-initialized value. +// In Godot 4.1+, placement new is used, requiring no prior value. +// This method abstracts over that. Outside of GodotFfi because it should not be overridden. + +/// # Safety +/// +/// See [`GodotFfi::from_sys_init`] and [`GodotFfi::from_sys_init_default`]. +#[cfg(gdextension_api = "4.0")] +pub unsafe fn from_sys_init_or_init_default( + init_fn: impl FnOnce(sys::GDExtensionTypePtr), +) -> T { + T::from_sys_init_default(init_fn) +} + +/// # Safety +/// +/// See [`GodotFfi::from_sys_init`] and [`GodotFfi::from_sys_init_default`]. +#[cfg(not(gdextension_api = "4.0"))] +pub unsafe fn from_sys_init_or_init_default( + init_fn: impl FnOnce(sys::GDExtensionUninitializedTypePtr), +) -> T { + T::from_sys_init(init_fn) +} + +// ---------------------------------------------------------------------------------------------------------------------------------------------- + /// Marks a type as having a nullable counterpart in Godot. /// /// This trait primarily exists to implement GodotFfi for `Option>`, which is not possible @@ -114,13 +140,6 @@ unsafe impl GodotFfi for Option where T: GodotNullablePtr, { - fn sys(&self) -> sys::GDExtensionTypePtr { - match self { - Some(value) => value.sys(), - None => ptr::null_mut() as sys::GDExtensionTypePtr, - } - } - unsafe fn from_sys(ptr: sys::GDExtensionTypePtr) -> Self { ptr_then(ptr, |ptr| T::from_sys(ptr)) } @@ -132,6 +151,13 @@ where Self::from_sys(raw.assume_init()) } + fn sys(&self) -> sys::GDExtensionTypePtr { + match self { + Some(value) => value.sys(), + None => ptr::null_mut() as sys::GDExtensionTypePtr, + } + } + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, call_type: PtrcallType) -> Self { ptr_then(ptr, |ptr| T::from_arg_ptr(ptr, call_type)) } @@ -143,6 +169,8 @@ where } } +// ---------------------------------------------------------------------------------------------------------------------------------------------- + /// An indication of what type of pointer call is being made. #[derive(Default, Copy, Clone, Eq, PartialEq, Debug)] pub enum PtrcallType { diff --git a/godot-ffi/src/lib.rs b/godot-ffi/src/lib.rs index 50beaee36..961e09acc 100644 --- a/godot-ffi/src/lib.rs +++ b/godot-ffi/src/lib.rs @@ -32,7 +32,9 @@ use std::ffi::CStr; #[doc(hidden)] pub use paste; -pub use crate::godot_ffi::{GodotFfi, GodotFuncMarshal, GodotNullablePtr, PtrcallType}; +pub use crate::godot_ffi::{ + from_sys_init_or_init_default, GodotFfi, GodotFuncMarshal, GodotNullablePtr, PtrcallType, +}; pub use gen::central::*; pub use gen::gdextension_interface::*; pub use gen::interface::*; From 27a3a7f3ea85cd0475ced5d8c6b8e3ab9fc4a407 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 3 Jun 2023 12:36:11 +0200 Subject: [PATCH 6/6] Variant FFI: use conditionally compiled from_var_sys_init[_default] --- godot-core/src/builtin/variant/impls.rs | 62 +++++--------------- godot-core/src/builtin/variant/mod.rs | 78 ++++++++++--------------- 2 files changed, 47 insertions(+), 93 deletions(-) diff --git a/godot-core/src/builtin/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index 8645ff4dc..cbad9c7e2 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -37,11 +37,7 @@ macro_rules! impl_variant_traits { impl_variant_traits!(@@ $T, $from_fn, $to_fn, $variant_type;); }; - ($T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident, init) => { - impl_variant_traits!(@@ $T, $from_fn, $to_fn, $variant_type, init;); - }; - - ($T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident, metadata = $param_metadata:ident) => { + ($T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident, $param_metadata:ident) => { impl_variant_traits!(@@ $T, $from_fn, $to_fn, $variant_type; fn param_metadata() -> sys::GDExtensionClassMethodArgumentMetadata { sys::$param_metadata @@ -49,15 +45,7 @@ macro_rules! impl_variant_traits { ); }; - ($T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident, init, metadata = $param_metadata:ident) => { - impl_variant_traits!(@@ $T, $from_fn, $to_fn, $variant_type, init; - fn param_metadata() -> sys::GDExtensionClassMethodArgumentMetadata { - sys::$param_metadata - } - ); - }; - - (@@ $T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident $(, $init:ident)?; $($extra:tt)*) => { + (@@ $T:ty, $from_fn:ident, $to_fn:ident, $variant_type:ident; $($extra:tt)*) => { impl ToVariant for $T { fn to_variant(&self) -> Variant { let variant = unsafe { @@ -87,9 +75,7 @@ macro_rules! impl_variant_traits { // // This was changed in 4.1. let result = unsafe { - impl_variant_traits!(@@from_sys_init, from_sys_init $(, $init)?); - - from_sys_init(|self_ptr| { + sys::from_sys_init_or_init_default(|self_ptr| { let converter = sys::builtin_fn!($to_fn); converter(self_ptr, variant.var_sys()); }) @@ -101,22 +87,6 @@ macro_rules! impl_variant_traits { impl_variant_metadata!($T, $variant_type; $($extra)*); }; - - (@@from_sys_init, $from_sys_init:ident, init) => { - #[cfg(gdextension_api = "4.0")] - let $from_sys_init = Self::from_sys_init_default; - #[cfg(not(gdextension_api = "4.0"))] - let $from_sys_init = Self::from_sys_init; - }; - - (@@from_sys_init, $from_sys_init:ident) => { - // let $from_sys_init = Self::from_sys_init; - - #[cfg(gdextension_api = "4.0")] - let $from_sys_init = Self::from_sys_init_default; - #[cfg(not(gdextension_api = "4.0"))] - let $from_sys_init = Self::from_sys_init; - }; } macro_rules! impl_variant_traits_int { @@ -184,7 +154,7 @@ mod impls { impl_variant_traits!(Aabb, aabb_to_variant, aabb_from_variant, Aabb); impl_variant_traits!(bool, bool_to_variant, bool_from_variant, Bool); impl_variant_traits!(Basis, basis_to_variant, basis_from_variant, Basis); - impl_variant_traits!(Callable, callable_to_variant, callable_from_variant, Callable, init); + impl_variant_traits!(Callable, callable_to_variant, callable_from_variant, Callable); impl_variant_traits!(Vector2, vector2_to_variant, vector2_from_variant, Vector2); impl_variant_traits!(Vector3, vector3_to_variant, vector3_from_variant, Vector3); impl_variant_traits!(Vector4, vector4_to_variant, vector4_from_variant, Vector4); @@ -192,20 +162,20 @@ mod impls { impl_variant_traits!(Vector3i, vector3i_to_variant, vector3i_from_variant, Vector3i); impl_variant_traits!(Quaternion, quaternion_to_variant, quaternion_from_variant, Quaternion); impl_variant_traits!(Color, color_to_variant, color_from_variant, Color); - impl_variant_traits!(GodotString, string_to_variant, string_from_variant, String, init); + impl_variant_traits!(GodotString, string_to_variant, string_from_variant, String); impl_variant_traits!(StringName, string_name_to_variant, string_name_from_variant, StringName); impl_variant_traits!(NodePath, node_path_to_variant, node_path_from_variant, NodePath); // TODO use impl_variant_traits!, as soon as `Default` is available. Also consider auto-generating. impl_variant_metadata!(Signal, /* signal_to_variant, signal_from_variant, */ Signal); - impl_variant_traits!(PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant, PackedByteArray, init); - impl_variant_traits!(PackedInt32Array, packed_int32_array_to_variant, packed_int32_array_from_variant, PackedInt32Array, init); - impl_variant_traits!(PackedInt64Array, packed_int64_array_to_variant, packed_int64_array_from_variant, PackedInt64Array, init); - impl_variant_traits!(PackedFloat32Array, packed_float32_array_to_variant, packed_float32_array_from_variant, PackedFloat32Array, init); - impl_variant_traits!(PackedFloat64Array, packed_float64_array_to_variant, packed_float64_array_from_variant, PackedFloat64Array, init); - impl_variant_traits!(PackedStringArray, packed_string_array_to_variant, packed_string_array_from_variant, PackedStringArray, init); - impl_variant_traits!(PackedVector2Array, packed_vector2_array_to_variant, packed_vector2_array_from_variant, PackedVector2Array, init); - impl_variant_traits!(PackedVector3Array, packed_vector3_array_to_variant, packed_vector3_array_from_variant, PackedVector3Array, init); - impl_variant_traits!(PackedColorArray, packed_color_array_to_variant, packed_color_array_from_variant, PackedColorArray, init); + impl_variant_traits!(PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant, PackedByteArray); + impl_variant_traits!(PackedInt32Array, packed_int32_array_to_variant, packed_int32_array_from_variant, PackedInt32Array); + impl_variant_traits!(PackedInt64Array, packed_int64_array_to_variant, packed_int64_array_from_variant, PackedInt64Array); + impl_variant_traits!(PackedFloat32Array, packed_float32_array_to_variant, packed_float32_array_from_variant, PackedFloat32Array); + impl_variant_traits!(PackedFloat64Array, packed_float64_array_to_variant, packed_float64_array_from_variant, PackedFloat64Array); + impl_variant_traits!(PackedStringArray, packed_string_array_to_variant, packed_string_array_from_variant, PackedStringArray); + impl_variant_traits!(PackedVector2Array, packed_vector2_array_to_variant, packed_vector2_array_from_variant, PackedVector2Array); + impl_variant_traits!(PackedVector3Array, packed_vector3_array_to_variant, packed_vector3_array_from_variant, PackedVector3Array); + impl_variant_traits!(PackedColorArray, packed_color_array_to_variant, packed_color_array_from_variant, PackedColorArray); impl_variant_traits!(Plane, plane_to_variant, plane_from_variant, Plane); impl_variant_traits!(Projection, projection_to_variant, projection_from_variant, Projection); impl_variant_traits!(Rid, rid_to_variant, rid_from_variant, Rid); @@ -215,7 +185,7 @@ mod impls { impl_variant_traits!(Transform3D, transform_3d_to_variant, transform_3d_from_variant, Transform3D); impl_variant_traits!(Dictionary, dictionary_to_variant, dictionary_from_variant, Dictionary); - impl_variant_traits!(i64, int_to_variant, int_from_variant, Int, metadata = GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT64); + impl_variant_traits!(i64, int_to_variant, int_from_variant, Int, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT64); impl_variant_traits_int!(i8, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT8); impl_variant_traits_int!(i16, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT16); impl_variant_traits_int!(i32, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT32); @@ -225,7 +195,7 @@ mod impls { impl_variant_traits_int!(u32, GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_UINT32); // u64 is not supported, because it cannot be represented on GDScript side, and implicitly converting to i64 is error-prone. - impl_variant_traits!(f64, float_to_variant, float_from_variant, Float, metadata = GDEXTENSION_METHOD_ARGUMENT_METADATA_REAL_IS_DOUBLE); + impl_variant_traits!(f64, float_to_variant, float_from_variant, Float, GDEXTENSION_METHOD_ARGUMENT_METADATA_REAL_IS_DOUBLE); impl_variant_traits_float!(f32, GDEXTENSION_METHOD_ARGUMENT_METADATA_REAL_IS_FLOAT); } diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index 0b3b0890e..0a8e94a40 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -108,28 +108,8 @@ impl Variant { let args_sys: Vec<_> = args.iter().map(|v| v.var_sys_const()).collect(); let mut error = sys::default_call_error(); - // #[cfg(gdextension_api = "4.0")] - // let result = { - // #[allow(unused_mut)] - // let mut result = Variant::nil(); - // - // use sys::AsUninit; - // unsafe { - // interface_fn!(variant_call)( - // self.var_sys(), - // method.string_sys(), - // args_sys.as_ptr(), - // args_sys.len() as i64, - // result.var_sys().as_uninit(), - // ptr::addr_of_mut!(error), - // ) - // }; - // result - // }; - // - // #[cfg(not(gdextension_api = "4.0"))] let result = unsafe { - Variant::from_var_sys_init(|variant_ptr| { + Variant::from_var_sys_init_or_init_default(|variant_ptr| { interface_fn!(variant_call)( self.var_sys(), method.string_sys(), @@ -152,24 +132,8 @@ impl Variant { let op_sys = op.sys(); let mut is_valid = false as u8; - #[cfg(gdextension_api = "4.0")] - let result = { - #[allow(unused_mut)] - let mut result = Variant::nil(); - unsafe { - interface_fn!(variant_evaluate)( - op_sys, - self.var_sys(), - rhs.var_sys(), - result.var_sys(), - ptr::addr_of_mut!(is_valid), - ) - }; - result - }; - #[cfg(not(gdextension_api = "4.0"))] let result = unsafe { - Variant::from_var_sys_init(|variant_ptr| { + Self::from_var_sys_init_or_init_default(|variant_ptr| { interface_fn!(variant_evaluate)( op_sys, self.var_sys(), @@ -230,15 +194,35 @@ impl Variant { fn var_sys = sys; } - // #[doc(hidden)] - // pub unsafe fn from_var_sys_init_default( - // init_fn: impl FnOnce(sys::GDExtensionVariantPtr), - // ) -> Self { - // #[allow(unused_mut)] - // let mut variant = Variant::nil(); - // init_fn(variant.var_sys()); - // variant - // } + #[doc(hidden)] + pub unsafe fn from_var_sys_init_default( + init_fn: impl FnOnce(sys::GDExtensionVariantPtr), + ) -> Self { + #[allow(unused_mut)] + let mut variant = Variant::nil(); + init_fn(variant.var_sys()); + variant + } + + /// # Safety + /// + /// See [`GodotFfi::from_sys_init`] and [`GodotFfi::from_sys_init_default`]. + #[cfg(gdextension_api = "4.0")] + pub unsafe fn from_var_sys_init_or_init_default( + init_fn: impl FnOnce(sys::GDExtensionVariantPtr), + ) -> Self { + Self::from_var_sys_init_default(init_fn) + } + + /// # Safety + /// + /// See [`GodotFfi::from_sys_init`] and [`GodotFfi::from_sys_init_default`]. + #[cfg(not(gdextension_api = "4.0"))] + pub unsafe fn from_var_sys_init_or_init_default( + init_fn: impl FnOnce(sys::GDExtensionUninitializedVariantPtr), + ) -> Self { + Self::from_var_sys_init(init_fn) + } #[doc(hidden)] pub fn var_sys_const(&self) -> sys::GDExtensionConstVariantPtr {