diff --git a/plugin/.luacheckrc b/plugin/.luacheckrc index 5e011987..9a0c40a1 100644 --- a/plugin/.luacheckrc +++ b/plugin/.luacheckrc @@ -13,6 +13,7 @@ stds.roblox = { -- Types "Vector2", "Vector3", + "Vector2int16", "Vector3int16", "Color3", "UDim", "UDim2", "Rect", diff --git a/plugin/src/InstanceMap.lua b/plugin/src/InstanceMap.lua new file mode 100644 index 00000000..5356ac7b --- /dev/null +++ b/plugin/src/InstanceMap.lua @@ -0,0 +1,71 @@ +local Logging = require(script.Parent.Logging) + +--[[ + A bidirectional map between instance IDs and Roblox instances. It lets us + keep track of every instance we know about. + + TODO: Track ancestry to catch when stuff moves? +]] +local InstanceMap = {} +InstanceMap.__index = InstanceMap + +function InstanceMap.new() + local self = { + fromIds = {}, + fromInstances = {}, + } + + return setmetatable(self, InstanceMap) +end + +function InstanceMap:insert(id, instance) + self.fromIds[id] = instance + self.fromInstances[instance] = id +end + +function InstanceMap:removeId(id) + local instance = self.fromIds[id] + + if instance ~= nil then + self.fromIds[id] = nil + self.fromInstances[instance] = nil + else + Logging.warn("Attempted to remove nonexistant ID %s", tostring(id)) + end +end + +function InstanceMap:removeInstance(instance) + local id = self.fromInstances[instance] + + if id ~= nil then + self.fromInstances[instance] = nil + self.fromIds[id] = nil + else + Logging.warn("Attempted to remove nonexistant instance %s", tostring(instance)) + end +end + +function InstanceMap:destroyId(id) + local instance = self.fromIds[id] + self:removeId(id) + + if instance ~= nil then + local descendantsToDestroy = {} + + for otherInstance in pairs(self.fromInstances) do + if otherInstance:IsDescendantOf(instance) then + table.insert(descendantsToDestroy, otherInstance) + end + end + + for _, otherInstance in ipairs(descendantsToDestroy) do + self:removeInstance(otherInstance) + end + + instance:Destroy() + else + Logging.warn("Attempted to destroy nonexistant ID %s", tostring(id)) + end +end + +return InstanceMap \ No newline at end of file diff --git a/plugin/src/Reconciler.lua b/plugin/src/Reconciler.lua index 07e9db76..e3a7bc40 100644 --- a/plugin/src/Reconciler.lua +++ b/plugin/src/Reconciler.lua @@ -1,108 +1,14 @@ +local InstanceMap = require(script.Parent.InstanceMap) local Logging = require(script.Parent.Logging) - -local function makeInstanceMap() - local self = { - fromIds = {}, - fromInstances = {}, - } - - function self:insert(id, instance) - self.fromIds[id] = instance - self.fromInstances[instance] = id - end - - function self:removeId(id) - local instance = self.fromIds[id] - - if instance ~= nil then - self.fromIds[id] = nil - self.fromInstances[instance] = nil - else - Logging.warn("Attempted to remove nonexistant ID %s", tostring(id)) - end - end - - function self:removeInstance(instance) - local id = self.fromInstances[instance] - - if id ~= nil then - self.fromInstances[instance] = nil - self.fromIds[id] = nil - else - Logging.warn("Attempted to remove nonexistant instance %s", tostring(instance)) - end - end - - function self:destroyId(id) - local instance = self.fromIds[id] - self:removeId(id) - - if instance ~= nil then - local descendantsToDestroy = {} - - for otherInstance in pairs(self.fromInstances) do - if otherInstance:IsDescendantOf(instance) then - table.insert(descendantsToDestroy, otherInstance) - end - end - - for _, otherInstance in ipairs(descendantsToDestroy) do - self:removeInstance(otherInstance) - end - - instance:Destroy() - else - Logging.warn("Attempted to destroy nonexistant ID %s", tostring(id)) - end - end - - return self -end - -local function setProperty(instance, key, value) - -- The 'Contents' property of LocalizationTable isn't directly exposed, but - -- has corresponding (deprecated) getters and setters. - if key == "Contents" and instance.ClassName == "LocalizationTable" then - instance:SetContents(value) - return - end - - -- If we don't have permissions to access this value at all, we can skip it. - local readSuccess, existingValue = pcall(function() - return instance[key] - end) - - if not readSuccess then - -- An error will be thrown if there was a permission issue or if the - -- property doesn't exist. In the latter case, we should tell the user - -- because it's probably their fault. - if existingValue:find("lacking permission") then - Logging.trace("Permission error reading property %s on class %s", tostring(key), instance.ClassName) - return - else - error(("Invalid property %s on class %s: %s"):format(tostring(key), instance.ClassName, existingValue), 2) - end - end - - local writeSuccess, err = pcall(function() - if existingValue ~= value then - instance[key] = value - end - end) - - if not writeSuccess then - error(("Cannot set property %s on class %s: %s"):format(tostring(key), instance.ClassName, err), 2) - end - - return true -end +local setProperty = require(script.Parent.setProperty) +local rojoValueToRobloxValue = require(script.Parent.rojoValueToRobloxValue) local Reconciler = {} Reconciler.__index = Reconciler function Reconciler.new() local self = { - instanceMap = makeInstanceMap(), + instanceMap = InstanceMap.new(), } return setmetatable(self, Reconciler) @@ -140,7 +46,7 @@ function Reconciler:reconcile(virtualInstancesById, id, instance) setProperty(instance, "Name", virtualInstance.Name) for key, value in pairs(virtualInstance.Properties) do - setProperty(instance, key, value.Value) + setProperty(instance, key, rojoValueToRobloxValue(value)) end local existingChildren = instance:GetChildren() @@ -195,9 +101,6 @@ function Reconciler:reconcile(virtualInstancesById, id, instance) -- Some instances, like services, don't like having their Parent -- property poked, even if we're setting it to the same value. setProperty(instance, "Parent", parent) - if instance.Parent ~= parent then - instance.Parent = parent - end end return instance @@ -217,8 +120,7 @@ function Reconciler:__reify(virtualInstancesById, id, parent) local instance = Instance.new(virtualInstance.ClassName) for key, value in pairs(virtualInstance.Properties) do - -- TODO: Branch on value.Type - setProperty(instance, key, value.Value) + setProperty(instance, key, rojoValueToRobloxValue(value)) end instance.Name = virtualInstance.Name diff --git a/plugin/src/rojoValueToRobloxValue.lua b/plugin/src/rojoValueToRobloxValue.lua new file mode 100644 index 00000000..995a8e4b --- /dev/null +++ b/plugin/src/rojoValueToRobloxValue.lua @@ -0,0 +1,32 @@ +local primitiveTypes = { + String = true, + Bool = true, + Int32 = true, + Float32 = true, + Enum = true, +} + +local directConstructors = { + CFrame = CFrame.new, + Color3 = Color3.new, + Vector2 = Vector2.new, + Vector2int16 = Vector2int16.new, + Vector3 = Vector3.new, + Vector3int16 = Vector3int16.new, +} + +local function rojoValueToRobloxValue(value) + if primitiveTypes[value.Type] then + return value.Value + end + + local constructor = directConstructors[value.Type] + if constructor ~= nil then + return constructor(unpack(value.Value)) + end + + local errorMessage = ("The Rojo plugin doesn't know how to handle values of type %q yet!"):format(tostring(value.Type)) + error(errorMessage) +end + +return rojoValueToRobloxValue \ No newline at end of file diff --git a/plugin/src/setProperty.lua b/plugin/src/setProperty.lua new file mode 100644 index 00000000..76a44f82 --- /dev/null +++ b/plugin/src/setProperty.lua @@ -0,0 +1,45 @@ +local Logging = require(script.Parent.Logging) + +--[[ + Attempts to set a property on the given instance, correctly handling + 'virtual properties', which aren't reflected directly to Lua. +]] +local function setProperty(instance, key, value) + -- The 'Contents' property of LocalizationTable isn't directly exposed, but + -- has corresponding (deprecated) getters and setters. + if instance.ClassName == "LocalizationTable" and key == "Contents" then + instance:SetContents(value) + return + end + + -- If we don't have permissions to access this value at all, we can skip it. + local readSuccess, existingValue = pcall(function() + return instance[key] + end) + + if not readSuccess then + -- An error will be thrown if there was a permission issue or if the + -- property doesn't exist. In the latter case, we should tell the user + -- because it's probably their fault. + if existingValue:find("lacking permission") then + Logging.trace("Permission error reading property %s on class %s", tostring(key), instance.ClassName) + return + else + error(("Invalid property %s on class %s: %s"):format(tostring(key), instance.ClassName, existingValue), 2) + end + end + + local writeSuccess, err = pcall(function() + if existingValue ~= value then + instance[key] = value + end + end) + + if not writeSuccess then + error(("Cannot set property %s on class %s: %s"):format(tostring(key), instance.ClassName, err), 2) + end + + return true +end + +return setProperty \ No newline at end of file