Fix erroring when setting nil attributes on instances in roblox lib

CHANGELOG.md

@@ -21,8 +21,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Fixed
 
-- Fixed formatted values in tables not being separated by newlines
+- Fixed formatted values in tables not being separated by newlines.
 - Fixed panicking (crashing) when using `process.spawn` with a program that does not exist.
+- Fixed `instance:SetAttribute("name", nil)` throwing an error and not removing the attribute.
 
 ## `0.8.4` - May 12th, 2024
 

crates/lune-roblox/src/instance/base.rs

@@ -155,13 +155,18 @@ pub fn add_methods<'lua, M: LuaUserDataMethods<'lua, Instance>>(m: &mut M) {
         |lua, this, (attribute_name, lua_value): (String, LuaValue)| {
             ensure_not_destroyed(this)?;
             ensure_valid_attribute_name(&attribute_name)?;
-            match lua_value.lua_to_dom_value(lua, None) {
+            if lua_value.is_nil() || lua_value.is_null() {
-                Ok(dom_value) => {
+                this.remove_attribute(attribute_name);
-                    ensure_valid_attribute_value(&dom_value)?;
+                Ok(())
-                    this.set_attribute(attribute_name, dom_value);
+            } else {
-                    Ok(())
+                match lua_value.lua_to_dom_value(lua, None) {
+                    Ok(dom_value) => {
+                        ensure_valid_attribute_value(&dom_value)?;
+                        this.set_attribute(attribute_name, dom_value);
+                        Ok(())
+                    }
+                    Err(e) => Err(e.into()),
                 }
-                Err(e) => Err(e.into()),
             }
         },
     );

crates/lune-roblox/src/instance/mod.rs

@@ -442,6 +442,29 @@ impl Instance {
         }
     }
 
+    /**
+        Removes an attribute from the instance.
+
+        Note that this does not have an equivalent in the Roblox engine API,
+        but separating this from `set_attribute` lets `set_attribute` be more
+        ergonomic and not require an `Option<DomValue>` for the value argument.
+        The equivalent in the Roblox engine API would be `instance:SetAttribute(name, nil)`.
+    */
+    pub fn remove_attribute(&self, name: impl AsRef<str>) {
+        let mut dom = INTERNAL_DOM.lock().expect("Failed to lock document");
+        let inst = dom
+            .get_by_ref_mut(self.dom_ref)
+            .expect("Failed to find instance in document");
+        if let Some(DomValue::Attributes(attributes)) =
+            inst.properties.get_mut(PROPERTY_NAME_ATTRIBUTES)
+        {
+            attributes.remove(name.as_ref());
+            if attributes.is_empty() {
+                inst.properties.remove(PROPERTY_NAME_ATTRIBUTES);
+            }
+        }
+    }
+
     /**
         Adds a tag to the instance.
 

tests/roblox/instance/attributes.luau

@@ -101,6 +101,11 @@ local folder = Instance.new("Folder")
 folder:SetAttribute("Foo", "Bar")
 assert(folder:GetAttribute("Foo") == "Bar")
 
+-- Setting attributes to nil should work
+
+folder:SetAttribute("Foo", nil)
+assert(folder:GetAttribute("Foo") == nil)
+
 -- Writing files with modified attributes should work
 
 local game = Instance.new("DataModel")