Skip to content

Commit e005e0f

Browse files
committed
Always implement WithUserSignals for user classes, if Base<T> field is present
1 parent 086f7e6 commit e005e0f

File tree

5 files changed

+135
-9
lines changed

5 files changed

+135
-9
lines changed

godot-core/src/obj/gd.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,8 @@ impl<T: GodotClass> Gd<T> {
326326

327327
/// Equivalent to [`upcast::<Object>()`][Self::upcast], but without bounds.
328328
// Not yet public because it might need _mut/_ref overloads, and 6 upcast methods are a bit much...
329-
pub(crate) fn upcast_object(self) -> Gd<classes::Object> {
329+
#[doc(hidden)] // no public API, but used by #[signal].
330+
pub fn upcast_object(self) -> Gd<classes::Object> {
330331
self.owned_cast()
331332
.expect("Upcast to Object failed. This is a bug; please report it.")
332333
}

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

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ pub fn make_signal_registrations(
211211
}
212212

213213
#[cfg(since_api = "4.2")]
214-
let signal_symbols = make_signal_symbols(class_name, collection_api);
214+
let signal_symbols = Some(make_signal_symbols(class_name, collection_api));
215215
#[cfg(before_api = "4.2")]
216216
let signal_symbols = None;
217217

@@ -311,7 +311,7 @@ impl SignalCollection {
311311
.push(make_signal_individual_struct(details))
312312
}
313313

314-
pub fn is_empty(&self) -> bool {
314+
fn is_empty(&self) -> bool {
315315
self.individual_structs.is_empty()
316316
}
317317
}
@@ -380,18 +380,33 @@ fn make_signal_individual_struct(details: &SignalDetails) -> TokenStream {
380380

381381
/// Generates symbolic API for signals:
382382
/// * collection (unspecified-name struct holding methods to access each signal)
383+
/// * can be absent if the class declares no own #[signal]s.
383384
/// * individual signal types
384385
/// * trait impls
385386
fn make_signal_symbols(
386387
class_name: &Ident,
387388
collection_api: SignalCollection,
388389
// max_visibility: SignalVisibility,
389-
) -> Option<TokenStream> {
390-
// Future note: if we add Rust->Rust inheritance, then the WithSignals trait must be unconditionally implemented.
390+
) -> TokenStream {
391+
// If class declares no own signals, just implement the traits, no collection APIs.
391392
if collection_api.is_empty() {
392-
return None;
393+
let with_signals_impl = make_with_signals_impl_delegated_to_base(class_name);
394+
let base_field_macro = util::format_class_base_field_macro(class_name);
395+
396+
// base_field_macro! is a macro that expands to all input tokens if the class declares a Base<T> field, and to nothing otherwise.
397+
// This makes sure that WithSignals is only implemented for classes with a base field, and avoids compile errors about it.
398+
// Note: this doesn't work with `impl nested::MyClass` style remote impls, which can be enabled via #[hint(signals=true/false)].
399+
400+
return quote! {
401+
#base_field_macro! {
402+
#with_signals_impl
403+
}
404+
};
393405
}
394406

407+
// For the regular flow, we *expect* that a Base<T> field is present. Having a concise error message "missing Base<T> field" is
408+
// better than magic tricks, since the user explicitly asked for #[signal] and thus should be guided to complete the missing parts.
409+
395410
let collection_struct_name = format_ident!("__godot_Signals_{}", class_name);
396411
let collection_struct_methods = &collection_api.provider_methods;
397412
let with_signals_impl = make_with_signals_impl(class_name, &collection_struct_name);
@@ -455,9 +470,10 @@ fn make_signal_symbols(
455470
#( #individual_structs )*
456471
};
457472

458-
Some(code)
473+
code
459474
}
460475

476+
/// Declare `impl WithSignals` and `impl WithUserSignals` with own signal collection.
461477
fn make_with_signals_impl(class_name: &Ident, collection_struct_name: &Ident) -> TokenStream {
462478
quote! {
463479
impl ::godot::obj::WithSignals for #class_name {
@@ -467,10 +483,10 @@ fn make_with_signals_impl(class_name: &Ident, collection_struct_name: &Ident) ->
467483
type __SignalObj<'c> = ::godot::private::UserSignalObject<'c, Self>;
468484

469485
#[doc(hidden)]
470-
fn __signals_from_external(external: &mut Gd<Self>) -> Self::SignalCollection<'_, Self> {
486+
fn __signals_from_external(external: &mut ::godot::obj::Gd<Self>) -> Self::SignalCollection<'_, Self> {
471487
Self::SignalCollection {
472488
__internal_obj: Some(::godot::private::UserSignalObject::External {
473-
gd: external.clone().upcast::<Object>()
489+
gd: external.clone().upcast_object()
474490
})
475491
}
476492
}
@@ -486,6 +502,39 @@ fn make_with_signals_impl(class_name: &Ident, collection_struct_name: &Ident) ->
486502
}
487503
}
488504

505+
/// Declare `impl WithSignals` and `impl WithUserSignals`, but without own signal collection; instead delegate to base collection.
506+
fn make_with_signals_impl_delegated_to_base(class_name: &Ident) -> TokenStream {
507+
quote! {
508+
impl ::godot::obj::WithSignals for #class_name {
509+
type SignalCollection<'c, C: ::godot::obj::WithSignals> =
510+
<<Self as ::godot::obj::GodotClass>::Base as ::godot::obj::WithSignals>::SignalCollection<'c, C>;
511+
512+
#[doc(hidden)]
513+
type __SignalObj<'c> =
514+
<<Self as ::godot::obj::GodotClass>::Base as ::godot::obj::WithSignals>::__SignalObj<'c>;
515+
516+
517+
#[doc(hidden)]
518+
fn __signals_from_external(external: &mut ::godot::obj::Gd<Self>) -> Self::SignalCollection<'_, Self> {
519+
// This will need updating when allowing Rust->Rust inheritance.
520+
Self::SignalCollection {
521+
__internal_obj: Some(external.clone().upcast())
522+
}
523+
}
524+
}
525+
526+
impl ::godot::obj::WithUserSignals for #class_name {
527+
fn signals(&mut self) -> Self::SignalCollection<'_, Self> {
528+
// This will need updating when allowing Rust->Rust inheritance.
529+
let self_gd = <Self as ::godot::obj::WithBaseField>::to_gd(self);
530+
Self::SignalCollection {
531+
__internal_obj: Some(self_gd.upcast()),
532+
}
533+
}
534+
}
535+
}
536+
}
537+
489538
fn make_upcast_deref_impl(class_name: &Ident, collection_struct_name: &Ident) -> TokenStream {
490539
quote! {
491540
impl<'c, C: ::godot::obj::WithSignals> std::ops::Deref for #collection_struct_name<'c, C> {

godot-macros/src/class/derive_godot_class.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,9 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
142142
pub struct #funcs_collection_struct_name {}
143143
};
144144

145+
// Note: one limitation is that macros don't work for `impl nested::MyClass` blocks.
145146
let visibility_macro = make_visibility_macro(class_name, class.vis_marker.as_ref());
147+
let base_field_macro = make_base_field_macro(class_name, fields.base_field.is_some());
146148

147149
Ok(quote! {
148150
impl ::godot::obj::GodotClass for #class_name {
@@ -174,6 +176,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
174176
#user_class_impl
175177
#init_expecter
176178
#visibility_macro
179+
#base_field_macro
177180
#( #deprecations )*
178181
#( #errors )*
179182

@@ -211,6 +214,25 @@ fn make_visibility_macro(
211214
}
212215
}
213216

217+
/// Generates code for a decl-macro, which evaluates to nothing if the class has no base field.
218+
fn make_base_field_macro(class_name: &Ident, has_base_field: bool) -> TokenStream {
219+
let macro_name = util::format_class_base_field_macro(class_name);
220+
221+
if has_base_field {
222+
quote! {
223+
macro_rules! #macro_name {
224+
( $( $tt:tt )* ) => { $( $tt )* };
225+
}
226+
}
227+
} else {
228+
quote! {
229+
macro_rules! #macro_name {
230+
( $( $tt:tt )* ) => {};
231+
}
232+
}
233+
}
234+
}
235+
214236
/// Checks at compile time that a function with the given name exists on `Self`.
215237
#[must_use]
216238
pub fn make_existence_check(ident: &Ident) -> TokenStream {

godot-macros/src/util/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,3 +409,8 @@ pub fn format_funcs_collection_struct(class_name: &Ident) -> Ident {
409409
pub fn format_class_visibility_macro(class_name: &Ident) -> Ident {
410410
format_ident!("__godot_{class_name}_vis_macro")
411411
}
412+
413+
/// Returns the name of the macro used to communicate whether the `struct` (class) contains a base field.
414+
pub fn format_class_base_field_macro(class_name: &Ident) -> Ident {
415+
format_ident!("__godot_{class_name}_has_base_field_macro")
416+
}

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,53 @@ fn signal_symbols_engine_inherited_internal() {
327327
node.free();
328328
}
329329

330+
// Test that Node signals are accessible from a derived class, when the class itself has no #[signal] declarations.
331+
// Verifies the code path that only generates the traits, no dedicated signal collection.
332+
#[cfg(since_api = "4.2")]
333+
#[itest]
334+
fn signal_symbols_engine_inherited_no_own_signals(ctx: &crate::framework::TestContext) {
335+
let mut node = Receiver::new_alloc();
336+
337+
// Add to tree, so signals are propagated.
338+
ctx.scene_tree.clone().add_child(&node);
339+
340+
let mut sig = node.signals().renamed();
341+
sig.connect_self(|this: &mut Emitter| {
342+
this.last_received_int = 887;
343+
});
344+
345+
node.set_name("new name");
346+
347+
assert_eq!(node.bind().last_received_int, 887);
348+
349+
// Remove from tree for other tests.
350+
node.free();
351+
}
352+
353+
// Test that trait is implemented even without own #[signal] declarations (to access base signals).
354+
#[cfg(since_api = "4.2")]
355+
const fn __type_check<'c>() {
356+
// Needs WithUserSignals, not just WithSignals, to allow self.signals().
357+
const fn has_with_user_signals<T: godot::obj::WithUserSignals>() {}
358+
359+
trait SameType {}
360+
impl<T> SameType for (T, T) {}
361+
const fn is_same_type<T, U>()
362+
where
363+
(T, U): SameType,
364+
{
365+
}
366+
367+
has_with_user_signals::<Receiver>();
368+
369+
// Checks whether there is no new collection defined, but instead the base collection is reused.
370+
// This reduces the amount of proc-macro generated code.
371+
is_same_type::<
372+
Receiver::SignalCollection<'c, Receiver>,
373+
Object::SignalCollection<'c, Receiver>, //.
374+
>();
375+
}
376+
330377
#[itest]
331378
fn signal_construction_and_id() {
332379
let mut object = RefCounted::new_gd();
@@ -451,6 +498,8 @@ struct Receiver {
451498

452499
#[godot_api]
453500
impl Receiver {
501+
// Do not declare any #[signal]s here -- explicitly test this implements WithSignal without them.
502+
454503
fn last_received(&self) -> LastReceived {
455504
self.last_received.get()
456505
}

0 commit comments

Comments
 (0)