Skip to content

to_gd(), Gd::try_cast() result, formatting #497

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

Merged
merged 8 commits into from
Nov 26, 2023
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: 0 additions & 3 deletions .github/bors.toml

This file was deleted.

9 changes: 3 additions & 6 deletions .github/workflows/full-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ name: Full CI

on:
merge_group:
push:
branches:
- staging
- trying
# push:

env:
GDEXT_FEATURES: ''
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
5 changes: 1 addition & 4 deletions examples/dodge-the-creeps/rust/src/main_scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,7 @@ impl Main {
mob.set_linear_velocity(lin_vel);

let mut hud = self.base.get_node_as::<Hud>("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 {
Expand Down
2 changes: 1 addition & 1 deletion godot-bindings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion godot-codegen/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion godot-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
4 changes: 3 additions & 1 deletion godot-core/src/builtin/callable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T, S>(object: Gd<T>, method_name: S) -> Self
pub fn from_object_method<T, S>(object: &Gd<T>, method_name: S) -> Self
where
T: GodotClass, // + Inherits<Object>,
S: Into<StringName>,
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/meta/godot_convert/convert_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl Error for ConvertError {
}
}

#[derive(Debug, PartialEq, Eq)]
#[derive(Eq, PartialEq, Debug)]
pub(crate) enum ErrorKind {
FromGodot(FromGodotError),
FromFfi(FromFfiError),
Expand Down
6 changes: 3 additions & 3 deletions godot-core/src/builtin/vectors/vector_axis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions godot-core/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl PackedSceneExt for PackedScene {
where
T: Inherits<Node>,
{
self.instantiate().and_then(|gd| gd.try_cast::<T>())
self.instantiate().and_then(|gd| gd.try_cast::<T>().ok())
}
}

Expand Down Expand Up @@ -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::<T>())
.and_then(|node| node.try_cast::<T>().ok())
}
}

Expand Down Expand Up @@ -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::<T>())
.and_then(|res| res.try_cast::<T>().ok())
}
11 changes: 7 additions & 4 deletions godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,12 +308,12 @@ impl<T: GodotClass> Gd<T> {
/// 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<Gd<Derived>, Self> so that user can still use original object (e.g. to free if manual)
pub fn try_cast<Derived>(self) -> Option<Gd<Derived>>
pub fn try_cast<Derived>(self) -> Result<Gd<Derived>, Self>
where
Derived: GodotClass + Inherits<T>,
{
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.
Expand Down Expand Up @@ -385,9 +385,12 @@ impl<T: GodotClass> Gd<T> {
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<S: Into<StringName>>(&self, method_name: S) -> Callable {
Callable::from_object_method(self.clone(), method_name)
Callable::from_object_method(self, method_name)
}
}

Expand Down
23 changes: 12 additions & 11 deletions godot-core/src/obj/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self>` from `Self`.
// Possible alternative for builder APIs, although even less ergonomic: Base<T> could be Base<T, Self> and return Gd<Self>.
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<Self>;
}

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

/// Capability traits, providing dedicated functionalities for Godot classes
Expand Down Expand Up @@ -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<Self::Base>;

#[doc(hidden)]
fn __godot_base_mut(&mut self) -> &mut Gd<Self::Base>;
}

// TODO Evaluate whether we want this public or not
#[doc(hidden)]
pub trait GodotToString: GodotClass {
Expand Down
14 changes: 9 additions & 5 deletions godot-fmt/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
4 changes: 4 additions & 0 deletions godot-fmt/benches/gdext_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
2 changes: 1 addition & 1 deletion godot-fmt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 14 additions & 3 deletions godot-macros/src/class/derive_godot_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult<TokenStream> {
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<Self> {
::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);
Expand Down Expand Up @@ -78,11 +90,10 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult<TokenStream> {
::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

Expand Down
4 changes: 2 additions & 2 deletions godot-macros/src/derive/derive_from_variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions godot-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ use crate::util::ident;
/// #[class(base = Node2D)]
/// struct MyStruct {
/// #[base]
/// base: Gd<Node2D>,
/// base: Base<Node2D>,
/// }
/// ```
///
Expand Down Expand Up @@ -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,
Expand Down
36 changes: 19 additions & 17 deletions godot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.<br><br>
//!
//! * **`double-precision`**
//!
//! Use `f64` instead of `f32` for the floating-point type [`real`][type@builtin::real]. Requires Godot to be compiled with the
Expand All @@ -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.
//! <br><br>
//!
//! * **`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`.<br><br>
//!
//! * **`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.<br><br>
//!
//! * **`experimental-threads`**
//!
//! Experimental threading support. This enables `Send`/`Sync` traits for `Gd<T>` 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.<br><br>
//!
//! * **`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.<br><br>
//!
//! * **`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.<br><br>
//!
//! # Public API
//!
Expand Down Expand Up @@ -234,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()
}
Loading