Improve property access for roblox instances

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<Self> {
+        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<str>,
         name: impl AsRef<str>,
@@ -42,43 +55,9 @@ impl EnumItem {
         Self::from_enum_and_name(&parent, name)
     }
 
-    /**
+    pub(crate) fn from_enum_name_and_value(enum_name: impl AsRef<str>, value: u32) -> Option<Self> {
-        Converts an instance property into an [`EnumItem`] datatype, if the property is known.
+        let parent = Enum::from_name(enum_name)?;
-
+        Self::from_enum_and_value(&parent, value)
-        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<str>,
-        prop_name: impl AsRef<str>,
-        value: u32,
-    ) -> Option<Self> {
-        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,
-        })
     }
 }
 

packages/lib-roblox/src/instance/mod.rs

@@ -15,9 +15,7 @@ use crate::{
         types::EnumItem,
         userdata_impl_eq, userdata_impl_to_string,
     },
-    shared::instance::{
+    shared::instance::{class_exists, class_is_a, find_property_info},
-        class_exists, class_is_a, find_property_enum, find_property_type, property_is_enum,
-    },
 };
 
 #[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
+            2. Check if a property exists for the wanted name
-            3. Try to get a current child of the instance
+                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) {
+            if let Some(info) = find_property_info(&this.class_name, &prop_name) {
-                match LuaValue::dom_value_to_lua(lua, &prop) {
+                if let Some(prop) = this.get_property(&prop_name) {
-                    Ok(value) => Ok(value),
+                    if let DomValue::Enum(enum_value) = prop {
-                    Err(e) => Err(e.into()),
+                        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
+                2a. Set a strict enum from a given EnumItem OR
-            3b. Set a normal property from a given value
+                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 {
+                if let Some(enum_name) = info.enum_name {
-                    let enum_name = find_property_enum(&this.class_name, &prop_name).unwrap();
                     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.{}",
+                            "Failed to set property '{}' - expected Enum.{}, got Enum.{}",
-                            enum_name, given_enum.name
+                            prop_name, enum_name, given_enum.name
                         ))),
                         Err(e) => Err(e),
                     }
-                } else {
+                } else if let Some(dom_type) = info.value_type {
-                    let dom_type = find_property_type(&this.class_name, &prop_name).unwrap();
                     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
+                    )))
                 }
             },
         );

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<Cow<'static, str>>,
+    pub enum_default: Option<u32>,
+    pub value_type: Option<DomType>,
+    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<str>,
     property_name: impl AsRef<str>,
-) -> Option<bool> {
+) -> Option<PropertyInfo> {
     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);
 
-/**
+    match &prop_definition.data_type {
-    Finds the type of a property of the given class.
+        DataType::Enum(enum_name) => Some(PropertyInfo {
-
+            enum_name: Some(Cow::Borrowed(enum_name)),
-    Returns `None` if the class or property does not exist or if the property is an enum.
+            enum_default: prop_default.and_then(|default| match default {
-*/
+                DomValue::Enum(enum_default) => Some(enum_default.to_u32()),
-pub fn find_property_type(
+                _ => None,
-    instance_class: impl AsRef<str>,
+            }),
-    property_name: impl AsRef<str>,
+            value_type: None,
-) -> Option<DomType> {
+            value_default: None,
-    let db = rbx_reflection_database::get();
+        }),
-    let class = db.classes.get(instance_class.as_ref())?;
+        DataType::Value(value_type) => Some(PropertyInfo {
-    let prop = class.properties.get(property_name.as_ref())?;
+            enum_name: None,
-
+            enum_default: None,
-    if let DataType::Value(typ) = prop.data_type {
+            value_type: Some(*value_type),
-        Some(typ)
+            value_default: prop_default,
-    } else {
+        }),
-        None
+        _ => Some(PropertyInfo {
-    }
+            enum_name: None,
-}
+            enum_default: None,
-
+            value_type: None,
-/**
+            value_default: 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<str>,
-    property_name: impl AsRef<str>,
-) -> Option<Cow<'static, str>> {
-    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
     }
 }