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

Update Specta Implementation for glam & bevy_ecs Types based on Serialization Format (serde) #164

Closed
bennobuilder opened this issue Oct 26, 2023 · 2 comments · Fixed by #165

Comments

@bennobuilder
Copy link
Contributor

bennobuilder commented Oct 26, 2023

The current specta implementation for glam types defines each type according to its original structure. However, this does not align with the serialized format of these types as provided by glam's serde implementation. The serialized format represents these types as arrays rather than individual struct fields.

For instance, glam::DVec2 is represented as a struct with fields x and y in the current specta implementation, but its serialized form is an array [x, y].

// Current specta implementation

#[derive(Type)]
#[specta(remote = glam::DVec2, crate = crate, export = false)]
#[allow(dead_code)]
struct DVec2 {
    x: f64,
    y: f64,
}

Proposed Solution

Update the specta implementations to match the serialization format of glam's serde implementation. This will ensure consistency and correctness when working with these types in serialized form. The updated implementation should represent the serialized format as arrays:

// Updated specta implementation

#[derive(Type)]
#[specta(remote = glam::DVec2, crate = crate, export = false)]
#[allow(dead_code)]
struct DVec2([f64; 2]);

Expected implementation for wrongly implemented glam types:

DVec2: Ok("[1.0,2.0]")
IVec2: Ok("[1,2]")
DMat2: Ok("[1.0,2.0,3.0,4.0]")
DAffine2: Ok("[1.0,2.0,3.0,4.0,1.0,2.0]")
Vec2: Ok("[1.0,2.0]")
Vec3: Ok("[1.0,2.0,3.0]")
Vec3A: Ok("[1.0,2.0,3.0]")
Vec4: Ok("[1.0,2.0,3.0,4.0]")
Mat2: Ok("[1.0,2.0,3.0,4.0]")
Mat3: Ok("[1.0,2.0,3.0,4.0,5.0,6.0,7.0,8.0,9.0]")
Mat3A: Ok("[1.0,2.0,3.0,4.0,5.0,6.0,7.0,8.0,9.0]")
Mat4: Ok("[1.0,2.0,3.0,4.0,5.0,6.0,7.0,8.0,9.0,10.0,11.0,12.0,13.0,14.0,15.0,16.0]")
Quat: Ok("[1.0,2.0,3.0,4.0]")
Affine2: Ok("[1.0,2.0,3.0,4.0,1.0,2.0]")
Affine3A: Ok("[1.0,2.0,3.0,4.0,5.0,6.0,7.0,8.0,9.0,1.0,2.0,3.0]")
Based on
fn serialize_and_log_glam_types() {
    // Create instances of glam types
    let dvec2 = DVec2::new(1.0, 2.0);
    let ivec2 = IVec2::new(1, 2);
    let dmat2 = DMat2::from_cols_array(&[1.0, 2.0, 3.0, 4.0]);
    let daffine2 = DAffine2 {
        matrix2: dmat2,
        translation: dvec2,
    };
    let vec2 = Vec2::new(1.0, 2.0);
    let vec3 = Vec3::new(1.0, 2.0, 3.0);
    let vec3a = Vec3A::new(1.0, 2.0, 3.0);
    let vec4 = Vec4::new(1.0, 2.0, 3.0, 4.0);
    let mat2 = Mat2::from_cols_array(&[1.0, 2.0, 3.0, 4.0]);
    let mat3 = Mat3::from_cols_array(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0]);
    let mat3a = Mat3A::from_cols_array(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0]);
    let mat4 = Mat4::from_cols_array(&[
        1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0, 16.0,
    ]);
    let quat = Quat::from_xyzw(1.0, 2.0, 3.0, 4.0);
    let affine2 = Affine2 {
        matrix2: mat2,
        translation: vec2,
    };
    let affine3a = Affine3A {
        matrix3: mat3a,
        translation: vec3a,
    };

    // Serialize and log each glam type
    log_glam_type("DVec2", &dvec2);
    log_glam_type("IVec2", &ivec2);
    log_glam_type("DMat2", &dmat2);
    log_glam_type("DAffine2", &daffine2);
    log_glam_type("Vec2", &vec2);
    log_glam_type("Vec3", &vec3);
    log_glam_type("Vec3A", &vec3a);
    log_glam_type("Vec4", &vec4);
    log_glam_type("Mat2", &mat2);
    log_glam_type("Mat3", &mat3);
    log_glam_type("Mat3A", &mat3a);
    log_glam_type("Mat4", &mat4);
    log_glam_type("Quat", &quat);
    log_glam_type("Affine2", &affine2);
    log_glam_type("Affine3A", &affine3a);
}

fn log_glam_type<T: serde::Serialize>(name: &str, value: &T) {
    let serialized = serde_json::to_string(value);
    println!("{}: {:?}", name, serialized);
}
@oscartbeaumont
Copy link
Member

Would happily accept a PR, else when I have a few minutes will make the changes. We definitely need to stick to Serde definitions! I was not aware so many crates have custom impls for them.

@bennobuilder
Copy link
Contributor Author

Would happily accept a PR, else when I have a few minutes will make the changes. We definitely need to stick to Serde definitions! I was not aware so many crates have custom impls for them.

I'm on it :)

bennobuilder added a commit to bennobuilder/specta that referenced this issue Oct 26, 2023
bennobuilder added a commit to bennobuilder/specta that referenced this issue Oct 26, 2023
bennobuilder added a commit to bennobuilder/specta that referenced this issue Oct 26, 2023
bennobuilder added a commit to bennobuilder/specta that referenced this issue Oct 26, 2023
bennobuilder added a commit to bennobuilder/specta that referenced this issue Oct 26, 2023
bennobuilder added a commit to bennobuilder/specta that referenced this issue Oct 26, 2023
bennobuilder added a commit to bennobuilder/specta that referenced this issue Nov 5, 2023
oscartbeaumont added a commit that referenced this issue Nov 22, 2023
…ion (#165)

* #164 updated glam & bevy_ecs types to match their `serde` implementation

* #164 fixed typo

* #164 improved error message and removed WorldId (no serde implemented)

* #164 fixed test

* #164 updated array to tuple

* #164 fixed to long tuple for Mat4 and added basic tests

* #164 bumped bevy version

* avoid string allocate when error not hit

* small cleanup

---------

Co-authored-by: Oscar Beaumont <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants