Skip to content

Commit f38eb34

Browse files
committed
Virtual dispatch: fix incorrect matching of renamed virtual methods
A handful of virtual methods are renamed during codegen, however #[godot_api] always matches against the Rust name (with added `_` prefix). Now stores the Godot names in the same global constants as the hashes.
1 parent 6e0f41c commit f38eb34

File tree

3 files changed

+21
-17
lines changed

3 files changed

+21
-17
lines changed

godot-codegen/src/generator/virtual_hashes.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,10 @@ fn make_virtual_hashes_for_class(class: &Class, ctx: &mut Context) -> TokenStrea
4343
return None;
4444
};
4545

46-
let method_name = method.name_ident();
46+
let rust_name = method.name_ident();
47+
let godot_name_str = method.godot_name();
4748
let constant = quote! {
48-
pub const #method_name: u32 = #hash;
49+
pub const #rust_name: (&'static str, u32) = (#godot_name_str, #hash);
4950
};
5051

5152
Some(constant)

godot-macros/src/class/data_models/interface_trait_impl.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,14 @@ pub fn transform_trait_impl(mut original_impl: venial::Impl) -> ParseResult<Toke
102102
&& !decls
103103
.overridden_virtuals
104104
.iter()
105-
.any(|v| v.method_name == "_ready")
105+
.any(|v| v.rust_method_name == "_ready")
106106
{
107107
let match_arm = OverriddenVirtualFn {
108108
cfg_attrs: vec![],
109-
method_name: "_ready".to_string(),
109+
rust_method_name: "_ready".to_string(),
110110
// Can't use `hashes::ready` here, as the base class might not be `Node` (see above why such a branch is still added).
111111
#[cfg(since_api = "4.4")]
112-
hash_constant: quote! { ::godot::sys::known_virtual_hashes::Node::ready },
112+
godot_name_hash_constant: quote! { ::godot::sys::known_virtual_hashes::Node::ready },
113113
signature_info: SignatureInfo::fn_ready(),
114114
before_kind: BeforeKind::OnlyBefore,
115115
interface_trait: None,
@@ -518,12 +518,12 @@ fn handle_regular_virtual_fn<'a>(
518518
// each distinct method.
519519
decls.overridden_virtuals.push(OverriddenVirtualFn {
520520
cfg_attrs,
521-
method_name: virtual_method_name,
521+
rust_method_name: virtual_method_name,
522522
// If ever the `I*` verbatim validation is relaxed (it won't work with use-renames or other weird edge cases), the approach
523523
// with known_virtual_hashes module could be changed to something like the following (GodotBase = nearest Godot base class):
524524
// __get_virtual_hash::<Self::GodotBase>("method")
525525
#[cfg(since_api = "4.4")]
526-
hash_constant: quote! { hashes::#method_name_ident },
526+
godot_name_hash_constant: quote! { hashes::#method_name_ident },
527527
signature_info,
528528
before_kind,
529529
interface_trait: Some(trait_path.clone()),
@@ -573,9 +573,10 @@ fn make_inactive_class_check(_return_value: TokenStream) -> TokenStream {
573573

574574
struct OverriddenVirtualFn<'a> {
575575
cfg_attrs: Vec<&'a venial::Attribute>,
576-
method_name: String,
576+
rust_method_name: String,
577+
/// Path to a pre-defined constant storing a `("_virtual_func", 123456789)` tuple with name and hash of the virtual function.
577578
#[cfg(since_api = "4.4")]
578-
hash_constant: TokenStream,
579+
godot_name_hash_constant: TokenStream,
579580
signature_info: SignatureInfo,
580581
before_kind: BeforeKind,
581582
interface_trait: Option<venial::TypeExpr>,
@@ -584,16 +585,16 @@ struct OverriddenVirtualFn<'a> {
584585
impl OverriddenVirtualFn<'_> {
585586
fn make_match_arm(&self, class_name: &Ident) -> TokenStream {
586587
let cfg_attrs = self.cfg_attrs.iter();
587-
let method_name_str = self.method_name.as_str();
588588

589589
#[cfg(since_api = "4.4")]
590590
let pattern = {
591-
let hash_constant = &self.hash_constant;
592-
quote! { (#method_name_str, #hash_constant) }
591+
let godot_name_hash_constant = &self.godot_name_hash_constant;
592+
quote! { #godot_name_hash_constant }
593593
};
594594

595+
// Note: this is wrong before 4.4, as some methods are renamed in Rust.
595596
#[cfg(before_api = "4.4")]
596-
let pattern = method_name_str;
597+
let pattern = self.rust_method_name.as_str();
597598

598599
// Lazily generate code for the actual work (calling user function).
599600
let method_callback = make_virtual_callback(

godot-macros/src/class/derive_godot_class.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,13 +400,15 @@ fn make_user_class_impl(
400400
// See also __virtual_call() codegen.
401401
// This doesn't explicitly check if the base class inherits from Node (and thus has `_ready`), but the derive-macro already does
402402
// this for the `OnReady` field declaration.
403-
let (hash_param, hash_check);
403+
let (hash_param, matches_ready_hash);
404404
if cfg!(since_api = "4.4") {
405405
hash_param = quote! { hash: u32, };
406-
hash_check = quote! { && hash == ::godot::sys::known_virtual_hashes::Node::ready };
406+
matches_ready_hash = quote! {
407+
(name, hash) == ::godot::sys::known_virtual_hashes::Node::ready
408+
};
407409
} else {
408410
hash_param = TokenStream::new();
409-
hash_check = TokenStream::new();
411+
matches_ready_hash = quote! { name == "_ready" }
410412
}
411413

412414
let default_virtual_fn = quote! {
@@ -417,7 +419,7 @@ fn make_user_class_impl(
417419
use ::godot::obj::UserClass as _;
418420
#tool_check
419421

420-
if name == "_ready" #hash_check {
422+
if #matches_ready_hash {
421423
#callback
422424
} else {
423425
None

0 commit comments

Comments
 (0)