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

#[func] should not pass types not representable in GDScript #327

Closed
Bromeon opened this issue Jul 2, 2023 · 3 comments
Closed

#[func] should not pass types not representable in GDScript #327

Bromeon opened this issue Jul 2, 2023 · 3 comments
Labels
bug c: register Register classes, functions and other symbols to GDScript

Comments

@Bromeon
Copy link
Member

Bromeon commented Jul 2, 2023

Since #326, we use the same mechanism to pass types from/to the engine API (Rust -> Godot) as well as from to the bind API (GDScript -> Rust, via #[func]). This is generally a good thing, but for some types it's problematic.

For example, u64 is not representable in GDScript, so we should discourage accepting and returning u64 in #[func]. GDScript always uses i64 as int, so there is going to be an implicit conversion which can lead to a different value.

However, such a #[func] API may have uses in the rare cases where another language (C++, C# or even Rust?) calls our Rust API.

There are multiple options:

  • two separate traits for GodotFuncMarshal, one for bind and one for engine API
  • a wrapper newtype UnstableU64(u64) or so
    • #[func] API would require explicit packing/unpacking
    • engine API could accept impl Into<UnstableU64> parameters -- but what about return types?
    • instead of going via trait, the engine codegen could use a hardcoded special case
@Bromeon Bromeon added bug c: register Register classes, functions and other symbols to GDScript labels Jul 2, 2023
@Bromeon
Copy link
Member Author

Bromeon commented Jul 2, 2023

Relevant code:

// Some places Godot will pass along 64-bit numbers that are intended to be interpreted as unsigned
// integers despite Godot integers being signed by default.
impl_godot_marshalling!(u64 as i64; lossy);

@Bromeon
Copy link
Member Author

Bromeon commented Jan 5, 2024

Related: #105 and #125

@Bromeon
Copy link
Member Author

Bromeon commented May 6, 2024

Closed by #700.

We still allow u64 in #[func] parameter and return type positions, but panic if the value is larger than i64::MAX. For now, I think this is a good trade-off and avoids duplicating the entire marshalling mechanism for such edge cases.

Users are generally encouraged to use i64 when interfacing GDScript.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: register Register classes, functions and other symbols to GDScript
Projects
None yet
Development

No branches or pull requests

1 participant