Skip to content

Implement Gd::try_dynify. #1255

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 1 commit into from
Aug 6, 2025

Conversation

Yarwin
Copy link
Contributor

@Yarwin Yarwin commented Jul 31, 2025

Adds Gd::try_dynify, available for engine classes (currently we can validate if user-defined class implements AsDyn<D> on compile time, thus making try_dynify pointless – just use into_dyn instead).

@Yarwin Yarwin added feature Adds functionality to the library c: core Core components labels Jul 31, 2025
@Yarwin Yarwin force-pushed the implement_gd_try_into_dyn branch from e26700f to 7c45e5e Compare July 31, 2025 22:39
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1255

@Yarwin Yarwin force-pushed the implement_gd_try_into_dyn branch from 7c45e5e to c2ecacc Compare July 31, 2025 22:40
@Yarwin Yarwin marked this pull request as draft August 1, 2025 07:01
@Yarwin Yarwin force-pushed the implement_gd_try_into_dyn branch 2 times, most recently from bac8697 to f72710a Compare August 1, 2025 07:22
@Yarwin Yarwin marked this pull request as ready for review August 1, 2025 07:22
Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

So, try_xy() should always have the same semantics as xy() but returning Result/Option instead of panic. Since into_dyn is already taken, we need a different name.

Maybe try_dynify? Ideally a comment in docs would explain the relation + difference between the two.

@@ -512,6 +513,21 @@ impl<T: GodotClass> Gd<T> {
DynGd::<T, D>::from_gd(self)
}

/// Tries to upgrade to a `DynGd<T, D>` pointer, enabling the `D` abstraction.
///
/// If `T`'s class' dynamic type doesn't implement `AsDyn<D>`, `Err(self)` is returned, meaning you can reuse the original
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"T's class' dynamic type" is a bit imprecise, since T is a compile-time type (given by the generic parameter). You probably mean the dynamic type of self?

Copy link
Contributor Author

@Yarwin Yarwin Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant the "original" class 🤔 – maybe "T's dynamic class"?

@Yarwin Yarwin force-pushed the implement_gd_try_into_dyn branch from f72710a to 0a500a0 Compare August 1, 2025 08:42
@Yarwin Yarwin changed the title Implement Gd::try_into_dyn. Implement Gd::try_dynify. Aug 1, 2025
Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 🙂

@Bromeon Bromeon added this pull request to the merge queue Aug 6, 2025
Merged via the queue into godot-rust:master with commit a3e2cdf Aug 6, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: core Core components feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants