Merge pull request #1286 from godot-rust/qol/remove-conversions

Bromeon · web-flow · commit e6d317e3d9ab · 2025-09-18T22:02:01.000Z
Remove by-value `From` conversions between strings
diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs
@@ -700,7 +700,7 @@ mod custom_callable {
         r_out: sys::GDExtensionStringPtr,
     ) {
         let c: &T = CallableUserdata::inner_from_raw(callable_userdata);
-        let s = crate::builtin::GString::from(c.to_string());
+        let s = GString::from(&c.to_string());
 
         s.move_into_string_ptr(r_out);
         *r_is_valid = sys::conv::SYS_TRUE;
diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs
@@ -1368,7 +1368,7 @@ impl<T: ArrayElement> GodotType for Array<T> {
         // Typed arrays use type hint.
         PropertyHintInfo {
             hint: crate::global::PropertyHint::ARRAY_TYPE,
-            hint_string: GString::from(element_godot_type_name::<T>()),
+            hint_string: GString::from(&element_godot_type_name::<T>()),
         }
     }
 }
diff --git a/godot-core/src/builtin/string/gstring.rs b/godot-core/src/builtin/string/gstring.rs
@@ -359,12 +359,6 @@ impl From<&[char]> for GString {
     }
 }
 
-impl From<String> for GString {
-    fn from(value: String) -> Self {
-        value.as_str().into()
-    }
-}
-
 impl From<&String> for GString {
     fn from(value: &String) -> Self {
         value.as_str().into()
@@ -424,15 +418,6 @@ impl From<&StringName> for GString {
     }
 }
 
-impl From<StringName> for GString {
-    /// Converts this `StringName` to a `GString`.
-    ///
-    /// This is identical to `GString::from(&string_name)`, and as such there is no performance benefit.
-    fn from(string_name: StringName) -> Self {
-        Self::from(&string_name)
-    }
-}
-
 impl From<&NodePath> for GString {
     fn from(path: &NodePath) -> Self {
         unsafe {
@@ -445,15 +430,6 @@ impl From<&NodePath> for GString {
     }
 }
 
-impl From<NodePath> for GString {
-    /// Converts this `NodePath` to a `GString`.
-    ///
-    /// This is identical to `GString::from(&path)`, and as such there is no performance benefit.
-    fn from(path: NodePath) -> Self {
-        Self::from(&path)
-    }
-}
-
 #[cfg(feature = "serde")]
 mod serialize {
     use std::fmt::Formatter;
diff --git a/godot-core/src/builtin/string/node_path.rs b/godot-core/src/builtin/string/node_path.rs
@@ -233,20 +233,16 @@ impl fmt::Debug for NodePath {
 impl_rust_string_conv!(NodePath);
 
 impl From<&str> for NodePath {
+    // NodePath doesn't offer direct construction from bytes; go via GString.
     fn from(s: &str) -> Self {
-        GString::from(s).into()
-    }
-}
-
-impl From<String> for NodePath {
-    fn from(s: String) -> Self {
-        GString::from(s).into()
+        Self::from(&GString::from(s))
     }
 }
 
 impl From<&String> for NodePath {
     fn from(s: &String) -> Self {
-        GString::from(s).into()
+        // NodePath doesn't offer direct construction from bytes; go via GString.
+        Self::from(&GString::from(s))
     }
 }
 
@@ -262,27 +258,9 @@ impl From<&GString> for NodePath {
     }
 }
 
-impl From<GString> for NodePath {
-    /// Converts this `GString` to a `NodePath`.
-    ///
-    /// This is identical to `NodePath::from(&string)`, and as such there is no performance benefit.
-    fn from(string: GString) -> Self {
-        Self::from(&string)
-    }
-}
-
 impl From<&StringName> for NodePath {
-    fn from(string_name: &StringName) -> Self {
-        Self::from(GString::from(string_name))
-    }
-}
-
-impl From<StringName> for NodePath {
-    /// Converts this `StringName` to a `NodePath`.
-    ///
-    /// This is identical to `NodePath::from(&string_name)`, and as such there is no performance benefit.
-    fn from(string_name: StringName) -> Self {
-        Self::from(GString::from(string_name))
+    fn from(s: &StringName) -> Self {
+        Self::from(&GString::from(s))
     }
 }
 
diff --git a/godot-core/src/builtin/string/string_name.rs b/godot-core/src/builtin/string/string_name.rs
@@ -125,7 +125,7 @@ impl StringName {
                 // This branch is short-circuited if invoked for CStr, which uses `string_name_new_with_latin1_chars`
                 // (requires nul-termination). In general, fall back to GString conversion.
                 GString::try_from_bytes_with_nul_check(bytes, Encoding::Latin1, check_nul)
-                    .map(Self::from)
+                    .map(|s| Self::from(&s))
             }
             Encoding::Utf8 => {
                 // from_utf8() also checks for intermediate NUL bytes.
@@ -335,12 +335,6 @@ impl From<&str> for StringName {
     }
 }
 
-impl From<String> for StringName {
-    fn from(value: String) -> Self {
-        value.as_str().into()
-    }
-}
-
 impl From<&String> for StringName {
     fn from(value: &String) -> Self {
         value.as_str().into()
@@ -360,27 +354,9 @@ impl From<&GString> for StringName {
     }
 }
 
-impl From<GString> for StringName {
-    /// Converts this `GString` to a `StringName`.
-    ///
-    /// This is identical to `StringName::from(&string)`, and as such there is no performance benefit.
-    fn from(string: GString) -> Self {
-        Self::from(&string)
-    }
-}
-
 impl From<&NodePath> for StringName {
     fn from(path: &NodePath) -> Self {
-        Self::from(GString::from(path))
-    }
-}
-
-impl From<NodePath> for StringName {
-    /// Converts this `NodePath` to a `StringName`.
-    ///
-    /// This is identical to `StringName::from(&path)`, and as such there is no performance benefit.
-    fn from(path: NodePath) -> Self {
-        Self::from(GString::from(path))
+        Self::from(&GString::from(path))
     }
 }
 
diff --git a/godot-core/src/meta/property_info.rs b/godot-core/src/meta/property_info.rs
@@ -151,7 +151,7 @@ impl PropertyInfo {
     {
         self.variant_type == VariantType::ARRAY
             && self.hint_info.hint == PropertyHint::ARRAY_TYPE
-            && self.hint_info.hint_string == T::Via::godot_type_name().into()
+            && self.hint_info.hint_string == GString::from(&T::Via::godot_type_name())
     }
 
     // ------------------------------------------------------------------------------------------------------------------------------------------
@@ -291,7 +291,7 @@ impl PropertyHintInfo {
         let hint_string = if sys::GdextBuild::since_api("4.3") {
             GString::new()
         } else {
-            GString::from(type_name)
+            GString::from(&type_name)
         };
 
         Self {
@@ -304,23 +304,23 @@ impl PropertyHintInfo {
     pub fn var_array_element<T: ArrayElement>() -> Self {
         Self {
             hint: PropertyHint::ARRAY_TYPE,
-            hint_string: GString::from(element_godot_type_name::<T>()),
+            hint_string: GString::from(&element_godot_type_name::<T>()),
         }
     }
 
     /// Use for `#[export]` properties -- [`PROPERTY_HINT_TYPE_STRING`](PropertyHint::TYPE_STRING) with the **element** type string as hint string.
     pub fn export_array_element<T: ArrayElement>() -> Self {
         Self {
             hint: PropertyHint::TYPE_STRING,
-            hint_string: GString::from(T::element_type_string()),
+            hint_string: GString::from(&T::element_type_string()),
         }
     }
 
     /// Use for `#[export]` properties -- [`PROPERTY_HINT_TYPE_STRING`](PropertyHint::TYPE_STRING) with the **element** type string as hint string.
     pub fn export_packed_array_element<T: PackedArrayElement>() -> Self {
         Self {
             hint: PropertyHint::TYPE_STRING,
-            hint_string: GString::from(T::element_type_string()),
+            hint_string: GString::from(&T::element_type_string()),
         }
     }
 
@@ -349,7 +349,7 @@ impl PropertyHintInfo {
         D: ?Sized + 'static,
     {
         PropertyHintInfo {
-            hint_string: GString::from(get_dyn_property_hint_string::<T, D>()),
+            hint_string: GString::from(&get_dyn_property_hint_string::<T, D>()),
             ..PropertyHintInfo::export_gd::<T>()
         }
     }
diff --git a/godot-core/src/registry/property/mod.rs b/godot-core/src/registry/property/mod.rs
@@ -315,7 +315,7 @@ pub mod export_info_functions {
 
         PropertyHintInfo {
             hint: PropertyHint::RANGE,
-            hint_string: hint_string.into(),
+            hint_string: GString::from(&hint_string),
         }
     }
 
@@ -381,7 +381,7 @@ pub mod export_info_functions {
 
         PropertyHintInfo {
             hint: PropertyHint::ENUM,
-            hint_string: hint_string.into(),
+            hint_string: GString::from(&hint_string),
         }
     }
 
@@ -390,7 +390,7 @@ pub mod export_info_functions {
 
         PropertyHintInfo {
             hint: PropertyHint::EXP_EASING,
-            hint_string: hint_string.into(),
+            hint_string: GString::from(&hint_string),
         }
     }
 
@@ -414,7 +414,7 @@ pub mod export_info_functions {
 
         PropertyHintInfo {
             hint: PropertyHint::FLAGS,
-            hint_string: hint_string.into(),
+            hint_string: GString::from(&hint_string),
         }
     }
 
@@ -493,7 +493,7 @@ pub mod export_info_functions {
 
         PropertyHintInfo {
             hint: PropertyHint::TYPE_STRING,
-            hint_string: format!("{hint_string}:{filter}").into(),
+            hint_string: GString::from(&format!("{hint_string}:{filter}")),
         }
     }
 
diff --git a/godot-core/src/storage/instance_storage.rs b/godot-core/src/storage/instance_storage.rs
@@ -301,7 +301,7 @@ pub unsafe fn destroy_storage<T: GodotClass>(instance_ptr: sys::GDExtensionClass
         // In Debug mode, crash which may trigger breakpoint.
         // In Release mode, leak player object (Godot philosophy: don't crash if somehow avoidable). Likely leads to follow-up issues.
         if cfg!(debug_assertions) {
-            let error = crate::builtin::GString::from(error);
+            let error = crate::builtin::GString::from(&error);
             crate::classes::Os::singleton().crash(&error);
         } else {
             leak_rust_object = true;
diff --git a/godot-macros/src/derive/data_models/c_style_enum.rs b/godot-macros/src/derive/data_models/c_style_enum.rs
@@ -115,7 +115,8 @@ impl CStyleEnum {
         }
 
         quote! {
-            format!(#fmt, #(#fmt_args),*)
+            // & because it is passed to GString::from().
+            &format!(#fmt, #(#fmt_args),*)
         }
     }
 
diff --git a/itest/rust/src/builtin_tests/string/gstring_test.rs b/itest/rust/src/builtin_tests/string/gstring_test.rs
@@ -28,11 +28,6 @@ fn string_conversion() {
     let back = String::from(&second);
 
     assert_eq!(string, back);
-
-    let second = GString::from(string.clone());
-    let back = String::from(second);
-
-    assert_eq!(string, back);
 }
 
 #[itest]
@@ -75,7 +70,7 @@ fn string_chars() {
 
     let string = String::from("ö🍎A💡");
     let string_chars: Vec<char> = string.chars().collect();
-    let gstring = GString::from(string);
+    let gstring = GString::from(&string);
 
     assert_eq!(gstring.chars(), string_chars.as_slice());
     assert_eq!(
diff --git a/itest/rust/src/builtin_tests/string/node_path_test.rs b/itest/rust/src/builtin_tests/string/node_path_test.rs
@@ -26,11 +26,6 @@ fn node_path_conversion() {
     let back = GString::from(&name);
 
     assert_eq!(string, back);
-
-    let second = NodePath::from(string.clone());
-    let back = GString::from(second);
-
-    assert_eq!(string, back);
 }
 
 #[itest]
diff --git a/itest/rust/src/builtin_tests/string/string_name_test.rs b/itest/rust/src/builtin_tests/string/string_name_test.rs
@@ -28,11 +28,6 @@ fn string_name_conversion() {
     let back = GString::from(&name);
 
     assert_eq!(string, back);
-
-    let second = StringName::from(string.clone());
-    let back = GString::from(second);
-
-    assert_eq!(string, back);
 }
 
 #[itest]
@@ -42,11 +37,6 @@ fn string_name_node_path_conversion() {
     let back = StringName::from(&name);
 
     assert_eq!(string, back);
-
-    let second = NodePath::from(string.clone());
-    let back = StringName::from(second);
-
-    assert_eq!(string, back);
 }
 
 #[itest]
diff --git a/itest/rust/src/object_tests/dyn_gd_test.rs b/itest/rust/src/object_tests/dyn_gd_test.rs
@@ -4,7 +4,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
-
+use godot::global::godot_str;
 // Test that all important dyn-related symbols are in the prelude.
 use godot::prelude::*;
 
@@ -554,7 +554,7 @@ struct RefcHealth {
 #[godot_api]
 impl IRefCounted for RefcHealth {
     fn to_string(&self) -> GString {
-        format!("RefcHealth(hp={})", self.hp).into()
+        godot_str!("RefcHealth(hp={})", self.hp)
     }
 }
 
diff --git a/itest/rust/src/object_tests/object_test.rs b/itest/rust/src/object_tests/object_test.rs
@@ -15,6 +15,7 @@ use godot::builtin::{GString, StringName, Variant, Vector3};
 use godot::classes::{
     file_access, Engine, FileAccess, IRefCounted, Node, Node2D, Node3D, Object, RefCounted,
 };
+use godot::global::godot_str;
 #[allow(deprecated)]
 use godot::meta::{FromGodot, GodotType, ToGodot};
 use godot::obj::{Base, Gd, Inherits, InstanceId, NewAlloc, NewGd, RawGd};
@@ -942,7 +943,7 @@ impl IRefCounted for RefcPayload {
     }
 
     fn to_string(&self) -> GString {
-        format!("value={}", self.value).into()
+        godot_str!("value={}", self.value)
     }
 }
 
diff --git a/itest/rust/src/object_tests/virtual_methods_test.rs b/itest/rust/src/object_tests/virtual_methods_test.rs
@@ -17,6 +17,7 @@ use godot::classes::{
     IEditorPlugin, INode, INode2D, IPrimitiveMesh, IRefCounted, InputEvent, InputEventAction, Node,
     Node2D, Object, PrimitiveMesh, RefCounted, Window,
 };
+use godot::global::godot_str;
 use godot::meta::ToGodot;
 use godot::obj::{Base, Gd, NewAlloc, NewGd};
 use godot::private::class_macros::assert_eq_approx;
@@ -44,7 +45,7 @@ struct VirtualMethodTest {
 #[godot_api]
 impl IRefCounted for VirtualMethodTest {
     fn to_string(&self) -> GString {
-        format!("VirtualMethodTest[integer={}]", self.integer).into()
+        godot_str!("VirtualMethodTest[integer={}]", self.integer)
     }
 }
 
diff --git a/itest/rust/src/register_tests/func_virtual_test.rs b/itest/rust/src/register_tests/func_virtual_test.rs

Original file line number	Diff line number	Diff line change
`@@ -1368,7 +1368,7 @@ impl<T: ArrayElement> GodotType for Array<T> {`
`1368`	`1368`	`// Typed arrays use type hint.`
`1369`	`1369`	`PropertyHintInfo {`
`1370`	`1370`	`hint: crate::global::PropertyHint::ARRAY_TYPE,`
`1371`		`- hint_string: GString::from(element_godot_type_name::<T>()),`
	`1371`	`+ hint_string: GString::from(&element_godot_type_name::<T>()),`
`1372`	`1372`	`}`
`1373`	`1373`	`}`
`1374`	`1374`	`}`
Original file line number	Diff line number	Diff line change
`@@ -151,7 +151,7 @@ impl PropertyInfo {`
`151`	`151`	`{`
`152`	`152`	`self.variant_type == VariantType::ARRAY`
`153`	`153`	`&& self.hint_info.hint == PropertyHint::ARRAY_TYPE`
`154`		`- && self.hint_info.hint_string == T::Via::godot_type_name().into()`
	`154`	`+ && self.hint_info.hint_string == GString::from(&T::Via::godot_type_name())`
`155`	`155`	`}`
`156`	`156`
`157`	`157`	`// ------------------------------------------------------------------------------------------------------------------------------------------`
`@@ -291,7 +291,7 @@ impl PropertyHintInfo {`
`291`	`291`	`let hint_string = if sys::GdextBuild::since_api("4.3") {`
`292`	`292`	`GString::new()`
`293`	`293`	`} else {`
`294`		`- GString::from(type_name)`
	`294`	`+ GString::from(&type_name)`
`295`	`295`	`};`
`296`	`296`
`297`	`297`	`Self {`
`@@ -304,23 +304,23 @@ impl PropertyHintInfo {`
`304`	`304`	`pub fn var_array_element<T: ArrayElement>() -> Self {`
`305`	`305`	`Self {`
`306`	`306`	`hint: PropertyHint::ARRAY_TYPE,`
`307`		`- hint_string: GString::from(element_godot_type_name::<T>()),`
	`307`	`+ hint_string: GString::from(&element_godot_type_name::<T>()),`
`308`	`308`	`}`
`309`	`309`	`}`
`310`	`310`
`311`	`311`	/// Use for `#[export]` properties -- [`PROPERTY_HINT_TYPE_STRING`](PropertyHint::TYPE_STRING) with the element type string as hint string.
`312`	`312`	`pub fn export_array_element<T: ArrayElement>() -> Self {`
`313`	`313`	`Self {`
`314`	`314`	`hint: PropertyHint::TYPE_STRING,`
`315`		`- hint_string: GString::from(T::element_type_string()),`
	`315`	`+ hint_string: GString::from(&T::element_type_string()),`
`316`	`316`	`}`
`317`	`317`	`}`
`318`	`318`
`319`	`319`	/// Use for `#[export]` properties -- [`PROPERTY_HINT_TYPE_STRING`](PropertyHint::TYPE_STRING) with the element type string as hint string.
`320`	`320`	`pub fn export_packed_array_element<T: PackedArrayElement>() -> Self {`
`321`	`321`	`Self {`
`322`	`322`	`hint: PropertyHint::TYPE_STRING,`
`323`		`- hint_string: GString::from(T::element_type_string()),`
	`323`	`+ hint_string: GString::from(&T::element_type_string()),`
`324`	`324`	`}`
`325`	`325`	`}`
`326`	`326`
`@@ -349,7 +349,7 @@ impl PropertyHintInfo {`
`349`	`349`	`D: ?Sized + 'static,`
`350`	`350`	`{`
`351`	`351`	`PropertyHintInfo {`
`352`		`- hint_string: GString::from(get_dyn_property_hint_string::<T, D>()),`
	`352`	`+ hint_string: GString::from(&get_dyn_property_hint_string::<T, D>()),`
`353`	`353`	`..PropertyHintInfo::export_gd::<T>()`
`354`	`354`	`}`
`355`	`355`	`}`
Original file line number	Diff line number	Diff line change
`@@ -315,7 +315,7 @@ pub mod export_info_functions {`
`315`	`315`
`316`	`316`	`PropertyHintInfo {`
`317`	`317`	`hint: PropertyHint::RANGE,`
`318`		`- hint_string: hint_string.into(),`
	`318`	`+ hint_string: GString::from(&hint_string),`
`319`	`319`	`}`
`320`	`320`	`}`
`321`	`321`
`@@ -381,7 +381,7 @@ pub mod export_info_functions {`
`381`	`381`
`382`	`382`	`PropertyHintInfo {`
`383`	`383`	`hint: PropertyHint::ENUM,`
`384`		`- hint_string: hint_string.into(),`
	`384`	`+ hint_string: GString::from(&hint_string),`
`385`	`385`	`}`
`386`	`386`	`}`
`387`	`387`
`@@ -390,7 +390,7 @@ pub mod export_info_functions {`
`390`	`390`
`391`	`391`	`PropertyHintInfo {`
`392`	`392`	`hint: PropertyHint::EXP_EASING,`
`393`		`- hint_string: hint_string.into(),`
	`393`	`+ hint_string: GString::from(&hint_string),`
`394`	`394`	`}`
`395`	`395`	`}`
`396`	`396`
`@@ -414,7 +414,7 @@ pub mod export_info_functions {`
`414`	`414`
`415`	`415`	`PropertyHintInfo {`
`416`	`416`	`hint: PropertyHint::FLAGS,`
`417`		`- hint_string: hint_string.into(),`
	`417`	`+ hint_string: GString::from(&hint_string),`
`418`	`418`	`}`
`419`	`419`	`}`
`420`	`420`
`@@ -493,7 +493,7 @@ pub mod export_info_functions {`
`493`	`493`
`494`	`494`	`PropertyHintInfo {`
`495`	`495`	`hint: PropertyHint::TYPE_STRING,`
`496`		`- hint_string: format!("{hint_string}:{filter}").into(),`
	`496`	`+ hint_string: GString::from(&format!("{hint_string}:{filter}")),`
`497`	`497`	`}`
`498`	`498`	`}`
`499`	`499`
Original file line number	Diff line number	Diff line change
`@@ -115,7 +115,8 @@ impl CStyleEnum {`
`115`	`115`	`}`
`116`	`116`
`117`	`117`	`quote! {`
`118`		`- format!(#fmt, #(#fmt_args),*)`
	`118`	`+ // & because it is passed to GString::from().`
	`119`	`+ &format!(#fmt, #(#fmt_args),*)`
`119`	`120`	`}`
`120`	`121`	`}`
`121`	`122`