Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,8 @@ impl<T: GodotClass> Gd<T> {

/// Equivalent to [`upcast::<Object>()`][Self::upcast], but without bounds.
// Not yet public because it might need _mut/_ref overloads, and 6 upcast methods are a bit much...
pub(crate) fn upcast_object(self) -> Gd<classes::Object> {
#[doc(hidden)] // no public API, but used by #[signal].
pub fn upcast_object(self) -> Gd<classes::Object> {
self.owned_cast()
.expect("Upcast to Object failed. This is a bug; please report it.")
}
Expand Down
34 changes: 29 additions & 5 deletions godot-macros/src/class/data_models/inherent_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,17 @@ struct SignalAttr {
pub no_builder: bool,
}

// ----------------------------------------------------------------------------------------------------------------------------------------------

pub struct InherentImplAttr {
pub(crate) struct InherentImplAttr {
/// For implementation reasons, there can be a single 'primary' impl block and 0 or more 'secondary' impl blocks.
/// For now, this is controlled by a key in the 'godot_api' attribute.
pub secondary: bool,

/// When typed signal generation is explicitly disabled by the user.
pub no_typed_signals: bool,
}

// ----------------------------------------------------------------------------------------------------------------------------------------------

/// Codegen for `#[godot_api] impl MyType`
pub fn transform_inherent_impl(
meta: InherentImplAttr,
Expand Down Expand Up @@ -107,8 +110,12 @@ pub fn transform_inherent_impl(

// For each #[func] in this impl block, create one constant.
let func_name_constants = make_funcs_collection_constants(&funcs, &class_name);
let (signal_registrations, signal_symbol_types) =
make_signal_registrations(&signals, &class_name, &class_name_obj)?;
let (signal_registrations, signal_symbol_types) = make_signal_registrations(
&signals,
&class_name,
&class_name_obj,
meta.no_typed_signals,
)?;

#[cfg(feature = "codegen-full")]
let rpc_registrations = crate::class::make_rpc_registrations_fn(&class_name, &funcs);
Expand Down Expand Up @@ -206,6 +213,23 @@ pub fn transform_inherent_impl(
}
}

/* Re-enable if we allow controlling declarative macros for signals (base_field_macro, visibility_macros).
fn extract_hint_attribute(impl_block: &mut venial:: Impl) -> ParseResult<GodotApiHints> {
// #[hint(has_base_field = BOOL)]
let has_base_field;
if let Some(mut hints) = KvParser::parse_remove(&mut impl_block.attributes, "hint")? {
has_base_field = hints.handle_bool("has_base_field")?;
} else {
has_base_field = None;
}

// #[hint(class_visibility = pub(crate))]
// ...

Ok(GodotApiHints { has_base_field })
}
*/

fn process_godot_fns(
class_name: &Ident,
impl_block: &mut venial::Impl,
Expand Down
46 changes: 35 additions & 11 deletions godot-macros/src/class/data_models/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ pub fn make_signal_registrations(
signals: &[SignalDefinition],
class_name: &Ident,
class_name_obj: &TokenStream,
no_typed_signals: bool,
) -> ParseResult<(Vec<TokenStream>, Option<TokenStream>)> {
let mut signal_registrations = Vec::new();

Expand Down Expand Up @@ -210,8 +211,11 @@ pub fn make_signal_registrations(
signal_registrations.push(registration);
}

// Rewrite the above using #[cfg].
#[cfg(since_api = "4.2")]
let signal_symbols = make_signal_symbols(class_name, collection_api);
let signal_symbols =
(!no_typed_signals).then(|| make_signal_symbols(class_name, collection_api));

#[cfg(before_api = "4.2")]
let signal_symbols = None;

Expand Down Expand Up @@ -311,7 +315,7 @@ impl SignalCollection {
.push(make_signal_individual_struct(details))
}

pub fn is_empty(&self) -> bool {
fn is_empty(&self) -> bool {
self.individual_structs.is_empty()
}
}
Expand Down Expand Up @@ -380,18 +384,26 @@ fn make_signal_individual_struct(details: &SignalDetails) -> TokenStream {

/// Generates symbolic API for signals:
/// * collection (unspecified-name struct holding methods to access each signal)
/// * can be absent if the class declares no own #[signal]s.
/// * individual signal types
/// * trait impls
fn make_signal_symbols(
class_name: &Ident,
collection_api: SignalCollection,
// max_visibility: SignalVisibility,
) -> Option<TokenStream> {
// Future note: if we add Rust->Rust inheritance, then the WithSignals trait must be unconditionally implemented.
if collection_api.is_empty() {
return None;
}
) -> TokenStream {
// Earlier implementation generated a simplified code when no #[signal] was declared: only WithSignals/WithUserSignals impl, but no own
// collection, instead the associated type pointing to the base class. This has however some problems:
// * Part of the reason for user-defined collection is to store UserSignalObject instead of Gd, which can store &mut self.
// This is necessary for self.signals().some_base_signal().emit(), if such a signal is connected to Self::method_mut;
// Gd would cause a borrow error.
// * Once we add Rust-Rust inheritance, we'd need to differentiate case again, which can be tricky since #[godot_api] has no information
// about the base class.
//
// As compromise, we always generate a collection struct if either at least 1 #[signal] is declared or the struct has a Base<T> field.
// We also provide opt-out via #[godot_api(no_typed_signals)].

let declares_no_signals = collection_api.is_empty();
let collection_struct_name = format_ident!("__godot_Signals_{}", class_name);
let collection_struct_methods = &collection_api.provider_methods;
let with_signals_impl = make_with_signals_impl(class_name, &collection_struct_name);
Expand Down Expand Up @@ -431,7 +443,7 @@ fn make_signal_symbols(

let visibility_macro = util::format_class_visibility_macro(class_name);

let code = quote! {
let mut code = quote! {
#visibility_macro! {
#[allow(non_camel_case_types)]
#[doc(hidden)] // Only on struct, not methods, to allow completion in IDEs.
Expand All @@ -455,9 +467,21 @@ fn make_signal_symbols(
#( #individual_structs )*
};

Some(code)
// base_field_macro! is a macro that expands to all input tokens if the class declares a Base<T> field, and to nothing otherwise.
// This makes sure that WithSignals is only implemented for classes with a base field, and avoids compile errors about it.
// Only when no #[signal] is declared -> otherwise the user explicitly requests it, and a compile error is better to guide them.
if declares_no_signals {
let base_field_macro = util::format_class_base_field_macro(class_name);

code = quote! {
#base_field_macro! { #code }
};
}

code
}

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

#[doc(hidden)]
fn __signals_from_external(external: &mut Gd<Self>) -> Self::SignalCollection<'_, Self> {
fn __signals_from_external(external: &mut ::godot::obj::Gd<Self>) -> Self::SignalCollection<'_, Self> {
Self::SignalCollection {
__internal_obj: Some(::godot::private::UserSignalObject::External {
gd: external.clone().upcast::<Object>()
gd: external.clone().upcast_object()
})
}
}
Expand Down
22 changes: 22 additions & 0 deletions godot-macros/src/class/derive_godot_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
pub struct #funcs_collection_struct_name {}
};

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

Ok(quote! {
impl ::godot::obj::GodotClass for #class_name {
Expand Down Expand Up @@ -174,6 +176,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
#user_class_impl
#init_expecter
#visibility_macro
#base_field_macro
#( #deprecations )*
#( #errors )*

Expand Down Expand Up @@ -211,6 +214,25 @@ fn make_visibility_macro(
}
}

/// Generates code for a decl-macro, which evaluates to nothing if the class has no base field.
fn make_base_field_macro(class_name: &Ident, has_base_field: bool) -> TokenStream {
let macro_name = util::format_class_base_field_macro(class_name);

if has_base_field {
quote! {
macro_rules! #macro_name {
( $( $tt:tt )* ) => { $( $tt )* };
}
}
} else {
quote! {
macro_rules! #macro_name {
( $( $tt:tt )* ) => {};
}
}
}
}

/// Checks at compile time that a function with the given name exists on `Self`.
#[must_use]
pub fn make_existence_check(ident: &Ident) -> TokenStream {
Expand Down
13 changes: 12 additions & 1 deletion godot-macros/src/class/godot_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,20 @@ fn parse_inherent_impl_attr(meta: TokenStream) -> Result<super::InherentImplAttr
let item = venial_parse_meta(&meta, format_ident!("godot_api"), &quote! { fn func(); })?;
let mut attr = KvParser::parse_required(item.attributes(), "godot_api", &meta)?;
let secondary = attr.handle_alone("secondary")?;
let no_typed_signals = attr.handle_alone("no_typed_signals")?;
attr.finish()?;

Ok(super::InherentImplAttr { secondary })
if no_typed_signals && secondary {
return bail!(
meta,
"#[godot_api]: keys `secondary` and `no_typed_signals` are mutually exclusive; secondary blocks allow no signals anyway"
)?;
}

Ok(super::InherentImplAttr {
secondary,
no_typed_signals,
})
}

pub fn attribute_godot_api(
Expand Down
38 changes: 37 additions & 1 deletion godot-macros/src/util/kv_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,16 @@ impl KvParser {
}

/// Create a new parser which checks for presence of an `#[expected]` attribute.
///
/// Returns `Ok(None)` if the attribute is not present.
pub fn parse(attributes: &[venial::Attribute], expected: &str) -> ParseResult<Option<Self>> {
let mut found_attr: Option<Self> = None;

for attr in attributes.iter() {
let path = &attr.path;
if path_is_single(path, expected) {
if found_attr.is_some() {
return bail!(attr, "only a single #[{expected}] attribute allowed",);
return bail!(attr, "only a single #[{expected}] attribute allowed");
}

let attr_name = expected.to_string();
Expand All @@ -59,6 +61,40 @@ impl KvParser {
Ok(found_attr)
}

/// Like `parse()`, but removes the attribute from the list.
///
/// Useful for `#[proc_macro_attributes]`, where handled attributes must not show up in resulting code.
// Currently unused.
pub fn parse_remove(
attributes: &mut Vec<venial::Attribute>,
expected: &str,
) -> ParseResult<Option<Self>> {
let mut found_attr: Option<Self> = None;
let mut found_index: Option<usize> = None;

for (i, attr) in attributes.iter().enumerate() {
let path = &attr.path;
if path_is_single(path, expected) {
if found_attr.is_some() {
return bail!(attr, "only a single #[{expected}] attribute allowed");
}

let attr_name = expected.to_string();
found_index = Some(i);
found_attr = Some(Self {
span: attr.tk_brackets.span,
map: ParserState::parse(attr_name, &attr.value)?,
});
}
}

if let Some(index) = found_index {
attributes.remove(index);
}

Ok(found_attr)
}

pub fn span(&self) -> Span {
self.span
}
Expand Down
5 changes: 5 additions & 0 deletions godot-macros/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,3 +409,8 @@ pub fn format_funcs_collection_struct(class_name: &Ident) -> Ident {
pub fn format_class_visibility_macro(class_name: &Ident) -> Ident {
format_ident!("__godot_{class_name}_vis_macro")
}

/// Returns the name of the macro used to communicate whether the `struct` (class) contains a base field.
pub fn format_class_base_field_macro(class_name: &Ident) -> Ident {
format_ident!("__godot_{class_name}_has_base_field_macro")
}
20 changes: 20 additions & 0 deletions itest/rust/src/builtin_tests/containers/signal_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,24 @@ fn signal_symbols_engine_inherited_internal() {
node.free();
}

// Test that Node signals are accessible from a derived class, when the class itself has no #[signal] declarations.
// Verifies the code path that only generates the traits, no dedicated signal collection.
#[cfg(since_api = "4.2")]
#[itest]
fn signal_symbols_engine_inherited_no_own_signals() {
let mut obj = Receiver::new_alloc();

let mut sig = obj.signals().property_list_changed();
sig.connect_self(|this: &mut Receiver| {
this.receive_int(941);
});

obj.notify_property_list_changed();
assert_eq!(obj.bind().last_received.get(), LastReceived::Int(941));

obj.free();
}

#[itest]
fn signal_construction_and_id() {
let mut object = RefCounted::new_gd();
Expand Down Expand Up @@ -451,6 +469,8 @@ struct Receiver {

#[godot_api]
impl Receiver {
// Do not declare any #[signal]s here -- explicitly test this implements WithSignal without them.

fn last_received(&self) -> LastReceived {
self.last_received.get()
}
Expand Down
4 changes: 3 additions & 1 deletion itest/rust/src/object_tests/object_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,9 @@ pub mod object_test_gd {
}
use nested::ObjectTest;

#[godot_api]
// Disabling signals allows nested::ObjectTest, which would fail otherwise due to generated decl-macro being out-of-scope.
#[godot_api(no_typed_signals)]
// #[hint(has_base_field = false)] // if we allow more fine-grained control in the future
impl nested::ObjectTest {
#[func]
fn pass_object(&self, object: Gd<Object>) -> i64 {
Expand Down
Loading