Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#[base] attribute is no longer necessary #577

Merged
merged 4 commits into from
Jan 25, 2024
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
1 change: 0 additions & 1 deletion examples/dodge-the-creeps/rust/src/hud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use godot::prelude::*;
#[derive(GodotClass)]
#[class(base=CanvasLayer)]
pub struct Hud {
#[base]
base: Base<CanvasLayer>,
}

Expand Down
1 change: 0 additions & 1 deletion examples/dodge-the-creeps/rust/src/main_scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ pub struct Main {
music: Option<Gd<AudioStreamPlayer>>,
death_sound: Option<Gd<AudioStreamPlayer>>,
score: i64,
#[base]
base: Base<Node>,
}

Expand Down
1 change: 0 additions & 1 deletion examples/dodge-the-creeps/rust/src/mob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ pub struct Mob {
pub min_speed: real,
pub max_speed: real,

#[base]
base: Base<RigidBody2D>,
}

Expand Down
1 change: 0 additions & 1 deletion examples/dodge-the-creeps/rust/src/player.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ pub struct Player {
speed: real,
screen_size: Vector2,

#[base]
base: Base<Area2D>,
}

Expand Down
2 changes: 1 addition & 1 deletion godot-codegen/src/generator/virtual_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ fn special_virtual_methods(notification_enum_name: &Ident) -> TokenStream {

/// Godot constructor, accepting an injected `base` object.
///
/// `base` refers to the base instance of the class, which can either be stored in a `#[base]` field or discarded.
/// `base` refers to the base instance of the class, which can either be stored in a `Base<T>` field or discarded.
/// This method returns a fully-constructed instance, which will then be moved into a [`Gd<T>`][crate::obj::Gd] pointer.
///
/// If the class has a `#[class(init)]` attribute, this method will be auto-generated and must not be overridden.
Expand Down
23 changes: 23 additions & 0 deletions godot-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,29 @@ pub mod private {

/// Ensure `T` is an editor plugin.
pub const fn is_editor_plugin<T: crate::obj::Inherits<crate::engine::EditorPlugin>>() {}

// ------------------------------------------------------------------------------------------------------------------------------------------
// Compatibility

// Code generated by Rust derive macros cannot cause any deprecation warnings, due to questionable "feature"
// https://github.com/rust-lang/rust/pull/58994. Fortunately, an extra layer of indirection solves most problems: we generate a declarative
// macro that itself isn't deprecated, but _its_ expansion is. Since the expansion happens in a later step, the warning is emitted.

#[doc(hidden)]
#[inline(always)]
#[deprecated = "#[base] is no longer needed; Base<T> is recognized directly. \n\
More information on https://github.com/godot-rust/gdext/pull/577."]
pub const fn base_attribute_warning() {}

#[doc(hidden)]
#[macro_export]
macro_rules! __base_attribute_warning_expand {
() => {
const _: () = $crate::private::base_attribute_warning();
};
}

pub use crate::__base_attribute_warning_expand;
}

macro_rules! generate_gdextension_api_version {
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/obj/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<T: GodotClass> Base<T> {
// This obj does not contribute to the strong count, otherwise we create a reference cycle:
// 1. RefCounted (dropped in GDScript)
// 2. holds user T (via extension instance and storage)
// 3. holds #[base] RefCounted (last ref, dropped in T destructor, but T is never destroyed because this ref keeps storage alive)
// 3. holds Base<T> RefCounted (last ref, dropped in T destructor, but T is never destroyed because this ref keeps storage alive)
// Note that if late-init never happened on self, we have the same behavior (still a raw pointer instead of weak Gd)
Base::from_obj(obj)
}
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/obj/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub(super) mod private {
/// <strong>Never</strong> implement this trait manually.
/// </div>
///
/// Most of the time, this trait is covered by [`#[derive(GodotClass)`](../bind/derive.GodotClass.html).
/// Most of the time, this trait is covered by [`#[derive(GodotClass)]`](../register/derive.GodotClass.html).
/// If you implement `GodotClass` manually, use the [`implement_godot_bounds!`][crate::implement_godot_bounds] macro.
///
/// There are two reasons to avoid a hand-written `impl Bounds`:
Expand Down
7 changes: 3 additions & 4 deletions godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ use crate::{callbacks, engine, out};
///
/// * [`Gd::default()`] for reference-counted types that are constructible. For user types, this means they must expose an `init` function
/// or have a generated one. `Gd::<T>::default()` is equivalent to the shorter `T::new_gd()` and primarily useful for derives or generics.
/// * [`Gd::from_init_fn(function)`][Gd::from_init_fn] for Rust objects with `#[base]` field, which are constructed inside the smart pointer.
/// * [`Gd::from_init_fn(function)`][Gd::from_init_fn] for Rust objects with `Base<T>` field, which are constructed inside the smart pointer.
/// This is a very handy function if you want to pass extra parameters to your object upon construction.
/// * [`Gd::from_object(rust_obj)`][Gd::from_object] for existing Rust objects without a `#[base]` field that are moved _into_ the smart pointer.
/// * [`Gd::from_object(rust_obj)`][Gd::from_object] for existing Rust objects without a `Base<T>` field that are moved _into_ the smart pointer.
/// * [`Gd::from_instance_id(id)`][Gd::from_instance_id] and [`Gd::try_from_instance_id(id)`][Gd::try_from_instance_id]
/// to obtain a pointer to an object which is already alive in the engine.
///
Expand Down Expand Up @@ -108,7 +108,7 @@ where
{
/// Creates a `Gd<T>` using a function that constructs a `T` from a provided base.
///
/// Imagine you have a type `T`, which has a `#[base]` field that you cannot default-initialize.
/// Imagine you have a type `T`, which has a base field that you cannot default-initialize.
/// The `init` function provides you with a `Base<T::Base>` object that you can use inside your `T`, which
/// is then wrapped in a `Gd<T>`.
///
Expand All @@ -118,7 +118,6 @@ where
/// #[derive(GodotClass)]
/// #[class(init, base=Node2D)]
/// struct MyClass {
/// #[base]
/// my_base: Base<Node2D>,
/// other_field: i32,
/// }
Expand Down
10 changes: 4 additions & 6 deletions godot-core/src/obj/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ use godot_ffi as sys;
/// Makes `T` eligible to be managed by Godot and stored in [`Gd<T>`][crate::obj::Gd] pointers.
///
/// The behavior of types implementing this trait is influenced by the associated types; check their documentation for information.
///
/// Normally, you don't need to implement this trait yourself; use [`#[derive(GodotClass)]`](../register/derive.GodotClass.html) instead.
// Above intra-doc link to the derive-macro only works as HTML, not as symbol link.
pub trait GodotClass: Bounds + 'static
where
Expand Down Expand Up @@ -208,7 +210,7 @@ pub trait IndexEnum: EngineEnum {
}
}

/// Trait that's implemented for user-defined classes that provide a `#[base]` field.
/// Trait that's implemented for user-defined classes that provide a `Base<T>` field.
///
/// Gives direct access to the containing `Gd<Self>` from `Self`.
// Possible alternative for builder APIs, although even less ergonomic: Base<T> could be Base<T, Self> and return Gd<Self>.
Expand All @@ -232,7 +234,6 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
/// #[derive(GodotClass)]
/// #[class(init, base = Node)]
/// struct MyClass {
/// #[base]
/// base: Base<Node>,
/// }
///
Expand All @@ -259,8 +260,7 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
/// #[derive(GodotClass)]
/// #[class(init, base = Node)]
/// struct MyClass {
/// #[base]
/// base: Base<Node>,
/// /// base: Base<Node>,
/// }
///
/// #[godot_api]
Expand Down Expand Up @@ -298,7 +298,6 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
/// #[derive(GodotClass)]
/// #[class(init, base = Node)]
/// struct MyClass {
/// #[base]
/// base: Base<Node>,
/// }
///
Expand All @@ -324,7 +323,6 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
/// #[derive(GodotClass)]
/// #[class(init, base = Node)]
/// struct MyClass {
/// #[base]
/// base: Base<Node>,
/// }
///
Expand Down
5 changes: 4 additions & 1 deletion godot-macros/src/class/data_models/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ pub struct Fields {
/// All fields except `base_field`.
pub all_fields: Vec<Field>,

/// The field annotated with `#[base]`.
/// The field with type `Base<T>`, if available.
pub base_field: Option<Field>,

/// Whether a deprecated `#[base]` was used.
pub has_deprecated_base: bool,
}
92 changes: 75 additions & 17 deletions godot-macros/src/class/derive_godot_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult<TokenStream> {
quote! {}
};

let deprecated_base_warning = if fields.has_deprecated_base {
quote! { ::godot::private::__base_attribute_warning_expand!(); }
} else {
TokenStream::new()
};

let (user_class_impl, has_default_virtual) =
make_user_class_impl(class_name, struct_cfg.is_tool, &fields.all_fields);

Expand Down Expand Up @@ -134,6 +140,7 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult<TokenStream> {
});

#prv::class_macros::#inherits_macro!(#class_name);
#deprecated_base_warning
})
}

Expand Down Expand Up @@ -228,8 +235,14 @@ fn parse_struct_attributes(class: &Struct) -> ParseResult<ClassAttributes> {
is_tool = true;
}

// TODO: better error message when using in Godot 4.0
if parser.handle_alone_ident("editor_plugin")?.is_some() {
if let Some(attr_key) = parser.handle_alone_ident("editor_plugin")? {
if cfg!(before_api = "4.1") {
return bail!(
attr_key,
"#[class(editor_plugin)] is not supported in Godot 4.0"
);
}

is_editor_plugin = true;
}

Expand All @@ -256,6 +269,7 @@ fn parse_struct_attributes(class: &Struct) -> ParseResult<ClassAttributes> {
fn parse_fields(class: &Struct) -> ParseResult<Fields> {
let mut all_fields = vec![];
let mut base_field = Option::<Field>::None;
let mut has_deprecated_base = false;

let named_fields: Vec<(NamedField, Punct)> = match &class.fields {
StructFields::Unit => {
Expand All @@ -273,17 +287,15 @@ fn parse_fields(class: &Struct) -> ParseResult<Fields> {
let mut is_base = false;
let mut field = Field::new(&named_field);

// #[base]
if let Some(parser) = KvParser::parse(&named_field.attributes, "base")? {
if let Some(prev_base) = base_field.as_ref() {
return bail!(
parser.span(),
"#[base] allowed for at most 1 field, already applied to `{}`",
prev_base.name
);
}
// Base<T> type inference
if path_ends_with_complex(&field.ty, "Base") {
is_base = true;
}

// deprecated #[base]
if KvParser::parse(&named_field.attributes, "base")?.is_some() {
has_deprecated_base = true;
is_base = true;
parser.finish()?;
}

// OnReady<T> type inference
Expand Down Expand Up @@ -312,23 +324,69 @@ fn parse_fields(class: &Struct) -> ParseResult<Fields> {
parser.finish()?;
}

// Exported or Rust-only fields
// #[hint] to override type inference (must be at the end).
if let Some(mut parser) = KvParser::parse(&named_field.attributes, "hint")? {
if let Some(override_base) = handle_opposite_keys(&mut parser, "base")? {
is_base = override_base;
}

if let Some(override_onready) = handle_opposite_keys(&mut parser, "onready")? {
field.is_onready = override_onready;
}
parser.finish()?;
}

// Extra validation; eventually assign to base_fields or all_fields.
if is_base {
base_field = Some(field);
if field.is_onready
|| field.var.is_some()
|| field.export.is_some()
|| field.default.is_some()
{
return bail!(
&named_field,
"base field cannot have type `OnReady<T>` or attributes #[var], #[export] or #[init]"
);
}

if let Some(prev_base) = base_field.replace(field) {
// Ensure at most one Base<T>.
return bail!(
// base_field.unwrap().name,
named_field,
"at most 1 field can have type Base<T>; previous is `{}`",
prev_base.name
);
}
} else {
all_fields.push(field);
}
}

// TODO detect #[base] based on type instead of attribute
// Edge cases (type aliases, user types with same name, ...) could be handled with #[hint(base)] or #[hint(no_base)].

Ok(Fields {
all_fields,
base_field,
has_deprecated_base,
})
}

fn handle_opposite_keys(parser: &mut KvParser, key: &str) -> ParseResult<Option<bool>> {
let antikey = format!("no_{}", key);

let is_key = parser.handle_alone(key)?;
let is_no_key = parser.handle_alone(&antikey)?;

match (is_key, is_no_key) {
(true, false) => Ok(Some(true)),
(false, true) => Ok(Some(false)),
(false, false) => Ok(None),
(true, true) => bail!(
parser.span(),
"#[hint] attribute keys `{key}` and `{antikey}` are mutually exclusive",
),
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
// General helpers

Expand Down
Loading
Loading