From f1814459203aca805ef674485d4be364b14a5631 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Sun, 19 Mar 2023 15:48:39 +0100 Subject: [PATCH] Improve property access for roblox instances --- .../src/datatypes/types/enum_item.rs | 55 ++++--------- packages/lib-roblox/src/instance/mod.rs | 71 +++++++++++----- packages/lib-roblox/src/shared/instance.rs | 80 +++++++++---------- 3 files changed, 106 insertions(+), 100 deletions(-) diff --git a/packages/lib-roblox/src/datatypes/types/enum_item.rs b/packages/lib-roblox/src/datatypes/types/enum_item.rs index 68ce7ab..1eb1c49 100644 --- a/packages/lib-roblox/src/datatypes/types/enum_item.rs +++ b/packages/lib-roblox/src/datatypes/types/enum_item.rs @@ -2,7 +2,6 @@ use core::fmt; use mlua::prelude::*; use rbx_dom_weak::types::Enum as DomEnum; -use rbx_reflection::DataType as DomDataType; use super::{super::*, Enum}; @@ -34,6 +33,20 @@ impl EnumItem { }) } + pub(crate) fn from_enum_and_value(parent: &Enum, value: u32) -> Option { + parent.desc.items.iter().find_map(|(name, v)| { + if *v == value { + Some(Self { + parent: parent.clone(), + name: name.to_string(), + value, + }) + } else { + None + } + }) + } + pub(crate) fn from_enum_name_and_name( enum_name: impl AsRef, name: impl AsRef, @@ -42,43 +55,9 @@ impl EnumItem { Self::from_enum_and_name(&parent, name) } - /** - Converts an instance property into an [`EnumItem`] datatype, if the property is known. - - Enums are not strongly typed which means we can not convert directly from a [`rbx_dom_weak::types::Enum`] - into an `EnumItem` without losing information about its parent [`Enum`] and the `EnumItem` name. - - This constructor exists as a shortcut to perform a [`rbx_reflection_database`] lookup for a particular - instance class and property to construct a strongly typed `EnumItem` with no loss of information. - */ - #[allow(dead_code)] - fn from_instance_property( - class_name: impl AsRef, - prop_name: impl AsRef, - value: u32, - ) -> Option { - let db = rbx_reflection_database::get(); - let prop = db - .classes - .get(class_name.as_ref())? - .properties - .get(prop_name.as_ref())?; - let prop_enum = match &prop.data_type { - DomDataType::Enum(name) => db.enums.get(name.as_ref()), - _ => None, - }?; - let enum_name = prop_enum.items.iter().find_map(|(name, v)| { - if v == &value { - Some(name.to_string()) - } else { - None - } - })?; - Some(Self { - parent: prop_enum.into(), - name: enum_name, - value, - }) + pub(crate) fn from_enum_name_and_value(enum_name: impl AsRef, value: u32) -> Option { + let parent = Enum::from_name(enum_name)?; + Self::from_enum_and_value(&parent, value) } } diff --git a/packages/lib-roblox/src/instance/mod.rs b/packages/lib-roblox/src/instance/mod.rs index bbe1c27..2e10437 100644 --- a/packages/lib-roblox/src/instance/mod.rs +++ b/packages/lib-roblox/src/instance/mod.rs @@ -15,9 +15,7 @@ use crate::{ types::EnumItem, userdata_impl_eq, userdata_impl_to_string, }, - shared::instance::{ - class_exists, class_is_a, find_property_enum, find_property_type, property_is_enum, - }, + shared::instance::{class_exists, class_is_a, find_property_info}, }; #[derive(Debug, Clone)] @@ -392,8 +390,10 @@ impl LuaUserData for Instance { Getting a value does the following: 1. Check if it is a special property like "ClassName", "Name" or "Parent" - 2. Try to get a known instance property - 3. Try to get a current child of the instance + 2. Check if a property exists for the wanted name + 2a. Get an existing instance property OR + 2b. Get a property from a known default value + 3. Get a current child of the instance 4. No valid property or instance found, throw error */ methods.add_meta_method(LuaMetaMethod::Index, |lua, this, prop_name: String| { @@ -410,10 +410,42 @@ impl LuaUserData for Instance { _ => {} } - if let Some(prop) = this.get_property(&prop_name) { - match LuaValue::dom_value_to_lua(lua, &prop) { - Ok(value) => Ok(value), - Err(e) => Err(e.into()), + if let Some(info) = find_property_info(&this.class_name, &prop_name) { + if let Some(prop) = this.get_property(&prop_name) { + if let DomValue::Enum(enum_value) = prop { + let enum_name = info.enum_name.ok_or_else(|| { + LuaError::RuntimeError(format!( + "Failed to get property '{}' - encountered unknown enum", + prop_name + )) + })?; + EnumItem::from_enum_name_and_value(&enum_name, enum_value.to_u32()) + .ok_or_else(|| { + LuaError::RuntimeError(format!( + "Failed to get property '{}' - Enum.{} does not contain numeric value {}", + prop_name, enum_name, enum_value.to_u32() + )) + })? + .to_lua(lua) + } else { + Ok(LuaValue::dom_value_to_lua(lua, &prop)?) + } + } else if let (Some(enum_name), Some(enum_value)) = (info.enum_name, info.enum_default) { + EnumItem::from_enum_name_and_value(&enum_name, enum_value) + .ok_or_else(|| { + LuaError::RuntimeError(format!( + "Failed to get property '{}' - Enum.{} does not contain numeric value {}", + prop_name, enum_name, enum_value + )) + })? + .to_lua(lua) + } else if let Some(prop_default) = info.value_default { + Ok(LuaValue::dom_value_to_lua(lua, prop_default)?) + } else { + Err(LuaError::RuntimeError(format!( + "Failed to get property '{}' - malformed property info", + prop_name + ))) } } else if let Some(inst) = this.find_child(|inst| inst.name == prop_name) { Ok(LuaValue::UserData(lua.create_userdata(inst)?)) @@ -429,8 +461,8 @@ impl LuaUserData for Instance { 1. Check if it is a special property like "ClassName", "Name" or "Parent" 2. Check if a property exists for the wanted name - 3a. Set a strict enum from a given EnumItem OR - 3b. Set a normal property from a given value + 2a. Set a strict enum from a given EnumItem OR + 2b. Set a normal property from a given value */ methods.add_meta_method_mut( LuaMetaMethod::NewIndex, @@ -459,7 +491,7 @@ impl LuaUserData for Instance { _ => {} } - let is_enum = match property_is_enum(&this.class_name, &prop_name) { + let info = match find_property_info(&this.class_name, &prop_name) { Some(b) => b, None => { return Err(LuaError::RuntimeError(format!( @@ -469,21 +501,19 @@ impl LuaUserData for Instance { } }; - if is_enum { - let enum_name = find_property_enum(&this.class_name, &prop_name).unwrap(); + if let Some(enum_name) = info.enum_name { match EnumItem::from_lua(prop_value, lua) { Ok(given_enum) if given_enum.name == enum_name => { this.set_property(prop_name, DomValue::Enum(given_enum.into())); Ok(()) } Ok(given_enum) => Err(LuaError::RuntimeError(format!( - "Expected Enum.{}, got Enum.{}", - enum_name, given_enum.name + "Failed to set property '{}' - expected Enum.{}, got Enum.{}", + prop_name, enum_name, given_enum.name ))), Err(e) => Err(e), } - } else { - let dom_type = find_property_type(&this.class_name, &prop_name).unwrap(); + } else if let Some(dom_type) = info.value_type { match prop_value.lua_to_dom_value(lua, dom_type) { Ok(dom_value) => { this.set_property(prop_name, dom_value); @@ -491,6 +521,11 @@ impl LuaUserData for Instance { } Err(e) => Err(e.into()), } + } else { + Err(LuaError::RuntimeError(format!( + "Failed to set property '{}' - malformed property info", + prop_name + ))) } }, ); diff --git a/packages/lib-roblox/src/shared/instance.rs b/packages/lib-roblox/src/shared/instance.rs index 1a71cb6..d833dda 100644 --- a/packages/lib-roblox/src/shared/instance.rs +++ b/packages/lib-roblox/src/shared/instance.rs @@ -1,61 +1,53 @@ use std::borrow::{Borrow, Cow}; -use rbx_dom_weak::types::VariantType as DomType; +use rbx_dom_weak::types::{Variant as DomValue, VariantType as DomType}; use rbx_reflection::{ClassTag, DataType}; +pub(crate) struct PropertyInfo { + pub enum_name: Option>, + pub enum_default: Option, + pub value_type: Option, + pub value_default: Option<&'static DomValue>, +} + /** - Checks if the given property is an enum. + Finds the info of a property of the given class. Returns `None` if the class or property does not exist. */ -pub fn property_is_enum( +pub(crate) fn find_property_info( instance_class: impl AsRef, property_name: impl AsRef, -) -> Option { +) -> Option { let db = rbx_reflection_database::get(); let class = db.classes.get(instance_class.as_ref())?; - let prop = class.properties.get(property_name.as_ref())?; - Some(matches!(prop.data_type, DataType::Enum(_))) -} + let property_name = property_name.as_ref(); + let prop_definition = class.properties.get(property_name)?; + let prop_default = class.default_properties.get(property_name); -/** - Finds the type of a property of the given class. - - Returns `None` if the class or property does not exist or if the property is an enum. -*/ -pub fn find_property_type( - instance_class: impl AsRef, - property_name: impl AsRef, -) -> Option { - let db = rbx_reflection_database::get(); - let class = db.classes.get(instance_class.as_ref())?; - let prop = class.properties.get(property_name.as_ref())?; - - if let DataType::Value(typ) = prop.data_type { - Some(typ) - } else { - None - } -} - -/** - Finds the enum name of a property of the given class. - - Returns `None` if the class or property does not exist or if the property is *not* an enum. -*/ -pub fn find_property_enum( - instance_class: impl AsRef, - property_name: impl AsRef, -) -> Option> { - let db = rbx_reflection_database::get(); - let class = db.classes.get(instance_class.as_ref())?; - let prop = class.properties.get(property_name.as_ref())?; - - if let DataType::Enum(name) = &prop.data_type { - Some(Cow::Borrowed(name)) - } else { - None + match &prop_definition.data_type { + DataType::Enum(enum_name) => Some(PropertyInfo { + enum_name: Some(Cow::Borrowed(enum_name)), + enum_default: prop_default.and_then(|default| match default { + DomValue::Enum(enum_default) => Some(enum_default.to_u32()), + _ => None, + }), + value_type: None, + value_default: None, + }), + DataType::Value(value_type) => Some(PropertyInfo { + enum_name: None, + enum_default: None, + value_type: Some(*value_type), + value_default: prop_default, + }), + _ => Some(PropertyInfo { + enum_name: None, + enum_default: None, + value_type: None, + value_default: None, + }), } }