Merge pull request #477 from godot-rust/qol/rename-get-prefixes

Strip `get_` prefixes from engine getters

Author: Bromeon

examples/dodge-the-creeps/rust/src/main_scene.rs

@@ -45,7 +45,7 @@ impl Main {
 
         self.score = 0;
 
-        player.bind_mut().start(start_position.get_position());
+        player.bind_mut().start(start_position.position());
         start_timer.start();
 
         let mut hud = self.base.get_node_as::<Hud>("Hud");
@@ -84,9 +84,9 @@ impl Main {
         let progress = rng.gen_range(u32::MIN..u32::MAX);
 
         mob_spawn_location.set_progress(progress as f32);
-        mob_scene.set_position(mob_spawn_location.get_position());
+        mob_scene.set_position(mob_spawn_location.position());
 
-        let mut direction = mob_spawn_location.get_rotation() + PI / 2.0;
+        let mut direction = mob_spawn_location.rotation() + PI / 2.0;
         direction += rng.gen_range(-PI / 4.0..PI / 4.0);
 
         mob_scene.set_rotation(direction);
@@ -101,7 +101,7 @@ impl Main {
         };
 
         mob.set_linear_velocity(Vector2::new(range, 0.0));
-        let lin_vel = mob.get_linear_velocity().rotated(real::from_f32(direction));
+        let lin_vel = mob.linear_velocity().rotated(real::from_f32(direction));
         mob.set_linear_velocity(lin_vel);
 
         let mut hud = self.base.get_node_as::<Hud>("Hud");

examples/dodge-the-creeps/rust/src/mob.rs

@@ -41,7 +41,7 @@ impl IRigidBody2D for Mob {
             .get_node_as::<AnimatedSprite2D>("AnimatedSprite2D");
 
         sprite.play();
-        let anim_names = sprite.get_sprite_frames().unwrap().get_animation_names();
+        let anim_names = sprite.sprite_frames().unwrap().animation_names();
 
         // TODO use pick_random() once implemented
         let anim_names = anim_names.to_vec();

examples/dodge-the-creeps/rust/src/player.rs

@@ -52,7 +52,7 @@ impl IArea2D for Player {
     }
 
     fn ready(&mut self) {
-        let viewport = self.base.get_viewport_rect();
+        let viewport = self.base.viewport_rect();
         self.screen_size = viewport.size;
         self.base.hide();
     }
@@ -101,7 +101,7 @@ impl IArea2D for Player {
         }
 
         let change = velocity * real::from_f64(delta);
-        let position = self.base.get_global_position() + change;
+        let position = self.base.global_position() + change;
         let position = Vector2::new(
             position.x.clamp(0.0, self.screen_size.x),
             position.y.clamp(0.0, self.screen_size.y),

godot-codegen/src/class_generator.rs

@@ -8,6 +8,7 @@
 
 use proc_macro2::{Ident, TokenStream};
 use quote::{format_ident, quote};
+use std::borrow::Cow;
 use std::path::Path;
 
 use crate::api_parser::*;
@@ -1125,22 +1126,34 @@ fn make_class_method_definition(
     if special_cases::is_deleted(class_name, method, ctx) {
         return FnDefinition::none();
     }
-    /*if method.map_args(|args| args.is_empty()) {
-        // Getters (i.e. 0 arguments) will be stripped of their `get_` prefix, to conform to Rust convention
-        if let Some(remainder) = method_name.strip_prefix("get_") {
-            // TODO Do not apply for FileAccess::get_16, StreamPeer::get_u16, etc
-            if !remainder.chars().nth(0).unwrap().is_ascii_digit() {
-                method_name = remainder;
+
+    let class_name_str = &class_name.godot_ty;
+    let godot_method_name = &method.name;
+    let renamed_method_name = special_cases::maybe_renamed(class_name, godot_method_name);
+
+    let mut rust_method_name = Cow::Borrowed(renamed_method_name);
+    let mut override_is_const = None;
+
+    if method.map_args(|args| args.is_empty()) {
+        // Getters (i.e. 0 arguments) are stripped of their `get_` prefix, to conform to Rust convention.
+        // Currently also applies to static methods, but NOT to methods which have default parameters but can be called with 0 arguments.
+        // TODO(bromeon): should we add #[doc(alias)]?
+        if let Some(remainder) = renamed_method_name.strip_prefix("get_") {
+            // Do not apply for FileAccess::get_16, StreamPeer::get_u16, etc.
+            if !special_cases::keeps_get_prefix(class_name, method) {
+                rust_method_name = Cow::Owned(remainder.to_string());
+
+                // Many getters are mutably qualified (GltfAccessor::get_max, CameraAttributes::get_exposure_multiplier, ...).
+                override_is_const = Some(true);
             }
         }
-    }*/
+    }
 
-    let class_name_str = &class_name.godot_ty;
-    let method_name_str = special_cases::maybe_renamed(class_name, &method.name);
+    let rust_method_name = rust_method_name.as_ref();
 
     let receiver = make_receiver(
         method.is_static,
-        method.is_const,
+        override_is_const.unwrap_or(method.is_const),
         quote! { self.object_ptr },
     );
 
@@ -1161,7 +1174,7 @@ fn make_class_method_definition(
         quote! {
             fptr_by_key(sys::lazy_keys::ClassMethodKey {
                 class_name: #class_name_str,
-                method_name: #method_name_str,
+                method_name: #godot_method_name,
                 hash: #hash,
             })
         }
@@ -1175,7 +1188,7 @@ fn make_class_method_definition(
 
         <CallSig as PtrcallSignatureTuple>::out_class_ptrcall::<RetMarshal>(
             method_bind,
-            #method_name_str,
+            #rust_method_name,
             #object_ptr,
             #maybe_instance_id,
             args,
@@ -1187,7 +1200,7 @@ fn make_class_method_definition(
 
         <CallSig as VarcallSignatureTuple>::out_class_varcall(
             method_bind,
-            #method_name_str,
+            #rust_method_name,
             #object_ptr,
             #maybe_instance_id,
             args,
@@ -1197,9 +1210,9 @@ fn make_class_method_definition(
 
     make_function_definition(
         &FnSignature {
-            function_name: method_name_str,
+            function_name: rust_method_name,
             surrounding_class: Some(class_name),
-            is_private: special_cases::is_private(class_name, &method.name),
+            is_private: special_cases::is_private(class_name, godot_method_name),
             is_virtual: false,
             is_vararg: method.is_vararg,
             qualifier: FnQualifier::for_method(method.is_const, method.is_static),

godot-codegen/src/special_cases.rs

@@ -176,6 +176,69 @@ pub(crate) fn is_excluded_from_default_params(class_name: Option<&TyName>, godot
     }
 }
 
+#[rustfmt::skip]
+pub(crate) fn keeps_get_prefix(class_name: &TyName, method: &ClassMethod) -> bool {
+    // Also list those which have default parameters and can be called with 0 arguments. Those are anyway
+    // excluded at the moment, but this is more robust if the outer logic changes.
+
+    match (class_name.godot_ty.as_str(), method.name.as_str()) {
+        // For Object
+        // https://docs.godotengine.org/en/stable/classes/class_object.html#methods
+        | ("Object", "get_class")
+        | ("Object", "get_instance_id") // currently removed, but would be shadowed by Gd::instance_id().
+        | ("Object", "get_script")
+        | ("Object", "get_script_instance")
+        // The following ones often have slight variations with parameters, so it's more consistent to have get_signal_list() and
+        // get_signal_connection_list(signal). This may change in the future.
+        | ("Object", "get_incoming_connections")
+        | ("Object", "get_meta_list")
+        | ("Object", "get_method_list")
+        | ("Object", "get_property_list")
+        | ("Object", "get_signal_list")
+
+        // For Node
+        // https://docs.godotengine.org/en/stable/classes/class_node.html#methods
+        // TODO get_child_count?
+
+        // https://docs.godotengine.org/en/stable/classes/class_fileaccess.html#methods
+        | ("FileAccess", "get_16")
+        | ("FileAccess", "get_32")
+        | ("FileAccess", "get_64")
+        | ("FileAccess", "get_8")
+        | ("FileAccess", "get_as_text")
+        | ("FileAccess", "get_csv_line")
+        | ("FileAccess", "get_double")
+        | ("FileAccess", "get_error") // If this has side effects, should definitely keep prefix. Not clear.
+        | ("FileAccess", "get_float")
+        | ("FileAccess", "get_line")
+        | ("FileAccess", "get_open_error")
+        | ("FileAccess", "get_pascal_string")
+        | ("FileAccess", "get_real")
+        | ("FileAccess", "get_var")
+
+        // https://docs.godotengine.org/en/stable/classes/class_streampeer.html#methods
+        // do for 8,16,32,64 and u*
+        | ("StreamPeer", "get_16")
+        | ("StreamPeer", "get_32")
+        | ("StreamPeer", "get_64")
+        | ("StreamPeer", "get_8")
+        | ("StreamPeer", "get_double")
+        | ("StreamPeer", "get_float")
+        | ("StreamPeer", "get_string")
+        | ("StreamPeer", "get_u16")
+        | ("StreamPeer", "get_u32")
+        | ("StreamPeer", "get_u64")
+        | ("StreamPeer", "get_u8")
+        | ("StreamPeer", "get_utf8_string")
+        | ("StreamPeer", "get_var")
+
+        // Others that conflict with a verb:
+        | ("AnimationPlayer", "get_queue")
+
+        => true, _ => false,
+    }
+}
+
 /// True if builtin method is excluded. Does NOT check for type exclusion; use [`is_builtin_type_deleted`] for that.
 pub(crate) fn is_builtin_deleted(_class_name: &TyName, method: &BuiltinClassMethod) -> bool {
     // Currently only deleted if codegen.

godot-core/src/obj/traits.rs

@@ -106,7 +106,7 @@ pub trait Share {
 ///     T: Inherits<Node>,
 /// {
 ///     let up = node.upcast(); // type Gd<Node> inferred
-///     println!("Node #{} with name {}", up.instance_id(), up.get_name());
+///     println!("Node #{} with name {}", up.instance_id(), up.name());
 ///     up.free();
 /// }
 ///

itest/godot/SpecialTests.gd

@@ -33,7 +33,7 @@ func test_collision_object_2d_input_event():
 	window.add_child(collision_object)
 
 	assert_that(not collision_object.input_event_called())
-	assert_eq(collision_object.get_viewport(), null)
+	assert_eq(collision_object.viewport(), null)
 
 	var event := InputEventMouseMotion.new()
 	event.global_position = Vector2.ZERO
@@ -43,7 +43,7 @@ func test_collision_object_2d_input_event():
 	await root.get_tree().physics_frame
 
 	assert_that(collision_object.input_event_called())
-	assert_eq(collision_object.get_viewport(), window)
+	assert_eq(collision_object.viewport(), window)
 
 	window.queue_free()
 

itest/rust/src/builtin_tests/containers/array_test.rs

@@ -401,7 +401,7 @@ fn typed_array_pass_to_godot_func() {
     let error = texture.create_from_images(images);
 
     assert_eq!(error, Error::OK);
-    assert_eq!((texture.get_width(), texture.get_height()), (2, 4));
+    assert_eq!((texture.width(), texture.height()), (2, 4));
 }
 
 #[itest]

itest/rust/src/builtin_tests/containers/callable_test.rs

@@ -120,7 +120,7 @@ fn callable_call_engine() {
     // TODO once varargs is available
     // let pos = Vector2::new(5.0, 7.0);
     // inner.call(&[pos.to_variant()]);
-    // assert_eq!(obj.get_position(), pos);
+    // assert_eq!(obj.position(), pos);
     //
     // inner.bindv(array);
 

itest/rust/src/engine_tests/node_test.rs

@@ -52,7 +52,7 @@ fn node_get_node_fail() {
 fn node_path_from_str(ctx: &TestContext) {
     let child = ctx.scene_tree.clone();
     assert_eq!(
-        child.get_path().to_string(),
+        child.path().to_string(),
         NodePath::from_str("/root/TestRunner").unwrap().to_string()
     );
 }
@@ -84,7 +84,7 @@ fn node_scene_tree() {
 #[itest]
 fn node_call_group(ctx: &TestContext) {
     let mut node = ctx.scene_tree.clone();
-    let mut tree = node.get_tree().unwrap();
+    let mut tree = node.tree().unwrap();
 
     node.add_to_group("group".into());
     tree.call_group("group".into(), "set_name".into(), &[Variant::from("name")]);