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

Implement core types #6

Closed
Bromeon opened this issue Oct 3, 2022 · 7 comments
Closed

Implement core types #6

Bromeon opened this issue Oct 3, 2022 · 7 comments
Labels
c: core Core components feature Adds functionality to the library

Comments

@Bromeon
Copy link
Member

Bromeon commented Oct 3, 2022

Core structs (GodotString, Vector2, Vector3 etc.) are currently stubs that lack functionality.

To make them useful, expose (parts of) the Godot API via Rust methods.
Several options:

  1. Call Godot equivalents through FFI (slower and less idiomatic, but automatable and fast to implement)
  2. Implement them in Rust (for geometric primitives)
  3. Reuse implementation from GDNative-Rust (needs author permission for license)
@Bromeon Bromeon added feature Adds functionality to the library c: core Core components labels Oct 3, 2022
@RealAstolfo
Copy link
Contributor

RealAstolfo commented Dec 28, 2022

I did some work and created an example for 2. this is what it seems it would look like
EDIT: some of the functions were made in an external math.rs file for things that glam didnt have (or i didnt think they
had)

Fixed minor things with syntax (Thanks chitoyuu!)

/*
 * This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/.
 */
use std::ops::*;

use godot_ffi as sys;
use sys::{ffi_methods, GodotFfi};

use super::math::*;

type Inner = glam::f32::Vec2;
//type Inner = glam::f64::DVec2;

#[derive(Default, Copy, Clone, Debug, PartialEq)]
#[repr(C)]
pub struct Vector2 {
    inner: Inner,
}

impl Vector2 {
    pub fn new(x: f32, y: f32) -> Self {
        Self {
            inner: Inner::new(x, y),
        }
    }

    pub fn abs(self) -> Self {
        Self::from_inner(glam::Vector2::abs(self.inner))
    }

    pub fn angle(self) -> f32 {
        self.inner.y.atan2(self.inner.x)
    }

    pub fn angle_to(self, to: Self) -> f32 {
        self.inner.angle_between(to.inner)
    }

    pub fn angle_to_point(self, to: Self) -> f32 {
        Self::from_inner(to - self).angle();
    }

    pub fn aspect(self) -> f32 {
        self.inner.x / self.inner.y
    }

    pub fn bezier_derivative(
        mut self,
        control_1: Self,
        control_2: Self,
        end: Self,
        t: f32,
    ) -> Self {
        self.inner.x = bezier_derivative(self.inner.x, control_1.x, control_2.x, end.x, t);
        self.inner.y = bezier_derivative(self.inner.y, control_1.y, control_2.y, end.y, t);
        self
    }

    pub fn bezier_interpolate(
        mut self,
        control_1: Self,
        control_2: Self,
        end: Self,
        t: f32,
    ) -> Self {
        self.inner.x = bezier_interpolate(self.inner.x, control_1.x, control_2.x, end.x, t);
        self.inner.y = bezier_interpolate(self.inner.y, control_1.y, control_2.y, end.y, t);
        self
    }

    pub fn bounce(self, normal: Self) -> Self {
        -self.reflect(normal)
    }

    pub fn ceil(self) -> Self {
        Self::from_inner(self.inner.ceil())
    }

    pub fn clamp(mut self, min: Self, max: Self) -> Self {
        self.inner.x = if self.inner.x > max.inner.x {
            max.inner.x
        } else if self.inner.x < min.inner.x {
            min.inner.x
        } else {
            self.inner.x
        };
        self.inner.x = if self.inner.y > max.inner.y {
            max.inner.y
        } else if self.inner.y < min.inner.y {
            min.inner.y
        } else {
            self.inner.y
        };
        self
    }

    pub fn cross(self, with: Self) -> f32 {
        Self::from_inner(self.inner.perp_dot(with.inner))
    }

    pub fn cubic_interpolate(mut self, b: Self, pre_a: Self, post_b: Self, weight: f32) -> Self {
        self.inner.x = cubic_interpolate(
            self.inner.x,
            b.inner.x,
            pre_a.inner.x,
            post_b.inner.x,
            weight,
        );
        self.inner.y = cubic_interpolate(
            self.inner.y,
            b.inner.y,
            pre_a.inner.y,
            post_b.inner.y,
            weight,
        );
        self
    }

    pub fn cubic_interpolate_in_time(
        mut self,
        b: Self,
        pre_a: Self,
        post_b: Self,
        weight: f32,
        b_t: f32,
        pre_a_t: f32,
        post_b_t: f32,
    ) -> Self {
        self.inner.x = cubic_interpolate_in_time(
            self.inner.x,
            b.inner.x,
            pre_a.inner.x,
            post_b.inner.x,
            weight,
            b_t,
            pre_a_t,
            post_b_t,
        );
        self.inner.y = cubic_interpolate_in_time(
            self.inner.y,
            b.inner.y,
            pre_a.inner.y,
            post_b.inner.y,
            weight,
            b_t,
            pre_a_t,
            post_b_t,
        );
        self
    }

    pub fn direction_to(self, to: Self) -> Self {
        (to - self).normalize()
    }

    pub fn distance_squared_to(self, to: Self) -> Self {
        Self::from_inner(self.inner.distance_squared(to.inner))
    }

    pub fn distance_to(self, to: Self) -> Self {
        Self::from_inner(self.inner.distance(to.inner))
    }

    pub fn dot(self, other: Self) -> Self {
        Self::from_inner(self.inner.dot(other.inner))
    }

    pub fn floor(self) -> Self {
        Self::from_inner(self.inner.floor())
    }

    pub fn from_angle(angle: f32) -> Self {
        Self::from_inner(Inner::from_angle(angle))
    }

    pub fn is_equal_approx(self, to: Self) -> bool {
        is_equal_approx(self.inner.x, to.inner.x) && is_equal_approx(self.inner.y, to.inner.y);
    }

    pub fn is_finite(self) -> bool {
        self.inner.is_finite()
    }

    pub fn is_normalized(self) -> bool {
        self.inner.is_normalized()
    }

    pub fn is_zero_approx(self) -> bool {
        is_zero_approx(self.inner.x) && is_zero_approx(self.inner.y)
    }

    pub fn length(self) -> f32 {
        self.inner.length()
    }

    pub fn length_squared(self) -> f32 {
        self.inner.length_squared()
    }

    pub fn lerp(self, to: Self, weight: f32) -> Self {
        Self::from_inner(self.inner.lerp(to, weight))
    }

    pub fn limit_length(self, length: Option<f32>) -> Self {
        Self::from_inner(self.inner.clamp_length_max(length.unwrap_or(1.0)))
    }

    pub fn max_axis_index(self) -> i32 {
        if self.inner.max_element() == self.inner.x {
            0
        } else {
            1
        }
    }

    pub fn min_axis_index(self) -> i32 {
        if self.inner.min_element() == self.inner.x {
            0
        } else {
            1
        }
    }

    pub fn move_toward(mut self, to: Self, delta: f32) -> Self {
        let vd = to - self;
        let len = vd.length();
        Self::from_inner(if len <= delta || len < CMP_EPSILON {
            to
        } else {
            self + vd / len * delta
        });
    }

    pub fn normalized(self) -> Self {
        Self::from_inner(self.inner.normalized())
    }

    pub fn orthogonal(self) -> Self {
        Self::from_inner(self.inner.orthogonal())
    }

    pub fn posmod(self, pmod: f32) -> Self {
        Self::new(fposmod(self.inner.x, pmod), fposmod(self.inner.y, pmod))
    }

    pub fn posmodv(self, modv: Self) -> Self {
        Self::new(
            fposmod(self.inner.x, modv.inner.x),
            fposmod(self.inner.y, modv.inner.y),
        )
    }

    pub fn project(self, b: Self) -> Self {
        Self::from_inner(self.inner.project_onto(b.inner))
    }

    pub fn reflect(self, normal: Self) -> Self {
        //Self::from_inner(2.0 * normal.inner * self.dot(normal) - self.inner)
        Self::from_inner(self.inner.reject_from(normal.inner))
    }

    pub fn rotated(self, angle: f32) -> Self {
        Self::from_inner(self.inner.rotate(Self::from_angle(angle)))
    }

    pub fn round(self) -> Self {
        Self::from_inner(self.inner.round())
    }

    pub fn sign(self) -> Self {
        -self
    }

    pub fn slerp(self, to: Self, weight: f32) -> Self {
        let start_length_sq = self.length_squared();
        let end_length_sq = to.length_squared();
        if start_length_sq == 0.0 || end_length_sq == 0.0 {
            self.lerp(to, weight)
        }
        let start_length = start_length_sq.sqrt();
        let result_length = lerp(start_length, end_length_sq.sqrt(), weight);
        let angle = self.angle_to(to);
        self.rotated(angle * weight) * (result_length / start_length)
    }

    pub fn slide(self, normal: Self) -> Self {
        self - normal * self.dot(normal)
    }

    pub fn snapped(self, step: Self) -> Self {
        Self::new(snapped(self.inner.x, step), snapped(self.inner.y, step))
    }

    fn from_inner(inner: Inner) -> Self {
        Self { inner }
    }
}

impl Add for Vector2 {
    type Output = Self;

    fn add(self, other: Self) -> Self {
        Self::new(self.inner.x + other.inner.x, self.inner.y + other.inner.y)
    }
}

impl AddAssign for Vector2 {
    fn add_assign(&mut self, other: Self) {
        *self = self + other
    }
}

impl Sub for Vector2 {
    type Output = Self;

    fn sub(self, other: Self) -> Self {
        Self::new(self.inner.x - other.inner.x, self.inner.y - other.inner.y)
    }
}

impl SubAssign for Vector2 {
    fn sub_assign(&mut self, other: Self) {
        *self = self - other
    }
}

impl Mul for Vector2 {
    type Output = Self;

    fn mul(self, other: f32) -> Self {
        Self::new(self.inner.x * other, self.inner.y * other)
    }
}

impl MulAssign for Vector2 {
    fn mul_assign(&mut self, other: f32) {
        *self = self * other
    }
}

impl Div for Vector2 {
    type Output = Self;

    fn div(self, other: f32) -> Self {
        Self::new(self.inner.x / other, self.inner.y / other)
    }
}

impl DivAssign for Vector2 {
    fn div_assign(&mut self, other: f32) {
        *self = self / other
    }
}

impl Neg for Vector2 {
    type Output = Self;

    fn neg(self) -> Self {
        Self::new(-self.inner.x, -self.inner.y)
    }
}

impl GodotFfi for Vector2 {
    ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}

impl std::fmt::Display for Vector2 {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        self.inner.fmt(f)
    }
}

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

type IInner = glam::IVec2;

#[derive(Default, Copy, Clone, Debug, Eq, PartialEq)]
#[repr(C)]
pub struct Vector2i {
    inner: IInner,
}

impl Vector2i {
    pub fn new(x: i32, y: i32) -> Self {
        Self {
            inner: IInner::new(x, y),
        }
    }
}

impl GodotFfi for Vector2i {
    ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}

impl std::fmt::Display for Vector2i {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        self.inner.fmt(f)
    }
}

@chitoyuu
Copy link

chitoyuu commented Dec 28, 2022

Talked a bit on Discord with @RealAstolfo about some non-idiomatic expressions in the code.

I think it's a nice beginning if we end up taking option 2 (fresh re-implementation). It feels bizarre to have to consider licensing within the same general project indeed.

Option 1 is interesting, although also a tricky one to end users. I'd imagine it to be hard to wrap one's head around if we make the full API interface available through FFI, only to suggest users to ignore them all and convert away from the type immediately if they want their code to be fast.

I suppose it isn't exactly against the project's goals, that are still completeness and correctness first and foremost, and we aren't generally inclined to be opinionated, but it still feels weird.

@ttencate
Copy link
Contributor

ttencate commented Jan 6, 2023

A native implementation (option 2/3) seems like the best way to me. It's a bit of work to implement these types once, but the implementations tend to be simple, and these classes are pretty stable in the engine so it's not a lot of work to keep up.

The Vector* types are currently implemented as wrappers around glam, which is nice for performance and for compatibility with other crates. But it does mean that we can't have public fields like x; we'd need accessor methods like x() and set_x() instead. I think this is idiomatic Rust anyway, and it allows to switch the implementation (e.g. SIMD types) without breaking the API.

I don't see why explicit permission from the gdnative author(s) would be needed in order to reuse implementations from godot-rust/gdnative. It's MIT-licensed, so as long as the original license and copyright notice are included, it should be fine. (Offtopic: why is gdextension licensed under MPL2, instead of MIT like gdnative and Godot itself?)

In any case, I have a current need of working Vector*, Color and *Array classes so I'll start working on those, mirroring the Godot API.

@Bromeon
Copy link
Member Author

Bromeon commented Jan 15, 2023

@RealAstolfo thanks a lot, that looks already very nice! 🙂

Could you submit this code as a pull request, so I can review it line-by-line? I saw you and @ttencate were working on some geometric types. Maybe submit only a PR for Vector2 in the beginning, as that will accelerate the review process, and we could take away some learnings for other types.

I think we should provide x and y as public fields, like in GDNative. There are no invariants to upheld on a vector and dealing with setters/getters makes user code extremely verbose. Then, we can have two private functions that would mem::transmute() between godot-rust and glam representations. Of course Vector2 needs to be #[repr(C)] for that.


chitoyuu:
Option 1 is interesting, although also a tricky one to end users. I'd imagine it to be hard to wrap one's head around if we make the full API interface available through FFI, only to suggest users to ignore them all and convert away from the type immediately if they want their code to be fast.

ttencate:
A native implementation (option 2/3) seems like the best way to me. It's a bit of work to implement these types once, but the implementations tend to be simple, and these classes are pretty stable in the engine so it's not a lot of work to keep up.

Fully agree, in addition:

  • Two ways to do the same thing -> why do both exist, when to use which, ...
  • Another place where SemVer can be broken through Godot changes
  • It only helps in the transition phase; once the API is mapped, it is almost always preferable due to performance. I'd rather spend effort on getting that done quicker than having a temporary API of which we already know it's going to break/disappear.

Maybe we can add a generated code layer internally for testing, but I would do that later.


I don't see why explicit permission from the gdnative author(s) would be needed in order to reuse implementations from godot-rust/gdnative. It's MIT-licensed, so as long as the original license and copyright notice are included, it should be fine.

Yes, and this is going to be a pain. MPL2 is file based, while MIT is typically project-based. So we would need to split code depending on license, and for all future make sure we carefully consider this.

What we could do is do a git blame on a line per line, get permission from users who contributed most, and only use their contributions. Then it doesn't matter if 10 people have each fixed a single typo. We would still need to make sure we attribute their work, e.g. as Co-authored-by: in the commit message.

But some APIs have also changed between Godot 3 and 4. All things considered, might be easier to rewrite, but I'm open for input here!


Offtopic: why is gdextension licensed under MPL2, instead of MIT like gdnative and Godot itself?

It's briefly explained in the ReadMe, and there was a longer discussion in #dev-gdextension on Discord a while ago. TLDR: it encourages that improvements of the library flow back upstream, without restricting commercial closed-source game development.


In any case, I have a current need of working Vector*, Color and *Array classes so I'll start working on those, mirroring the Godot API.

Those are great news, looking forward 🙂

Also here, maybe consider submitting early PRs as drafts instead of waiting until you have "the whole perfect thing". That way, we can use the time for early adjustments, and don't risk to throw away a lot of work if it doesn't pass review.

ttencate added a commit to ttencate/gdext that referenced this issue Jan 17, 2023
For Vector{2,3,4}{,i} this implements:
- public fields
- constructors
- constants
- operators
- indexing by axis
- (private) conversions to/from glam types
- Display
- a couple of functions like `abs()` and `length()` for demonstration

See also godot-rust#6.
@ttencate
Copy link
Contributor

ttencate commented Jan 17, 2023

Here's a spreadsheet tracking current progress:
https://docs.google.com/spreadsheets/d/10P37AawtSNGH_zZ4DkpJhTpL55pRMR0koz3i1OGdPXA/edit#gid=0
It reflects the state after #71.

ttencate added a commit to ttencate/gdext that referenced this issue Jan 17, 2023
For Vector{2,3,4}{,i} this implements:
- public fields
- constructors
- constants
- operators
- indexing by axis
- (private) conversions to/from glam types
- Display
- a couple of functions like `abs()` and `length()` for demonstration

See also godot-rust#6.
ttencate added a commit to ttencate/gdext that referenced this issue Jan 17, 2023
For Vector{2,3,4}{,i} this implements:
- public fields
- constructors
- constants
- operators
- indexing by axis
- (private) conversions to/from glam types
- Display
- a couple of functions like `abs()` and `length()` for demonstration

See also godot-rust#6.
ttencate added a commit to ttencate/gdext that referenced this issue Jan 17, 2023
For Vector{2,3,4}{,i} this implements:
- public fields
- constructors
- constants
- operators
- indexing by axis
- (private) conversions to/from glam types
- Display
- a couple of functions like `abs()` and `length()` for demonstration

See also godot-rust#6.
ttencate added a commit to ttencate/gdext that referenced this issue Jan 19, 2023
For Vector{2,3,4}{,i} this implements:
- public fields
- constructors
- constants
- operators
- indexing by axis
- (private) conversions to/from glam types
- Display
- a couple of functions like `abs()` and `length()` for demonstration

See also godot-rust#6.
bors bot added a commit that referenced this issue Jan 20, 2023
71: Implement the basics of built-in vector types r=Bromeon a=ttencate

For Vector{2,3,4}{,i} this implements:
- public fields
- constructors
- constants
- operators
- indexing by axis
- (private) conversions to/from glam types
- Display
- a couple of functions like `abs()` and `length()` for demonstration

See also #6.

This PR supersedes #66.

Co-authored-by: Thomas ten Cate <[email protected]>
Co-authored-by: Jan Haller <[email protected]>
bors bot added a commit that referenced this issue Feb 19, 2023
123: Implement Color functions r=Bromeon a=ttencate

Another small step towards #6.

This implements everything except the large list of named color constants and the getters/setters for hsv, since there are no autogenerated wrappers for either of those yet.

Co-authored-by: Thomas ten Cate <[email protected]>
@lilizoey
Copy link
Member

We are now only missing Callable and Signal. As the last four, Rect2, Rect2i, Aabb, and Plane now have a basic implementation and are being tracked in #209.

A lot of the work on Callable and Signal is likely gonna be hashing out some good APIs to use them though, so will be harder to fully reimplement.

Hapenia-Lans pushed a commit to Hapenia-Lans/gdextension that referenced this issue May 26, 2023
# This is the 1st commit message:

Parse gdextension_interface.h declarations using regex

# This is the commit message #2:

AsUninit trait to convert FFI pointers to their uninitialized versions

# This is the commit message godot-rust#3:

GodotFfi::from_sys_init() now uses uninitialized pointer types

# This is the commit message godot-rust#4:

Introduce GDExtensionUninitialized*Ptr, without changing semantics

# This is the commit message godot-rust#5:

Adjust init code to new get_proc_address mechanism

# This is the commit message godot-rust#6:

Make `trace` feature available in godot-ffi, fix interface access before initialization

# This is the commit message godot-rust#7:

Compatibility layer between Godot 4.0 and 4.1 (different GDExtension APIs)

# This is the commit message godot-rust#8:

Add GdextBuild to access build/runtime metadata

# This is the commit message godot-rust#9:

Detect 4.0 <-> 4.1 mismatches in both directions + missing `compatibility_minimum = 4.1`

# This is the commit message godot-rust#10:

Detect legacy/modern version of C header (also without `custom-godot` feature)

# This is the commit message godot-rust#11:

CI: add jobs that use patched 4.0.x versions

# This is the commit message godot-rust#12:

Remove several memory leaks by constructing into uninitialized pointers

# This is the commit message godot-rust#13:

CI: memcheck jobs for both 4.0.3 and nightly

# This is the commit message godot-rust#14:

Remove ToVariant, FromVariant, and VariantMetadata impls for pointers

This commit splits SignatureTuple into two separate traits:
PtrcallSignatureTuple and VarcallSignatureTuple. The latter is a child
of the former. PtrcallSignatureTuple is used for ptrcall and only
demands GodotFuncMarshall of its arguments. VarcallSignatureTuple is
used for varcall and additionally demands ToVariant, FromVariant, and
VariantMetadata of its arguments, so pointers cannot benefit from the
optimizations provided by varcall over ptrcall.

# This is the commit message godot-rust#15:

Adds FromVariant and ToVariant proc macros

# This is the commit message godot-rust#16:

godot-core: builtin: reimplement Plane functions/methods

# This is the commit message godot-rust#17:

impl GodotFfi for Option<T> when T is pointer sized and nullable godot-rust#240

Additionally FromVariant and ToVariant are also implemented for Option<Gd<T>>
to satisfy all the requirements for ffi and godot_api.

# This is the commit message godot-rust#18:

Fix UB in virtual method calls that take objects
Fix incorrect incrementing of refcount when calling in to godot
Fix refcount not being incremented when we receive a refcounted object in virtual methods

# This is the commit message godot-rust#19:

fix UB caused by preload weirdness

# This is the commit message godot-rust#20:

Implements swizzle and converts from/to tuples
@Bromeon
Copy link
Member Author

Bromeon commented Jun 14, 2023

Closed in favor of #310.

@Bromeon Bromeon closed this as completed Jun 14, 2023
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

No branches or pull requests

5 participants