From e8f25ee73649ba86aa77b9bd8f12848fedade395 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 25 Nov 2023 15:02:37 +0100 Subject: [PATCH 1/8] Improve Cargo feature docs --- godot/src/lib.rs | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/godot/src/lib.rs b/godot/src/lib.rs index 414101781..31838c4a6 100644 --- a/godot/src/lib.rs +++ b/godot/src/lib.rs @@ -106,11 +106,6 @@ //! //! The following features can be enabled for this crate. All off them are off by default. //! -//! * **`formatted`** -//! -//! Format the generated bindings with `rustfmt`. This significantly increases initial compile times and is -//! mostly useful when you actively contribute to the library and/or want to inspect generated files.

-//! //! * **`double-precision`** //! //! Use `f64` instead of `f32` for the floating-point type [`real`][type@builtin::real]. Requires Godot to be compiled with the @@ -131,32 +126,38 @@ //! //! * **`serde`** //! -//! Implement the [serde](https://docs.rs/serde) traits `Serialize` and `Deserialize` traits for certain built-in types. +//! Implement the [serde](https://serde.rs/) traits `Serialize` and `Deserialize` traits for certain built-in types. //! The serialized representation underlies **no stability guarantees** and may change at any time, even without a SemVer-breaking change. //!

//! +//! * **`lazy-function-tables`** +//! +//! Instead of loading all engine function pointers at startup, load them lazily on first use. This reduces startup time and RAM usage, but +//! incurs additional overhead in each FFI call. Also, you lose the guarantee that once the library has booted, all function pointers are +//! truly available. Function calls may thus panic only at runtime, possibly in deeply nested code paths. +//! This feature is not yet thread-safe and can thus not be combined with `experimental-threads`.

+//! +//! * **`formatted`** +//! +//! Format the generated binding code with a custom-built formatter, which aims to strike a balance between runtime and human readability. +//! rustfmt generates nice output, but it is unfortunately excessively slow across hundreds of Godot classes.

+//! //! * **`experimental-threads`** //! //! Experimental threading support. This enables `Send`/`Sync` traits for `Gd` and makes the guard types `Gd`/`GdMut` aware of //! multi-threaded references. There safety aspects are not ironed out yet; there is a high risk of unsoundness at the moment. -//! As this evolves, it is very likely that the API becomes more strict. +//! As this evolves, it is very likely that the API becomes more strict.

//! //! * **`experimental-godot-api`** //! //! Access to `godot::engine` APIs that Godot marks "experimental". These are under heavy development and may change at any time. -//! If you opt in to this feature, expect breaking changes at compile and runtime. +//! If you opt in to this feature, expect breaking changes at compile and runtime.

//! //! * **`experimental-wasm`** //! //! Support for WebAssembly exports is still a work-in-progress and is not yet well tested. This feature is in place for users -//! to explicitly opt-in to any instabilities or rough edges that may result. -//! -//! * **`lazy-function-tables`** -//! -//! Instead of loading all engine function pointers at startup, load them lazily on first use. This reduces startup time and RAM usage, but -//! incurs additional overhead in each FFI call. Also, you lose the guarantee that once the library has booted, all function pointers are -//! truly available. Function calls may thus panic only at runtime, possibly in deeply nested code paths. -//! This feature is not yet thread-safe and can thus not be combined with `experimental-threads`. +//! to explicitly opt-in to any instabilities or rough edges that may result. Due to a limitation in Godot, it might currently not +//! work Firefox browser.

//! //! # Public API //! From 8f8adb04ff5ad3a6be01f621036b249708260427 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 25 Nov 2023 16:20:25 +0100 Subject: [PATCH 2/8] Consistent #[derive] order and minor formatting --- godot-bindings/src/lib.rs | 2 +- godot-codegen/src/util.rs | 2 +- godot-core/src/builtin/array.rs | 2 +- .../src/builtin/meta/godot_convert/convert_error.rs | 2 +- godot-core/src/builtin/vectors/vector_axis.rs | 6 +++--- godot-fmt/src/lib.rs | 2 +- godot-macros/src/derive/derive_from_variant.rs | 4 ++-- godot-macros/src/lib.rs | 2 +- itest/rust/build.rs | 2 +- itest/rust/src/builtin_tests/convert_test.rs | 2 +- itest/rust/src/object_tests/property_test.rs | 2 +- itest/rust/src/register_tests/derive_variant_test.rs | 10 +++++----- 12 files changed, 19 insertions(+), 19 deletions(-) diff --git a/godot-bindings/src/lib.rs b/godot-bindings/src/lib.rs index 8a3d6a62b..15ab7a414 100644 --- a/godot-bindings/src/lib.rs +++ b/godot-bindings/src/lib.rs @@ -19,7 +19,7 @@ compile_error!( // This is outside of `godot_version` to allow us to use it even when we don't have the `custom-godot` // feature enabled. -#[derive(Debug, Eq, PartialEq)] +#[derive(Eq, PartialEq, Debug)] pub struct GodotVersion { /// the original string (trimmed, stripped of text around) pub full_string: String, diff --git a/godot-codegen/src/util.rs b/godot-codegen/src/util.rs index 1aad94f0a..eeab039c1 100644 --- a/godot-codegen/src/util.rs +++ b/godot-codegen/src/util.rs @@ -15,7 +15,7 @@ use quote::{format_ident, quote, ToTokens}; use std::fmt; -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Eq, PartialEq, Debug)] pub struct NativeStructuresField { pub field_type: String, pub field_name: String, diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs index 7a420847b..bcf1084e8 100644 --- a/godot-core/src/builtin/array.rs +++ b/godot-core/src/builtin/array.rs @@ -1022,7 +1022,7 @@ macro_rules! varray { /// [`set_typed`](https://docs.godotengine.org/en/latest/classes/class_array.html#class-array-method-set-typed). /// /// We ignore the `script` parameter because it has no impact on typing in Godot. -#[derive(PartialEq, Eq)] +#[derive(Eq, PartialEq)] pub(crate) struct TypeInfo { variant_type: VariantType, diff --git a/godot-core/src/builtin/meta/godot_convert/convert_error.rs b/godot-core/src/builtin/meta/godot_convert/convert_error.rs index a173fb68f..345fcd4ac 100644 --- a/godot-core/src/builtin/meta/godot_convert/convert_error.rs +++ b/godot-core/src/builtin/meta/godot_convert/convert_error.rs @@ -108,7 +108,7 @@ impl Error for ConvertError { } } -#[derive(Debug, PartialEq, Eq)] +#[derive(Eq, PartialEq, Debug)] pub(crate) enum ErrorKind { FromGodot(FromGodotError), FromFfi(FromFfiError), diff --git a/godot-core/src/builtin/vectors/vector_axis.rs b/godot-core/src/builtin/vectors/vector_axis.rs index e213f05bf..0f71a1684 100644 --- a/godot-core/src/builtin/vectors/vector_axis.rs +++ b/godot-core/src/builtin/vectors/vector_axis.rs @@ -73,7 +73,7 @@ pub trait ToVector: Sized { } /// Enumerates the axes in a [`Vector2`]. -#[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug)] #[repr(i32)] pub enum Vector2Axis { /// The X axis. @@ -117,7 +117,7 @@ impl FromGodot for Vector2Axis { /// Enumerates the axes in a [`Vector3`]. // TODO auto-generate this, alongside all the other builtin type's enums -#[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug)] #[repr(i32)] pub enum Vector3Axis { /// The X axis. @@ -164,7 +164,7 @@ impl FromGodot for Vector3Axis { // ---------------------------------------------------------------------------------------------------------------------------------------------- /// Enumerates the axes in a [`Vector4`]. -#[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug)] #[repr(i32)] pub enum Vector4Axis { /// The X axis. diff --git a/godot-fmt/src/lib.rs b/godot-fmt/src/lib.rs index 90988efdf..3b7b0c90f 100644 --- a/godot-fmt/src/lib.rs +++ b/godot-fmt/src/lib.rs @@ -88,7 +88,7 @@ fn indent(n: usize) -> &'static str { /// State that is kept between processing `TokenTree`s, used to decide /// how to insert whitespace. -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Copy, Clone, Eq, PartialEq)] enum FormatState { /// Starting state, meaning no whitespace is needed Start, diff --git a/godot-macros/src/derive/derive_from_variant.rs b/godot-macros/src/derive/derive_from_variant.rs index 2038a3a0a..b61f4f31b 100644 --- a/godot-macros/src/derive/derive_from_variant.rs +++ b/godot-macros/src/derive/derive_from_variant.rs @@ -261,8 +261,8 @@ fn make_enum_tuple( } else { quote! { let #ident = variant.pop_front() - .ok_or(ConvertError::with_cause_value("missing expected value", variant.clone()))? - .try_to::<#field_type>()?; + .ok_or(ConvertError::with_cause_value("missing expected value", variant.clone()))? + .try_to::<#field_type>()?; } }; (ident.to_token_stream(), set_ident) diff --git a/godot-macros/src/lib.rs b/godot-macros/src/lib.rs index 53ad2c299..bd6e5d6f7 100644 --- a/godot-macros/src/lib.rs +++ b/godot-macros/src/lib.rs @@ -529,7 +529,7 @@ pub fn derive_from_godot(input: TokenStream) -> TokenStream { /// # use godot::prelude::*; /// #[repr(i32)] /// #[derive(Property)] -/// # #[derive(PartialEq, Eq, Debug)] +/// # #[derive(Eq, PartialEq, Debug)] /// enum TestEnum { /// A = 0, /// B = 1, diff --git a/itest/rust/build.rs b/itest/rust/build.rs index 8ed8724bd..cb2dc05f5 100644 --- a/itest/rust/build.rs +++ b/itest/rust/build.rs @@ -77,7 +77,7 @@ macro_rules! push_newtype { pushs!( $inputs; $GDScriptTy, $name, $gdscript_val, $rust_val, false, false, None; - #[derive(Debug, Clone, PartialEq)] + #[derive(Clone, PartialEq, Debug)] pub struct $name($T); impl godot::builtin::meta::GodotConvert for $name { diff --git a/itest/rust/src/builtin_tests/convert_test.rs b/itest/rust/src/builtin_tests/convert_test.rs index 5367fc956..ad644a4d1 100644 --- a/itest/rust/src/builtin_tests/convert_test.rs +++ b/itest/rust/src/builtin_tests/convert_test.rs @@ -92,7 +92,7 @@ fn error_maintains_value() { } // Manual implementation of `GodotConvert` and related traits to ensure conversion works. -#[derive(Debug, PartialEq)] +#[derive(PartialEq, Debug)] struct Foo { a: i32, b: f32, diff --git a/itest/rust/src/object_tests/property_test.rs b/itest/rust/src/object_tests/property_test.rs index 74f3193e3..483e65c35 100644 --- a/itest/rust/src/object_tests/property_test.rs +++ b/itest/rust/src/object_tests/property_test.rs @@ -302,7 +302,7 @@ struct CheckAllExports { impl CheckAllExports {} #[repr(i64)] -#[derive(Property, Debug, PartialEq, Eq, Export)] +#[derive(Property, Export, Eq, PartialEq, Debug)] pub enum TestEnum { A = 0, B = 1, diff --git a/itest/rust/src/register_tests/derive_variant_test.rs b/itest/rust/src/register_tests/derive_variant_test.rs index d567c8145..abe36de71 100644 --- a/itest/rust/src/register_tests/derive_variant_test.rs +++ b/itest/rust/src/register_tests/derive_variant_test.rs @@ -41,10 +41,10 @@ trait Bound {} #[derive(FromGodot, ToGodot, GodotConvert, PartialEq, Debug)] struct StructGenBound(T); -#[derive(FromGodot, ToGodot, GodotConvert, PartialEq, Debug, Clone)] +#[derive(FromGodot, ToGodot, GodotConvert, Clone, PartialEq, Debug)] enum Uninhabited {} -#[derive(FromGodot, ToGodot, GodotConvert, PartialEq, Debug, Clone)] +#[derive(FromGodot, ToGodot, GodotConvert, Clone, PartialEq, Debug)] enum Enum { Unit, OneTuple(i32), @@ -145,7 +145,7 @@ macro_rules! roundtrip_with_skip { }; } -#[derive(Debug, Default, Clone, PartialEq, ToGodot, FromGodot, GodotConvert)] +#[derive(ToGodot, FromGodot, GodotConvert, Default, Clone, PartialEq, Debug)] enum EnumWithSkip { #[variant(skip)] Skipped(String), @@ -209,10 +209,10 @@ roundtrip_with_skip!( // ---------------------------------------------------------------------------------------------------------------------------------------------- // Skipping of structs -#[derive(Debug, Default, PartialEq, ToGodot, FromGodot, GodotConvert)] +#[derive(ToGodot, FromGodot, GodotConvert, Default, PartialEq, Debug)] struct NewTypeStructWithSkip(#[variant(skip)] String); -#[derive(Debug, Default, PartialEq, ToGodot, FromGodot, GodotConvert)] +#[derive(ToGodot, FromGodot, GodotConvert, Default, PartialEq, Debug)] struct StructWithSkip { #[variant(skip)] skipped_field: String, From a971839c1073e502c515f0f2b5eb36846dafdd20 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 25 Nov 2023 16:17:55 +0100 Subject: [PATCH 3/8] Gd::try_cast() now returns Err(self) instead of None on failure --- godot-core/src/engine/mod.rs | 6 +++--- godot-core/src/obj/gd.rs | 6 +++--- itest/rust/src/object_tests/object_test.rs | 17 ++++++++++------- .../src/object_tests/virtual_methods_test.rs | 2 +- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/godot-core/src/engine/mod.rs b/godot-core/src/engine/mod.rs index 42fe2ddf7..9ebdaeb58 100644 --- a/godot-core/src/engine/mod.rs +++ b/godot-core/src/engine/mod.rs @@ -59,7 +59,7 @@ impl PackedSceneExt for PackedScene { where T: Inherits, { - self.instantiate().and_then(|gd| gd.try_cast::()) + self.instantiate().and_then(|gd| gd.try_cast::().ok()) } } @@ -102,7 +102,7 @@ impl NodeExt for Node { // TODO differentiate errors (not found, bad type) with Result self.get_node_or_null(path) - .and_then(|node| node.try_cast::()) + .and_then(|node| node.try_cast::().ok()) } } @@ -252,5 +252,5 @@ where .load_ex(path.clone()) .type_hint(T::class_name().to_godot_string()) .done() // TODO unclone - .and_then(|res| res.try_cast::()) + .and_then(|res| res.try_cast::().ok()) } diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 33c131674..f7f708758 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -308,12 +308,12 @@ impl Gd { /// If `T`'s dynamic type is not `Derived` or one of its subclasses, `None` is returned /// and the reference is dropped. Otherwise, `Some` is returned and the ownership is moved /// to the returned value. - // TODO consider Result, Self> so that user can still use original object (e.g. to free if manual) - pub fn try_cast(self) -> Option> + pub fn try_cast(self) -> Result, Self> where Derived: GodotClass + Inherits, { - self.owned_cast().ok() + // Separate method due to more restrictive bounds. + self.owned_cast() } /// ⚠️ **Downcast:** convert into a smart pointer to a derived class. Panics on error. diff --git a/itest/rust/src/object_tests/object_test.rs b/itest/rust/src/object_tests/object_test.rs index 558d119d7..3de509fe7 100644 --- a/itest/rust/src/object_tests/object_test.rs +++ b/itest/rust/src/object_tests/object_test.rs @@ -552,7 +552,7 @@ fn object_reject_invalid_downcast() { let instance = Gd::from_object(CustomClassA {}); let object = instance.upcast::(); - assert!(object.try_cast::().is_none()); + assert!(object.try_cast::().is_err()); } #[itest] @@ -577,11 +577,12 @@ fn object_engine_downcast_reflexive() { #[itest] fn object_engine_bad_downcast() { let object: Gd = Object::new_alloc(); - let free_ref = object.clone(); - let node3d: Option> = object.try_cast::(); + let object2 = object.clone(); - assert!(node3d.is_none()); - free_ref.free(); + let node3d: Result, Gd> = object.try_cast::(); + + assert_eq!(node3d, Err(object2.clone())); + object2.free(); } #[itest] @@ -665,9 +666,11 @@ fn object_user_downcast() { fn object_user_bad_downcast() { let obj = user_refc_instance(); let object = obj.upcast::(); - let node3d: Option> = object.try_cast::(); + let object2 = object.clone(); + + let node3d: Result, Gd> = object.try_cast::(); - assert!(node3d.is_none()); + assert_eq!(node3d, Err(object2)); } #[itest] diff --git a/itest/rust/src/object_tests/virtual_methods_test.rs b/itest/rust/src/object_tests/virtual_methods_test.rs index 9f0fdec52..21005f8c5 100644 --- a/itest/rust/src/object_tests/virtual_methods_test.rs +++ b/itest/rust/src/object_tests/virtual_methods_test.rs @@ -376,7 +376,7 @@ fn test_format_loader(_test_context: &TestContext) { .cache_mode(CacheMode::CACHE_MODE_IGNORE) .done() .unwrap(); - assert!(resource.try_cast::().is_some()); + assert!(resource.try_cast::().is_ok()); loader.remove_resource_format_loader(format_loader.upcast()); } From bf402a3e835c8b1ba58dd870871ff246d662dfbf Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 25 Nov 2023 16:18:19 +0100 Subject: [PATCH 4/8] Callable::from_object_method() now accepts Gd parameter by shared-ref --- examples/dodge-the-creeps/rust/src/main_scene.rs | 5 +---- godot-core/src/builtin/callable.rs | 4 +++- godot-core/src/obj/gd.rs | 5 ++++- itest/rust/src/builtin_tests/containers/callable_test.rs | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/examples/dodge-the-creeps/rust/src/main_scene.rs b/examples/dodge-the-creeps/rust/src/main_scene.rs index a6e120f68..eb5db2fae 100644 --- a/examples/dodge-the-creeps/rust/src/main_scene.rs +++ b/examples/dodge-the-creeps/rust/src/main_scene.rs @@ -105,10 +105,7 @@ impl Main { mob.set_linear_velocity(lin_vel); let mut hud = self.base.get_node_as::("Hud"); - hud.connect( - "start_game".into(), - Callable::from_object_method(mob, "on_start_game"), - ); + hud.connect("start_game".into(), mob.callable("on_start_game")); } fn music(&mut self) -> &mut AudioStreamPlayer { diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index 4c56a0634..7bde47fd2 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -35,8 +35,10 @@ impl Callable { /// Create a callable for the method `object::method_name`. /// + /// See also [`Gd::callable()`]. + /// /// _Godot equivalent: `Callable(Object object, StringName method)`_ - pub fn from_object_method(object: Gd, method_name: S) -> Self + pub fn from_object_method(object: &Gd, method_name: S) -> Self where T: GodotClass, // + Inherits, S: Into, diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index f7f708758..6d10f5a18 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -385,9 +385,12 @@ impl Gd { pub(crate) fn obj_sys(&self) -> sys::GDExtensionObjectPtr { self.raw.obj_sys() } + /// Returns a callable referencing a method from this object named `method_name`. + /// + /// This is shorter syntax for [`Callable::from_object_method(self, method_name)`][Callable::from_object_method]. pub fn callable>(&self, method_name: S) -> Callable { - Callable::from_object_method(self.clone(), method_name) + Callable::from_object_method(self, method_name) } } diff --git a/itest/rust/src/builtin_tests/containers/callable_test.rs b/itest/rust/src/builtin_tests/containers/callable_test.rs index 54761dc6a..6399304f2 100644 --- a/itest/rust/src/builtin_tests/containers/callable_test.rs +++ b/itest/rust/src/builtin_tests/containers/callable_test.rs @@ -110,7 +110,7 @@ fn callable_call_return() { #[itest] fn callable_call_engine() { let obj = Node2D::new_alloc(); - let cb = Callable::from_object_method(obj.clone(), "set_position"); + let cb = Callable::from_object_method(&obj, "set_position"); let inner: InnerCallable = cb.as_inner(); assert!(!inner.is_null()); From d72512373b54cb7ad47c63f6e18b084699efe5b1 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 25 Nov 2023 17:28:23 +0100 Subject: [PATCH 5/8] For classes with #[base] field, add to_gd() utility method --- godot-core/src/obj/traits.rs | 23 ++++++++++---------- godot-macros/src/class/derive_godot_class.rs | 17 ++++++++++++--- godot-macros/src/lib.rs | 2 +- godot/src/lib.rs | 3 ++- itest/rust/src/object_tests/base_test.rs | 20 +++++++++++++++++ 5 files changed, 49 insertions(+), 16 deletions(-) diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index 5b68754b3..dcdbc51d3 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -206,6 +206,18 @@ pub trait IndexEnum: EngineEnum { } } +/// Trait that's implemented for user-defined classes that provide a `#[base]` field. +/// +/// Gives direct access to the containing `Gd` from `Self`. +// Possible alternative for builder APIs, although even less ergonomic: Base could be Base and return Gd. +pub trait WithBaseField: GodotClass { + /// Returns the `Gd` pointer containing this object. + /// + /// This is intended to be stored or passed to engine methods. You cannot call `bind()` or `bind_mut()` on it, while the method + /// calling `to_gd()` is still running; that would lead to a double borrow panic. + fn to_gd(&self) -> Gd; +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- /// Capability traits, providing dedicated functionalities for Godot classes @@ -257,17 +269,6 @@ pub mod cap { } } - /// Trait that's implemented for user-defined classes that provide a `#[base]` field. - /// - /// Gives direct access to the base pointer without going through upcast FFI. - pub trait WithBaseField: GodotClass { - #[doc(hidden)] - fn __godot_base(&self) -> &Gd; - - #[doc(hidden)] - fn __godot_base_mut(&mut self) -> &mut Gd; - } - // TODO Evaluate whether we want this public or not #[doc(hidden)] pub trait GodotToString: GodotClass { diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs index 78d5439fe..e0fe49a78 100644 --- a/godot-macros/src/class/derive_godot_class.rs +++ b/godot-macros/src/class/derive_godot_class.rs @@ -50,6 +50,18 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult { quote! {} }; + let godot_withbase_impl = if let Some(Field { name, .. }) = &fields.base_field { + quote! { + impl ::godot::obj::WithBaseField for #class_name { + fn to_gd(&self) -> Gd { + ::godot::obj::Gd::clone(&*self.#name).cast() + } + } + } + } else { + quote! {} + }; + let (godot_init_impl, create_fn, recreate_fn); if struct_cfg.has_generated_init { godot_init_impl = make_godot_init_impl(class_name, fields); @@ -78,11 +90,10 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult { ::godot::builtin::meta::ClassName::from_ascii_cstr(#class_name_cstr) } } - impl ::godot::obj::UserClass for #class_name { - - } + impl ::godot::obj::UserClass for #class_name {} #godot_init_impl + #godot_withbase_impl #godot_exports_impl #config_impl diff --git a/godot-macros/src/lib.rs b/godot-macros/src/lib.rs index bd6e5d6f7..c2a281668 100644 --- a/godot-macros/src/lib.rs +++ b/godot-macros/src/lib.rs @@ -100,7 +100,7 @@ use crate::util::ident; /// #[class(base = Node2D)] /// struct MyStruct { /// #[base] -/// base: Gd, +/// base: Base, /// } /// ``` /// diff --git a/godot/src/lib.rs b/godot/src/lib.rs index 31838c4a6..235c71cab 100644 --- a/godot/src/lib.rs +++ b/godot/src/lib.rs @@ -235,5 +235,6 @@ pub mod prelude { // Make trait methods available pub use super::engine::NodeExt as _; pub use super::obj::EngineEnum as _; - pub use super::obj::UserClass as _; // new_gd(), self_gd() + pub use super::obj::UserClass as _; // new_gd(), alloc_gd() + pub use super::obj::WithBaseField as _; // to_gd() } diff --git a/itest/rust/src/object_tests/base_test.rs b/itest/rust/src/object_tests/base_test.rs index d98447fdf..70bd2537c 100644 --- a/itest/rust/src/object_tests/base_test.rs +++ b/itest/rust/src/object_tests/base_test.rs @@ -93,6 +93,19 @@ fn base_with_init() { obj.free(); } +#[itest] +fn base_gd_self() { + let obj = Based::alloc_gd(); + let obj2 = obj.bind().access_gd_self(); + + assert_eq!(obj, obj2); + assert_eq!(obj.instance_id(), obj2.instance_id()); + + obj.free(); +} + +// ---------------------------------------------------------------------------------------------------------------------------------------------- + #[derive(GodotClass)] #[class(init, base=Node2D)] struct Based { @@ -102,6 +115,13 @@ struct Based { i: i32, } +impl Based { + fn access_gd_self(&self) -> Gd { + use godot::obj::WithBaseField as _; + self.to_gd() + } +} + #[derive(GodotClass)] #[class(init, base=Node2D)] struct Baseless { From 3f5788ecacbdb7f04606ec71fa89ec84bedf715a Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 25 Nov 2023 19:27:18 +0100 Subject: [PATCH 6/8] Disable godot-fmt benchmark; its criterion pulls in too many dependencies although never used Can be manually re-enabled. Unfortunately, Cargo doesn't offer optional dev-dependencies. Also remove unused serde_json dependency. --- godot-core/Cargo.toml | 1 - godot-fmt/Cargo.toml | 14 +++++++++----- godot-fmt/benches/gdext_bench.rs | 4 ++++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/godot-core/Cargo.toml b/godot-core/Cargo.toml index a270cc0c7..652a50428 100644 --- a/godot-core/Cargo.toml +++ b/godot-core/Cargo.toml @@ -31,7 +31,6 @@ serde = { version = "1", features = ["derive"], optional = true } # Reverse dev dependencies so doctests can use `godot::` prefix [dev-dependencies] godot = { path = "../godot" } -serde_json = "1.0" [build-dependencies] godot-bindings = { path = "../godot-bindings" } diff --git a/godot-fmt/Cargo.toml b/godot-fmt/Cargo.toml index 5f52db925..bfab77b56 100644 --- a/godot-fmt/Cargo.toml +++ b/godot-fmt/Cargo.toml @@ -8,9 +8,13 @@ edition = "2021" [dependencies] proc-macro2 = "1" -[dev-dependencies] -criterion = "0.3" -[[bench]] -name = "gdext_bench" -harness = false +# Disabled below, since it pulls in too many dependencies during `cargo test` but is not really used. +# Dev-dependencies cannot be optional and feature-gated. Enable manually when needed. + +#[[bench]] +#name = "gdext_bench" +#harness = false +# +#[dev-dependencies] +#criterion = "0.5" diff --git a/godot-fmt/benches/gdext_bench.rs b/godot-fmt/benches/gdext_bench.rs index 6b4037055..bfaa99212 100644 --- a/godot-fmt/benches/gdext_bench.rs +++ b/godot-fmt/benches/gdext_bench.rs @@ -4,6 +4,10 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +// IMPORTANT: to enable this benchmark, uncomment the corresponding lines in Cargo.toml. +// First tried with #![allow(clippy::all)], but clippy still tries to compile the code and fails on imports. +#![cfg(FALSE)] + use std::{path::PathBuf, str::FromStr}; use criterion::{black_box, criterion_group, criterion_main, Criterion}; From 8800257e9915e50979fee44fae1d8d076521c91e Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Wed, 22 Nov 2023 21:52:49 +0100 Subject: [PATCH 7/8] Farewell, bors. --- .github/bors.toml | 3 --- .github/workflows/full-ci.yml | 9 +++------ 2 files changed, 3 insertions(+), 9 deletions(-) delete mode 100644 .github/bors.toml diff --git a/.github/bors.toml b/.github/bors.toml deleted file mode 100644 index 5290b12aa..000000000 --- a/.github/bors.toml +++ /dev/null @@ -1,3 +0,0 @@ -block_labels = ["status: duplicate", "status: wontfix"] -delete_merged_branches = true -status = ["ci-status"] diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index 6f8fa0f28..c8e6c9d8f 100644 --- a/.github/workflows/full-ci.yml +++ b/.github/workflows/full-ci.yml @@ -8,10 +8,7 @@ name: Full CI on: merge_group: - push: - branches: - - staging - - trying +# push: env: GDEXT_FEATURES: '' @@ -328,9 +325,9 @@ jobs: # CI status report # Job to notify merge queue about success/failure - # 'push' is for workflows still triggered by bors ci-status: - if: always() && (github.event_name == 'merge_group' || github.event_name == 'push') + # Check for 'merge_group' not strictly necessary, but helpful when adding add-hoc `push:` trigger to `on:` for testing branch. + if: always() && github.event_name == 'merge_group' needs: - rustfmt - doc-lints From c8397b36d003bbffe6e69f75cb5d53485695a403 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 26 Nov 2023 11:38:49 +0100 Subject: [PATCH 8/8] Enable clippy for all targets locally, matching CI --- check.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check.sh b/check.sh index 6528b0874..75ab87704 100755 --- a/check.sh +++ b/check.sh @@ -125,7 +125,7 @@ function cmd_fmt() { } function cmd_clippy() { - run cargo clippy "${extraCargoArgs[@]}" -- \ + run cargo clippy --all-targets "${extraCargoArgs[@]}" -- \ -D clippy::suspicious \ -D clippy::style \ -D clippy::complexity \