From 7fd32e67c2ad904c92047d4d7ac550eaf3fd5a29 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Sat, 11 Mar 2023 20:50:53 +0100 Subject: [PATCH] Improve error reporting & ergonomics for datatype conversion --- .../lib-roblox/src/datatypes/conversion.rs | 69 +++++++++++++------ .../lib-roblox/src/datatypes/types/udim.rs | 12 ++-- .../lib-roblox/src/datatypes/types/udim2.rs | 48 ++++--------- .../lib-roblox/src/datatypes/types/vector2.rs | 8 +-- .../src/datatypes/types/vector2int16.rs | 8 +-- .../lib-roblox/src/datatypes/types/vector3.rs | 8 +-- .../src/datatypes/types/vector3int16.rs | 8 +-- 7 files changed, 86 insertions(+), 75 deletions(-) diff --git a/packages/lib-roblox/src/datatypes/conversion.rs b/packages/lib-roblox/src/datatypes/conversion.rs index 16602e8..f4c7574 100644 --- a/packages/lib-roblox/src/datatypes/conversion.rs +++ b/packages/lib-roblox/src/datatypes/conversion.rs @@ -109,11 +109,12 @@ impl<'lua> LuaToRbxVariant<'lua> for LuaValue<'lua> { */ impl<'lua> RbxVariantToLua<'lua> for LuaAnyUserData<'lua> { + #[rustfmt::skip] fn rbx_variant_to_lua(lua: &'lua Lua, variant: &RbxVariant) -> DatatypeConversionResult { use super::types::*; use RbxVariant as Rbx; - Ok(match variant { + Ok(match variant.clone() { // Not yet implemented datatypes // Rbx::Axes(_) => todo!(), // Rbx::BrickColor(_) => todo!(), @@ -131,18 +132,19 @@ impl<'lua> RbxVariantToLua<'lua> for LuaAnyUserData<'lua> { // Rbx::Rect(_) => todo!(), // Rbx::Region3(_) => todo!(), // Rbx::Region3int16(_) => todo!(), - Rbx::UDim(value) => lua.create_userdata(UDim::from(value))?, + + Rbx::UDim(value) => lua.create_userdata(UDim::from(value))?, Rbx::UDim2(value) => lua.create_userdata(UDim2::from(value))?, - Rbx::Vector2(value) => lua.create_userdata(Vector2::from(value))?, + Rbx::Vector2(value) => lua.create_userdata(Vector2::from(value))?, Rbx::Vector2int16(value) => lua.create_userdata(Vector2int16::from(value))?, - Rbx::Vector3(value) => lua.create_userdata(Vector3::from(value))?, + Rbx::Vector3(value) => lua.create_userdata(Vector3::from(value))?, Rbx::Vector3int16(value) => lua.create_userdata(Vector3int16::from(value))?, v => { return Err(DatatypeConversionError::FromRbxVariant { from: v.variant_name(), - to: "LuaValue", + to: "userdata", detail: Some("Type not supported".to_string()), }) } @@ -151,31 +153,54 @@ impl<'lua> RbxVariantToLua<'lua> for LuaAnyUserData<'lua> { } impl<'lua> LuaToRbxVariant<'lua> for LuaAnyUserData<'lua> { + #[rustfmt::skip] fn lua_to_rbx_variant( &self, _: &'lua Lua, variant_type: RbxVariantType, ) -> DatatypeConversionResult { use super::types::*; + use rbx_dom_weak::types as rbx; - Ok(if let Ok(value) = self.borrow::() { - RbxVariant::UDim((&*value).into()) - } else if let Ok(value) = self.borrow::() { - RbxVariant::UDim2((&*value).into()) - } else if let Ok(value) = self.borrow::() { - RbxVariant::Vector2((&*value).into()) - } else if let Ok(value) = self.borrow::() { - RbxVariant::Vector2int16((&*value).into()) - } else if let Ok(value) = self.borrow::() { - RbxVariant::Vector3((&*value).into()) - } else if let Ok(value) = self.borrow::() { - RbxVariant::Vector3int16((&*value).into()) - } else { - return Err(DatatypeConversionError::ToRbxVariant { + let f = match variant_type { + RbxVariantType::UDim => convert::, + RbxVariantType::UDim2 => convert::, + + RbxVariantType::Vector2 => convert::, + RbxVariantType::Vector2int16 => convert::, + RbxVariantType::Vector3 => convert::, + RbxVariantType::Vector3int16 => convert::, + + _ => return Err(DatatypeConversionError::ToRbxVariant { to: variant_type.variant_name(), from: "userdata", - detail: None, - }); - }) + detail: Some("Type not supported".to_string()), + }), + }; + + f(self, variant_type) + } +} + +fn convert( + userdata: &LuaAnyUserData, + variant_type: RbxVariantType, +) -> DatatypeConversionResult +where + Datatype: LuaUserData + Clone + 'static, + RbxType: From + Into, +{ + match userdata.borrow::() { + Ok(value) => Ok(RbxType::from(value.clone()).into()), + Err(LuaError::UserDataTypeMismatch) => Err(DatatypeConversionError::ToRbxVariant { + to: variant_type.variant_name(), + from: "userdata", + detail: Some("Type mismatch".to_string()), + }), + Err(e) => Err(DatatypeConversionError::ToRbxVariant { + to: variant_type.variant_name(), + from: "userdata", + detail: Some(format!("Internal error: {e}")), + }), } } diff --git a/packages/lib-roblox/src/datatypes/types/udim.rs b/packages/lib-roblox/src/datatypes/types/udim.rs index 1c41b6c..36a768d 100644 --- a/packages/lib-roblox/src/datatypes/types/udim.rs +++ b/packages/lib-roblox/src/datatypes/types/udim.rs @@ -18,6 +18,10 @@ pub struct UDim { } impl UDim { + pub(super) fn new(scale: f32, offset: i32) -> Self { + Self { scale, offset } + } + pub(crate) fn make_table(lua: &Lua, datatype_table: &LuaTable) -> LuaResult<()> { datatype_table.set( "new", @@ -91,8 +95,8 @@ impl ops::Sub for UDim { } } -impl From<&RbxUDim> for UDim { - fn from(v: &RbxUDim) -> Self { +impl From for UDim { + fn from(v: RbxUDim) -> Self { UDim { scale: v.scale, offset: v.offset, @@ -100,8 +104,8 @@ impl From<&RbxUDim> for UDim { } } -impl From<&UDim> for RbxUDim { - fn from(v: &UDim) -> Self { +impl From for RbxUDim { + fn from(v: UDim) -> Self { RbxUDim { scale: v.scale, offset: v.offset, diff --git a/packages/lib-roblox/src/datatypes/types/udim2.rs b/packages/lib-roblox/src/datatypes/types/udim2.rs index 0a3f129..88a6f12 100644 --- a/packages/lib-roblox/src/datatypes/types/udim2.rs +++ b/packages/lib-roblox/src/datatypes/types/udim2.rs @@ -24,14 +24,8 @@ impl UDim2 { "fromScale", lua.create_function(|_, (x, y): (Option, Option)| { Ok(UDim2 { - x: UDim { - scale: x.unwrap_or_default(), - offset: 0, - }, - y: UDim { - scale: y.unwrap_or_default(), - offset: 0, - }, + x: UDim::new(x.unwrap_or_default(), 0), + y: UDim::new(y.unwrap_or_default(), 0), }) })?, )?; @@ -39,14 +33,8 @@ impl UDim2 { "fromOffset", lua.create_function(|_, (x, y): (Option, Option)| { Ok(UDim2 { - x: UDim { - scale: 0f32, - offset: x.unwrap_or_default(), - }, - y: UDim { - scale: 0f32, - offset: y.unwrap_or_default(), - }, + x: UDim::new(0f32, x.unwrap_or_default()), + y: UDim::new(0f32, y.unwrap_or_default()), }) })?, )?; @@ -62,17 +50,11 @@ impl UDim2 { }) } else if let Ok((sx, ox, sy, oy)) = ArgsNums::from_lua_multi(args, lua) { Ok(UDim2 { - x: UDim { - scale: sx.unwrap_or_default(), - offset: ox.unwrap_or_default(), - }, - y: UDim { - scale: sy.unwrap_or_default(), - offset: oy.unwrap_or_default(), - }, + x: UDim::new(sx.unwrap_or_default(), ox.unwrap_or_default()), + y: UDim::new(sy.unwrap_or_default(), oy.unwrap_or_default()), }) } else { - // TODO: Better error message here using arg types + // FUTURE: Better error message here using given arg types Err(LuaError::RuntimeError( "Invalid arguments to constructor".to_string(), )) @@ -158,20 +140,20 @@ impl ops::Sub for UDim2 { } } -impl From<&RbxUDim2> for UDim2 { - fn from(v: &RbxUDim2) -> Self { +impl From for UDim2 { + fn from(v: RbxUDim2) -> Self { UDim2 { - x: (&v.x).into(), - y: (&v.y).into(), + x: v.x.into(), + y: v.y.into(), } } } -impl From<&UDim2> for RbxUDim2 { - fn from(v: &UDim2) -> Self { +impl From for RbxUDim2 { + fn from(v: UDim2) -> Self { RbxUDim2 { - x: (&v.x).into(), - y: (&v.y).into(), + x: v.x.into(), + y: v.y.into(), } } } diff --git a/packages/lib-roblox/src/datatypes/types/vector2.rs b/packages/lib-roblox/src/datatypes/types/vector2.rs index 384f3e1..1e1d8e1 100644 --- a/packages/lib-roblox/src/datatypes/types/vector2.rs +++ b/packages/lib-roblox/src/datatypes/types/vector2.rs @@ -138,14 +138,14 @@ impl ops::Sub for Vector2 { } } -impl From<&RbxVector2> for Vector2 { - fn from(v: &RbxVector2) -> Self { +impl From for Vector2 { + fn from(v: RbxVector2) -> Self { Vector2(Vec2 { x: v.x, y: v.y }) } } -impl From<&Vector2> for RbxVector2 { - fn from(v: &Vector2) -> Self { +impl From for RbxVector2 { + fn from(v: Vector2) -> Self { RbxVector2 { x: v.0.x, y: v.0.y } } } diff --git a/packages/lib-roblox/src/datatypes/types/vector2int16.rs b/packages/lib-roblox/src/datatypes/types/vector2int16.rs index b70065a..08aa5c3 100644 --- a/packages/lib-roblox/src/datatypes/types/vector2int16.rs +++ b/packages/lib-roblox/src/datatypes/types/vector2int16.rs @@ -113,8 +113,8 @@ impl ops::Sub for Vector2int16 { } } -impl From<&RbxVector2int16> for Vector2int16 { - fn from(v: &RbxVector2int16) -> Self { +impl From for Vector2int16 { + fn from(v: RbxVector2int16) -> Self { Vector2int16(IVec2 { x: v.x.clamp(i16::MIN, i16::MAX) as i32, y: v.y.clamp(i16::MIN, i16::MAX) as i32, @@ -122,8 +122,8 @@ impl From<&RbxVector2int16> for Vector2int16 { } } -impl From<&Vector2int16> for RbxVector2int16 { - fn from(v: &Vector2int16) -> Self { +impl From for RbxVector2int16 { + fn from(v: Vector2int16) -> Self { RbxVector2int16 { x: v.0.x.clamp(i16::MIN as i32, i16::MAX as i32) as i16, y: v.0.y.clamp(i16::MIN as i32, i16::MAX as i32) as i16, diff --git a/packages/lib-roblox/src/datatypes/types/vector3.rs b/packages/lib-roblox/src/datatypes/types/vector3.rs index 4ebe7b6..1d90f88 100644 --- a/packages/lib-roblox/src/datatypes/types/vector3.rs +++ b/packages/lib-roblox/src/datatypes/types/vector3.rs @@ -152,8 +152,8 @@ impl ops::Sub for Vector3 { } } -impl From<&RbxVector3> for Vector3 { - fn from(v: &RbxVector3) -> Self { +impl From for Vector3 { + fn from(v: RbxVector3) -> Self { Vector3(Vec3 { x: v.x, y: v.y, @@ -162,8 +162,8 @@ impl From<&RbxVector3> for Vector3 { } } -impl From<&Vector3> for RbxVector3 { - fn from(v: &Vector3) -> Self { +impl From for RbxVector3 { + fn from(v: Vector3) -> Self { RbxVector3 { x: v.0.x, y: v.0.y, diff --git a/packages/lib-roblox/src/datatypes/types/vector3int16.rs b/packages/lib-roblox/src/datatypes/types/vector3int16.rs index b3a97bf..3e70164 100644 --- a/packages/lib-roblox/src/datatypes/types/vector3int16.rs +++ b/packages/lib-roblox/src/datatypes/types/vector3int16.rs @@ -115,8 +115,8 @@ impl ops::Sub for Vector3int16 { } } -impl From<&RbxVector3int16> for Vector3int16 { - fn from(v: &RbxVector3int16) -> Self { +impl From for Vector3int16 { + fn from(v: RbxVector3int16) -> Self { Vector3int16(IVec3 { x: v.x.clamp(i16::MIN, i16::MAX) as i32, y: v.y.clamp(i16::MIN, i16::MAX) as i32, @@ -125,8 +125,8 @@ impl From<&RbxVector3int16> for Vector3int16 { } } -impl From<&Vector3int16> for RbxVector3int16 { - fn from(v: &Vector3int16) -> Self { +impl From for RbxVector3int16 { + fn from(v: Vector3int16) -> Self { RbxVector3int16 { x: v.0.x.clamp(i16::MIN as i32, i16::MAX as i32) as i16, y: v.0.y.clamp(i16::MIN as i32, i16::MAX as i32) as i16,