Skip to content
This repository has been archived by the owner on Jul 22, 2023. It is now read-only.

Add CFrame type #48

Merged
merged 7 commits into from
May 10, 2021
Merged

Add CFrame type #48

merged 7 commits into from
May 10, 2021

Conversation

jeparlefrancais
Copy link
Contributor

@jeparlefrancais jeparlefrancais commented May 8, 2021

This adds the CFrame.new constructor. The pattern matching is a bit scary, but I wanted to make sure to cover at least:

  • CFrame.new(x, y, z) with all combinations of integer and float
  • CFrame.new(position)

Related to #15

)| {
match triplet {
(None, None, None) => Ok(Self::from_position(0.0, 0.0, 0.0)),
(Some(LuaValue::Number(x)), None, None) => {
Copy link
Member

Choose a reason for hiding this comment

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

There's gotta be a better way than all this, especially when you'll probably want ALL the CFrame constructors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I wish I could just overload the function and rlua could figure it out by itself, that'd be cool!

Do you have something in mind? I can make a new either type with a number or a vector and convert the LuaValue to that type. Then I only have to match on number vs vector instead of having to do all number combinations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just pushed something that makes it better, but let me know if you have something else in mind! Not sure what to call the Either type so if you have a better name let me know 🙂

Thanks for your help by the way 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can simplify this code a lot using option combinators. I threw together some rough code that probably doesn't compile, but try something along these lines:

fn try_into_f32(value: LuaValue<'_>) -> Option<f32> {
    match value {
        LuaValue::Number(num) => Some(num as f32),
        LuaValue::Integer(int) => Some(int as f32),
        _ => None,
    }
}

fn to_position(
    triplet: (
        Option<LuaValue<'_>>,
        Option<LuaValue<'_>>,
        Option<LuaValue<'_>>,
    ),
) {
    match &triplet {
        (Some(LuaValue::UserData(user_data)), None, None) => {
            let position = &*user_data.borrow::<Vector3Value>()?;

            return Ok(CFrameValue::new(CFrame::new(
                position.0,
                // TODO: replace with `rbx_dom_weak::types::Matrix3::identity()` once
                // a version higher than 0.3.0 of rbx_types ships
                Matrix3::new(
                    Vector3::new(1.0, 0.0, 0.0),
                    Vector3::new(0.0, 1.0, 0.0),
                    Vector3::new(0.0, 0.0, 1.0),
                ),
            )));
        }

        _ => {}
    }

    let x = triplet.0.and_then(try_into_f32);
    let y = triplet.1.and_then(try_into_f32);
    let z = triplet.2.and_then(try_into_f32);

    match (x, y, z) {
        (Some(x), Some(y), Some(z)) => Ok(Self::from_position(x, y, z)),
        _ => Err(rlua::Error::external(
            "invalid argument #1 to 'new' (Vector3 expected)",
        )),
    }
}

src/value.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@LPGhatguy LPGhatguy left a comment

Choose a reason for hiding this comment

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

Some comments on idiomatic Rust here and there.

The 12 argument constructor is gonna be awful to write, heh.

)| {
match triplet {
(None, None, None) => Ok(Self::from_position(0.0, 0.0, 0.0)),
(Some(LuaValue::Number(x)), None, None) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can simplify this code a lot using option combinators. I threw together some rough code that probably doesn't compile, but try something along these lines:

fn try_into_f32(value: LuaValue<'_>) -> Option<f32> {
    match value {
        LuaValue::Number(num) => Some(num as f32),
        LuaValue::Integer(int) => Some(int as f32),
        _ => None,
    }
}

fn to_position(
    triplet: (
        Option<LuaValue<'_>>,
        Option<LuaValue<'_>>,
        Option<LuaValue<'_>>,
    ),
) {
    match &triplet {
        (Some(LuaValue::UserData(user_data)), None, None) => {
            let position = &*user_data.borrow::<Vector3Value>()?;

            return Ok(CFrameValue::new(CFrame::new(
                position.0,
                // TODO: replace with `rbx_dom_weak::types::Matrix3::identity()` once
                // a version higher than 0.3.0 of rbx_types ships
                Matrix3::new(
                    Vector3::new(1.0, 0.0, 0.0),
                    Vector3::new(0.0, 1.0, 0.0),
                    Vector3::new(0.0, 0.0, 1.0),
                ),
            )));
        }

        _ => {}
    }

    let x = triplet.0.and_then(try_into_f32);
    let y = triplet.1.and_then(try_into_f32);
    let z = triplet.2.and_then(try_into_f32);

    match (x, y, z) {
        (Some(x), Some(y), Some(z)) => Ok(Self::from_position(x, y, z)),
        _ => Err(rlua::Error::external(
            "invalid argument #1 to 'new' (Vector3 expected)",
        )),
    }
}

src/roblox_api/cframe.rs Outdated Show resolved Hide resolved
src/roblox_api/mod.rs Outdated Show resolved Hide resolved
src/value.rs Outdated Show resolved Hide resolved
src/value.rs Show resolved Hide resolved
src/roblox_api/cframe.rs Outdated Show resolved Hide resolved
@LPGhatguy LPGhatguy merged commit 51edda7 into rojo-rbx:master May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants