From eebaa4d69d35f198a78075f5cfe42cd05ee3248c Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Mon, 6 May 2024 21:40:39 +0200 Subject: [PATCH] Change type of process() + physics_process() delta from f64 to real --- examples/dodge-the-creeps/rust/src/player.rs | 4 +-- .../src/generator/functions_common.rs | 2 +- godot-codegen/src/models/domain.rs | 21 +++++++++++++--- godot-codegen/src/models/domain_mapping.rs | 8 +++--- .../src/special_cases/special_cases.rs | 25 +++++++++++++++++++ godot-core/src/obj/traits.rs | 8 +++--- itest/rust/src/engine_tests/codegen_test.rs | 2 +- itest/rust/src/object_tests/onready_test.rs | 8 +++--- 8 files changed, 59 insertions(+), 19 deletions(-) diff --git a/examples/dodge-the-creeps/rust/src/player.rs b/examples/dodge-the-creeps/rust/src/player.rs index a8355ce70..914c88ecd 100644 --- a/examples/dodge-the-creeps/rust/src/player.rs +++ b/examples/dodge-the-creeps/rust/src/player.rs @@ -56,7 +56,7 @@ impl IArea2D for Player { self.base_mut().hide(); } - fn process(&mut self, delta: f64) { + fn process(&mut self, delta: f32) { let mut animated_sprite = self .base() .get_node_as::("AnimatedSprite2D"); @@ -99,7 +99,7 @@ impl IArea2D for Player { animated_sprite.stop(); } - let change = velocity * real::from_f64(delta); + let change = velocity * delta; let position = self.base().get_global_position() + change; let position = Vector2::new( position.x.clamp(0.0, self.screen_size.x), diff --git a/godot-codegen/src/generator/functions_common.rs b/godot-codegen/src/generator/functions_common.rs index 003ec88e3..f9c0475e7 100644 --- a/godot-codegen/src/generator/functions_common.rs +++ b/godot-codegen/src/generator/functions_common.rs @@ -90,7 +90,7 @@ pub fn make_function_definition( let has_default_params = default_parameters::function_uses_default_params(sig); let vis = if has_default_params { // Public API mapped by separate function. - // Needs to be crate-public because default-arg builder lives outside of the module. + // Needs to be crate-public because default-arg builder lives outside the module. quote! { pub(crate) } } else { make_vis(sig.is_private()) diff --git a/godot-codegen/src/models/domain.rs b/godot-codegen/src/models/domain.rs index 238f68e9a..1df2f93d4 100644 --- a/godot-codegen/src/models/domain.rs +++ b/godot-codegen/src/models/domain.rs @@ -477,10 +477,14 @@ pub struct FnParam { } impl FnParam { - pub fn new_range(method_args: &Option>, ctx: &mut Context) -> Vec { + pub fn new_range( + method_args: &Option>, + meta_overrides: &HashMap, + ctx: &mut Context, + ) -> Vec { option_as_slice(method_args) .iter() - .map(|arg| Self::new(arg, ctx)) + .map(|arg| Self::new(arg, ctx, meta_overrides)) .collect() } @@ -494,9 +498,18 @@ impl FnParam { .collect() } - pub fn new(method_arg: &JsonMethodArg, ctx: &mut Context) -> FnParam { + pub fn new( + method_arg: &JsonMethodArg, + ctx: &mut Context, + meta_overrides: &HashMap, + ) -> FnParam { + // Some methods like _process(f64) have manual overrides to f32. + let forced_meta = meta_overrides.get(&method_arg.name); + let meta = forced_meta.or(method_arg.meta.as_ref()); + let name = safe_ident(&method_arg.name); - let type_ = conv::to_rust_type(&method_arg.type_, method_arg.meta.as_ref(), ctx); + + let type_ = conv::to_rust_type(&method_arg.type_, meta, ctx); let default_value = method_arg .default_value .as_ref() diff --git a/godot-codegen/src/models/domain_mapping.rs b/godot-codegen/src/models/domain_mapping.rs index 63fe82638..72b5cfcde 100644 --- a/godot-codegen/src/models/domain_mapping.rs +++ b/godot-codegen/src/models/domain_mapping.rs @@ -438,7 +438,6 @@ impl ClassMethod { } let is_private = special_cases::is_method_private(class_name, &method.name); - let godot_method_name = method.name.clone(); let qualifier = { @@ -451,11 +450,14 @@ impl ClassMethod { FnQualifier::from_const_static(is_actually_const, method.is_static) }; + let param_meta_overrides = + special_cases::get_class_method_meta_overrides(class_name, &method.name); + Some(Self { common: FunctionCommon { name: rust_method_name.to_string(), godot_name: godot_method_name, - parameters: FnParam::new_range(&method.arguments, ctx), + parameters: FnParam::new_range(&method.arguments, ¶m_meta_overrides, ctx), return_value: FnReturn::new(&method.return_value, ctx), is_vararg: method.is_vararg, is_private, @@ -494,7 +496,7 @@ impl UtilityFunction { common: FunctionCommon { name: rust_method_name, godot_name: godot_method_name, - parameters: FnParam::new_range(&function.arguments, ctx), + parameters: FnParam::new_range(&function.arguments, &HashMap::new(), ctx), return_value: FnReturn::new(&return_value, ctx), is_vararg: function.is_vararg, is_private: false, diff --git a/godot-codegen/src/special_cases/special_cases.rs b/godot-codegen/src/special_cases/special_cases.rs index 4112e6a47..0d1af8b66 100644 --- a/godot-codegen/src/special_cases/special_cases.rs +++ b/godot-codegen/src/special_cases/special_cases.rs @@ -27,6 +27,7 @@ use crate::models::domain::TyName; use crate::models::json::{JsonBuiltinMethod, JsonClassMethod, JsonUtilityFunction}; use crate::special_cases::codegen_special_cases; use crate::Context; +use std::collections::HashMap; // Deliberately private -- all checks must go through `special_cases`. @@ -354,3 +355,27 @@ pub fn is_class_level_server(class_name: &str) -> bool { => true, _ => false } } + +/// For certain methods like `_process()`, this can influence a parameter type's meta, e.g. represent `delta: f64` as `f32`. +/// +/// Deliberately only operates on meta level and doesn't replace types, at it's very easy to introduce UB otherwise. +#[rustfmt::skip] +pub(crate) fn get_class_method_meta_overrides( + class: &TyName, + method_name: &str, +) -> HashMap { + let mut map = HashMap::new(); + + match (class.godot_ty.as_str(), method_name) { + // Could also be `real`, but currently most docs use the resolved float type, e.g. VectorN x,y,z,w fields. + | ("Node", "_process") + | ("Node", "_physics_process") => { + #[cfg(not(feature = "double-precision"))] + map.insert("delta".to_string(), "float".to_string()); + } + + _ => {} + } + + map +} diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index 0888efc89..10987723c 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -242,7 +242,7 @@ pub trait WithBaseField: GodotClass + Bounds { /// /// #[godot_api] /// impl INode for MyClass { - /// fn process(&mut self, _delta: f64) { + /// fn process(&mut self, _delta: f32) { /// let name = self.base().get_name(); /// godot_print!("name is {name}"); /// } @@ -268,7 +268,7 @@ pub trait WithBaseField: GodotClass + Bounds { /// /// #[godot_api] /// impl INode for MyClass { - /// fn process(&mut self, _delta: f64) { + /// fn process(&mut self, _delta: f32) { /// let node = Node::new_alloc(); /// // fails because `add_child` requires a mutable reference. /// self.base().add_child(node); @@ -306,7 +306,7 @@ pub trait WithBaseField: GodotClass + Bounds { /// /// #[godot_api] /// impl INode for MyClass { - /// fn process(&mut self, _delta: f64) { + /// fn process(&mut self, _delta: f32) { /// let node = Node::new_alloc(); /// self.base_mut().add_child(node); /// } @@ -331,7 +331,7 @@ pub trait WithBaseField: GodotClass + Bounds { /// /// #[godot_api] /// impl INode for MyClass { - /// fn process(&mut self, _delta: f64) { + /// fn process(&mut self, _delta: f32) { /// self.base_mut().call("other_method".into(), &[]); /// } /// } diff --git a/itest/rust/src/engine_tests/codegen_test.rs b/itest/rust/src/engine_tests/codegen_test.rs index b42a44c07..5da944a14 100644 --- a/itest/rust/src/engine_tests/codegen_test.rs +++ b/itest/rust/src/engine_tests/codegen_test.rs @@ -93,7 +93,7 @@ impl IHttpRequest for CodegenTest { } // Test unnamed parameter in virtual function - fn process(&mut self, _: f64) {} + fn process(&mut self, _: real) {} } // ---------------------------------------------------------------------------------------------------------------------------------------------- diff --git a/itest/rust/src/object_tests/onready_test.rs b/itest/rust/src/object_tests/onready_test.rs index 0e46a5956..6c237d211 100644 --- a/itest/rust/src/object_tests/onready_test.rs +++ b/itest/rust/src/object_tests/onready_test.rs @@ -6,12 +6,12 @@ */ use crate::framework::{expect_panic, itest}; +use godot::builtin::meta::ToGodot; +use godot::builtin::real; use godot::engine::notify::NodeNotification; use godot::engine::INode; -use godot::register::{godot_api, GodotClass}; - use godot::obj::{Gd, OnReady}; -use godot::prelude::ToGodot; +use godot::register::{godot_api, GodotClass}; #[itest] fn onready_deref() { @@ -232,5 +232,5 @@ impl OnReadyWithImplWithoutReady { #[godot_api] impl INode for OnReadyWithoutImpl { // Declare another function to ensure virtual getter must be provided. - fn process(&mut self, _delta: f64) {} + fn process(&mut self, _delta: real) {} }